ietf-wg-gnap / gnap-core-protocol

141 stars 26 forks source link

Support for Refresh Tokens? #485

Closed blue-ringed-octopus-guy closed 1 year ago

blue-ringed-octopus-guy commented 1 year ago

In section 6.1. of the document where it discusses Access Token Rotation, I'm slightly surprised by the lack of support for Refresh Tokens? From my interpretation of the specification, the only valid way to rotate an expired Access Token is to pass in this same expired Token (signing the request with the appropriate cryptographic key), and then the AS expected to validate this expired Access Token and conclude that it is valid? I also can't see any provision within the specification for the AS to (optionally) issue a Refresh Token along with the Access Token(s) that it sends to Client?

There is something that feels quite wrong about expecting the AS to determine that an expired Access Token has successfully passed validation. On top of this, the lack of Refresh Token support could be confusing for someone who is familiar with using OAuth 2.0 - at the very least, I think Refresh Tokens should be mentioned in the document with a short explanation as to why they aren't supported (if this is indeed the case, and if this is a deliberate decision). I know the aim here is to have a fresh start and to not automatically replicate all of the concepts and details from OAuth, but I wasn't aware that Refresh Tokens was one of those concepts that needed removing.

I'd welcome your thoughts on these points.

jricher commented 1 year ago

There are no refresh tokens in GNAP as they are no longer required due to the existence of a comprehensive grant API. The equivalent action to using a refresh token would be to use a continuation call for an existing grant to get a new access token (section 5), specifically something like that what's listed here (section 5.2): https://www.ietf.org/archive/id/draft-ietf-gnap-core-protocol-12.html#name-continuing-during-pending-i

GNAP also separately allows for the rotation of an existing access token, which is the section you're referring to above. This would change the value and reset the expiration of an access token, not issue a new one from the existing grant.

blue-ringed-octopus-guy commented 1 year ago

Thanks for the response Justin.

I'm not sure I completely follow your logic here. I'm specifically talking about the scenario described in section 1.6.6. In this section it describes the flow that needs to be followed when the Access Token to call the RS has expired. In the text for step 8 of the flow it says the following:

The AS validates the rotation request including the signature and keys presented in (7) and returns a [new access token](https://www.ietf.org/archive/id/draft-ietf-gnap-core-protocol-12.html#response-token-single) ([Section 3.2.1](https://www.ietf.org/archive/id/draft-ietf-gnap-core-protocol-12.html#response-token-single)). The response includes a new access token...

Given that the value and the expiration date of the Access Token are being changed, this is a new Access Token that is being issued, as it says in the text quoted above. This contradicts your response above.

So my general point here is that when the Token Management URI is called within the flow described in section 1.6.6, I'd expect to have the option to pass a Refresh Token (that is associated with the expired Access Token) into the Token Management URI (or at least, the option to pass in a different Access Token linked to the other Access Token that is specially used to access the Token Management URI), and use that separate (non-expired) Token as the means of getting my expired Access Token rotated. This would then mean that the Token Management URI wouldn't be required to have to treat an expired Access Token as "valid" - as doing this seems to be slightly clumsy (and bad practice). If an Access Token has expired then it shouldn't used to successfully access anything - even if that thing is a Token Management URI.

jricher commented 1 year ago

The quoted text should not say "a new access token", and that should be fixed, thanks for pointing out that inconsistency. We'd gone through and fixed a lot of that language, but we seem to have missed this one.

It might help to think of it in terms of implementation. Let's say I have a grant, G1, that has two access tokens, A1 and A2. These tokens have values and expiration dates associated with them:

G1
 - A1
   - value: abc123456
   - exp: 500
 - A2
   - value: xyz67890
   - exp: 600

When the client rotates access token A1, it changes the data structure like so:

G1
 - A1
   - value: qrs48239
   - exp: 1000
 - A2
   - value: xyz67890
   - exp: 600

But if the client instead calls the grant continuation endpoint to create a new access token, the data structure changes like so:

G1
 - A1
   - value: abc123456
   - exp: 500
 - A2
   - value: xyz67890
   - exp: 600
 - A3
   - value: vhf457103
   - exp: 1000

We'd briefly talked about having a separate access token to access the token management API, like we do with the continuation URI, but that seemed like it would be a much more significant overhead in terms of complexity and the amount of items the client needs to track to perform basic functions. The AS already needs to associate the presented access token with the token management URI and dereference it to a previously-granted set of access rights, which are represented by that access token. The thought was that we've already got an access token value, let's just re-use that in place. The AS can determine whether or not it wants to rotate an expired access token. This isn't any more complex for the client -- it takes the token value and drops it into the request in the right way.

Also note that all calls to token rotation are signed by the client's key, so the AS has a strong association with the caller. The signature acts as the proof of the caller's right to call, the token value merely points at which data record above the action is performed on.

blue-ringed-octopus-guy commented 1 year ago

In your example above where you rotate token A1, I view the Access Token that is returned from that call as a "new" token. Access Tokens are immutable - therefore, if you need to change any item within the token data structure (whether that be the expiry date, or the value) then you're generating a new token (even if that new token has the same name or label, it is a new token). I think this is just about differing interpretations of what "new" means. It's not a big deal, but I do have a slight concern that by not referring to the new version of token A1 in your example above as "new" then readers of the specification may get the impression that Access Tokens are mutable, when clearly they aren't mutable.

Regarding the discussion about whether the Token Management URI should have a separate Access Token that is used to access it - I completely understand the concern that it would add significant overhead and complexity. I agree that it would add a lot of extra complexity to the protocol to add this in, however, I don't think that argument alone satisfies my original concern here. If the expectation is the token rotation calls to the Token Management URI are being protected only by the client signing the request with their private key, then the expired Access Token shouldn't be being passed into the request within the Authorization header - perhaps it would be more appropriate to instead pass the Access Token within a separate parameter in the body of the POST request in this instance. Putting the Access Token in the Authorization gives the impression that this token is expected to be fully validated by the AS and that it is being used to evidence that the client is authorized to make that particular call.

blue-ringed-octopus-guy commented 1 year ago

I accidentally closed the issue - I didn't intend to do that. Can it be re-opened?

jricher commented 1 year ago

I wouldn't say it's necessarily clear or universal that access tokens are not mutable in the wild -- but it depends on what you're talking about when you say "access token". If you're talking about the value that goes over the wire, then yes, that's probably pretty accurate. If you're talking about what that token represents, then that's the kind of thing that can and does change in real systems today. I don't think we see it explicitly out there a lot because you usually see short-lived access tokens in use. Plus the use of self-contained structured tokens like JWTs conflates things a bit, because the value of the token contains everything that the token represents, including its own expiry.