-
Notifications
You must be signed in to change notification settings - Fork 5
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
Specify which keys are valid when verifying requests #38
Comments
I like this idea! I've handled this in application code but here is much nicer. 👍 I wonder if there's a convenient way to handle key rotation, so that there's no need to update the key ID in both places, eg with a regex: JWTSignedRequest.configure_keys do |config|
config.add_verification_key(
key_id: 'identity_key.v1',
key: OpenSSL::PKey::EC.new(identity_v1_public_key),
algorithm: 'ES256',
)
end
JWTSignedRequest.verify(request: request, allowed_key_ids: [/^identity_key/]) |
I haven't given this much thought yet, but I know that @Shervanator, @fraserxu, and @lukerobins have opinions on this from their recent work with adding API endpoints to marketplace for shopfront. |
Just registering the same input I gave on Slack: it feels quite a bit like we're reinventing a wheel that's been designed a number of times. For example, GitHub would handle this via scopes in access tokens (as per OAuth2). I might give this some thought as an issue in the architecture repo a little later. |
There's a pair of pr for some more context
|
hey @fraserxu would the proposed change help with your use case? |
👍 for more flexibility for those that wish to adopt it |
@yoshdog I don't think this change is directly of use for SSO server. We don't currently have a need to lock down endpoints based on which key is accessing them. From a purist point-of-view doesn't this change mix authorisation into JWT - which has so far only been used for authentication (I think)? I don't mind - if it makes things easier for users of this gem then it isn't a bad thing. As for SSO: we don't even make use of the key store - nor can we, as we can (and do) have multiple clients that use the same We have been using the An alternative that could work for SSO (if |
This issue has had no activity for 60 days and is now considered stale. It will be closed in 7 days if there is no further activity. |
Context
When we added support for multiple verification keys via the usage of a key store, we introduced an interesting side effect. When users added verification keys to the key store this allowed these keys to be used to verify requests on every endpoint. This served the use case where the server allowed all its clients access to all its endpoints, but there are some servers which only want to grant access to certain clients per endpoint.
In this issue we will propose a way forward to allow us to specify which keys are valid per endpoint.
Proposal
We will still add verification keys the same way:
But when we call the verify method, we have a new
allowed_key_ids
option where we can specify a list of allowed key idsIf the request is signed with a key_id not on this list, we will raise an
UnauthorizedRequestErrror
For cases where people are using the rack middleware
We will introduce a new
allowed_key_ids!
method which can be used per endpoint that checks that the request has been signed by a valid key_id. If it isn't we can halt and return anunauthorized
status code.The text was updated successfully, but these errors were encountered: