Improve error handling when upstream groups is invalid during refresh

This commit is contained in:
Ryan Richard 2022-01-19 12:57:47 -08:00
parent 78bdb1928a
commit a4ca44ca14
2 changed files with 65 additions and 2 deletions

View File

@ -169,7 +169,9 @@ func upstreamOIDCRefresh(ctx context.Context, session *psession.PinnipedSession,
// and let any old groups memberships in the session remain.
refreshedGroups, err := downstreamsession.GetGroupsFromUpstreamIDToken(p, mergedClaims)
if err != nil {
return err
return errorsx.WithStack(errUpstreamRefreshError.WithHintf(
"Upstream refresh error while extracting groups claim.").WithWrap(err).
WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType))
}
if refreshedGroups != nil {
session.Fosite.Claims.Extra[oidc.DownstreamGroupsClaim] = refreshedGroups

View File

@ -190,6 +190,13 @@ var (
}
`)
fositeUpstreamGroupClaimErrorBody = here.Doc(`
{
"error": "error",
"error_description": "Error during upstream refresh. Upstream refresh error while extracting groups claim."
}
`)
happyAuthRequest = &http.Request{
Form: url.Values{
"response_type": {"code"},
@ -1275,7 +1282,36 @@ func TestRefreshGrant(t *testing.T) {
},
},
{
name: "happy path refresh grant when the upstream refresh does not return new group memberships from the merged ID token and userinfo results, it keeps groups from initial login",
name: "happy path refresh grant when the upstream refresh returns new group memberships as an empty list from the merged ID token and userinfo results, it updates groups to be empty",
idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(
upstreamOIDCIdentityProviderBuilder().WithGroupsClaim("my-groups-claim").WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{
IDToken: &oidctypes.IDToken{
Claims: map[string]interface{}{
"sub": goodUpstreamSubject,
"my-groups-claim": []string{}, // refreshed groups claims is updated to be an empty list
},
},
}).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()),
authcodeExchange: authcodeExchangeInputs{
customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(),
modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") },
want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()),
},
refreshRequest: refreshRequestInputs{
want: tokenEndpointResponseExpectedValues{
wantStatus: http.StatusOK,
wantSuccessBodyFields: []string{"refresh_token", "access_token", "id_token", "token_type", "expires_in", "scope"},
wantRequestedScopes: []string{"openid", "offline_access"},
wantGrantedScopes: []string{"openid", "offline_access"},
wantGroups: []string{}, // the user no longer belongs to any groups
wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(),
wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens(), true),
wantCustomSessionDataStored: upstreamOIDCCustomSessionDataWithNewRefreshToken(oidcUpstreamRefreshedRefreshToken),
},
},
},
{
name: "happy path refresh grant when the upstream refresh does not return new group memberships from the merged ID token and userinfo results by omitting claim, it keeps groups from initial login",
idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(
upstreamOIDCIdentityProviderBuilder().WithGroupsClaim("my-groups-claim").WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{
IDToken: &oidctypes.IDToken{
@ -1303,6 +1339,31 @@ func TestRefreshGrant(t *testing.T) {
},
},
},
{
name: "error from refresh grant when the upstream refresh does not return new group memberships from the merged ID token and userinfo results by returning group claim with illegal nil value",
idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(
upstreamOIDCIdentityProviderBuilder().WithGroupsClaim("my-groups-claim").WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{
IDToken: &oidctypes.IDToken{
Claims: map[string]interface{}{
"sub": goodUpstreamSubject,
"my-groups-claim": nil,
},
},
}).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()),
authcodeExchange: authcodeExchangeInputs{
customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(),
modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") },
want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()),
},
refreshRequest: refreshRequestInputs{
want: tokenEndpointResponseExpectedValues{
wantStatus: http.StatusUnauthorized,
wantErrorResponseBody: fositeUpstreamGroupClaimErrorBody,
wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(),
wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens(), true),
},
},
},
{
name: "happy path refresh grant when the upstream refresh does not return a new refresh token",
idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(