diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index eca7472a..b56ff262 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -56,50 +56,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\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": "some-state-value", + "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", } 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\nThis client must include a code_challenge when performing the authorize code flow, but it is missing.", - "error_hint": "This client must include a code_challenge when performing the authorize code flow, but it is missing.", - "state": "some-state-value", + "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", } 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", + "state": "some-state-value-that-is-32-byte", } 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", + "state": "some-state-value-that-is-32-byte", } 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", + "state": "some-state-value-that-is-32-byte", } 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", + "state": "some-state-value-that-is-32-byte", } 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 8 characters long to ensure sufficient entropy.", - "error_hint": `Request parameter "state" must be at least be 8 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 32 characters long to ensure sufficient entropy.", + "error_hint": `Request parameter "state" must be at least be 32 characters long to ensure sufficient entropy.`, "state": "short", } @@ -107,7 +107,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", + "state": "some-state-value-that-is-32-byte", } ) @@ -129,7 +129,7 @@ func TestAuthorizationEndpoint(t *testing.T) { happyCSRF := "test-csrf" happyPKCE := "test-pkce" - happyNonce := "test-nonce" + happyNonce := "test-nonce-that-is-32-bytes-long" 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 } @@ -175,7 +175,7 @@ func TestAuthorizationEndpoint(t *testing.T) { "response_type": "code", "scope": "openid profile email", "client_id": "pinniped-cli", - "state": "some-state-value", + "state": "some-state-value-that-is-32-byte", "nonce": "some-nonce-value", "code_challenge": "some-challenge", "code_challenge_method": "S256", diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index c19f4990..69072067 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -40,7 +40,7 @@ const ( happyUpstreamAuthcode = "upstream-auth-code" - happyDownstreamState = "some-downstream-state" + happyDownstreamState = "some-downstream-state-with-at-least-32-bytes" happyDownstreamCSRF = "test-csrf" happyDownstreamPKCE = "test-pkce" happyDownstreamNonce = "test-nonce" @@ -714,8 +714,8 @@ func validateAuthcodeStorage( require.Empty(t, storedSessionFromAuthcode.Username) require.Empty(t, storedSessionFromAuthcode.Headers) - // The authcode that we are issuing should be good for 15 minutes, which is default for fosite. - testutil.RequireTimeInDelta(t, time.Now().Add(time.Minute*15), storedSessionFromAuthcode.ExpiresAt[fosite.AuthorizeCode], timeComparisonFudgeFactor) + // The authcode that we are issuing should be good for the length of time that we declare in the fosite config. + testutil.RequireTimeInDelta(t, time.Now().Add(time.Minute*3), storedSessionFromAuthcode.ExpiresAt[fosite.AuthorizeCode], timeComparisonFudgeFactor) require.Len(t, storedSessionFromAuthcode.ExpiresAt, 1) // Now confirm the ID token claims. diff --git a/internal/oidc/oidc.go b/internal/oidc/oidc.go index 83445ac4..1241444f 100644 --- a/internal/oidc/oidc.go +++ b/internal/oidc/oidc.go @@ -5,6 +5,8 @@ package oidc import ( + "time" + "github.com/ory/fosite" "github.com/ory/fosite/compose" @@ -88,8 +90,23 @@ func PinnipedCLIOIDCClient() *fosite.DefaultOpenIDConnectClient { func FositeOauth2Helper(oauthStore interface{}, issuer string, hmacSecretOfLengthAtLeast32 []byte) fosite.OAuth2Provider { oauthConfig := &compose.Config{ - EnforcePKCEForPublicClients: true, - IDTokenIssuer: issuer, + AuthorizeCodeLifespan: 3 * time.Minute, // seems more than long enough to exchange a code + + IDTokenLifespan: 5 * time.Minute, // match clientCertificateTTL since it has similar properties to this token + AccessTokenLifespan: 5 * time.Minute, // match clientCertificateTTL since it has similar properties to this token + + RefreshTokenLifespan: 16 * time.Hour, // long enough for a single workday + + IDTokenIssuer: issuer, + TokenURL: "", // TODO set once we have this endpoint written + + ScopeStrategy: fosite.ExactScopeStrategy, // be careful and only support exact string matching for scopes + AudienceMatchingStrategy: nil, // I believe the default is fine + EnforcePKCE: true, // follow current set of best practices and always require PKCE + AllowedPromptValues: []string{"none"}, // TODO unclear what we should set here + + RefreshTokenScopes: nil, // TODO decide what makes sense when we add refresh token support + MinParameterEntropy: 32, // 256 bits seems about right } return compose.Compose( diff --git a/internal/oidc/provider/manager/manager_test.go b/internal/oidc/provider/manager/manager_test.go index fdea39d5..44ac6398 100644 --- a/internal/oidc/provider/manager/manager_test.go +++ b/internal/oidc/provider/manager/manager_test.go @@ -179,7 +179,7 @@ func TestManager(t *testing.T) { "response_type": []string{"code"}, "scope": []string{"openid profile email"}, "client_id": []string{"pinniped-cli"}, - "state": []string{"some-state-value"}, + "state": []string{"some-state-value-that-is-32-byte"}, "nonce": []string{"some-nonce-value"}, "code_challenge": []string{"some-challenge"}, "code_challenge_method": []string{"S256"}, diff --git a/site/content/docs/demo.md b/site/content/docs/demo.md index 027e5b11..0f0b6209 100644 --- a/site/content/docs/demo.md +++ b/site/content/docs/demo.md @@ -122,7 +122,7 @@ as the identity provider. 1. Create a `WebhookAuthenticator` object to configure Pinniped to authenticate using local-user-authenticator. ```bash - cat < /tmp/pinniped-kubeconfig + pinniped get-kubeconfig --pinniped-namespace pinniped-concierge --token "pinny-the-seal:password123" --authenticator-type webhook --authenticator-name local-user-authenticator > /tmp/pinniped-kubeconfig ``` If you are using MacOS, you may get an error dialog that says @@ -163,7 +163,7 @@ as the identity provider. the `pinny-the-seal` user. ```bash - kubectl --kubeconfig /tmp/pinniped-kubeconfig get pods -n pinniped + kubectl --kubeconfig /tmp/pinniped-kubeconfig get pods -n pinniped-concierge ``` Because this user has no RBAC permissions on this cluster, the previous command @@ -180,7 +180,7 @@ as the identity provider. 1. Use the generated kubeconfig to issue arbitrary `kubectl` commands as the `pinny-the-seal` user. ```bash - kubectl --kubeconfig /tmp/pinniped-kubeconfig get pods -n pinniped + kubectl --kubeconfig /tmp/pinniped-kubeconfig get pods -n pinniped-concierge ``` The user has permission to list pods, so the command succeeds this time.