djpbessems/ContainerImage.Pinniped
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

authorize and callback endpoints now handle the offline_access scope

Browse Source 
- This is in preparation for the token endpoint to support the refresh
  grant

Signed-off-by: Ryan Richard <richardry@vmware.com>
This commit is contained in:
Aram Price 2020-12-07 17:22:34 -08:00 committed by Ryan Richard
parent 6a90a10123
commit d91baba240
6 changed files with 83 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
internal/oidc/auth/auth_handler.go
View File

@ -9,6 +9,7 @@ import (
"net/http"
"time"
coreosoidc "github.com/coreos/go-oidc"
"github.com/ory/fosite"
"github.com/ory/fosite/handler/openid"
"github.com/ory/fosite/token/jwt"
@ -57,7 +58,10 @@ func NewHandler(
}
// Grant the openid scope (for now) if they asked for it so that `NewAuthorizeResponse` will perform its OIDC validations.
grantOpenIDScopeIfRequested(authorizeRequester)
oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOpenID)
// There don't seem to be any validations inside `NewAuthorizeResponse` related to the offline_access scope
// at this time, however we will temporarily grant the scope just in case that changes in a future release of fosite.
oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOfflineAccess)
now := time.Now()
_, err = oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, &openid.DefaultSession{
@ -148,14 +152,6 @@ func readCSRFCookie(r *http.Request, codec oidc.Codec) csrftoken.CSRFToken {
return csrfFromCookie
}
func grantOpenIDScopeIfRequested(authorizeRequester fosite.AuthorizeRequester) {
for _, scope := range authorizeRequester.GetRequestedScopes() {
if scope == "openid" {
authorizeRequester.GrantScope(scope)
}
}
}
func chooseUpstreamIDP(idpListGetter oidc.IDPListGetter) (provider.UpstreamOIDCIdentityProviderI, error) {
allUpstreamIDPs := idpListGetter.GetIDPList()
if len(allUpstreamIDPs) == 0 {

22
internal/oidc/auth/auth_handler_test.go
View File

@ -119,7 +119,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
Name: "some-idp",
ClientID: "some-client-id",
AuthorizationURL: *upstreamAuthURL,
Scopes: []string{"scope1", "scope2"},
Scopes: []string{"scope1", "scope2"}, // the scopes to request when starting the upstream authorization flow
}
// Configure fosite the same way that the production code would, using NullStorage to turn off storage.
@ -372,6 +372,26 @@ func TestAuthorizationEndpoint(t *testing.T) {
wantUpstreamStateParamInLocationHeader: true,
wantBodyStringWithLocationInHref: true,
},
{
name: "happy path when downstream requested scopes include offline_access",
issuer: downstreamIssuer,
idpListGetter: oidctestutil.NewIDPListGetter(&upstreamOIDCIdentityProvider),
generateCSRF: happyCSRFGenerator,
generatePKCE: happyPKCEGenerator,
generateNonce: happyNonceGenerator,
stateEncoder: happyStateEncoder,
cookieEncoder: happyCookieEncoder,
method: http.MethodGet,
path: modifiedHappyGetRequestPath(map[string]string{"scope": "openid offline_access"}),
wantStatus: http.StatusFound,
wantContentType: "text/html; charset=utf-8",
wantCSRFValueInCookieHeader: happyCSRF,
wantLocationHeader: expectedRedirectLocation(expectedUpstreamStateParam(map[string]string{
"scope": "openid offline_access",
}, "", "")),
wantUpstreamStateParamInLocationHeader: true,
wantBodyStringWithLocationInHref: true,
},
{
name: "downstream redirect uri does not match what is configured for client",
issuer: downstreamIssuer,

14
internal/oidc/callback/callback_handler.go
View File

@ -11,6 +11,7 @@ import (
"net/url"
"time"
coreosoidc "github.com/coreos/go-oidc"
"github.com/ory/fosite"
"github.com/ory/fosite/handler/openid"
"github.com/ory/fosite/token/jwt"
@ -70,8 +71,9 @@ func NewHandler(
return httperr.New(http.StatusBadRequest, "error using state downstream auth params")
}
// Grant the openid scope only if it was requested.
grantOpenIDScopeIfRequested(authorizeRequester)
// Automatically grant the openid and offline_access scopes, but only if they were requested.
oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOpenID)
oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOfflineAccess)
token, err := upstreamIDPConfig.ExchangeAuthcodeAndValidateTokens(
r.Context(),
@ -189,14 +191,6 @@ func readState(r *http.Request, stateDecoder oidc.Decoder) (*oidc.UpstreamStateP
return &state, nil
}
func grantOpenIDScopeIfRequested(authorizeRequester fosite.AuthorizeRequester) {
for _, scope := range authorizeRequester.GetRequestedScopes() {
if scope == "openid" {
authorizeRequester.GrantScope(scope)
}
}
}
func getUsernameFromUpstreamIDToken(
upstreamIDPConfig provider.UpstreamOIDCIdentityProviderI,
idTokenClaims map[string]interface{},

56
internal/oidc/callback/callback_handler_test.go
View File

@ -66,7 +66,8 @@ const (
var (
upstreamGroupMembership = []string{"test-pinniped-group-0", "test-pinniped-group-1"}
happyDownstreamScopesRequested = []string{"openid", "profile", "email"}
happyDownstreamScopesRequested = []string{"openid"}
happyDownstreamScopesGranted = []string{"openid"}
happyDownstreamRequestParamsQuery = url.Values{
"response_type": []string{"code"},
@ -127,7 +128,7 @@ func TestCallbackEndpoint(t *testing.T) {
wantStatus int
wantBody string
wantRedirectLocationRegexp string
wantGrantedOpenidScope bool
wantDownstreamGrantedScopes []string
wantDownstreamIDTokenSubject string
wantDownstreamIDTokenGroups []string
wantDownstreamRequestedScopes []string
@ -145,11 +146,11 @@ func TestCallbackEndpoint(t *testing.T) {
csrfCookie: happyCSRFCookie,
wantStatus: http.StatusFound,
wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp,
wantGrantedOpenidScope: true,
wantBody: "",
wantDownstreamIDTokenSubject: upstreamUsername,
wantDownstreamIDTokenGroups: upstreamGroupMembership,
wantDownstreamRequestedScopes: happyDownstreamScopesRequested,
wantDownstreamGrantedScopes: happyDownstreamScopesGranted,
wantDownstreamNonce: downstreamNonce,
wantDownstreamPKCEChallenge: downstreamPKCEChallenge,
wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
@ -163,11 +164,11 @@ func TestCallbackEndpoint(t *testing.T) {
csrfCookie: happyCSRFCookie,
wantStatus: http.StatusFound,
wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp,
wantGrantedOpenidScope: true,
wantBody: "",
wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject,
wantDownstreamIDTokenGroups: nil,
wantDownstreamRequestedScopes: happyDownstreamScopesRequested,
wantDownstreamGrantedScopes: happyDownstreamScopesGranted,
wantDownstreamNonce: downstreamNonce,
wantDownstreamPKCEChallenge: downstreamPKCEChallenge,
wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
@ -181,11 +182,11 @@ func TestCallbackEndpoint(t *testing.T) {
csrfCookie: happyCSRFCookie,
wantStatus: http.StatusFound,
wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp,
wantGrantedOpenidScope: true,
wantBody: "",
wantDownstreamIDTokenSubject: upstreamSubject,
wantDownstreamIDTokenGroups: upstreamGroupMembership,
wantDownstreamRequestedScopes: happyDownstreamScopesRequested,
wantDownstreamGrantedScopes: happyDownstreamScopesGranted,
wantDownstreamNonce: downstreamNonce,
wantDownstreamPKCEChallenge: downstreamPKCEChallenge,
wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
@ -316,6 +317,28 @@ func TestCallbackEndpoint(t *testing.T) {
wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs,
},
{
name: "state's downstream auth params also included offline_access scope",
idp: happyUpstream().Build(),
method: http.MethodGet,
path: newRequestPath().
WithState(
happyUpstreamStateParam().
WithAuthorizeRequestParams(shallowCopyAndModifyQuery(happyDownstreamRequestParamsQuery, map[string]string{"scope": "openid offline_access"}).Encode()).
Build(t, happyStateCodec),
).String(),
csrfCookie: happyCSRFCookie,
wantStatus: http.StatusFound,
wantRedirectLocationRegexp: downstreamRedirectURI + `\?code=([^&]+)&scope=openid%20offline_access&state=` + happyDownstreamState,
wantDownstreamIDTokenSubject: upstreamUsername,
wantDownstreamRequestedScopes: []string{"openid", "offline_access"},
wantDownstreamGrantedScopes: []string{"openid", "offline_access"},
wantDownstreamIDTokenGroups: upstreamGroupMembership,
wantDownstreamNonce: downstreamNonce,
wantDownstreamPKCEChallenge: downstreamPKCEChallenge,
wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
wantExchangeAndValidateTokensCall: happyExchangeAndValidateTokensArgs,
},
{
name: "the UpstreamOIDCProvider CRD has been deleted",
idp: otherUpstreamOIDCIdentityProvider,
@ -481,7 +504,7 @@ func TestCallbackEndpoint(t *testing.T) {
// Several Secrets should have been created
expectedNumberOfCreatedSecrets := 2
if test.wantGrantedOpenidScope {
if includesOpenIDScope(test.wantDownstreamGrantedScopes) {
expectedNumberOfCreatedSecrets++
}
require.Len(t, client.Actions(), expectedNumberOfCreatedSecrets)
@ -493,7 +516,7 @@ func TestCallbackEndpoint(t *testing.T) {
t,
oauthStore,
authcodeDataAndSignature[1], // Authcode store key is authcode signature
test.wantGrantedOpenidScope,
test.wantDownstreamGrantedScopes,
test.wantDownstreamIDTokenSubject,
test.wantDownstreamIDTokenGroups,
test.wantDownstreamRequestedScopes,
@ -513,7 +536,7 @@ func TestCallbackEndpoint(t *testing.T) {
)
// One IDSession should have been stored, if the downstream actually requested the "openid" scope
if test.wantGrantedOpenidScope {
if includesOpenIDScope(test.wantDownstreamGrantedScopes) {
testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, 1)
validateIDSessionStorage(
@ -530,6 +553,15 @@ func TestCallbackEndpoint(t *testing.T) {
}
}
func includesOpenIDScope(scopes []string) bool {
for _, scope := range scopes {
if scope == "openid" {
return true
}
}
return false
}
type requestPath struct {
code, state *string
}
@ -704,7 +736,7 @@ func validateAuthcodeStorage(
t *testing.T,
oauthStore *oidc.KubeStorage,
storeKey string,
wantGrantedOpenidScope bool,
wantDownstreamGrantedScopes []string,
wantDownstreamIDTokenSubject string,
wantDownstreamIDTokenGroups []string,
wantDownstreamRequestedScopes []string,
@ -719,11 +751,7 @@ func validateAuthcodeStorage(
storedRequestFromAuthcode, storedSessionFromAuthcode := castStoredAuthorizeRequest(t, storedAuthorizeRequestFromAuthcode)
// Check which scopes were granted.
if wantGrantedOpenidScope {
require.Contains(t, storedRequestFromAuthcode.GetGrantedScopes(), "openid")
} else {
require.NotContains(t, storedRequestFromAuthcode.GetGrantedScopes(), "openid")
}
require.ElementsMatch(t, wantDownstreamGrantedScopes, storedRequestFromAuthcode.GetGrantedScopes())
// Check all the other fields of the stored request.
require.NotEmpty(t, storedRequestFromAuthcode.ID)

2
internal/oidc/nullstorage_test.go
View File

@ -28,7 +28,7 @@ func TestNullStorage_GetClient(t *testing.T) {
RedirectURIs: []string{"http://127.0.0.1/callback"},
ResponseTypes: []string{"code"},
GrantTypes: []string{"authorization_code"},
Scopes: []string{"openid", "profile", "email"},
Scopes: []string{"openid", "offline_access", "profile", "email"},
},
TokenEndpointAuthMethod: "none",
},

11
internal/oidc/oidc.go
View File

@ -7,6 +7,7 @@ package oidc
import (
"time"
coreosoidc "github.com/coreos/go-oidc"
"github.com/ory/fosite"
"github.com/ory/fosite/compose"
@ -84,7 +85,7 @@ func PinnipedCLIOIDCClient() *fosite.DefaultOpenIDConnectClient {
RedirectURIs: []string{"http://127.0.0.1/callback"},
ResponseTypes: []string{"code"},
GrantTypes: []string{"authorization_code"},
Scopes: []string{"openid", "profile", "email"},
Scopes: []string{coreosoidc.ScopeOpenID, coreosoidc.ScopeOfflineAccess, "profile", "email"},
},
TokenEndpointAuthMethod: "none",
}
@ -156,3 +157,11 @@ func FositeErrorForLog(err error) []interface{} {
type IDPListGetter interface {
GetIDPList() []provider.UpstreamOIDCIdentityProviderI
}
func GrantScopeIfRequested(authorizeRequester fosite.AuthorizeRequester, scopeName string) {
for _, scope := range authorizeRequester.GetRequestedScopes() {
if scope == scopeName {
authorizeRequester.GrantScope(scope)
}
}
}