search

doorkeeper-gem / doorkeeper-openid_connect

OpenID Connect extension for Doorkeeper
MIT License
175 stars 117 forks source link

Fixes incorrect strategy registration for "id_token" and "id_token token" flows #143

Closed paukul closed 3 years ago

paukul commented 3 years ago

Issue: the response_type_strategy was incorrectly set to Doorkeeper::OpenidConnect::IdToken instead of Doorkeeper::Request::IdToken. This only worked by accident because Doorkeeper < 5.5 builds the strategy by constantizing the request type so "token" becomes "Doorkeeper::Request::Token" even though Doorkeeper::OpenidConnect::IdToken has been (incorrectly) registered.

However, besides from the registration only working incidentally, this will break with Doorkeeper versions from 5.5 upwards.

Compare the 5.4 implementation with 5.5: 5.4: https://github.com/doorkeeper-gem/doorkeeper/blob/v5.4.0/lib/doorkeeper/request.rb#L30 5.5: https://github.com/doorkeeper-gem/doorkeeper/blob/v5.5.0/lib/doorkeeper/request.rb#L7

As soon as https://github.com/doorkeeper-gem/doorkeeper-openid_connect/pull/138 is merged the current implementation will cause issues.

An example for issues that will appear is that resource_owner_from_access_token will get passed the server (or Doorkeeper.configuration) object instead of an access token object for "id_token" and "id_token token" response types

toupeira commented 3 years ago

@paukul thanks for catching this! :heart:

We got some conflicts because https://github.com/doorkeeper-gem/doorkeeper-openid_connect/pull/138 changed the indentation, but I'll go ahead and cherry-pick this into a new PR :+1: