feat: Add rotation support for client secrets

[rplnt]

Related issue

Issue: #590
Discussed with: @aeneasr

Proposed changes

The aim of this PR is to introduce support for rotation of client's secrets.
New interface is created that extends fosite.Client so as not to break compatibility of custom clients. The fosite.DefaultClient implements this interface by introducing RotatedSecrets field. When not set (nil), the functionality stays the same as before.

Checklist

• I have read the contributing guidelines
  and signed the CLA.
• I have read the security policy.
• 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 green light (please contact
  [email protected]) 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 necessary documentation within the code base (if
  appropriate).

Further comments

Detail of how the change above is utilized in the fosite code:

1. check regular (main, current) secret by using GetHashedSecret method
2. if 1. fails, try to see if provided client implements fosite.ClientWithSecretRotation
3. if 2. succeeds, call GetRotatedHashes and loop over provided hashes to compare the secret

Some questions I have:

• I'm not sure if the addition of this feature is documented enough, or if there is some other place I could add it to?
• By having the checkClientSecret method be private, I'm unable to test it directly, so only a few basic scenarios were added testing it via the methods that use it. Should I opt for public method just in order to test it better? Any other preference?

[aeneasr]

Awesome, thank you for your contribution! This looks very good. Just one or two minor things :)

[aeneasr]

Do you want to enable this test or remove it? :)

[rplnt]

Tried to clean commit history and somewhat ended up with this there :)

Removed it now, but it relates to my last question.

[aeneasr]

Thank you! I think this looks good. Documentation could be added in the README, otherwise we don't really have docs. Regarding the tests, I think the current set up is sufficient!

Sorry for the long review time, I missed the force push in my notifications :/

[rplnt]

@aeneasr It's all right, just let me know if there's something else needed.

Haven't found a great section to put it into, but it's in the example that is in the readme at least.

[aeneasr]

Maybe you could do a follow up PR in https://github.com/ory/fosite-example/ ? With rotation enabled there. I think that's probably the best option.

Merging this one :)

[seamory]

I'm confuse what does the RotatedSecrets do? Anyone who can help me explain it? Thanks.

[rplnt]

@seamory You can set RotatedSecrets in DefaultClient as a slice of secrets and when the main secret check fails the rotated secrets are checked instead. The purpose is to be able to rotate secrets without any disruption (or synchronisation between services) as multiple secrets can be valid at the same time.

Have a look here ory/fosite-example#33 (it's using changes in NewExampleStore from this PR)

[seamory]

@seamory You can set RotatedSecrets in DefaultClient as a slice of secrets and when the main secret check fails the rotated secrets are checked instead. The purpose is to be able to rotate secrets without any disruption (or synchronisation between services) as multiple secrets can be valid at the same time.

Have a look here ory/fosite-example#33 (it's using changes in NewExampleStore from this PR)

Thanks, i'm clear now.