zalando / skipper

An HTTP router and reverse proxy for service composition, including use cases like Kubernetes Ingress
https://opensource.zalando.com/skipper/
Other
3.11k stars 351 forks source link

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

Open czhou-brex opened 1 month ago

czhou-brex commented 1 month ago

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

Observed behavior

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

AlexanderYastrebov commented 1 month ago

Hello.

There is a -oauth2-tokeninfo-subject-key flag: https://github.com/zalando/skipper/blob/bd87e31f76fbbf400aaeb4f7a345e277df8f0028/config/config.go#L497

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

AlexanderYastrebov commented 1 month ago

I also think this https://github.com/zalando/skipper/blob/bd87e31f76fbbf400aaeb4f7a345e277df8f0028/filters/auth/grant.go#L166 should be set only when TokeninfoSubjectKey is not empty https://github.com/zalando/skipper/blob/bd87e31f76fbbf400aaeb4f7a345e277df8f0028/filters/auth/grant.go#L158-L164 such that existing sub claim is not overwritten when -oauth2-tokeninfo-subject-key=''

czhou-brex commented 1 month ago

Hello.

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

https://github.com/zalando/skipper/blob/bd87e31f76fbbf400aaeb4f7a345e277df8f0028/config/config.go#L497

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.