Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client_options.redirect_uri should fallback to sensible default when missing #77

Closed
maxbes opened this issue Jan 19, 2021 · 1 comment
Closed
Labels

Comments

@maxbes
Copy link

maxbes commented Jan 19, 2021

Context: I am setting up Greenlight for use with OpenIDConnect through your library

The initialization happens this way:

      provider :openid_connect,
        issuer: ENV["OPENID_CONNECT_ISSUER"],
        discovery: true,
        scope: [:email, :profile],
        response_type: :code,
        uid_field: ENV["OPENID_CONNECT_UID_FIELD"] || "preferred_username",
        client_options: {
          identifier: ENV['OPENID_CONNECT_CLIENT_ID'],
          secret: ENV['OPENID_CONNECT_CLIENT_SECRET'],
          redirect_uri: redirect
        },

Unless explicitly configured by the user, redirect will have a nil value, which omniauth_openid_connect will handle by constructing the following authorization uri:

http://my.provider/authorize?client_id=1234&nonce=8225bfbd740e8909a54ff97f536c3c18&response_type=code&scope=email%20profile%20openid&state=8f8c4887392f8bcbdcdc195704e53f44

redirect_uri is missing, which is not valid per the OIDC spec

Other Omniauth providers I have looked at use Omniauth's callback_url method to set the redirect_uri to a sensible default value when the user does not explicitely specify it. This makes configuration of OpenID Connect a little easier, by relieving the users from having to try and guess the correct callback URL when configuring an application that uses Omniauth.

I have tried the following change:

diff --git a/lib/omniauth/strategies/openid_connect.rb b/lib/omniauth/strategies/openid_connect.rb
index c7e86f7..ec37c7f 100644
--- a/lib/omniauth/strategies/openid_connect.rb
+++ b/lib/omniauth/strategies/openid_connect.rb
@@ -156,7 +156,7 @@ module OmniAuth
       end
 
       def authorize_uri
-        client.redirect_uri = redirect_uri
+        client.redirect_uri = redirect_uri || callback_url
         opts = {
           response_type: options.response_type,
           response_mode: options.response_mode,

which works for my use case, but breaks the unit tests in ways I cannot understand as someone not very familiar with Ruby

@stale
Copy link

stale bot commented Mar 20, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Mar 20, 2021
@stale stale bot closed this as completed Mar 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant