rafiki icon indicating copy to clipboard operation
rafiki copied to clipboard

Check request access during token introspection

Open wilsonianb opened this issue 2 years ago • 6 comments

  • https://github.com/interledger/rafiki/issues/798#issuecomment-1335314019
  • https://github.com/interledger/rafiki/issues/798#issuecomment-1335342847

Pass the Open Payments request access type/action in the token introspection request access field. The AS should return { active: false } if the token's grant does not include the specified type/action. Otherwise, the token introspection response's access field should only include the access permitting the request.

https://datatracker.ietf.org/doc/html/draft-ietf-gnap-resource-servers#name-token-introspection

Note: make sure to update the token-introspection README while completing this. See:

  • https://github.com/interledger/rafiki/pull/1271#discussion_r1149291844

wilsonianb avatar Dec 09 '22 17:12 wilsonianb

Hi @wilsonianb @mkurapov @sabineschaller , After going through the issue, I think motive is to match args.access == tokenInfo.access (By this condition we're verifying if the token's grant does not include the specified type/action) , if yes then return tokenInfo , if not then return {active : false} . I'm not sure about else condition as in comments its mentioned that we need to throw error.

changes needs to be done in introspection.ts

export const validateTokenInfo = (tokenInfo: TokenInfo): TokenInfo => { // TODO: tokenInfo.access must include args.access // https://github.com/interledger/rafiki/issues/835 // throw new Error( // 'Token info access does not match request access' // ) return tokenInfo }

Let me know if i'm understanding this correctly, then i'll proceed.

Yashdeep-Jain avatar Oct 09 '23 00:10 Yashdeep-Jain

Hi @tyash71, Thanks for picking this up. You assessment about returning {active: false} is correct. That is also what the issue says. The comment in the code needs to be removed. Would you like to work on this?

sabineschaller avatar Oct 23 '23 07:10 sabineschaller

Hi @sabineschaller , In this issue we're getting one tokeninfo parameter which has these properties ->

access_token: type: string description: The access token value presented to the RS by the client instance. proof: type: string description: The proofing method used by the client instance to bind the token to the RS request. resource_server: oneOf: - $ref: '#/components/schemas/key' - type: string description: 'The identification used to authenticate the resource server making this call, either by value or by reference.' access: $ref: ./schemas.yaml#/components/schemas/access

So from here, on which property basis should i declare IF ELSE case , like either it should be like

if(tokeninfo.access){ return tokeninfo }else{ return {active:false} }

OR

if(tokeninfo.access_token){ return tokeninfo }else{ return {active:false} }

Please clarify on this, like in the issue comment , there it says if the token's grant does not include the specified type/action. And as per my understanding its grant property is in response as mentioned in resource-server.yaml.

Yashdeep-Jain avatar Oct 24 '23 13:10 Yashdeep-Jain

@tyash71 my understanding of this is what that we should pass in access here:

https://github.com/interledger/rafiki/blob/16adf550f0b9e1e50b0c73f6353edffbb5313d33/packages/backend/src/open_payments/auth/middleware.ts#L68-L70

which allows this chunk of code below (that checks that the granted access that corresponds with the request):

https://github.com/interledger/rafiki/blob/16adf550f0b9e1e50b0c73f6353edffbb5313d33/packages/backend/src/open_payments/auth/middleware.ts#L78-L108

to be basically moved into the introspection service handler: https://github.com/interledger/rafiki/blob/16adf550f0b9e1e50b0c73f6353edffbb5313d33/packages/auth/src/accessToken/service.ts#L69

If it the check fails, we should return { active: false } at the authorization server level.

mkurapov avatar Oct 25 '23 14:10 mkurapov

I'm thinking of picking this up since this is in the createTokenIntrospectionMiddleware that would directly be part of

  • #2395

However, I think

Otherwise, the token introspection response's access field should only include the access permitting the request.

is not so straightforward. Following this logic exactly, since we either pass in RequestAction.Read or RequestAction.List into createTokenIntrospectionMiddleware when we register get & list routes respectively, we never pass in (and therefore never get back) Read ListAll, even though the grant might permit that action. In that case, if the

token introspection response's access field should only include the access permitting the request

should it return the "highest" possible access that the grant has? ie if passing in RequestAction.Read and the grant has access for ReadAll, should we just return ReadAll?

mkurapov avatar Feb 21 '24 12:02 mkurapov

I'm thinking of picking this up since this is in the createTokenIntrospectionMiddleware that would directly be part of

However, I think

Otherwise, the token introspection response's access field should only include the access permitting the request.

is not so straightforward. Following this logic exactly, since we either pass in RequestAction.Read or RequestAction.List into createTokenIntrospectionMiddleware when we register get & list routes respectively, we never pass in (and therefore never get back) Read ListAll, even though the grant might permit that action. In that case, if the

token introspection response's access field should only include the access permitting the request

should it return the "highest" possible access that the grant has? ie if passing in RequestAction.Read and the grant has access for ReadAll, should we just return ReadAll?

I think returning the most relevant access on the grant that permits the action would be the best way to go. Doing that still fits within the description of "only including the access permitting the request" in my opinion.

njlie avatar Feb 21 '24 14:02 njlie