Recent Change in Token Revocation Endpoint Behavior Breaks Revocation of Tokens for Public Client

[edmondchui]

Steps to reproduce

I have a mobile app using a Rails server, which in turn depends on Doorkeeper to generate access tokens for API calls. I used to be able to logout successfully from the client using a call to Doorkeeper::TokensController.revoke. However, after upgrading to version Doorkeeper 5.4.0, I got HTTP response 401 during logout.

Expected behavior

Token revocation using a valid access token from Resource Owner Password Credentials Flow should work.

Actual behavior

Token revocation fails with a valid access token from Resource Owner Password Credentials Flow.

System configuration

In version 5.3.3, before this change https://github.com/doorkeeper-gem/doorkeeper/pull/1370/commits/d194bc40e335464e67bc36dfd7e2e61247547545, a token granted to a public client (https://tools.ietf.org/html/rfc6749#section-2.1) via Resource Owner Password Credentials Flow works fine as the method Doorkeeper::TokensController.authorized? is used for authentication.

In version 5.4.0, the method Doorkeeper::TokensController.revoke is modified to require a server.client, which is a confidential client (https://tools.ietf.org/html/rfc6749#section-2.1).

Ruby version: 2.6.5

[nbulaj]

Hi @edmondchui .

Did you do this from your mobile app?

The client also includes its authentication credentials as described in Section 2.3. of [RFC6749].

server.client which you mentioned just stores client instance that performs a request. It is taken from the credentials used to authorize the request. And if it's blank - aka no auth passed or wrong credentials used - request is blocked as stated in the the RFC.

You need to call POST /token/revoke with ?token= value of access token you wanna to revoke and include authorization headers (HTTP-Authrozation: Basic Base64(client_id:client_secret)) to inform the server that this request is authorized to perform an action. Alternatively you could send client_id and client_secret via body params, but this method isn't recommended by the RFC. Don't forget that if the token you wanna to revoke was issued to some specific client - only this client could revoke the token (it's credentials must be used to authorize the request).

More details: section 2.1. Revocation Request of the RFC7009 and then section 2.3. Client Authentication of RFC6749

[AndKiel]

I've got a similar problem. I'm trying to upgrade from Doorkeeper 5.1 to latest.

Token revocation stopped working for me. I'm getting 403 with {"error"=>"unauthorized_client", "error_description"=>"You are not authorized to revoke this token"} in my spec.

Previously post oauth_revoke_path spec was passing, returning 200 with an empty response. I tried changing to post oauth_revoke_path, headers: { 'Authorization': "Bearer #{access_token.token}" } but this does not pass either (this is basically what I was doing in the UI when "logging out").

Current upgrade PR: https://github.com/AndKiel/tournaments-api/pull/8

PS allow_token_introspection false doesn't seem to prevent the route from appearing in annotations but that's separate issue.

[nbulaj]

@AndKiel does your token issued to public or private client? Do you use the same client for authorization that the token you wanna to revoke issued for?

This all covered by specs, would be great if you could provide a reproducable spec to fix the bug (if it's a bug)

[AndKiel]

@nbulaj, my project is public repository so I think it can be treated as reproducible? (you can see Travis running spec on the PR I linked) I'm using password grant flow, nothing more, there are no clients. It works fine on 5.2 and 5.3. The 5.4 version breaks token revocation for me.

[nbulaj]

Even password grant flow requires auth to execute a request. And if token was issued to some client (has application_id filled) - it could be revoked only by the same client.

If I have the time - I'll take a look at your repo, but it could take long time

Also did you try to perform a request using HTTP-Authorization: Basic client_id:client_secret header?

[AndKiel]

/token/info has "application":{"uid":null} in my case when I authenticate.

No, I haven't tried Basic client_id:client_secret. I have no such thing like client. Unless I'm misunderstanding something. I'm using doorkeeper with simple email + password authentication for users, nothing more.

[nbulaj]

Please take a look at section 2.1. Revocation Request of the RFC7009 and then section 2.3. Client Authentication of RFC6749.

Before the changes Doorkeeper had a bug (as well as a security issue) and allowed to revoke token without any authentication. That was fixed according to OAuth RFC. You can find specs for changes here https://github.com/doorkeeper-gem/doorkeeper/pull/1370/files to find use cases for public/private clients and authentication.

[nbulaj]

@AndKiel if you didn't use (aka create) any client (Doorkeeper Application) instances before - you may need to create a new one (let it be non-confidential) and use it's credentials to authorize the request. Your mobile app or whatever could store this pair (client ID and client secret) to revoke the tokens

[AndKiel]

Okay, I've taken a look.

RFC7009#2.1 says that token is required and authentication must be included as stated in RFC6749#2.3.

RFC7009#2.1 also provides an example that is considered valid:

     POST /revoke HTTP/1.1
     Host: server.example.com
     Content-Type: application/x-www-form-urlencoded
     Authorization: Basic czZCaGRSa3F0MzpnWDFmQmF0M2JW

     token=45ghiukldjahdnhzdauz&token_type_hint=refresh_token

So Basic Authorization header would be the client_id + client_secret if I understand correctly.

But there's RFC6749#2.3. First paragraph - only for confidential clients. Second paragraph - continuation of that. Third paragraph:

The authorization server MAY establish a client authentication method with public clients.

May. Doesn't have to. So shouldn't this have worked when testing:

post oauth_revoke_path, 
     params: { token: access_token.token }

It has token which is required. That token has no application (client). Wouldn't lack of application impact the flow somehow? Before the changes in latest version, token was derived from headers: { 'Authorization': "Bearer #{access_token.token}" } implicitly somehow.

I feel like I'm still getting this wrong or missing some crucial part here.

[nbulaj]

Public client means it doesn't have s secret, just ID. Public client means that client exists and it could be identified just by ID. Please check also section 5. Security Considerations of RFC7009:

A malicious client may attempt to guess valid tokens on this endpoint by making revocation requests against potential token strings. According to this specification, a client's request must contain a valid client_id, in the case of a public client, or valid client credentials, in the case of a confidential client. The token being revoked must also belong to the requesting client.

Sorry, but I wouldn't support this "May. Doesn't have to" for the security reasons. Nothing actually stops you from creating a public client and using it's credentials to authorize the request.

Wouldn't lack of application impact the flow somehow?

Nope. Lack of authentication just open a security hole for brute-forcing (throtling) any tokens you have.

[edmondchui]

Hi @nbulaj,

Did you do this from your mobile app?

Yes. My mobile app issues a logout request to my app server (the confidential client). My app server then calls the authorization server TokenController#revoke to revoke the access token (should I do refresh token as well?) previously granted by the authorization server.

Before version 5.4.0, it works successfully as the authorization server TokenController#revoke only uses the authorized? method, which considers my mobile app a public client. Is that public client assumption correct?

One more observation: The token granted by the authorization server to the mobile app via the Resource Owner Password Credentials Flow does not have the application_id set in the database. Am I missing something here?

The client also includes its authentication credentials as described in Section 2.3. of [RFC6749].

server.client which you mentioned just stores client instance that performs a request. It is taken from the credentials used to authorize the request. And if it's blank - aka no auth passed or wrong credentials used - request is blocked as stated in the the RFC.

You need to call POST /token/revoke with ?token= value of access token you wanna to revoke and include authorization headers (HTTP-Authrozation: Basic Base64(client_id:client_secret)) to inform the server that this request is authorized to perform an action. Alternatively you could send client_id and client_secret via body params, but this method isn't recommended by the RFC. Don't forget that if the token you wanna to revoke was issued to some specific client - only this client could revoke the token (it's credentials must be used to authorize the request).

Do you mean to use the client_id and client_secret issued by the authorization server to my app server?

Thanks, Edmond

[nbulaj]

Before version 5.4.0, it works successfully as the authorization server TokenController#revoke only uses the authorized? method

Nope @edmondchui, it's just because Doorkeeper < 5.4 had a bug and security issue and didn't conform the RFC. Authorization server didn't perform any check except if token belongs to a client that requests the revocation.

Do you mean to use the client_id and client_secret issued by the authorization server to my app server?

Exactly. You need to add a new header Authorization: Basic Base64(client_id:client_secret) to your revocation request (or at least provide them in request body but this is not recommended by the RFC). That way Doorkeeper will see who is interacting with the server and if the token was issued to this client (in case access token has application_id filled).

Here - https://github.com/doorkeeper-gem/doorkeeper/commit/d194bc40e335464e67bc36dfd7e2e61247547545 - you could find specs at the end of the PR which have all the test cases for authorizing the request

[edmondchui]

Do you mean to use the client_id and client_secret issued by the authorization server to my app server?

Exactly. You need to add a new header Authorization: Basic Base64(client_id:client_secret) to your revocation request (or at least provide them in request body but this is not recommended by the RFC). That way Doorkeeper will see who is interacting with the server and if the token was issued to this client (in case access token has application_id filled).

I found out that the token granted by the authorization server to the mobile app via the Resource Owner Password Credentials Flow does not have the application_id set in the database. Is that a bug? If not, how to work around the issue?

[nbulaj]

There are no issue with Password flow, it just allows to authenticate without passing client credentials. If the access token doesn't have application_id - it's OK, it means that any application could revoke it (because it wasn't issued by some specific app).

But to revoke the token you need to authorize. Authorization server requires authentication in order to protect your from bruteforcing your tokens. Without protection I could just revoke all the tokens you have on the server.

As I already answered above for AndKiel, you could just create a new Doorkeeper Application instance and use it's credentials in the revocation request from your mobile/web application. All you need is:

1) Some application registered on authorization server (where Doorkeeper installed) - it's ID and secret 2) New header for revocation request that will include this headers

[nbulaj]

Also it's a question for me how both of you could obtain an access token using password grant flow without requiring client_id (for public) or client_id + client_secret (for private) :thinking: I need to check if Doorkeeper has some bug here...

I remember is was here: https://github.com/doorkeeper-gem/doorkeeper/issues/1347

UPD: ok the culprit is https://github.com/doorkeeper-gem/doorkeeper/commit/ea66661ac405192689e5151105a5f8f53e1743af from 2013 :fearful:

I'll check if it conforms with https://tools.ietf.org/html/rfc6749#section-4.3

[nbulaj]

Oh guys, I think I have bad news for you (and everyone else).

I think we must fix this too and break a lot of clients that creates tokens without any client authorization :( I foresee much more such issues in a near future...

If the client type is confidential or the client was issued client credentials (or assigned other authentication requirements), the client MUST authenticate with the authorization server as described in Section 3.2.1.

We have confidential column that indicates if application is public/private and always generate an ID (and secret in case of private client). So we must always require authorization for this grant flow

[AndKiel]

You're saying that all those years I was using doorkeeper with password grant flow and without creating an application... was a bug. That bug is probably the reason a lot of people love doorkeeper. Simplicity and literally zero setup to have email+password authentication in any project. :D

[nbulaj]

Yep, it's sad, but true :(

I don't sure why it's so hard to create one record and use it's credentials in requests... Anyway I need to conform the RFC. So I must fix this. I'll try to do it in a backward compatible way with first just a deprecation message and then with a totally removed credentials-free behavior.

More examples:

• https://auth0.com/docs/api-auth/tutorials/password-grant#ask-for-a-token
• https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-oauth-ropc
• https://docs.apigee.com/api-platform/security/oauth/implementing-password-grant-type
• https://help.salesforce.com/articleView?id=remoteaccess_oauth_username_password_flow.htm&type=5
• https://docs.oracle.com/cd/E39820_01/doc.11121/gateway_docs/content/oauth_flows.html

[edmondchui]

Thanks for digging into the root cause, @nbulaj.

Your proposed fix solves my mystery about the inconsistency.

For the record, the confidential column was introduced in https://github.com/doorkeeper-gem/doorkeeper/issues/891, which is doorkeeper version 4.4.0.

[AndKiel]

I don't sure why it's so hard to create one record and use it's credentials in requests...

It's not exactly about it being hard. It's pointless when it's for a front-end app that's all in a browser. Right now I can have two dockerized projects that require running docker-compose up and everything works out of the box. UI communicates with the API and everything's great. I don't have to go into oauth applications endpoint, then create an app, then update env var so the UI knows client_id (or I don't have to provide client_id at build time so compiled code has it) and so on.

[nbulaj]

I don't have to go into oauth applications endpoint, then create an app, then update env var so the UI knows client_id (or I don't have to provide client_id at build time so compiled code has it) and so on.

You could just use seeds for this:

Doorkeeper::Application.find_or_create!(name: "freontend", uid "set your long own value", secret: "the same", ...)

And use this credentials in the FE application. All you need is to also to run seeds (rails db:seed) after your database is up. Nothing special btw

[AndKiel]

I know that seeding kinda solves the problem but IMHO enforcing application makes little sense here. It provides no additional security in this particular use-case because front-end code is fully accessible in the browser so neither client_id nor client_secret would actually be secret.

[nbulaj]

Yep, and this is one of the reasons (except sharing user credentials) why this grant flow doesn't recommended for use by the RFC (and even more - forbidden in latest Security considerations document), and could be used only by highly trusted applications.

Btw whether we want it or not - we have a document that specifies the behavior and we need to follow it. Everybody who don't wanna to do it could path Doorkeeper internals and use any desired behavior :)

[AndKiel]

Personally, I'd rather have an option to disable this client/application requirement in the configuration so Doorkeeper still would be usable as is with such front-end apps. With it being required by default, of course, and documenting how bad disabling it is. Such front-end apps for public-facing APIs are valid real-life use-case.

[nbulaj]

Yep, this is one of the options I think about. I just don't like to bloat the configuration with a lot of options, especially when there is no need in them.

So I think I'll add such option, but with deprecation and it will be removed in some major update.

[nbulaj]

@edmondchui can we close this issue now? Do you need more help with it?

[edmondchui]

Since there is nothing much I can do except to downgrade to version 5.3.3, I guess you can make this a known issue on the release notes before closing this. That could save other's time to research on this. Thanks.

[nbulaj]

I guess you can make this a known issue on the release notes before closing this. That could save other's time to research on this.

It's not an issue actually, and it's a known breaking change:

• It's pointed in upgrade guides - https://github.com/doorkeeper-gem/doorkeeper/wiki/Migration-from-old-versions#from-5x-to-54x (see API changes)
• It's pointed in the CHANGELOG - https://github.com/doorkeeper-gem/doorkeeper/blob/master/CHANGELOG.md#540rc1 (see #1370)

Known (at least now) issue is that you guys could use ROPC grant without using a client which would be fixed soon, yep.

I think I can close it for now, just need to remember issue number for the same threads in the near future.

[jpaas]

So if I am interpreting the current status of things correctly. I can request tokens without client credentials. These tokens will have no Application client association.

But when I revoke a token it will fail without a client id. I can't image that a revoke request with a client id is able to revoke a token with no associated client. So how would anyone ever revoke a clientless token?

[ghiculescu]

FYI for anyone interested in this - https://github.com/doorkeeper-gem/doorkeeper/pull/1458