ory / fosite

Extensible security first OAuth 2.0 and OpenID Connect SDK for Go.
https://www.ory.sh/?utm_source=github&utm_medium=banner&utm_campaign=fosite
Apache License 2.0
2.33k stars 365 forks source link

Add support for JWT Bearer authorization grant #305

Closed gperinazzo closed 3 years ago

gperinazzo commented 6 years ago

Feature request:

RFC 7523 defines the usage of JWTs as authorization grants in order to obtain access tokens through the assertion framework (described in RFC 7521). This could be implemented as a grant type handler. The library currently only supports JWT assertions for client authentication.

aeneasr commented 6 years ago

Hi there, I'm open to discuss how this can be added to the project. As part of that I'd like to ask some things from you:

  1. Can you please describe the use case you are trying to solve and why it is important to have this flow implemented?
  2. Can you comment on the applicability of this spec with regard to OpenID Connect and it's certification?

We also welcome contributions to this project. If you plan to contribute a PR for this please lay out a plan in this issue how you would like to approach it. This will save both of us a ton of time!

gperinazzo commented 6 years ago

The flow allows a user to grant access to an application by sharing a private key with it. The holder of the private key can use it to sign a JWT that would be used as an authorization grant to request access tokens with a set of scopes.

My use case is to create "API keys" that users can create to grant access to our resources (as that specific user) to their own applications without requiring user interaction.

As an example, Google uses this flow in their OAuth2 server for their API service accounts.

For the second question: this spec is an extension to OAuth2, and is not related to OpenID Connect. Using the assertion framework, you can only guarantee that the user has authorized the application. As far as I can tell, this grant type can not be used with OpenID Connect as it explicitly skips the authentication step that OpenID relies on. Implementing this flow should have no effect in acquiring or maintaining OpenID Connect certification.

I am currently implementing the grant type handler. I would be happy to contribute. Following is my overall plan, would appreciate some feedback on it:

  1. Create the handler following the pattern laid by the handlers inside handler/oauth2. The handler should implement HandleTokenEndpointRequest, where the JWT assertion is checked against the spec requirements, and PopulateTokenEndpointResponse, where an access token is issued.
  2. Create a mechanism for the library to find the user-specific public key and allowed scopes for that public key. I'm not sure what would be the best way to accomplish this. I am currently using an interface to ensure that the storage has functions to retrieve these values:
    type JWTBearerGrantStorage interface {
    oauth2.AccessTokenStorage
    GetPublicKey(ctx context.Context, name string) (*rsa.PublicKey, error)
    GetPublicKeyScopes(ctx context.Context, name string) ([]string, error)
    }
  3. Create the compose helpers to build the grant

I am currently doing this from my client code, and it's enough to implement the flow with a small issue that may not be solvable from client code: the assertion framework says that client authentication for this grant is optional, however the library does not seem to allow access to the token endpoint without at least the client id. What would be the best way to allow this?

aeneasr commented 6 years ago

Thank you for the explanation and sorry for the long response time. I'm wondering whether this isn't simply OAuth 2.0 Client Credentials where the authentication credentials are a JWT signed with a private key? Could you shed some light on the differences?

aeneasr commented 6 years ago

The part where we use assertsions as authorization grants is also implemented already by another spec, OpenID Connect request object or request url, see: https://openid.net/specs/openid-connect-core-1_0.html#JWTRequests

gperinazzo commented 6 years ago

Thank you for the explanation and sorry for the long response time. I'm wondering whether this isn't simply OAuth 2.0 Client Credentials where the authentication credentials are a JWT signed with a private key? Could you shed some light on the differences?

The difference between the two is that the JWT Bearer grant allows someone to acquire access tokens on behalf of a specific user without user interaction, while the Client Credentials grant allows clients to acquire access tokens that are not related to any user. The JWT Bearer grant also allows anonymous clients to requests such tokens (with no client authentication required from the token endpoint). It's similar to using a JWT to authenticate the client, but it authenticates a user instead.

The part where we use assertsions as authorization grants is also implemented already by another spec, OpenID Connect request object or request url, see: https://openid.net/specs/openid-connect-core-1_0.html#JWTRequests

That spec defines how to use a JWT to send request parameters to the authorization endpoint. The JWT Bearer flow skips the authorization endpoint as possession of a user's private key is considered implicit authorization.

gperinazzo commented 6 years ago

I guess it's easier to define it as Resource Owner Password Grant with a JWT instead of password :thinking:

aeneasr commented 6 years ago

Ok, so I read a bit more on the spec. There are multiple flows:

I think the second part is both valuable and potentially dangerous. On the one hand, it allows privileged users (with the proper private keys) to authorize credentials on a user's behalf without consent. On the other, careless use of this feature ("let's give every internal dev access to that secret key") is a blatant security and privacy violation. I was first confused because I thought a "Service Account" is actually a programmatic identity (read client credentials) and not considered a user - but I was apparently wrong here.

If I remember correctly, you created this issue originally at the ORY Hydra repository, indicating that this is a functionality you'd like to see in ORY Hydra. So, on the one hand it would be a great addition because it allows you to get access credentials without user interaction, which can be useful. On the other hand, many safeguards (like hitting login and consent endpoints) can not be implemented with this. Instead we'd just issue a token for whatever subject we were given, without any way of checking if the user is banned, inactive, flagged, or whatever.

Another issue I see is that developers love solving things with "least resistance". What I've seen often is a "oh can't I just issue the token I mean I am the dev so I should be allowed to do that" (no you're not, otherwise Google would have leaked a hundred times already). I fear that this feature may push people in that direction.

To conclude, it definitely has it's validity but - if the goal is to add it to Hydra - we have some obstacles to overcome. I'm open for ideas and input.

aeneasr commented 6 years ago

Regarding your underlying use case - the personal access token (at least that's what it's called in GitHub) is a totally valid flow. I don't think that this RFC is necessary for that. The flows I know require user interaction in the form that you're clicking a button "generate personal access token" which then gives you the access token. Performing the login and (optional) consent dance is IMO a good security measure. If I recall correctly, GitHub prompts you for your password when you generate such a token. This reduces risk of someone using your existing login session to issue long living access credentials without your knowledge. From my standpoint, it would be valid to perform the authcode flow here.

But again, open to ideas and suggestions and criticism of my thinking.

aeneasr commented 6 years ago

I researched a bit more and there is an important limitation which will prevent your use case if I understand it correctly. RFC7521 (Using Assertions as Authorization Grants) limits the lifespan of credentials issued by an assertion:

An assertion used in this context is generally a short-lived representation of the authorization grant, and authorization servers SHOULD NOT issue access tokens with a lifetime that exceeds the validity period of the assertion by a significant period. In practice, that will usually mean that refresh tokens are not issued in response to assertion grant requests, and access tokens will be issued with a reasonably short lifetime. Clients can refresh an expired access token by requesting a new one using the same assertion, if it is still valid, or with a new assertion.

This is not mentioned in RFC7523 but since it's a direct expansion of 7521 my take is that this limitation is true for RFC7523 as well. If that stands true, long-living API keys are nothing this flow should provide. Please correct me if you read this differently or have other insights into this.

gperinazzo commented 6 years ago

I agree 100% that it can be a dangerous feature, and understand that it may not be able to be done in Hydra. I asked about it on the discord before opening this mostly to see if there was another flow to accomplish the same without having to implement this one. I understand that there are security implications (from bad practices that could emerge) and technical limitations (hydra not being an identity provider) that prevent the flow from being supported there.

My team and I are already committed to building our own solution using fosite (which we are loving so far, awesome design!).

Regarding your underlying use case - the personal access token (at least that's what it's called in GitHub) is a totally valid flow. I don't think that this RFC is necessary for that. The flows I know require user interaction in the form that you're clicking a button "generate personal access token" which then gives you the access token. Performing the login and (optional) consent dance is IMO a good security measure. If I recall correctly, GitHub prompts you for your password when you generate such a token.

Yes, this flow is well known and used in a lot of places. I see a few advantages to using the JWT Bearer instead of normal API keys:

Of course creation of the public/private key pair necessary for the flow should be locked behind a forced login, but that is beyond the scope of the library.

I researched a bit more and there is an important limitation which will prevent your use case if I understand it correctly. RFC7521 (Using Assertions as Authorization Grants) limits the lifespan of credentials issued by an assertion:

I don't see that as a limitation. Possession of a private key means you can always create a new JWT to request a new access token once the one you have expires. We plan on providing our own library to manage the access token in client applications. The access token does not (and should not) have be long lived at all.

I'm already implementing it for our use, thanks to how easily we can extend fosite, but if you think it would make sense to support this flow in the library I would be glad to contribute! And feedback on how to better do this is appreciated.

aeneasr commented 6 years ago

Great to hear that, the handler design was specifically built to support extensions like this. There are some issues left with this library which I simply had not the time to address and also partly because they are primarily implemented in ORY Hydra which make the implementation tricky at times, specifically regarding OIDC validation and some session issues.

I'm debating if this should be added as a standard handler to this library or not, and if so if it should be enabled by default. I'm currently converging on "it should be added but not enabled by default". I think the risk of implementing this incorrectly is high but the benefit of using it correctly is high too.

From a quick glance, the interface looks ok. Does the name correspond to the sub field?

I was also thinking about connecting the API Key to the private key which is then used to exchange the access token, but I think (this is my take, not a criticism of your work) is too bloated and has several disadvantages:

I have been thinking for a long time about personal access tokens. The topic has come up several times. There are two camps:

  1. We need personal access tokens because it makes user interaction easier. This is the case for GitHub where you need PAT for the git password portion as git does not support oauth2 mechanisms.
  2. We have OAuth 2.0 Flows and don't need personal access tokens because we have full control over all clients can implement OAuth 2.0 Flows. This is the case for the Google Cloud SDK which uses OAuth 2.0 to get the access token. The user flow is very nice.

I haven't set up a CI integration with Google Cloud recently so I'm not sure how that works at the moment. I remember a JWT config with some private key but I don't recall if it was used as an assertion or something else.

If you feel that I have a point with my criticism on your approach I am more than happy to discuss further ideas. I think that PATs can generally be created as long-living access tokens. That would allow all the regular mechanisms and on top a way to natively revoke them. I am keen on solving this because again - it's a valid use case. Even if with just come up with a guideline to solve this, it would be more than exists currently and both you and the community will benefit from it.

aeneasr commented 6 years ago

Ok but you are definitely right, in the case where we're creating a GCloud service account it is possible to issue access/refresh tokens without user interaction (well doesn't make sense anyways because the service account is not a user) and it's done with the aforementioned RFC and assertion framework. This solves the case where we have a CI integration (programmatic client) that needs to access resources in a long-living manner. The SDK then exchanges the assertion for tokens which are understood by the API authorization layer.

I'm guessing that Google has a special account type (Service Account) here because they separate between OAuth2 Clients which are not seen as "Identities" and their Service Accounts. I'm not sure what design philosophy is behind this but it's maybe that other end-users can also act on behalf of a service account.

Anyways, it seems like Google is not exposing a way of exchanging an assertion for an end-user, only for programmatic identities.

gperinazzo commented 6 years ago

The way Google seem to make their service accounts (according to this article) is:

I guess that the OAuth2 server is the only place in the system where a service account is seen as different from an end user. After the flow, the resource server that receives the token probably sees either: the service account itself as a valid end user or an actual end user in the case of impersonation.

Anyways, it seems like Google is not exposing a way of exchanging an assertion for an end-user, only for programmatic identities.

You can impersonate an end-user with the assertion. It does require the service account to be authorized to do so inside the GCloud Admin console.

From a quick glance, the interface looks ok. Does the name correspond to the sub field?

I am currently using the iss field, since sub may be an anonymous value, I guess relating the public key to the issuer makes more sense.

I tried to make the interface not assume what the public key is, that should be up to the library client to define. It could be a personal token (issuer is the user) or a separate entity (issuer is the entity id).

You're not relying on revokation to revoke tokens. This has to be implemented as additional logic.

That is true. You could revoke the private key by forgetting the public one, and revoke every access token created for that specific account, but this would probably be up to the library client.

You have to resolve the API Key and perform the relevant flows (if refreshing is required) on every API hit. In my opinion, this should be much easier. I don't have a clear answer HOW it would be easier, but is HAS to be easier, at best without any additional processing.

This does feel awkward, especially if you're only making requests infrequently, but there are some advantages if you're reusing the same access token more than once:

Having a long lived access token going around in the network doesn't sit well with me. That is down to mostly personal opinion though. It would however work for my use-case and allow for scoping to reduce the risk in case it's leaked.

aeneasr commented 6 years ago

Thank you for the summary. I had a good night's sleep and time to think about this more. First of all, I appreciate the effort and think it would make sense to be added to this repository. I want to propose however that the signature changes. This is due to the fact that you're assuming RSA keys but we really want to think beyond that. I think we need a dedicated validation strategy. The strategy has then access to e.g. the store or some other means of retrieving the key that's required. Maybe the validation strategy holds the keys itself or maybe it retrieves them from a store. I'd leave that up to the strategy and not assume this in the storage interface.

aeneasr commented 6 years ago

Oh and second, I think we could solve the API Keys with long-living assertions that allow long-living tokens. I share your sentiment against that, on the same page though it greatly improves developer UX if all you need is to include a token in the HTTP Header (as opposed to performing the assertion dance).

gperinazzo commented 6 years ago

Oh right, I forgot about that, we were only going to support RSA keys internally so that's why it's like that. For the library, the interface should return interface{} that could be any type of public key. I guess a better signature for a key-finding function would be to receive the parsed JWT so it can implement whatever logic for key retrieval it wants (like checking the JWT header to see what type of key it expects, or using a claim other than the issuer to find it). Changing it to a dedicated validation strategy, what parts of the JWT validation would you want to be left to it? Do we expect it to do the full validation expected by the spec or do we define parts of the validation that it would outsource to the strategy (like key and scopes retrieval, maybe time validation so it can allow long lived tokens)?

aeneasr commented 6 years ago

Actually, I think we should implement this with a bit of a broader spec, specifically with having RFC7521 in mind. As such, we would probably have different strategies that register for different assertion types. They expose ValidateAssertion(context.Context, assertion string) (error) (and optionally an argument to modify the response).

The standard JWT implementation could then simply use RS256 and have a field for pre-defined public keys and use the issuer for lookup (e.g. Keys map[string]rsa.PublicKey.

Regarding your question with client auth, this is a more difficult topic. Throughout the framework, we assume that client authentication is a given. They say the original spec has this as optional, I'm not sure where they get that from:

image

But apart from that there are certain specifics that cause trouble here. One is that token introspection doesn't have a way to look up the client_id. I thought that we could use the assertion to get the client_id but that doesn't seem to be the case. Not sure how to solve this. Any ideas?

gperinazzo commented 6 years ago

That's a great idea! It'll make it easier if someone needs to implement RFC 7522 Security Assertion Markup Language (SAML) 2.0 Profile for OAuth 2.0 Client Authentication and Authorization Grants that also uses the assertion framework.

We'll have then a generic AssertionGrantHandler that holds a set of strategies for assertion validation and an access token strategy.

type AssertionGrantHandler struct {
    AssertionStrategies []AssertionStrategy
    *HandleHelper
}

On HandleTokenEndpointRequest we'll query each strategy to see if it can understand the grant type, validate if the assertion request is valid (i.e contains the assertion field, check for scope field), and call to the strategy to validate the assertion and scopes. The AssertionStrategy interface would be something like this:

type AssertionStrategy interface {
    CanHandleGrant(grantType string) bool
    ValidateAssertion(context.Context, assertion string, scopes Arguments) error
}

I don't think we'll need anything from the strategy to write PopulateTokenEndpointResponse.

I would like to write the JWTAssertionStrategy in a way that allows a library client to reuse it easily should they want to only replace the public key finding algorithm, so I'll probably need another interface there with the default implementation using a static map.

About allowing anonymous clients, this is the interpretation I belive they're using:

Confidential clients or other clients issued client credentials MUST authenticate with the authorization

This means that if I've never been issued any credentials (thus am also not a public or confidential client), this sentence does not apply. It's a bit weird but reading the spec that's the only place I think you could interpret in a way to allow optional client authentication.

I don't think simply allowing anonymous requests to the token endpoint would be a good idea. Maybe having a way to only allow it for certain grant types would be best.

Why is not having a client an issue for token introspection? The endpoint itself requires client authentication (and that's fine), but I can't find where it would require the access token itself to be linked to a client. Could you point me to the file?

mattes commented 5 years ago

A lot of knowledge in this thread. Impressed. What happened to this ticket though? Was this feature added eventually?

aeneasr commented 5 years ago

I don't think any work has been done on this issue.

gperinazzo commented 5 years ago

I changed jobs shortly after this and don't have access to the code anymore. I thought someone else from the company was going to continue this issue but it seems they didn't.

mitar commented 4 years ago

Isn't this now available in Hydra, but not in Fosite? Any reason not to expose it on Fosite as well?

aeneasr commented 3 years ago

Only for client authentication - which is also implemented in ORY Fosite!