akhq icon indicating copy to clipboard operation
akhq copied to clipboard

feat(helm): added jwt secret auto gen (#1062)

Open danielhass opened this issue 3 years ago • 6 comments

Introduced a new auto generated secret key that will only be generated on first install called "jwtSignatureGeneratorSecret". The value of this secret key is added via an environment variable setting to the depoyment only if no user defined variable with the same name is found. This should avoid duplicate env var names that can cause trouble with subsequent changes to the deployment object.

I would like to get some feedback on the implementation design. I tried to introduce the new feature without breaking existing behavior.

Summary of changes:

  • the release-name-akhq-secrets is now created always and the previous conditional is moved closer to the application-secrets.yml key
  • a Helm lookup is used to only generate the secret once on first install and reuse it's value on subsequent upgrades. This should make it possible to slowly shift traffic between pods on e.g. upgrades as all instances use the same signature generator secret
  • the deployment env section now incorporates a bunch of logic to check if a user as explicitly set the MICRONAUT_SECURITY_TOKEN_JWT_SIGNATURES_SECRET_GENERATOR_SECRET env var. In this case the auto generated value will not be used (however it will still be generated and written to the secret).

One point of uncertainty at this point is the question if we should include a general switch to completely disable this new feature. However with the default setting being active as the goal of #1062 is to make the chart secure per default.

danielhass avatar May 15 '22 11:05 danielhass

@tchiotludo would love to hear your feedback on this before moving on.

danielhass avatar May 15 '22 11:05 danielhass

hi @danielhass,

thanks for that. I look at quickly but I found it's some kind of breaking change. You will add extra env vars if the users to have defined it, but since most of people is using configuration (or secrets files), it will be override by your values.

I think I would prefer:

jwtSecret:
  create: true
  value: "xx"

to allow people to define one or to let autocreate one.

What do you think about that?

tchiotludo avatar May 15 '22 18:05 tchiotludo

@tchiotludo I get your concern. Actually I don't know enough about Micronaut to comment on the precedence of the different ways of setting the jwt generator secret. I don't know if a env variable would overwrite a configuration file supplied setting or vice versa.

Based on your feedback I will try to adapt my PR to a more explizit variant were users have to opt-in in order to use the autogen feature.

However my ultimate goal in #1062 was to provide akhq users with a secure installation out-of-the-box. Switching to a explicit setting somehow contradicts this. What would you think of an addition to the charts NOTES.txt that points users to the autogen flag if its not already set? Otherwise users would need to check the akhq container logs to find the Micronaut warning about the usage of the default JWT generator secret.

danielhass avatar May 17 '22 15:05 danielhass

@danielhass : any env vars will override any application files in micronaut. The better will be to add the generated jwt to secrets in order to create automatically the application-secrets.yml if not provide or to merge with provided one.

I think we could kept the conf for convenience :

jwtSecret:
  create: true
  value: "xx"

what do you think ?

tchiotludo avatar May 26 '22 20:05 tchiotludo

@tchiotludo thanks for the feedback and clarifying the precedence of configs in Micronaut.

I'm not sure if I get your proposal right but I would not be a huge fan of merging a generated secret into the user supplied values saved in the application-secrets.yml. I honestly think that could cause even more confusion. If we want to support the explicit way of enabling the JWT generator secret auto generation and additionally provide the user with the aforementioned convenience option I would suggest that the auto generated or user supplied value is saved as a separate key inside the akhq secret. Afterwards I would provide it via an secret based environment variable to the deployment.

This would have the additional charm that even if users use other ways of providing a secret the convenience option or auto generated value will always have a higher precedence. And as this values.yaml option is a clear user opt-in for me this sounds quite right.

What to you think about this proposal? If it sounds good to you I will go ahead and try to implement it this way.

danielhass avatar May 30 '22 13:05 danielhass

Best solution: https://github.com/tchiotludo/akhq/pull/1152/files#diff-9cc2bd3ea2b84a15de094176387d72447ead7aa612f26fd272f05e4996559752R16

Please see 16th line.

than just use

    micronaut:
      security:
        enabled: true
        token:
          jwt:
            signatures:
              secret:
                generator:
                  secret: ${JWT_TOKEN}

vutkin avatar Jul 22 '22 15:07 vutkin