From 020fbcf1907c785d357b9d49725c4b3da606f4b4 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Fri, 11 Dec 2020 17:39:58 -0800 Subject: [PATCH] Adjust some expectations about the state and nonce lengths --- internal/oidc/auth/auth_handler_test.go | 21 +++++++++++-------- .../oidc/callback/callback_handler_test.go | 4 +++- ...mic_open_id_connect_ecdsa_strategy_test.go | 2 +- .../oidc/provider/manager/manager_test.go | 6 +++--- internal/oidc/token/token_handler_test.go | 4 ++-- 5 files changed, 21 insertions(+), 16 deletions(-) diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index b937a228..ee07552b 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -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(` { @@ -59,42 +62,42 @@ func TestAuthorizationEndpoint(t *testing.T) { "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-that-is-32-byte", + "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{ @@ -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", diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index df0c2779..a0ecc996 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -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", diff --git a/internal/oidc/dynamic_open_id_connect_ecdsa_strategy_test.go b/internal/oidc/dynamic_open_id_connect_ecdsa_strategy_test.go index 85a63501..f0250e22 100644 --- a/internal/oidc/dynamic_open_id_connect_ecdsa_strategy_test.go +++ b/internal/oidc/dynamic_open_id_connect_ecdsa_strategy_test.go @@ -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) diff --git a/internal/oidc/provider/manager/manager_test.go b/internal/oidc/provider/manager/manager_test.go index 0720b719..9776c6fa 100644 --- a/internal/oidc/provider/manager/manager_test.go +++ b/internal/oidc/provider/manager/manager_test.go @@ -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}, diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index d0ae5f3a..698bc4db 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -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"},