msuliq / omniauth_oidc

Omniauth strategy to authenticate and retrieve user data using OpenID Connect (OIDC)
MIT License
4 stars 1 forks source link

Give precedence to config provided in initializer over config endpoint #6

Closed Augustin-Grenne closed 1 week ago

Augustin-Grenne commented 1 month ago

Having a config endpoint you can provide is nice, but one would expect settings provided (also) in the initializer to take precedence over those fetched from the config endpoint.

Describe the solution you'd like Any config provided in the client_options in the initializer should supplement or substitute the config from the fetched from the config_endpoint in that same client_options.

Additional Context More specifically My ACM provider requires a redirect_uri to be present on the authorization request. The redirect_uri isn't present on the config endpoint and should match a predefined value (agreed upon in an integration document).

Potential Implementation Potentially I don't see the full picture, but based on my current understanding of the strategy, something like this might work. (untested)

# Config is build from the json response from the OIDC config endpoint
def config
  [...]
  @config ||= OpenidConfigParser.fetch_openid_configuration(client_options.config_endpoint).merge(client_options)
end

This would technically make the config_endpoint optional and would allow you to remove the raising of the error, in the config method.

Additionally I noticed in the strategy (lines 190-192), the redirect_uri method, which seemingly isn't used, but could be a good fallback if it isn't provided via the initializer or config_endpoint.

def redirect_uri
  "#{request.base_url}/auth/#{name}/callback"
end

Extra Maybe the following strategy could serve as an inspiration.

msuliq commented 1 month ago

@Augustin-Grenne thank you for the insightful comment. Not long ago I had to spend a lot of time implementing OIDC authentication in a Rails application, and I had tried the strategy provided in your comment as one of the many possible solutions. But to my regret many libraries were either too complex with little documentation or did not fully works and seemed abandoned.

As you mentioned, any config provided in the client_options in the omniauth initializer does override the one fetched from config_endpoint. But it's a different case for redirect_uri.

From the examples in the readme you might notice that the gem does not accept any configuration for the redirect_uri. Instead it uses encapsulated non-configurable redirect_uri that follows the format of https://your_app.com/auth/<provider name defined in the omniauth initializer>/callback. This is happening because rack-oauth2 is configured to catch response of your OIDC provider before it hits your client application, and initiate subsequent steps of the authentication flow (code request, token exchange, etc.).

Let's assume provider has name: :acm_provider defined in omniauth.rb, so for your ACM provider a redirect_uri of https://your_app.com/auth/acm_provider/callbackwill provided in the authorization request. Sameredirect_urihas to be registered with your ACM provider as permittedredirect_uri`.

Since this redirect_uri value is caught by rack-oauth2 without it reaching your app, you need to use another method to process the ACM provider's final response with user data. For example I will assume that this method is located in callbacks_controller#acm_omniauth. So to receive the user data from ACM provider and then process it within the acm_omniauth method you will need to include the following in your routes.rb

# config/routes.rb
Rails.application.routes.draw do
  match 'auth/acm_provider/callback', via: :get, to: "callbacks#acm_omniauth"
end
Augustin-Grenne commented 3 weeks ago

Indeed many libraries are abandoned or are needlessly complex and provide little to no documentation. Sorry if I jumped to the wrong conclusion based on my flawed or incomplete understanding of your lib. In fact my issue is with the redirect_uri only. While https://my_app.com/auth/<provider_name>/callback is certainly a sensible default, we use a path prefix of 'voters' and thus it does not match our callback. More specifically, our ACM provider requires the redirect_uri to be present on the auth request (even if it isn't required by the OAuth specification). It does this as an extra security measure and will throw an error ("Missing or incorrect redirect uri", with status: 401) if the redirect_uri in the auth request does not match the 'registered permitted redirect uri' or is empty. Because our redirect_uri should be https://my_app.com/voters/auth/<provider_name>/callback, and not: https://my_app.com/auth/<provider_name>/callback we now see this 'auth error' from our ACM provider.

Being able to provide the specific full redirect_uri in the omniauth.rb config file and have it been used on all request would be a great advantage, and at least in our case necessary, for a successful OAuth flow.

This library with very little documentation, and possibly a lot of complexity, seems to do that with the redirect_uri: omniauth_openid_connect

msuliq commented 3 weeks ago

Hi @Augustin-Grenne , glad you are back :) The omniauth_openid_connect indeed implements a configurable redirect_uri but based on personal experience it did not work as expected when it comes to user authentication with OpenID Connect. I will look into what can be done about your case with the registered redirect_uri.

msuliq commented 1 week ago

Hi @Augustin-Grenne after weeks of trial and error, unfortunately I have not found any way to bypass this and allow prefixed custom redirect_uri to be passed with the initial request to the OIDC provider. This is a limitation of omniauth gem described in an issue here.