it is safer if the jwt.secret is not set by default

[punkle]

This change probably warrents attention from the user, as it may
invalidate the sessions of logged in users when this change is
uptaken.

However I believe it is an acceptable interference. As things are
now, if a deployment of vouch does not set the jwt.secret then the
cookie, copied to another domain would be accepted.

With this change, the default behaviour causes vouch to generate a
secret.

[punkle]

If you want to see the result of this change, take a look at the vouch logs after the change:

{"level":"info","ts":1612525742.3686314,"msg":"jwt.secret not found in /config/secret"}
{"level":"warn","ts":1612525742.3686361,"msg":"generating random jwt.secret and storing it in /config/secret"}
{"level":"debug","ts":1612525742.3686557,"msg":"open /config/secret: read-only file system"}

Reference https://github.com/vouch/vouch-proxy/blob/master/pkg/cfg/jwt.go#L19

[halkeye]

{"level":"debug","ts":1612525742.3686557,"msg":"open /config/secret: read-only file system"}

wouldn't it be better to just set it to a random string then?
or use {{ required }}

vouch was my first helm chart, so i'm certain there's lots of things that could be done better.

[punkle]

Hey @halkeye. Thanks for the quick response. That sounds like a good idea. I have two follow up questions. Is it possible in helm to generate a random string once only? Also, given that vouch itself is generating a password at startup when it is not explicitly set, how is generating one in helm a better?

The helm chart is great. Thank you for writing it.

[halkeye]

Is it possible in helm to generate a random string once only?
I don't think so. It might be possible if you do upgrades without specifiying values, but I never use that feature of helm

Also, given that vouch itself is generating a password at startup when it is not explicitly set, how is generating one in helm a better?
It would be once per install/upgrade, vs every startup since it can't write to the /config directory {"level":"debug","ts":1612525742.3686557,"msg":"open /config/secret: read-only file system"}

Also there's no reason you can't specify one, just if you don't specify one it would be random.

Personally I think required would be better to error if its not set.

How about at the top of the configmap do:

   {{- if le len(.Values.config.vouch.jwt.secret) 0 }}
      {{ fail "`config.vouch.jwt.secret` is not set and we are no longer providing a weak default" }}
   {{- end }}

and change the default value to ''

[punkle]

@halkeye thanks for your feedback and apologies for the delay. I have made your suggested changes to the pull request.

[halkeye]

Can yuou update the version in chart.yaml? then i'm going to just yolo and merge it, it looks good

[halkeye]

rawr, i'll fix the build don't worry

[halkeye]

You'll be happy to know newer vouch doesn't even start :)

vouch-7b76bb96db-rbws9 vouch {"level":"error","ts":1615659973.9340656,"msg":"Your secret is too short! (37 characters long). Please consider deleting vouch.jwt.secret to automatically generate a secret of 44 characters"}