Specify which keys are valid when verifying requests

[yoshdog]

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:

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',
  )

  config.add_verification_key(
    key_id: 'marketplace_key_v3',
    key: OpenSSL::PKey::EC.new(marketplace_v3_public_key),
    algorithm: 'ES256',
  )
end

But when we call the verify method, we have a new allowed_key_ids option where we can specify a list of allowed key ids

    begin
      JWTSignedRequest.verify(request: request, allowed_key_ids: ['identity_key_v1', 'marketplace_key_v3'])

    rescue JWTSignedRequest::UnauthorizedRequestError => e
      render :json => {}, :status => :unauthorized
    end

If 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

    use JWTSignedRequest::Middlewares::Rack
     
    app.get '/api/foo' do
          JWTSignedRequest.allowed_key_ids!(key_ids: ['identity_key_v1',  'marketplace_key_v3'], request: request)

          json {foo: 'bar'}
      end

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 an unauthorized status code.

[zubin]

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/])

[petervandoros]

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.

[liamdawson]

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.

[fraserxu]

There's a pair of pr for some more context

• in the monolith https://github.com/envato/marketplace/pull/23506/files
• in the job services https://github.com/envato/customer-market-homepage-jobs/pull/149

[yoshdog]

hey @fraserxu would the proposed change help with your use case?

[stevend]

👍 for more flexibility for those that wish to adopt it

[brushbox]

@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 key-id (e.g. public_key_one).

We have been using the secret_key and algorithm arguments in order to pass in the appropriate key for a given client. In order to use a single key_store for all client verification keys we would need to rename these key to ensure they are globally unique. It certainly could be done - but it will take coordination with the clients to make sure they have switched over to the new key-ids.

An alternative that could work for SSO (if secret_key/algorithm arguments are removed) is that verify provides an optional key_store: argument so that we could provide a (per client) key store instance. This could also work for the allowed keys functionality (the implementation would be a key store implementation that is a filter/delegate to the real key store). For us the simplest thing is to keep secret_key and algorithm arguments.

[github-actions[bot]]

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.