Merge branch 'main' into upstream-oidc-refresh-retries

This commit is contained in:
Ryan Richard 2022-01-20 13:14:06 -08:00
commit 7f9867598c
5 changed files with 80 additions and 6 deletions

2
go.mod
View File

@ -55,7 +55,7 @@ require (
github.com/joshlf/go-acl v0.0.0-20200411065538-eae00ae38531 github.com/joshlf/go-acl v0.0.0-20200411065538-eae00ae38531
github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826
github.com/ory/fosite v0.41.0 github.com/ory/fosite v0.41.0
github.com/ory/x v0.0.331 github.com/ory/x v0.0.334
github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8
github.com/pkg/errors v0.9.1 github.com/pkg/errors v0.9.1
github.com/sclevine/agouti v3.0.0+incompatible github.com/sclevine/agouti v3.0.0+incompatible

4
go.sum
View File

@ -1533,8 +1533,8 @@ github.com/ory/x v0.0.214/go.mod h1:aRl57gzyD4GF0HQCekovXhv0xTZgAgiht3o8eVhsm9Q=
github.com/ory/x v0.0.250/go.mod h1:jUJaVptu+geeqlb9SyQCogTKj5ztSDIF6APkhbKtwLc= github.com/ory/x v0.0.250/go.mod h1:jUJaVptu+geeqlb9SyQCogTKj5ztSDIF6APkhbKtwLc=
github.com/ory/x v0.0.272/go.mod h1:1TTPgJGQutrhI2OnwdrTIHE9ITSf4MpzXFzA/ncTGRc= github.com/ory/x v0.0.272/go.mod h1:1TTPgJGQutrhI2OnwdrTIHE9ITSf4MpzXFzA/ncTGRc=
github.com/ory/x v0.0.288/go.mod h1:APpShLyJcVzKw1kTgrHI+j/L9YM+8BRjHlcYObc7C1U= github.com/ory/x v0.0.288/go.mod h1:APpShLyJcVzKw1kTgrHI+j/L9YM+8BRjHlcYObc7C1U=
github.com/ory/x v0.0.331 h1:Elw9xiTXqRDkO+7b4NKTIXTPC2nyk70HosbF2jQzEOI= github.com/ory/x v0.0.334 h1:ZtxDKRjrRYadZGYIg7kFI4wuEpRX7n5eMBQnxRU07lw=
github.com/ory/x v0.0.331/go.mod h1:l9sL63RvxMWjhMIe9epl8SiqXoPh2TrwMXE4rNA2dOY= github.com/ory/x v0.0.334/go.mod h1:vRr+//Cmpcu4HwkYwstv4mzie65ss+r76+iXU9fqQiA=
github.com/pact-foundation/pact-go v1.0.4/go.mod h1:uExwJY4kCzNPcHRj+hCR/HBbOOIwwtUjcrb0b5/5kLM= github.com/pact-foundation/pact-go v1.0.4/go.mod h1:uExwJY4kCzNPcHRj+hCR/HBbOOIwwtUjcrb0b5/5kLM=
github.com/parnurzeal/gorequest v0.2.15/go.mod h1:3Kh2QUMJoqw3icWAecsyzkpY7UzRfDhbRdTjtNwNiUE= github.com/parnurzeal/gorequest v0.2.15/go.mod h1:3Kh2QUMJoqw3icWAecsyzkpY7UzRfDhbRdTjtNwNiUE=
github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc= github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=

View File

@ -194,7 +194,9 @@ func upstreamOIDCRefresh(
// and let any old groups memberships in the session remain. // and let any old groups memberships in the session remain.
refreshedGroups, err := downstreamsession.GetGroupsFromUpstreamIDToken(p, mergedClaims) refreshedGroups, err := downstreamsession.GetGroupsFromUpstreamIDToken(p, mergedClaims)
if err != nil { 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 { if refreshedGroups != nil {
session.Fosite.Claims.Extra[oidc.DownstreamGroupsClaim] = refreshedGroups 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{ happyAuthRequest = &http.Request{
Form: url.Values{ Form: url.Values{
"response_type": {"code"}, "response_type": {"code"},
@ -1271,7 +1278,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( idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(
upstreamOIDCIdentityProviderBuilder().WithGroupsClaim("my-groups-claim").WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{ upstreamOIDCIdentityProviderBuilder().WithGroupsClaim("my-groups-claim").WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{
IDToken: &oidctypes.IDToken{ IDToken: &oidctypes.IDToken{
@ -1299,6 +1335,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", name: "happy path refresh grant when the upstream refresh does not return a new refresh token",
idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC( idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(

View File

@ -136,6 +136,17 @@ func TestSupervisorLogin(t *testing.T) {
}, },
createIDP: func(t *testing.T) string { createIDP: func(t *testing.T) string {
t.Helper() t.Helper()
var additionalScopes []string
// keep all the scopes except for offline access so we can test the access token based refresh flow.
if len(env.ToolsNamespace) == 0 {
additionalScopes = env.SupervisorUpstreamOIDC.AdditionalScopes
} else {
for _, additionalScope := range env.SupervisorUpstreamOIDC.AdditionalScopes {
if additionalScope != "offline_access" {
additionalScopes = append(additionalScopes, additionalScope)
}
}
}
oidcIDP := testlib.CreateTestOIDCIdentityProvider(t, idpv1alpha1.OIDCIdentityProviderSpec{ oidcIDP := testlib.CreateTestOIDCIdentityProvider(t, idpv1alpha1.OIDCIdentityProviderSpec{
Issuer: env.SupervisorUpstreamOIDC.Issuer, Issuer: env.SupervisorUpstreamOIDC.Issuer,
TLS: &idpv1alpha1.TLSSpec{ TLS: &idpv1alpha1.TLSSpec{
@ -149,7 +160,7 @@ func TestSupervisorLogin(t *testing.T) {
Groups: env.SupervisorUpstreamOIDC.GroupsClaim, Groups: env.SupervisorUpstreamOIDC.GroupsClaim,
}, },
AuthorizationConfig: idpv1alpha1.OIDCAuthorizationConfig{ AuthorizationConfig: idpv1alpha1.OIDCAuthorizationConfig{
AdditionalScopes: []string{"email"}, // does not ask for offline_access. AdditionalScopes: additionalScopes,
}, },
}, idpv1alpha1.PhaseReady) }, idpv1alpha1.PhaseReady)
return oidcIDP.Name return oidcIDP.Name