refactor: jwk aquire lock when generating

[awill1988]

Update the locking behavior of jwk.GetOrGenerateKeys to acquire a read lock to fetch keys and only acquire the write lock when generating and persisting new keys.

Related issue(s)

3863

Checklist

• [x] I have read the contributing guidelines.
• [x] I have referenced an issue containing the design document if my change introduces a new feature.
• [x] I am following the contributing code guidelines.
• [x] I have read the security policy.
• [x] I confirm that this pull request does not address a security vulnerability. If this pull request addresses a security vulnerability, I confirm that I got the approval (please contact security@ory.sh) from the maintainers to push the changes.
• [ ] I have added tests that prove my fix is effective or that my feature works.
• [ ] I have added or changed the documentation.

Further Comments

I can attempt to write up more unit tests if that is required, but since this is not a significant change I am wondering if the existing tests can cover it?

I am also not sure what would need to be written for documentation but I am happy to add it where it is preferred.

On the related issue there are some reproducible steps that can be used to demonstrate the benefit of this refactor, but I can specify steps or update here if needed.

[alnr]

This whole code is unsafe in the presence of multiple hydra instances. We would need to change the database code to do an INSERT INTO ... ON UPDATE DO NOTHING RETURNING * to properly synchronize.

[aeneasr]

This whole code is unsafe in the presence of multiple hydra instances. We would need to change the database code to do an INSERT INTO ... ON UPDATE DO NOTHING RETURNING * to properly synchronize.

While this is true, it doesn't really matter, because the whole thing will eventually settle. Even if we created 20 keys, it will be like if we rotated through 20 keys and just use the newest key. So the impact is really minimal in my view

[alnr]

That is:

1. SELECT * FROM HYDRA_JWK
2. a) if found, return
3. b) if not found, generate a new one in memory (could be done under a mutex to throttle CPU usage)
4. INSERT INTO hydra_jwk ON CONFLICT DO NOTHING
5. SELECT * FROM hydra jwk
6. Continue with the key returned from the last select.

Postgres and other DBs which support RETURNING could do 4+5 in one step as an optimization (optional, since this is a rare event).

[aeneasr]

I think that analysis is correct. So all we need really is a lock for when we generate the key and not for reading, which also solves the issue here

[aeneasr]

https://github.com/ory/hydra/pull/3866/files

[awill1988]

closing in favor of #3866, 🙏🏻 for taking over this issue @aeneasr