github/pinniped
1
0
Fork 0
mirror of https://github.com/vmware-tanzu/pinniped.git synced 2026-02-14 10:00:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #2254 from vmware-tanzu/rr/callback_handler_form_post

Browse Source 
support response_mode=form_post in upstream OIDC IDPs
This commit is contained in:
Joshua Casey
2025-03-07 17:40:37 -06:00
committed by GitHub
parent 308c76cd69 749633e43c
commit f54834cbee
9 changed files with 308 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
internal/federationdomain/endpoints/auth/auth_handler.go
View File

@@ -1,4 +1,4 @@
// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
// Copyright 2020-2025 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
// Package auth provides a handler for the OIDC authorization endpoint.
@@ -510,12 +510,32 @@ func addCSRFSetCookieHeader(w http.ResponseWriter, csrfValue csrftoken.CSRFToken
}
http.SetCookie(w, &http.Cookie{
Name: oidc.CSRFCookieName,
Value: encodedCSRFValue,
// Because of the other settings below, this value can only be known by the end user's browser, not by other sites.
Value: encodedCSRFValue,
// Name starting with "__Host-" makes the cookie domain-locked (subdomains cannot set this cookie).
Name: oidc.CSRFCookieName,
// This cookie can't be accessed by JavaScript.
HttpOnly: true,
SameSite: http.SameSiteLaxMode,
Secure: true,
Path: "/",
// Okay for requests from other sites to cause the user's browser to send this cookie back to this site,
// for allowing response_mode=form_post, in which an upstream IDP needs to host a web form which POSTs back
// to the Supervisor's callback endpoint. Note that this allows a malicious 3rd party site to cause the user's
// browser to include this cookie on a request to the Supervisor. However, there is no way for 3rd party sites
// to create the corresponding state param to include on a callback request to the Supervisor to cause that
// callback request be allowed by the Supervisor's callback endpoint. That state param must include this cookie's
// value. A 3rd party site cannot receive this cookie (and therefore cannot know its value), and even if it somehow
// did learn its value, it could not sign the state param (cannot know the signing key for state params, which never
// leaves the Supervisor server). So although a 3rd party site could cause the user's cookie to be sent, that
// request will never be considered acceptable by the Supervisor.
// Note that SameSite=None was created in a 2019 draft standard, so it requires modern browsers to work.
// See https://datatracker.ietf.org/doc/html/draft-west-cookie-incrementalism-00.
SameSite: http.SameSiteNoneMode,
// This cookie may only be sent via HTTPS (required for domain-locked cookies).
Secure: true,
// Sending this cookie to any path of this server is acceptable (required for domain-locked cookies).
Path: "/",
// Note that we do not set "Domain", so this cookie should not be sent to any subdomains (required for domain-locked cookies).
// Also note that we do not set "Expires" or "MaxAge", so the client may keep the cookie as long as it likes,
// which prevents the cookie from expiring during login flows.
})
return nil

12
internal/federationdomain/endpoints/auth/auth_handler_test.go
View File

@@ -1,4 +1,4 @@
// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
// Copyright 2020-2025 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package auth
@@ -1551,7 +1551,7 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo
cookieEncoder: happyCookieEncoder,
method: http.MethodGet,
path: happyGetRequestPathForOIDCUpstream,
csrfCookie: "__Host-pinniped-csrf=" + encodedIncomingCookieCSRFValue + " ",
csrfCookie: "__Host-pinniped-csrf-v2=" + encodedIncomingCookieCSRFValue + " ",
wantStatus: http.StatusSeeOther,
wantContentType: htmlContentType,
wantLocationHeader: expectedRedirectLocationForUpstreamOIDC(expectedUpstreamStateParam(nil, incomingCookieCSRFValue, oidcUpstreamName, "oidc"), nil),
@@ -1568,7 +1568,7 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo
cookieEncoder: happyCookieEncoder,
method: http.MethodGet,
path: happyGetRequestPathForLDAPUpstream,
csrfCookie: "__Host-pinniped-csrf=" + encodedIncomingCookieCSRFValue + " ",
csrfCookie: "__Host-pinniped-csrf-v2=" + encodedIncomingCookieCSRFValue + " ",
wantStatus: http.StatusSeeOther,
wantContentType: htmlContentType,
wantLocationHeader: urlWithQuery(downstreamIssuer+"/login", map[string]string{"state": expectedUpstreamStateParam(nil, incomingCookieCSRFValue, ldapUpstreamName, "ldap")}),
@@ -1585,7 +1585,7 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo
cookieEncoder: happyCookieEncoder,
method: http.MethodGet,
path: happyGetRequestPathForADUpstream,
csrfCookie: "__Host-pinniped-csrf=" + encodedIncomingCookieCSRFValue + " ",
csrfCookie: "__Host-pinniped-csrf-v2=" + encodedIncomingCookieCSRFValue + " ",
wantStatus: http.StatusSeeOther,
wantContentType: htmlContentType,
wantLocationHeader: urlWithQuery(downstreamIssuer+"/login", map[string]string{"state": expectedUpstreamStateParam(nil, incomingCookieCSRFValue, activeDirectoryUpstreamName, "activedirectory")}),
@@ -1931,7 +1931,7 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo
cookieEncoder: happyCookieEncoder,
method: http.MethodGet,
path: happyGetRequestPathForOIDCUpstream,
csrfCookie: "__Host-pinniped-csrf=this-value-was-not-signed-by-pinniped",
csrfCookie: "__Host-pinniped-csrf-v2=this-value-was-not-signed-by-pinniped",
wantStatus: http.StatusSeeOther,
wantContentType: htmlContentType,
// Generated a new CSRF cookie and set it in the response.
@@ -4217,7 +4217,7 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo
if test.wantCSRFValueInCookieHeader != "" {
require.Len(t, rsp.Header().Values("Set-Cookie"), 1)
actualCookie := rsp.Header().Get("Set-Cookie")
regex := regexp.MustCompile("__Host-pinniped-csrf=([^;]+); Path=/; HttpOnly; Secure; SameSite=Lax")
regex := regexp.MustCompile("__Host-pinniped-csrf-v2=([^;]+); Path=/; HttpOnly; Secure; SameSite=None")
submatches := regex.FindStringSubmatch(actualCookie)
require.Len(t, submatches, 2)
captured := submatches[1]

35
internal/federationdomain/endpoints/callback/callback_handler.go
View File

@@ -1,10 +1,11 @@
// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
// Copyright 2020-2025 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
// Package callback provides a handler for the OIDC callback endpoint.
package callback
import (
"mime"
"net/http"
"net/url"
"strings"
@@ -136,8 +137,36 @@ func authcode(r *http.Request) string {
}
func validateRequest(r *http.Request, stateDecoder, cookieDecoder oidc.Decoder, auditLogger plog.AuditLogger) (*oidc.UpstreamStateParamData, error) {
if r.Method != http.MethodGet {
return nil, httperr.Newf(http.StatusMethodNotAllowed, "%s (try GET)", r.Method)
// An upstream OIDC provider will typically return a redirect with the authcode,
// which the user's browser will follow and therefore perform a GET to this endpoint.
// See https://openid.net/specs/openid-connect-core-1_0.html#AuthResponse.
//
// If the upstream provider supports using response_mode=form_post,
// and if the admin configured OIDCIdentityProvider.spec.authorizationConfig.additionalAuthorizeParameters
// to include response_mode=form_post, then the user's browser will POST the authcode to this endpoint.
// See https://openid.net/specs/oauth-v2-form-post-response-mode-1_0.html.
if r.Method != http.MethodGet && r.Method != http.MethodPost {
return nil, httperr.Newf(http.StatusMethodNotAllowed, "%s (try GET or POST)", r.Method)
}
if r.Method == http.MethodPost {
// https://openid.net/specs/oauth-v2-form-post-response-mode-1_0.html says
// "the result parameters being encoded in the body using the application/x-www-form-urlencoded format",
// so only accept that format. Since "multipart/form-data" is intended for binary data, there's no need to
// support it here.
contentType := r.Header.Get("Content-Type")
if contentType == "" {
return nil, httperr.New(http.StatusUnsupportedMediaType, "no Content-Type header (try Content-Type: application/x-www-form-urlencoded)")
}
parsedContentType, _, err := mime.ParseMediaType(contentType)
if err != nil {
// Note that it should not be possible to reach this line since AuditRequestParams() is already
// parsing the form and handling unparseable form types before we get here. This is just defensive coding.
return nil, httperr.Newf(http.StatusUnsupportedMediaType, "cannot parse Content-Type: %s (try Content-Type: application/x-www-form-urlencoded)", contentType)
}
if parsedContentType != "application/x-www-form-urlencoded" {
return nil, httperr.Newf(http.StatusUnsupportedMediaType, "%s (try application/x-www-form-urlencoded)", contentType)
}
}
encodedState, decodedState, err := oidc.ReadStateParamAndValidateCSRFCookie(r, cookieDecoder, stateDecoder)

221
internal/federationdomain/endpoints/callback/callback_handler_test.go
View File

@@ -1,4 +1,4 @@
// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
// Copyright 2020-2025 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package callback
@@ -7,6 +7,7 @@ import (
"context"
"errors"
"fmt"
"io"
"net/http"
"net/http/httptest"
"net/url"
@@ -194,7 +195,7 @@ func TestCallbackEndpoint(t *testing.T) {
encodedIncomingCookieCSRFValue, err := happyCookieCodec.Encode("csrf", happyDownstreamCSRF)
require.NoError(t, err)
happyCSRFCookie := "__Host-pinniped-csrf=" + encodedIncomingCookieCSRFValue
happyCSRFCookie := "__Host-pinniped-csrf-v2=" + encodedIncomingCookieCSRFValue
happyOIDCUpstreamExchangeAuthcodeAndValidateTokenArgs := &oidctestutil.ExchangeAuthcodeAndValidateTokenArgs{
Authcode: happyUpstreamAuthcode,
@@ -229,6 +230,8 @@ func TestCallbackEndpoint(t *testing.T) {
kubeResources func(t *testing.T, supervisorClient *supervisorfake.Clientset, kubeClient *fake.Clientset)
method string
path string
body string
headers map[string]string
csrfCookie string
wantStatus int
@@ -318,6 +321,78 @@ func TestCallbackEndpoint(t *testing.T) {
}
},
},
{
name: "OIDC: POST (like when upstream is using response_mode=form_post) with good state and cookie and successful upstream token exchange with response_mode=form_post returns 200 with HTML+JS form",
idps: testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream().Build()),
method: http.MethodPost,
body: url.Values{
"code": []string{happyUpstreamAuthcode},
"state": []string{
happyOIDCUpstreamStateParam().WithAuthorizeRequestParams(
shallowCopyAndModifyQuery(
happyDownstreamRequestParamsQuery,
map[string]string{"response_mode": "form_post"},
).Encode(),
).Build(t, happyStateCodec).String(),
},
}.Encode(),
path: (&requestPath{}).String(),
headers: map[string]string{"Content-Type": "application/x-www-form-urlencoded; charset=utf-8"},
csrfCookie: happyCSRFCookie,
wantStatus: http.StatusOK,
wantContentType: "text/html;charset=UTF-8",
wantBodyFormResponseRegexp: `<code id="manual-auth-code">(.+)</code>`,
wantDownstreamIDTokenSubject: oidcUpstreamIssuer + "?idpName=" + happyOIDCUpstreamIDPName + "&sub=" + oidcUpstreamSubjectQueryEscaped,
wantDownstreamIDTokenUsername: oidcUpstreamUsername,
wantDownstreamIDTokenGroups: oidcUpstreamGroupMembership,
wantDownstreamRequestedScopes: happyDownstreamScopesRequested,
wantDownstreamGrantedScopes: happyDownstreamScopesGranted,
wantDownstreamNonce: downstreamNonce,
wantDownstreamClientID: downstreamPinnipedClientID,
wantDownstreamPKCEChallenge: downstreamPKCEChallenge,
wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod,
wantDownstreamCustomSessionData: happyDownstreamCustomSessionDataForOIDCUpstream,
wantOIDCAuthcodeExchangeCall: &expectedOIDCAuthcodeExchange{
performedByUpstreamName: happyOIDCUpstreamIDPName,
args: happyOIDCUpstreamExchangeAuthcodeAndValidateTokenArgs,
},
wantAuditLogs: func(encodedStateParam stateparam.Encoded, sessionID string) []testutil.WantedAuditLog {
return []testutil.WantedAuditLog{
testutil.WantAuditLog("HTTP Request Parameters", map[string]any{
"params": map[string]any{"code": "redacted", "state": "redacted"},
}),
testutil.WantAuditLog("AuthorizeID From Parameters", map[string]any{
"authorizeID": encodedStateParam.AuthorizeID(),
}),
testutil.WantAuditLog("Using Upstream IDP", map[string]any{
"displayName": "upstream-oidc-idp-name",
"resourceName": "upstream-oidc-idp-name",
"resourceUID": "upstream-oidc-resource-uid",
"type": "oidc",
}),
testutil.WantAuditLog("Identity From Upstream IDP", map[string]any{
"upstreamIDPDisplayName": "upstream-oidc-idp-name",
"upstreamIDPType": "oidc",
"upstreamIDPResourceName": "upstream-oidc-idp-name",
"upstreamIDPResourceUID": "upstream-oidc-resource-uid",
"personalInfo": map[string]any{
"upstreamUsername": "test-pinniped-username",
"upstreamGroups": []any{"test-pinniped-group-0", "test-pinniped-group-1"},
},
}),
testutil.WantAuditLog("Session Started", map[string]any{
"sessionID": sessionID,
"warnings": []any{}, // json: []
"personalInfo": map[string]any{
"username": "test-pinniped-username",
"groups": []any{"test-pinniped-group-0", "test-pinniped-group-1"},
"subject": "https://my-upstream-issuer.com?idpName=upstream-oidc-idp-name&sub=abc123-some+guid",
"additionalClaims": map[string]any{}, // json: {}
},
}),
}
},
},
{
name: "GitHub: GET with good state and cookie and successful upstream token exchange with response_mode=form_post returns 200 with HTML+JS form",
idps: testidplister.NewUpstreamIDPListerBuilder().WithGitHub(happyGitHubUpstream().Build()),
@@ -1131,7 +1206,7 @@ func TestCallbackEndpoint(t *testing.T) {
path: newRequestPath().String(),
wantStatus: http.StatusMethodNotAllowed,
wantContentType: htmlContentType,
wantBody: "Method Not Allowed: PUT (try GET)\n",
wantBody: "Method Not Allowed: PUT (try GET or POST)\n",
wantAuditLogs: func(encodedStateParam stateparam.Encoded, sessionID string) []testutil.WantedAuditLog {
return []testutil.WantedAuditLog{
testutil.WantAuditLog("HTTP Request Parameters", map[string]any{
@@ -1140,15 +1215,6 @@ func TestCallbackEndpoint(t *testing.T) {
}
},
},
{
name: "POST method is invalid",
idps: testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream().Build()),
method: http.MethodPost,
path: newRequestPath().String(),
wantStatus: http.StatusMethodNotAllowed,
wantContentType: htmlContentType,
wantBody: "Method Not Allowed: POST (try GET)\n",
},
{
name: "PATCH method is invalid",
idps: testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream().Build()),
@@ -1156,7 +1222,7 @@ func TestCallbackEndpoint(t *testing.T) {
path: newRequestPath().String(),
wantStatus: http.StatusMethodNotAllowed,
wantContentType: htmlContentType,
wantBody: "Method Not Allowed: PATCH (try GET)\n",
wantBody: "Method Not Allowed: PATCH (try GET or POST)\n",
},
{
name: "DELETE method is invalid",
@@ -1165,7 +1231,7 @@ func TestCallbackEndpoint(t *testing.T) {
path: newRequestPath().String(),
wantStatus: http.StatusMethodNotAllowed,
wantContentType: htmlContentType,
wantBody: "Method Not Allowed: DELETE (try GET)\n",
wantBody: "Method Not Allowed: DELETE (try GET or POST)\n",
},
{
name: "params cannot be parsed",
@@ -1267,6 +1333,42 @@ func TestCallbackEndpoint(t *testing.T) {
}
},
},
{
name: "code param was not included on request and there is no error param when the request was a POST with body",
idps: testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream().Build()),
method: http.MethodPost,
body: url.Values{
"state": []string{
happyOIDCUpstreamStateParam().WithAuthorizeRequestParams(
shallowCopyAndModifyQuery(
happyDownstreamRequestParamsQuery,
map[string]string{"response_mode": "form_post"},
).Encode(),
).Build(t, happyStateCodec).String(),
},
}.Encode(),
path: (&requestPath{}).String(),
headers: map[string]string{"Content-Type": "application/x-www-form-urlencoded"},
csrfCookie: happyCSRFCookie,
wantStatus: http.StatusBadRequest,
wantContentType: htmlContentType,
wantBody: here.Doc(`Bad Request: code param not found
Something went wrong with your authentication attempt at your external identity provider.
Pinniped AuditID: fake-audit-id
`),
wantAuditLogs: func(encodedStateParam stateparam.Encoded, sessionID string) []testutil.WantedAuditLog {
return []testutil.WantedAuditLog{
testutil.WantAuditLog("HTTP Request Parameters", map[string]any{
"params": map[string]any{"state": "redacted"},
}),
testutil.WantAuditLog("AuthorizeID From Parameters", map[string]any{
"authorizeID": encodedStateParam.AuthorizeID(),
}),
}
},
},
{
name: "state param was not included on request",
idps: testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream().Build()),
@@ -1284,6 +1386,77 @@ func TestCallbackEndpoint(t *testing.T) {
}
},
},
{
name: "state param was not included on request when the request was a POST with body",
idps: testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream().Build()),
method: http.MethodPost,
body: url.Values{"code": []string{happyUpstreamAuthcode}}.Encode(),
path: (&requestPath{}).String(),
headers: map[string]string{"Content-Type": "application/x-www-form-urlencoded"},
csrfCookie: happyCSRFCookie,
wantStatus: http.StatusBadRequest,
wantContentType: htmlContentType,
wantBody: "Bad Request: state param not found\n",
wantAuditLogs: func(encodedStateParam stateparam.Encoded, sessionID string) []testutil.WantedAuditLog {
return []testutil.WantedAuditLog{
testutil.WantAuditLog("HTTP Request Parameters", map[string]any{
"params": map[string]any{"code": "redacted"},
}),
}
},
},
{
name: "wrong Content-Type header included on request when the request was a POST with body",
idps: testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream().Build()),
method: http.MethodPost,
body: "code=" + happyUpstreamAuthcode,
path: (&requestPath{}).String(),
headers: map[string]string{"Content-Type": "application/text"},
csrfCookie: happyCSRFCookie,
wantStatus: http.StatusUnsupportedMediaType,
wantContentType: htmlContentType,
wantBody: "Unsupported Media Type: application/text (try application/x-www-form-urlencoded)\n",
wantAuditLogs: func(encodedStateParam stateparam.Encoded, sessionID string) []testutil.WantedAuditLog {
return []testutil.WantedAuditLog{
testutil.WantAuditLog("HTTP Request Parameters", map[string]any{
"params": map[string]any{},
}),
}
},
},
{
name: "unparseable Content-Type header included on request when the request was a POST with body",
idps: testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream().Build()),
method: http.MethodPost,
body: "code=" + happyUpstreamAuthcode,
path: (&requestPath{}).String(),
headers: map[string]string{"Content-Type": "bogus ;========="}, // this is unparseable garbage
csrfCookie: happyCSRFCookie,
wantStatus: http.StatusBadRequest,
wantContentType: htmlContentType,
wantBody: "Bad Request: error parsing request params\n",
wantAuditLogs: func(encodedStateParam stateparam.Encoded, sessionID string) []testutil.WantedAuditLog {
return []testutil.WantedAuditLog{} // couldn't parse params for auditing of params
},
},
{
name: "no Content-Type header included on request when the request was a POST with body",
idps: testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream().Build()),
method: http.MethodPost,
body: "code=" + happyUpstreamAuthcode,
path: (&requestPath{}).String(),
csrfCookie: happyCSRFCookie,
wantStatus: http.StatusUnsupportedMediaType,
wantContentType: htmlContentType,
wantBody: "Unsupported Media Type: no Content-Type header (try Content-Type: application/x-www-form-urlencoded)\n",
wantAuditLogs: func(encodedStateParam stateparam.Encoded, sessionID string) []testutil.WantedAuditLog {
return []testutil.WantedAuditLog{
testutil.WantAuditLog("HTTP Request Parameters", map[string]any{
"params": map[string]any{},
}),
}
},
},
{
name: "state param was not signed correctly, has expired, or otherwise cannot be decoded for any reason",
idps: testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream().Build()),
@@ -1624,7 +1797,7 @@ func TestCallbackEndpoint(t *testing.T) {
idps: testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream().Build()),
method: http.MethodGet,
path: newRequestPath().WithState(happyOIDCState).String(),
csrfCookie: "__Host-pinniped-csrf=this-value-was-not-signed-by-pinniped",
csrfCookie: "__Host-pinniped-csrf-v2=this-value-was-not-signed-by-pinniped",
wantStatus: http.StatusForbidden,
wantContentType: htmlContentType,
wantBody: "Forbidden: error reading CSRF cookie\n",
@@ -2024,10 +2197,19 @@ func TestCallbackEndpoint(t *testing.T) {
)
reqContext := context.WithValue(context.Background(), struct{ name string }{name: "test"}, "request-context")
req := httptest.NewRequest(test.method, test.path, nil).WithContext(reqContext)
var bodyReader io.Reader
if test.body != "" {
bodyReader = strings.NewReader(test.body)
}
req := httptest.NewRequest(test.method, test.path, bodyReader).WithContext(reqContext)
if test.csrfCookie != "" {
req.Header.Set("Cookie", test.csrfCookie)
}
if test.headers != nil {
for k, v := range test.headers {
req.Header.Set(k, v)
}
}
req, _ = auditid.NewRequestWithAuditID(req, func() string { return "fake-audit-id" })
rsp := httptest.NewRecorder()
subject.ServeHTTP(rsp, req)
@@ -2116,7 +2298,12 @@ func TestCallbackEndpoint(t *testing.T) {
}
if test.wantAuditLogs != nil {
wantAuditLogs := test.wantAuditLogs(testutil.GetStateParam(t, test.path), sessionID)
encodedStateParam := testutil.GetStateParamFromRequestURL(t, test.path)
if test.body != "" {
// Assume that the state param was sent in a form post body.
encodedStateParam = testutil.GetStateParamFromRequestBody(t, test.body)
}
wantAuditLogs := test.wantAuditLogs(encodedStateParam, sessionID)
testutil.WantAuditIDOnEveryAuditLog(wantAuditLogs, "fake-audit-id")
testutil.CompareAuditLogs(t, wantAuditLogs, actualAuditLog.String())
}

10
internal/federationdomain/endpoints/login/login_handler_test.go
View File

@@ -1,4 +1,4 @@
// Copyright 2022-2024 the Pinniped contributors. All Rights Reserved.
// Copyright 2022-2025 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package login
@@ -109,7 +109,7 @@ func TestLoginEndpoint(t *testing.T) {
encodedIncomingCookieCSRFValue, err := happyCookieCodec.Encode("csrf", happyDownstreamCSRF)
require.NoError(t, err)
happyCSRFCookie := "__Host-pinniped-csrf=" + encodedIncomingCookieCSRFValue
happyCSRFCookie := "__Host-pinniped-csrf-v2=" + encodedIncomingCookieCSRFValue
tests := []struct {
name string
@@ -261,7 +261,7 @@ func TestLoginEndpoint(t *testing.T) {
name: "the CSRF cookie was not signed correctly, has expired, or otherwise cannot be decoded for any reason on GET request",
method: http.MethodGet,
path: happyPathWithState,
csrfCookie: "__Host-pinniped-csrf=this-value-was-not-signed-by-pinniped",
csrfCookie: "__Host-pinniped-csrf-v2=this-value-was-not-signed-by-pinniped",
wantStatus: http.StatusForbidden,
wantContentType: htmlContentType,
wantBody: "Forbidden: error reading CSRF cookie\n",
@@ -270,7 +270,7 @@ func TestLoginEndpoint(t *testing.T) {
name: "the CSRF cookie was not signed correctly, has expired, or otherwise cannot be decoded for any reason on POST request",
method: http.MethodPost,
path: happyPathWithState,
csrfCookie: "__Host-pinniped-csrf=this-value-was-not-signed-by-pinniped",
csrfCookie: "__Host-pinniped-csrf-v2=this-value-was-not-signed-by-pinniped",
wantStatus: http.StatusForbidden,
wantContentType: htmlContentType,
wantBody: "Forbidden: error reading CSRF cookie\n",
@@ -519,7 +519,7 @@ func TestLoginEndpoint(t *testing.T) {
require.Equal(t, test.wantBody, rsp.Body.String())
if test.wantAuditLogs != nil {
wantAuditLogs := test.wantAuditLogs(testutil.GetStateParam(t, test.path))
wantAuditLogs := test.wantAuditLogs(testutil.GetStateParamFromRequestURL(t, test.path))
testutil.WantAuditIDOnEveryAuditLog(wantAuditLogs, "fake-audit-id")
testutil.CompareAuditLogs(t, wantAuditLogs, actualAuditLog.String())
}

4
internal/federationdomain/endpointsmanager/manager_test.go
View File

@@ -187,7 +187,7 @@ func TestManager(t *testing.T) {
cookies := recorder.Result().Cookies()
r.Len(cookies, 1)
csrfCookie := cookies[0]
r.Equal("__Host-pinniped-csrf", csrfCookie.Name)
r.Equal("__Host-pinniped-csrf-v2", csrfCookie.Name)
r.NotEmpty(csrfCookie.Value)
// Return the important parts of the response so we can use them in our next request to the callback endpoint
@@ -201,7 +201,7 @@ func TestManager(t *testing.T) {
getRequest := newGetRequest(requestIssuer + oidc.CallbackEndpointPath + requestURLSuffix)
getRequest.AddCookie(&http.Cookie{
Name: "__Host-pinniped-csrf",
Name: "__Host-pinniped-csrf-v2",
Value: csrfCookieValue,
})
subject.HandlerChain().ServeHTTP(recorder, getRequest)

4
internal/federationdomain/oidc/oidc.go
View File

@@ -61,9 +61,11 @@ const (
UpstreamStateParamEncodingName = "s"
// CSRFCookieName is the name of the browser cookie which shall hold our CSRF value.
// The "-v2" suffix was added when the SameSite value was changed from Lax to None,
// to force the creation and use of a new cookie upon upgrade.
// The `__Host` prefix has a special meaning. See:
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#Cookie_prefixes.
CSRFCookieName = "__Host-pinniped-csrf"
CSRFCookieName = "__Host-pinniped-csrf-v2"
// CSRFCookieEncodingName is the `name` passed to the encoder for encoding and decoding the CSRF
// cookie contents.

10
internal/testutil/log_lines.go
View File

@@ -1,4 +1,4 @@
// Copyright 2024 the Pinniped contributors. All Rights Reserved.
// Copyright 2025 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package testutil
@@ -44,7 +44,7 @@ func WantAuditIDOnEveryAuditLog(wantedAuditLogs []WantedAuditLog, wantAuditID st
}
}
func GetStateParam(t *testing.T, fullURL string) stateparam.Encoded {
func GetStateParamFromRequestURL(t *testing.T, fullURL string) stateparam.Encoded {
if fullURL == "" {
var empty stateparam.Encoded
return empty
@@ -55,6 +55,12 @@ func GetStateParam(t *testing.T, fullURL string) stateparam.Encoded {
return stateparam.Encoded(path.Query().Get("state"))
}
func GetStateParamFromRequestBody(t *testing.T, body string) stateparam.Encoded {
values, err := url.ParseQuery(body)
require.NoError(t, err)
return stateparam.Encoded(values.Get("state"))
}
func CompareAuditLogs(t *testing.T, wantAuditLogs []WantedAuditLog, actualAuditLogsOneLiner string) {
t.Helper()

23
test/integration/supervisor_login_test.go
View File

@@ -1,4 +1,4 @@
// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
// Copyright 2020-2025 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package integration
@@ -362,6 +362,27 @@ func TestSupervisorLogin_Browser(t *testing.T) {
// the ID token Username should include the upstream user ID after the upstream issuer name
wantDownstreamIDTokenUsernameToMatch: func(_ string) string { return "^" + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Issuer+"?sub=") + ".+" },
},
{
name: "oidc with upstream response_mode=form_post",
maybeSkip: skipNever,
createIDP: func(t *testing.T) string {
providerSpec := basicOIDCIdentityProviderSpec()
providerSpec.AuthorizationConfig.AdditionalAuthorizeParameters = []idpv1alpha1.Parameter{{
// Note that at the time of writing this comment, Dex completely ignores this param
// and just treats it as the default response_mode by returning a redirect.
// However, when the external OIDC provider is Okta, this should actually test
// using this capability. Okta will cause the user's browser to POST the authcode
// to the Supervisor's callback endpoint.
Name: "response_mode",
Value: "form_post",
}}
return testlib.CreateTestOIDCIdentityProvider(t, providerSpec, idpv1alpha1.PhaseReady).Name
},
requestAuthorization: requestAuthorizationUsingBrowserAuthcodeFlowOIDC,
wantDownstreamIDTokenSubjectToMatch: expectedIDTokenSubjectRegexForUpstreamOIDC,
// the ID token Username should include the upstream user ID after the upstream issuer name
wantDownstreamIDTokenUsernameToMatch: func(_ string) string { return "^" + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Issuer+"?sub=") + ".+" },
},
{
name: "oidc IDP using secrets of type opaque to source ca bundle with default username and groups claim settings",
maybeSkip: skipExternalCABundleOIDCTestsWhenCABundleIsEmpty,