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

OAuthgrant filter does not work with GET /userinfo as the tokeninfo URL #3226

Open
czhou-brex opened this issue Sep 11, 2024 · 3 comments
Open

Comments

@czhou-brex
Copy link
Contributor

czhou-brex commented Sep 11, 2024

Describe the bug
The OAuthgrant filter requires a tokeninfo URL which is only called by a GET request. However some IDPs like Okta have already deprecated this call. Specifically it looks for a uid field and it it fails, it will do a redirect. This leads to a redirect loop.

Using the GET /userinfo does not resolve this issue, as in Okta's case specifically the uid field does not exist in GET /userinfo. https://developer.okta.com/docs/api/openapi/okta-oauth/oauth/tag/OrgAS/#tag/OrgAS/operation/userinfo

To Reproduce

skipper \
  -routes-file oauth.eskip \
  -enable-oauth2-grant-flow \
  -oauth2-auth-url https://myapp/oauth2/default/v1/authorize \
  -oauth2-token-url https://myapp/oauth2/default/v1/token \
  -oauth2-tokeninfo-url https://myapp/oauth2/default/v1/userinfo \
  -oauth2-secret-file=skipper-secrets \
  -oauth2-client-id=client-id \
  -oauth2-client-secret=client-secret \
  -oauth2-callback-path=/oauth/callback \
  -oauth2-auth-url-parameters=scope="openid profile offline_access" \
  -application-log-level DEBUG

oauth.eskip file
dashboard: * -> oauthGrant() -> inlineContent("It works!") -> <shunt>;

Expected behavior

  • should successfully complete the oauth2 login

Observed behavior

  • infinite loop caused by missing "uid" from calling GET /tokeninfo
    Failed to create token container: tokeninfo subject key 'uid' is missing.

I think it should not rely on the uid field and work with GET /userinfo like the oidc filter.

@szuecs szuecs added the bug label Sep 12, 2024
@AlexanderYastrebov
Copy link
Member

Hello.

There is a -oauth2-tokeninfo-subject-key flag:

flag.StringVar(&cfg.Oauth2TokeninfoSubjectKey, "oauth2-tokeninfo-subject-key", "uid", "sets the tokeninfo subject key")

Could you please try -oauth2-tokeninfo-subject-key=sub (or nickname or email) and see if it helps?

@AlexanderYastrebov
Copy link
Member

AlexanderYastrebov commented Sep 12, 2024

I also think this

tokeninfo["sub"] = subject

should be set only when TokeninfoSubjectKey is not empty
if f.config.TokeninfoSubjectKey != "" {
if s, ok := tokeninfo[f.config.TokeninfoSubjectKey].(string); ok {
subject = s
} else {
return fmt.Errorf("tokeninfo subject key '%s' is missing", f.config.TokeninfoSubjectKey)
}
}

such that existing sub claim is not overwritten when -oauth2-tokeninfo-subject-key=''

@czhou-brex
Copy link
Contributor Author

Hello.

There is a -oauth2-tokeninfo-subject-key flag:

flag.StringVar(&cfg.Oauth2TokeninfoSubjectKey, "oauth2-tokeninfo-subject-key", "uid", "sets the tokeninfo subject key")

Could you please try -oauth2-tokeninfo-subject-key=sub (or nickname or email) and see if it helps?

Yes, this worked with the Okta GET /userinfo endpoint.

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

No branches or pull requests

3 participants