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

Supporting multiple profiles for the same provider? #52

Open
lukaszkorecki opened this issue Sep 21, 2023 · 5 comments
Open

Supporting multiple profiles for the same provider? #52

lukaszkorecki opened this issue Sep 21, 2023 · 5 comments

Comments

@lukaszkorecki
Copy link

This might be somewhat an edge case, although I dealt with this in the past when working with Ruby's Omniauth where this issue was addressed IIRC.

Here's my setup:

I have two profiles for Google OAuth:

{:google-sso {:authorize-uri "https://accounts.google.com/o/oauth2/v2/auth?access_type=offline&prompt=select_account"
              :access-token-uri "https://www.googleapis.com/oauth2/v4/token"
              :client-id "test"
              :client-secret "test"
              :redirect-uri "/api/auth/google/callback"
              :landing-uri "/api/auth/google/finalize"

              ;; specific for SSO
              :launch-uri "/api/auth/google/auth"
              :scopes ["openid" "email" "profile"]}

 :google-calendar {:authorize-uri "https://accounts.google.com/o/oauth2/v2/auth?access_type=offline&prompt=select_account&include_granted_scopes=true"
                   :access-token-uri "https://www.googleapis.com/oauth2/v4/token"
                   :client-id "test"
                   :client-secret "test"
                   :redirect-uri "/api/auth/google/callback"
                   :landing-uri "/api/auth/google/finalize"

                   ;; for connecting Google Calendar
                   :launch-uri "/api/auth/google/auth"
                   :scopes ["https://www.googleapis.com/auth/calendar"]}}

Note that :redirect-uri are the same, but scopes, :authorize-uri and :launch-uri are different.

Why this setup? Because Google encourages staged authorization to different resources - as in, if all you need Google access for is signing-in, don't request calendar access as well - not every user will need it. It's something I dealt with in the past with multiple providers, not only Google but also Slack, Qualtrics and probably more that I'm forgetting.

The issue is - because of how the middleware detects the profile during redirect stage:

redirects (into {} (map (juxt parse-redirect-url identity)) profiles)]

a couple of things happen:

  • it will sometimes pick up the wrong profile (depending on ordering)
  • might pass wrong scopes
  • even if the authentication succeeds, the key to get the access token might be also wrong (e.g. I started with :google-sso profile, but access tokens end up in {:oauth2/access-tokens {:google-calendar ... }}

The workaround might look trivial: configure Google's OAuth screen to accept unique redirect URLs. That might work if there's only two, but in realistic scenarios there's at least six (for each environment) and there will be more if/when my application needs to support different regions. And no, Google verifies the whole URL, so appending query params won't work.
To make this even more difficult - any change to the OAuth screen requires going through Google's verification process, which in my experience can be anything from 2hrs to 2 months 😢

On top of that: some API providers support a fixed number of redirect urls (or just one, and you're forced to create separate workspaces/accounts/etc for different environments).

I have a fork which fixes this issue and while the code works, it's a bit hairy - you can see it here: https://github.com/weavejester/ring-oauth2/compare/master...lukaszkorecki:tmp/figure-out-profile-confusion?expand=1

It's all working as expected and all tests are passing, but:

  • the formatting is off
  • I need to come up with a unit test that reproduces this issue and verifies the fix

I'll be more than happy to take this to the finish line, but I'm wondering if there's another way to deal with this.

Thank you

@weavejester
Copy link
Owner

So because Google and other providers may not support an arbitrary number of redirect URLs, instead your solution is to set a session variable and use that to determine the profile being used? I think that's fine, but I also think it should be an option that's off by default. Something like :session-chooses-profile?. What do you think?

@lukaszkorecki
Copy link
Author

I think that could work - let me play this back, just so I understand your approach:

Since launch-uris have to be unique per-profile (as per the documentation), with that option we could set the session variable if the new option is active, and use that to find the right profile during the redirect phase. If the session var is missing, we fallback to the original approach of using the redirects map.

Something like that?
If that's the case - where does this option go? Currently wrap-oauth2 accepts the handler and profile map.

@weavejester
Copy link
Owner

Yes, I think that would work. I think the option would go in the profile, as you might not want to use the session approach for some providers. The session has the disadvantage of not being "thread safe", so you'd only want to use it where there's no other reasonable option.

@lukaszkorecki
Copy link
Author

Sounds good - I'll make changes in my fork and open a proper PR 👍

Thinking out loud though - when the authorization process starts, we know exactly which profile started it for the current user, so carrying over the profile ID should be safe, just like holding on to the state parameter is.
While yes, it's possible that the same person starts two auth flows at the same time and things will go south, but that's more of a user error based on my experience.

@weavejester
Copy link
Owner

Yeah, it's unlikely to occur, but I don't believe it's technically incorrect for two OAuth2 flows to be active for the same user at the same time. It's safer, if only incrementally, to use the redirect URL to determine the profile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants