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

Merge pull request #279 from vmware-tanzu/fosite-settings

Browse Source 
Update more fosite settings
This commit is contained in:
Ryan Richard 2020-12-11 18:19:50 -08:00 committed by GitHub
commit a5c07042c1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 97 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -121,13 +121,22 @@ func NewHandler(
}
}
authCodeOptions := []oauth2.AuthCodeOption{
oauth2.AccessTypeOffline,
nonceValue.Param(),
pkceValue.Challenge(),
pkceValue.Method(),
}
promptParam := r.Form.Get("prompt")
if promptParam != "" && oidc.ScopeWasRequested(authorizeRequester, coreosoidc.ScopeOpenID) {
authCodeOptions = append(authCodeOptions, oauth2.SetAuthURLParam("prompt", promptParam))
}
http.Redirect(w, r,
upstreamOAuthConfig.AuthCodeURL(
encodedStateParamValue,
oauth2.AccessTypeOffline,
nonceValue.Param(),
pkceValue.Challenge(),
pkceValue.Method(),
authCodeOptions...,
),
302,
)

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

@ -32,8 +32,11 @@ func TestAuthorizationEndpoint(t *testing.T) {
downstreamIssuer = "https://my-downstream-issuer.com/some-path"
downstreamRedirectURI = "http://127.0.0.1/callback"
downstreamRedirectURIWithDifferentPort = "http://127.0.0.1:42/callback"
happyState = "8b-state"
)
require.Len(t, happyState, 8, "we expect fosite to allow 8 byte state params, so we want to test that boundary case")
var (
fositeInvalidClientErrorBody = here.Doc(`
{
@ -57,50 +60,50 @@ func TestAuthorizationEndpoint(t *testing.T) {
fositePromptHasNoneAndOtherValueErrorQuery = map[string]string{
"error": "invalid_request",
"error_description": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed\n\nUsed unknown value \"[none login]\" for prompt parameter",
"error_hint": "Used unknown value \"[none login]\" for prompt parameter",
"state": "some-state-value-that-is-32-byte",
"error_description": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed\n\nParameter \"prompt\" was set to \"none\", but contains other values as well which is not allowed.",
"error_hint": "Parameter \"prompt\" was set to \"none\", but contains other values as well which is not allowed.",
"state": happyState,
}
fositeMissingCodeChallengeErrorQuery = map[string]string{
"error": "invalid_request",
"error_description": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed\n\nClients must include a code_challenge when performing the authorize code flow, but it is missing.",
"error_hint": "Clients must include a code_challenge when performing the authorize code flow, but it is missing.",
"state": "some-state-value-that-is-32-byte",
"state": happyState,
}
fositeMissingCodeChallengeMethodErrorQuery = map[string]string{
"error": "invalid_request",
"error_description": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed\n\nClients must use code_challenge_method=S256, plain is not allowed.",
"error_hint": "Clients must use code_challenge_method=S256, plain is not allowed.",
"state": "some-state-value-that-is-32-byte",
"state": happyState,
}
fositeInvalidCodeChallengeErrorQuery = map[string]string{
"error": "invalid_request",
"error_description": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed\n\nThe code_challenge_method is not supported, use S256 instead.",
"error_hint": "The code_challenge_method is not supported, use S256 instead.",
"state": "some-state-value-that-is-32-byte",
"state": happyState,
}
fositeUnsupportedResponseTypeErrorQuery = map[string]string{
"error": "unsupported_response_type",
"error_description": "The authorization server does not support obtaining a token using this method\n\nThe client is not allowed to request response_type \"unsupported\".",
"error_hint": `The client is not allowed to request response_type "unsupported".`,
"state": "some-state-value-that-is-32-byte",
"state": happyState,
}
fositeInvalidScopeErrorQuery = map[string]string{
"error": "invalid_scope",
"error_description": "The requested scope is invalid, unknown, or malformed\n\nThe OAuth 2.0 Client is not allowed to request scope \"tuna\".",
"error_hint": `The OAuth 2.0 Client is not allowed to request scope "tuna".`,
"state": "some-state-value-that-is-32-byte",
"state": happyState,
}
fositeInvalidStateErrorQuery = map[string]string{
"error": "invalid_state",
"error_description": "The state is missing or does not have enough characters and is therefore considered too weak\n\nRequest parameter \"state\" must be at least be 32 characters long to ensure sufficient entropy.",
"error_hint": `Request parameter "state" must be at least be 32 characters long to ensure sufficient entropy.`,
"error_description": "The state is missing or does not have enough characters and is therefore considered too weak\n\nRequest parameter \"state\" must be at least be 8 characters long to ensure sufficient entropy.",
"error_hint": `Request parameter "state" must be at least be 8 characters long to ensure sufficient entropy.`,
"state": "short",
}
@ -108,7 +111,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
"error": "unsupported_response_type",
"error_description": "The authorization server does not support obtaining a token using this method\n\nThe request is missing the \"response_type\"\" parameter.",
"error_hint": `The request is missing the "response_type"" parameter.`,
"state": "some-state-value-that-is-32-byte",
"state": happyState,
}
)
@ -131,7 +134,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
happyCSRF := "test-csrf"
happyPKCE := "test-pkce"
happyNonce := "test-nonce-that-is-32-bytes-long"
happyNonce := "test-nonce"
happyCSRFGenerator := func() (csrftoken.CSRFToken, error) { return csrftoken.CSRFToken(happyCSRF), nil }
happyPKCEGenerator := func() (pkce.Code, error) { return pkce.Code(happyPKCE), nil }
happyNonceGenerator := func() (nonce.Nonce, error) { return nonce.Nonce(happyNonce), nil }
@ -177,7 +180,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
"response_type": "code",
"scope": "openid profile email",
"client_id": "pinniped-cli",
"state": "some-state-value-that-is-32-byte",
"state": happyState,
"nonce": "some-nonce-value",
"code_challenge": "some-challenge",
"code_challenge_method": "S256",
@ -229,8 +232,8 @@ func TestAuthorizationEndpoint(t *testing.T) {
return encoded
}
expectedRedirectLocation := func(expectedUpstreamState string) string {
return urlWithQuery(upstreamAuthURL.String(), map[string]string{
expectedRedirectLocation := func(expectedUpstreamState string, expectedPrompt string) string {
query := map[string]string{
"response_type": "code",
"access_type": "offline",
"scope": "scope1 scope2",
@ -240,7 +243,11 @@ func TestAuthorizationEndpoint(t *testing.T) {
"code_challenge": expectedUpstreamCodeChallenge,
"code_challenge_method": "S256",
"redirect_uri": downstreamIssuer + "/callback",
})
}
if expectedPrompt != "" {
query["prompt"] = expectedPrompt
}
return urlWithQuery(upstreamAuthURL.String(), query)
}
incomingCookieCSRFValue := "csrf-value-from-cookie"
@ -288,7 +295,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
wantStatus: http.StatusFound,
wantContentType: "text/html; charset=utf-8",
wantCSRFValueInCookieHeader: happyCSRF,
wantLocationHeader: expectedRedirectLocation(expectedUpstreamStateParam(nil, "", "")),
wantLocationHeader: expectedRedirectLocation(expectedUpstreamStateParam(nil, "", ""), ""),
wantUpstreamStateParamInLocationHeader: true,
wantBodyStringWithLocationInHref: true,
},
@ -306,7 +313,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
csrfCookie: "__Host-pinniped-csrf=" + encodedIncomingCookieCSRFValue + " ",
wantStatus: http.StatusFound,
wantContentType: "text/html; charset=utf-8",
wantLocationHeader: expectedRedirectLocation(expectedUpstreamStateParam(nil, incomingCookieCSRFValue, "")),
wantLocationHeader: expectedRedirectLocation(expectedUpstreamStateParam(nil, incomingCookieCSRFValue, ""), ""),
wantUpstreamStateParamInLocationHeader: true,
wantBodyStringWithLocationInHref: true,
},
@ -327,7 +334,27 @@ func TestAuthorizationEndpoint(t *testing.T) {
wantContentType: "",
wantBodyString: "",
wantCSRFValueInCookieHeader: happyCSRF,
wantLocationHeader: expectedRedirectLocation(expectedUpstreamStateParam(nil, "", "")),
wantLocationHeader: expectedRedirectLocation(expectedUpstreamStateParam(nil, "", ""), ""),
wantUpstreamStateParamInLocationHeader: true,
},
{
name: "happy path with prompt param login passed through to redirect uri",
issuer: downstreamIssuer,
idpListGetter: oidctestutil.NewIDPListGetter(&upstreamOIDCIdentityProvider),
generateCSRF: happyCSRFGenerator,
generatePKCE: happyPKCEGenerator,
generateNonce: happyNonceGenerator,
stateEncoder: happyStateEncoder,
cookieEncoder: happyCookieEncoder,
method: http.MethodGet,
path: modifiedHappyGetRequestPath(map[string]string{"prompt": "login"}),
contentType: "application/x-www-form-urlencoded",
body: encodeQuery(happyGetRequestQueryMap),
wantStatus: http.StatusFound,
wantContentType: "text/html; charset=utf-8",
wantBodyStringWithLocationInHref: true,
wantCSRFValueInCookieHeader: happyCSRF,
wantLocationHeader: expectedRedirectLocation(expectedUpstreamStateParam(map[string]string{"prompt": "login"}, "", ""), "login"),
wantUpstreamStateParamInLocationHeader: true,
},
{
@ -346,7 +373,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
wantContentType: "text/html; charset=utf-8",
// Generated a new CSRF cookie and set it in the response.
wantCSRFValueInCookieHeader: happyCSRF,
wantLocationHeader: expectedRedirectLocation(expectedUpstreamStateParam(nil, "", "")),
wantLocationHeader: expectedRedirectLocation(expectedUpstreamStateParam(nil, "", ""), ""),
wantUpstreamStateParamInLocationHeader: true,
wantBodyStringWithLocationInHref: true,
},
@ -368,7 +395,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
wantCSRFValueInCookieHeader: happyCSRF,
wantLocationHeader: expectedRedirectLocation(expectedUpstreamStateParam(map[string]string{
"redirect_uri": downstreamRedirectURIWithDifferentPort, // not the same port number that is registered for the client
}, "", "")),
}, "", ""), ""),
wantUpstreamStateParamInLocationHeader: true,
wantBodyStringWithLocationInHref: true,
},
@ -388,7 +415,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
wantCSRFValueInCookieHeader: happyCSRF,
wantLocationHeader: expectedRedirectLocation(expectedUpstreamStateParam(map[string]string{
"scope": "openid offline_access",
}, "", "")),
}, "", ""), ""),
wantUpstreamStateParamInLocationHeader: true,
wantBodyStringWithLocationInHref: true,
},
@ -586,7 +613,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
wantCSRFValueInCookieHeader: happyCSRF,
wantLocationHeader: expectedRedirectLocation(expectedUpstreamStateParam(
map[string]string{"prompt": "none login", "scope": "email"}, "", "",
)),
), ""),
wantUpstreamStateParamInLocationHeader: true,
wantBodyStringWithLocationInHref: true,
},

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

@ -48,7 +48,7 @@ const (
happyUpstreamRedirectURI = "https://example.com/callback"
happyDownstreamState = "some-downstream-state-with-at-least-32-bytes"
happyDownstreamState = "8b-state"
happyDownstreamCSRF = "test-csrf"
happyDownstreamPKCE = "test-pkce"
happyDownstreamNonce = "test-nonce"
@ -84,6 +84,8 @@ var (
)
func TestCallbackEndpoint(t *testing.T) {
require.Len(t, happyDownstreamState, 8, "we expect fosite to allow 8 byte state params, so we want to test that boundary case")
otherUpstreamOIDCIdentityProvider := oidctestutil.TestUpstreamOIDCIdentityProvider{
Name: "other-upstream-idp-name",
ClientID: "other-some-client-id",

2
internal/oidc/dynamic_open_id_connect_ecdsa_strategy_test.go
View File

@ -30,7 +30,7 @@ func TestDynamicOpenIDConnectECDSAStrategy(t *testing.T) {
clientID = "some-client-id"
goodSubject = "some-subject"
goodUsername = "some-username"
goodNonce = "some-nonce-that-is-at-least-32-characters-to-meet-entropy-requirements"
goodNonce = "some-nonce-value-with-enough-bytes-to-exceed-min-allowed"
)
ecPrivateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)

35
internal/oidc/oidc.go
View File

@ -199,12 +199,20 @@ func FositeOauth2Helper(
AccessTokenLifespan: timeoutsConfiguration.AccessTokenLifespan,
RefreshTokenLifespan: timeoutsConfiguration.RefreshTokenLifespan,
ScopeStrategy: fosite.ExactScopeStrategy,
AudienceMatchingStrategy: nil,
EnforcePKCE: true,
AllowedPromptValues: []string{"none"}, // TODO unclear what we should set here
RefreshTokenScopes: []string{coreosoidc.ScopeOfflineAccess}, // as per https://openid.net/specs/openid-connect-core-1_0.html#OfflineAccess
MinParameterEntropy: 32, // TODO is 256 bits too high?
ScopeStrategy: fosite.ExactScopeStrategy,
EnforcePKCE: true,
// "offline_access" as per https://openid.net/specs/openid-connect-core-1_0.html#OfflineAccess
RefreshTokenScopes: []string{coreosoidc.ScopeOfflineAccess},
// The default is to support all prompt values from the spec.
// See https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
// We'll make a best effort to support these by passing the value of this prompt param to the upstream IDP
// and rely on its implementation of this param.
AllowedPromptValues: nil,
// Use the fosite default to make it more likely that off the shelf OIDC clients can work with the supervisor.
MinParameterEntropy: fosite.MinParameterEntropy,
}
return compose.Compose(
@ -251,9 +259,16 @@ type IDPListGetter interface {
}
func GrantScopeIfRequested(authorizeRequester fosite.AuthorizeRequester, scopeName string) {
for _, scope := range authorizeRequester.GetRequestedScopes() {
if scope == scopeName {
authorizeRequester.GrantScope(scope)
}
if ScopeWasRequested(authorizeRequester, scopeName) {
authorizeRequester.GrantScope(scopeName)
}
}
func ScopeWasRequested(authorizeRequester fosite.AuthorizeRequester, scopeName string) bool {
for _, scope := range authorizeRequester.GetRequestedScopes() {
if scope == scopeName {
return true
}
}
return false
}

6
internal/oidc/provider/manager/manager_test.go
View File

@ -142,7 +142,7 @@ func TestManager(t *testing.T) {
actualLocationQueryParams := parsedLocation.Query()
r.Contains(actualLocationQueryParams, "code")
r.Equal("openid", actualLocationQueryParams.Get("scope"))
r.Equal("some-state-value-that-is-32-byte", actualLocationQueryParams.Get("state"))
r.Equal("some-state-value-with-enough-bytes-to-exceed-min-allowed", actualLocationQueryParams.Get("state"))
// Make sure that we wired up the callback endpoint to use kube storage for fosite sessions.
r.Equal(len(kubeClient.Actions()), numberOfKubeActionsBeforeThisRequest+3,
@ -293,8 +293,8 @@ func TestManager(t *testing.T) {
"response_type": []string{"code"},
"scope": []string{"openid profile email"},
"client_id": []string{downstreamClientID},
"state": []string{"some-state-value-that-is-32-byte"},
"nonce": []string{"some-nonce-value-that-is-at-least-32-bytes"},
"state": []string{"some-state-value-with-enough-bytes-to-exceed-min-allowed"},
"nonce": []string{"some-nonce-value-with-enough-bytes-to-exceed-min-allowed"},
"code_challenge": []string{testutil.SHA256(downstreamPKCECodeVerifier)},
"code_challenge_method": []string{"S256"},
"redirect_uri": []string{downstreamRedirectURL},

4
internal/oidc/token/token_handler_test.go
View File

@ -52,7 +52,7 @@ const (
goodClient = "pinniped-cli"
goodRedirectURI = "http://127.0.0.1/callback"
goodPKCECodeVerifier = "some-pkce-verifier-that-must-be-at-least-43-characters-to-meet-entropy-requirements"
goodNonce = "some-nonce-that-is-at-least-32-characters-to-meet-entropy-requirements"
goodNonce = "some-nonce-value-with-enough-bytes-to-exceed-min-allowed"
goodSubject = "some-subject"
goodUsername = "some-username"
@ -213,7 +213,7 @@ var (
"response_type": {"code"},
"scope": {"openid profile email"},
"client_id": {goodClient},
"state": {"some-state-value-that-is-32-byte"},
"state": {"some-state-value-with-enough-bytes-to-exceed-min-allowed"},
"nonce": {goodNonce},
"code_challenge": {testutil.SHA256(goodPKCECodeVerifier)},
"code_challenge_method": {"S256"},