-
-
Notifications
You must be signed in to change notification settings - Fork 358
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for clients' secret rotation #590
Comments
The OAuth2 "compliant" (or rather OpenID Connect) way to do this is to create a copy of the client with a new password and update the client ID and the client secret in the OAuth2 consumers. This is because OpenID Connect Dynamic Client Registration currently does not have a spec for secret rotation and the API design more or less prohibits secret rotation. Alternative would be using JWT-based auth for clients ( |
Nevertheless - I completely understand the feature and think it's a good idea to support that! |
Great. Thanks! We want to automate entire process so having two active secrets is preferred way for us. What you think about a way how to get this feature done? Can we kindly ask you to add this functionality? |
That's not how open source works ;) But you can come up with an idea how of to implement it, discuss it in this issue, and then execute on the plan in a PR |
No problem :), we will prepare an idea PR and we are gonna share that with you. ;] |
@aeneasr Hi, we have looked into this and have the following proposal:
type ClientWithSecretRotation interface {
Client
// GetHashedSecretPrevious returns the previous hashed secret as it is stored in the store.
GetHashedSecretPrevious() []byte
}
func (f *Fosite) checkClientSecret(ctx context.Context, client Client, clientSecret []byte) error {
var err error
err = f.Hasher.Compare(ctx, client.GetHashedSecret(), []byte(clientSecret))
if err == nil {
return nil
}
cc, ok := client.(ClientWithSecretRotation)
if !ok {
return err
}
return f.Hasher.Compare(ctx, cc.GetHashedSecretPrevious(), []byte(clientSecret))
} This seems like a simple approach that won't break custom clients as it just adds a new interface and behavior for existing clients doesn't change. Having it separated like this also allows for instrumentation on the Is this something that would be an acceptable approach? There are some open questions:
|
0Awesome, this sounds like a great proposal! I think the naming could be We could also think of making return a slice of slice of bytes so Looking forward to the contrib! |
Closing because merged! |
Is your feature request related to a problem? Please describe.
Currently it's not possible to simultaneously rotate secrets of individual clients.
Describe the solution you'd like
That would be great to set two passwords while both of them are valid in that time. That allows user to smoothly rotate secret without any outage.
The text was updated successfully, but these errors were encountered: