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

400 invalid scope for google oidc #3005

Closed
creeram opened this issue Nov 27, 2024 Discussed in #3004 · 10 comments · Fixed by #3117
Closed

400 invalid scope for google oidc #3005

creeram opened this issue Nov 27, 2024 Discussed in #3004 · 10 comments · Fixed by #3117

Comments

@creeram
Copy link

creeram commented Nov 27, 2024

Discussed in #3004

Originally posted by creeram November 27, 2024
I'm using google OIDC config but while trying to login using the sso it says.

Some requested scopes were invalid. {valid=[openid, https://www.googleapis.com/auth/userinfo.profile, https://www.googleapis.com/auth/userinfo.email], invalid=[offline_access]} 
invalid=[offline_access]

values.yaml file

I have disabled the because we have the default SSL certificate configured for ingress nginx.

global:
  nodeSelector: {}


api:
  enabled: true
  replicas: 1
  host: kargo.example.com
  tls:
    enabled: false
  ingress:
    enabled: true
    ingressClassName: nginx
    tls:
      enabled: false

  adminAccount:
    enabled: false
  oidc:
    enabled: true
    issuerURL: https://accounts.google.com
    clientID: *********************************.apps.googleusercontent.com
    additionalScopes: []

    admins:
      claims:
         email:
         - hello@example.com
       
</div>
@krancour
Copy link
Member

Could you try logging in using the CLI instead of the UI please? I think I've spotted a bug in how the UI puts together the list of scopes it requests and the CLI should not have the same issue. If you can confirm my suspicion by successfully logging in with the CLI, this is probably an easy fix.

However, you may run into another issue afterwards because, to the best of my recollection, Google Identity Platform doesn't support PKCE without a client secret (which, last I new was a frequent complaint about Google Identity Platform since the whole point of PKCE is for use by clients, such as a mobile app or web SPA, which cannot be trusted to keep secrets).

If you do encounter that next, the solution is to simply use Dex as a middle-man, which is very easy.

If we can at least get you past this offline_access issue, then we'll have at least uncovered an easily fixable bug.

@krancour
Copy link
Member

krancour commented Dec 4, 2024

@Marvin9 this line looks suspicious to me:

...(oidcConfig.scopes.includes('offline_access') ? [] : ['offline_access'])

It looks as if we're unconditionally asking for offline_access.

Compare to what the CLI does, which is checking if the IDP supports that scope before deciding to request it:

// If the provider supports the "offline_access" scope, request it so that
// we can get a refresh token.
if slices.Contains(providerClaims.ScopesSupported, offlineAccessScope) {
scopes = append(scopes, offlineAccessScope)
}

I could tentatively move this decision to the back end so the back end determines if the IDP supports that scope and conditionally adds it to the list of scopes that the client should request, but my original thinking on this had been that it seemed more appropriate for the client to decide whether it wanted a refresh token or not.

If you can at least confirm that my reading of the UI code is correct, you and I can figure out which approach we want to take to solving.

@Marvin9
Copy link
Contributor

Marvin9 commented Dec 4, 2024

You are correct @krancour. UI will force append even if its not supported from IDP. We already might have some info from .well-known endpoint in UI and most probably that would help in this case. If I am reading this correct then this would remove refresh token mechanism due to IDP limitation?

@Marvin9
Copy link
Contributor

Marvin9 commented Dec 4, 2024

@krancour I don't know if you have already figured it out but this is not going to work without Dex. Plain OIDC config is going to complain client_secret missing on token request, I have verified it.

Screenshot 2024-12-04 at 1 41 08 PM (2)
Screenshot 2024-12-04 at 1 40 25 PM (2)

https://stackoverflow.com/a/63275535

@krancour
Copy link
Member

krancour commented Dec 4, 2024

We already might have some info from .well-known endpoint in UI and most probably that would help in this case. If I am reading this correct then this would remove refresh token mechanism due to IDP limitation?

Yes. The response from the .well-known endpoint should have "provider claims" and one of those should be a list of valid scopes that the IDP supports. We should only ask for the offline_access scope if we find it in that list.

this is not going to work without Dex

This is referring to PKCE? Yep. That was my guess.

to the best of my recollection, Google Identity Platform doesn't support PKCE without a client secret

It is a mystery to me why they don't support this yet. 🤷‍♂️

@krancour
Copy link
Member

krancour commented Dec 4, 2024

Here's a workaround for the client secret issue that doesn't involve Dex and worked for at least two people, but I haven't verified it myself:

https://stackoverflow.com/a/78414957/80901

@Marvin9
Copy link
Contributor

Marvin9 commented Dec 4, 2024

This actually worked @krancour 🤯. Even with offline_access scope Never mind, it was through dex.

Update: it worked with plain OIDC configuration and without offline_access.

@Marvin9 Marvin9 assigned Marvin9 and unassigned krancour Dec 4, 2024
@krancour
Copy link
Member

krancour commented Dec 5, 2024

That was using the SO workaround?

@Marvin9
Copy link
Contributor

Marvin9 commented Dec 5, 2024

Yes workaround. If we create client for UWP type

@krancour
Copy link
Member

krancour commented Dec 5, 2024

Good to know it worked. Thanks!

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

Successfully merging a pull request may close this issue.

3 participants