openstreetmap / openstreetmap-website

The Rails application that powers OpenStreetMap
https://www.openstreetmap.org/
GNU General Public License v2.0
2.2k stars 910 forks source link

oauth_filter intercepts OAuth2.0 endpoints meant for Doorkeeper #3245

Closed mmd-osm closed 3 years ago

mmd-osm commented 3 years ago

(Originally posted in #2145)

Now with OAuth 2.0 in production, I wanted to revisit the token introspection topic, which is handled by the /oauth2/introspect endpoint.

While the Doorkeeper config supports this endpoint, it seems the oauth_filter gem is intercepting the call. It redirects is to the Oauth2Token model, which is not related to Doorkeeper, and then fails with a 415 Unsupported Media Type error.

I'm not at all familiar with any oauth_filter details. All I could find was the query below over at: https://github.com/pelle/oauth-plugin/blob/master/lib/oauth/rack/oauth_filter.rb#L27

Does it still make sense to have this code in place for OAuth2.0, now that we're using Doorkeeper?

Started POST "/oauth2/introspect" for ::1 at 2021-07-02 12:12:20 +0200
  Oauth2Token Load (0.9ms)  SELECT "oauth_tokens".* FROM "oauth_tokens" WHERE "oauth_tokens"."type" = $1 AND (invalidated_at IS NULL and authorized_at IS NOT NULL and token = 'xyz') ORDER BY "oauth_tokens"."id" ASC LIMIT $2  [["type", "Oauth2Token"], ["LIMIT", 1]]
  ↳ config/initializers/compressed_requests.rb:27:in `call'
Processing by Doorkeeper::TokensController#introspect as JSON
  Parameters: {"token"=>"xyz"}
Filter chain halted as :enforce_content_type rendered or redirected
Completed 415 Unsupported Media Type in 0ms (Views: 0.1ms | ActiveRecord: 0.0ms | Allocations: 87)
mmd-osm commented 3 years ago

Use case would be to support steps 8 - 11 in the following diagram, where Authorization Server = OSM Website, and Resource Server = Overpass API (or some other external application).

oauth2-authz (picture source: https://backstage.forgerock.com/docs/am/6/oauth2-guide/)

tomhughes commented 3 years ago

Do we really want to be vouching for third party sites though?

As to the technical issues, obviously the old oauth plugin shouldn't be stealing OAuth 2 stuff, but it's still used for OAuth 1 so we can't just get rid of it.

I'm surprised it is intercepting /oauth2 though as it should be using /oauth for it's endpoints. I will have to see what I can do.

tomhughes commented 3 years ago

Why do you think this is related to oauth_plugin exactly? This line:

Processing by Doorkeeper::TokensController#introspect as JSON

suggests it has reached the doorkeeper controller.

tomhughes commented 3 years ago

The 415 response is because you didn't send a content type of application/x-www-form-urlencoded with your form data. Once I fix that I get 401 Unauthorized.

mmd-osm commented 3 years ago

Thanks, I need to try this again with the correct content type. It might be that I misinterpreted the error message. I was under the impression that oauth_tokens is the only table that is being checked for Bearer tokens, and because our tokens are in another table, this check would always fail. But I would agree now, the trace hints towards Doorkeeper.

tomhughes commented 3 years ago

Yes there's a redundant check from the filter but once the content type is right it reads the correct table:

Started POST "/oauth2/introspect" for 2001:8b0:bd:1:fce3:75ff:febd:6a8c at 2021-07-02 15:51:08 +0100
   (1.1ms)  SELECT "schema_migrations"."version" FROM "schema_migrations" ORDER BY "schema_migrations"."version" ASC
  ↳ config/initializers/cors.rb:9:in `call'
  Oauth2Token Load (0.7ms)  SELECT "oauth_tokens".* FROM "oauth_tokens" WHERE "oauth_tokens"."type" = $1 AND (invalidated_at IS NULL and authorized_at IS NOT NULL and token = '05sMxyhosM-gISCObOgdfSq7urPxGMlaGqS3VoeYse8') ORDER BY "oauth_tokens"."id" ASC LIMIT $2  [["type", "Oauth2Token"], ["LIMIT", 1]]
  ↳ config/initializers/compressed_requests.rb:27:in `call'
Processing by Oauth2TokensController#introspect as */*
  Parameters: {"token"=>"05sMxyhosM-gISCObOgdfSq7urPxGMlaGqS3VoeYse8"}
  Doorkeeper::AccessToken Load (0.7ms)  SELECT "oauth_access_tokens".* FROM "oauth_access_tokens" WHERE "oauth_access_tokens"."token" = $1 LIMIT $2  [["token", "05sMxyhosM-gISCObOgdfSq7urPxGMlaGqS3VoeYse8"], ["LIMIT", 1]]
  ↳ app/controllers/api_controller.rb:64:in `current_ability'
  User Load (1.2ms)  SELECT "users".* FROM "users" WHERE "users"."id" = $1 LIMIT $2  [["id", 1], ["LIMIT", 1]]
  ↳ app/abilities/api_capability.rb:9:in `initialize'
  UserRole Load (0.7ms)  SELECT "user_roles".* FROM "user_roles" WHERE "user_roles"."user_id" = $1  [["user_id", 1]]
  ↳ app/models/user.rb:238:in `has_role?'
  CACHE Doorkeeper::AccessToken Load (0.0ms)  SELECT "oauth_access_tokens".* FROM "oauth_access_tokens" WHERE "oauth_access_tokens"."token" = $1 LIMIT $2  [["token", "05sMxyhosM-gISCObOgdfSq7urPxGMlaGqS3VoeYse8"], ["LIMIT", 1]]
  ↳ app/controllers/application_controller.rb:333:in `better_errors_allow_inline'
  CACHE Doorkeeper::AccessToken Load (0.0ms)  SELECT "oauth_access_tokens".* FROM "oauth_access_tokens" WHERE "oauth_access_tokens"."token" = $1 LIMIT $2  [["token", "05sMxyhosM-gISCObOgdfSq7urPxGMlaGqS3VoeYse8"], ["LIMIT", 1]]
  ↳ app/controllers/application_controller.rb:333:in `better_errors_allow_inline'
Completed 401 Unauthorized in 136ms (Views: 0.4ms | ActiveRecord: 20.4ms | Allocations: 88287)

Still working on hacking our cancancan config to authorize the requests...

mmd-osm commented 3 years ago

That's interesting. When I tested this endpoint last year in October, it happily accepted the following request:

curl -F client_id=zQyq4UbbrCMjShugI1BbYmJ_JQZKnDLj3iZjMVSEB8o -F client_secret=rTDU2cPJ284WL41yYIiPXqzvre2MXjovU3B4WX-zbN4 -F token=ASIKSMtZ67n2d7FaM5pYRQOLkNqZOfaYDQn-aB1OCCE -X POST http://localhost:3000/oauth2/introspect

Now with the current code in place, I need to use:

curl --location --request POST 'http://localhost:3000/oauth2/introspect' \
--header 'Authorization: Bearer xNxih4jQ2s9rAJ_IeHr3XhupQFkv4B3EwchxVEXhVvg' \
--data-urlencode 'token=xNxih4jQ2s9rAJ_IeHr3XhupQFkv4B3EwchxVEXhVvg'

(curl automatically figured out it needs to set Content-Type: application/x-www-form-urlencoded)

And, yes, I'm getting the same HTTP 401 error.

tomhughes commented 3 years ago

Right, so the problem I was having is that you can't introspect a token using bearer authorization with the same token - it's not allowed.

I think you can use bearer auth with a different token though I haven't proven that yet.

What definitely works is client authentication as you did before, for example:

% curl -X POST -d "client_id=dlpQ_u2s49xM0anHw6C7CwiB8m7WLIoaH4cUdOIqBPo&client_secret=fBG3ZtN18eetoSm2qs-6788gvybBJxoC8oDRbXwc3vQ&token=05sMxyhosM-gISCObOgdfSq7urPxGMlaGqS3VoeYse8" https://dev.osm.compton.nu/oauth2/introspect
{"active":true,"scope":"read_prefs","client_id":"dlpQ_u2s49xM0anHw6C7CwiB8m7WLIoaH4cUdOIqBPo","token_type":"Bearer","exp":0,"iat":1625235997}
mmd-osm commented 3 years ago

I think you can use bearer auth with a different token though I haven't proven that yet.

Yes, that seems to work. My local set up has a number of different users, and when I request a new access token in Postman for another user, and use that new token for bearer auth, I can request some details for another token.

The good thing is that the introspect endpoint doesn't include any details about the user id, which I believe was one of the reasons for Roland to create an GDPR compliant API key dispenser.

tomhughes commented 3 years ago

So I think we can close this then - it's all actually working if you use it right ;-)

mmd-osm commented 3 years ago

Yes, totally agree, and sorry to keep you busy with this. In the end, it was really the content type which made the difference. Bummer.

mmd-osm commented 3 years ago

So based on https://github.com/zmartzone/mod_oauth2 I'm now able to send requests to an Overpass instance, and the Bearer token would be validated on-the-fly using introspection. Also, successful lookups are cached in a local Redis (or shm, file, memcache) instance, to avoid excessive load on the Rails port. :+1:

At one point I thought it might be helpful to have the subject info in the introspection response, e.g. to single out excessive users of a service.

Depending on privacy requirements we could return the user id, or use a more sophisticated approach based on a hashed user id, client id, and some server secret/or salt, maybe.

From doorkeeper.rb:

   custom_introspection_response do |token, context|
     user = User.find(token.resource_owner_id)
     {
        "sub": ::Digest::SHA256.hexdigest(
                   user.id.to_s + token.application.try(:uid ) + SERVER_SECRET)
     }
   end

This is low prio at the moment, and more of a nice to have.