mirrors/pinniped
1
0
Fork 0
mirror of https://github.com/vmware-tanzu/pinniped.git synced 2026-01-10 07:58:07 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

token handler uses common method to audit HTTP request parameters

Browse Source
This commit is contained in:
Ryan Richard
2024-11-13 11:56:00 -08:00
committed by Joshua Casey
parent eab3fde3af
commit c06141c871
8 changed files with 24 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -272,14 +272,14 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo
// Inject this into our test subject at the last second so we get a fresh storage for every test.
// Use lower minimum required bcrypt cost than we would use in production to keep unit the tests fast.
kubeOauthStore := storage.NewKubeStorage(secretsClient, oidcClientsClient, timeoutsConfiguration, bcrypt.MinCost)
return oidc.FositeOauth2Helper(kubeOauthStore, downstreamIssuer, hmacSecretFunc, jwksProviderIsUnused, timeoutsConfiguration, nil), kubeOauthStore
return oidc.FositeOauth2Helper(kubeOauthStore, downstreamIssuer, hmacSecretFunc, jwksProviderIsUnused, timeoutsConfiguration), kubeOauthStore
}
createOauthHelperWithNullStorage := func(secretsClient v1.SecretInterface, oidcClientsClient v1alpha1.OIDCClientInterface) (fosite.OAuth2Provider, *storage.NullStorage) {
// Configure fosite the same way that the production code would, using NullStorage to turn off storage.
// Use lower minimum required bcrypt cost than we would use in production to keep unit the tests fast.
nullOauthStore := storage.NewNullStorage(secretsClient, oidcClientsClient, bcrypt.MinCost)
return oidc.FositeOauth2Helper(nullOauthStore, downstreamIssuer, hmacSecretFunc, jwksProviderIsUnused, timeoutsConfiguration, nil), nullOauthStore
return oidc.FositeOauth2Helper(nullOauthStore, downstreamIssuer, hmacSecretFunc, jwksProviderIsUnused, timeoutsConfiguration), nullOauthStore
}
upstreamAuthURL, err := url.Parse("https://some-upstream-idp:8443/auth")

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

@@ -1961,7 +1961,7 @@ func TestCallbackEndpoint(t *testing.T) {
hmacSecretFunc := func() []byte { return []byte("some secret - must have at least 32 bytes") }
require.GreaterOrEqual(t, len(hmacSecretFunc()), 32, "fosite requires that hmac secrets have at least 32 bytes")
jwksProviderIsUnused := jwks.NewDynamicJWKSProvider()
oauthHelper := oidc.FositeOauth2Helper(oauthStore, downstreamIssuer, hmacSecretFunc, jwksProviderIsUnused, timeoutsConfiguration, nil)
oauthHelper := oidc.FositeOauth2Helper(oauthStore, downstreamIssuer, hmacSecretFunc, jwksProviderIsUnused, timeoutsConfiguration)
auditLogger, actualAuditLog := plog.TestAuditLogger(t)

2
internal/federationdomain/endpoints/login/post_login_handler_test.go
View File

@@ -1335,7 +1335,7 @@ func TestPostLoginEndpoint(t *testing.T) {
hmacSecretFunc := func() []byte { return []byte("some secret - must have at least 32 bytes") }
require.GreaterOrEqual(t, len(hmacSecretFunc()), 32, "fosite requires that hmac secrets have at least 32 bytes")
jwksProviderIsUnused := jwks.NewDynamicJWKSProvider()
oauthHelper := oidc.FositeOauth2Helper(kubeOauthStore, downstreamIssuer, hmacSecretFunc, jwksProviderIsUnused, timeoutsConfiguration, nil)
oauthHelper := oidc.FositeOauth2Helper(kubeOauthStore, downstreamIssuer, hmacSecretFunc, jwksProviderIsUnused, timeoutsConfiguration)
req := httptest.NewRequest(http.MethodPost, "/ignored", strings.NewReader(tt.formParams.Encode()))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")

17
internal/federationdomain/endpoints/token/token_handler.go
View File

@@ -31,6 +31,18 @@ import (
"go.pinniped.dev/internal/psession"
)
func paramsSafeToLog() sets.Set[string] {
return sets.New(
// Standard params from https://openid.net/specs/openid-connect-core-1_0.html for authcode and refresh grants.
// Redacting code, client_secret, refresh_token, and PKCE code_verifier params.
"grant_type", "client_id", "redirect_uri", "scope",
// Token exchange params from https://datatracker.ietf.org/doc/html/rfc8693.
// Redact subject_token and actor_token.
// We don't allow all of these, but they should be safe to log.
"audience", "resource", "scope", "requested_token_type", "actor_token_type", "subject_token_type",
)
}
func NewHandler(
idpLister federationdomainproviders.FederationDomainIdentityProvidersListerI,
oauthHelper fosite.OAuth2Provider,
@@ -39,6 +51,11 @@ func NewHandler(
auditLogger plog.AuditLogger,
) http.Handler {
return httperr.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
if err := auditLogger.AuditRequestParams(r, paramsSafeToLog()); err != nil {
oauthHelper.WriteAccessError(r.Context(), w, nil, err)
return nil
}
session := psession.NewPinnipedSession()
accessRequest, err := oauthHelper.NewAccessRequest(r.Context(), r, session)
if err != nil {

7
internal/federationdomain/endpoints/token/token_handler_test.go
View File

@@ -128,7 +128,7 @@ var (
fositeInvalidPayloadErrorBody = here.Doc(`
{
"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. Unable to parse HTTP body, make sure to send a properly formatted form request body."
"error_description": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed. Unable to parse form params, make sure to send a properly formatted query params or form request body."
}
`)
@@ -5117,7 +5117,7 @@ func exchangeAuthcodeForTokens(
var oauthHelper fosite.OAuth2Provider
// Note that makeHappyOauthHelper() calls simulateAuthEndpointHavingAlreadyRun() to preload the session storage.
oauthHelper, authCode, jwtSigningKey = makeHappyOauthHelper(t, authRequest, oauthStore, test.makeJwksSigningKeyAndProvider, test.customSessionData, test.modifySession, auditLogger)
oauthHelper, authCode, jwtSigningKey = makeHappyOauthHelper(t, authRequest, oauthStore, test.makeJwksSigningKeyAndProvider, test.customSessionData, test.modifySession)
subject = NewHandler(
idps,
@@ -5336,12 +5336,11 @@ func makeHappyOauthHelper(
makeJwksSigningKeyAndProvider MakeJwksSigningKeyAndProviderFunc,
initialCustomSessionData *psession.CustomSessionData,
modifySession func(session *psession.PinnipedSession),
auditLogger plog.AuditLogger,
) (fosite.OAuth2Provider, string, *ecdsa.PrivateKey) {
t.Helper()
jwtSigningKey, jwkProvider := makeJwksSigningKeyAndProvider(t, goodIssuer)
oauthHelper := oidc.FositeOauth2Helper(store, goodIssuer, hmacSecretFunc, jwkProvider, oidc.DefaultOIDCTimeoutsConfiguration(), auditLogger)
oauthHelper := oidc.FositeOauth2Helper(store, goodIssuer, hmacSecretFunc, jwkProvider, oidc.DefaultOIDCTimeoutsConfiguration())
authResponder := simulateAuthEndpointHavingAlreadyRun(t, authRequest, oauthHelper, initialCustomSessionData, modifySession)
return oauthHelper, authResponder.GetCode(), jwtSigningKey
}

61
internal/federationdomain/endpoints/tokenendpointauditor/parameter_auditor.go
View File

@@ -1,61 +0,0 @@
// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package tokenendpointauditor
import (
"context"
"github.com/ory/fosite"
"github.com/ory/fosite/compose"
"k8s.io/apimachinery/pkg/util/sets"
"go.pinniped.dev/internal/auditevent"
"go.pinniped.dev/internal/plog"
)
type parameterAuditorHandler struct {
auditLogger plog.AuditLogger
}
func AuditorHandlerFactory(auditLogger plog.AuditLogger) compose.Factory {
return func(_ fosite.Configurator, _ any, _ any) any {
return &parameterAuditorHandler{
auditLogger: auditLogger,
}
}
}
var _ fosite.TokenEndpointHandler = (*parameterAuditorHandler)(nil)
func (p parameterAuditorHandler) PopulateTokenEndpointResponse(_ context.Context, _ fosite.AccessRequester, _ fosite.AccessResponder) error {
return nil
}
func (p parameterAuditorHandler) HandleTokenEndpointRequest(_ context.Context, _ fosite.AccessRequester) error {
return nil
}
func (p parameterAuditorHandler) CanSkipClientAuth(_ context.Context, _ fosite.AccessRequester) bool {
return false
}
func paramsSafeToLogTokenEndpoint() sets.Set[string] {
return sets.New(
// Standard params from https://openid.net/specs/openid-connect-core-1_0.html for authcode and refresh grants.
// Redacting code, client_secret, refresh_token, and PKCE code_verifier params.
"grant_type", "client_id", "redirect_uri", "scope",
// Token exchange params from https://datatracker.ietf.org/doc/html/rfc8693.
// Redact subject_token and actor_token.
// We don't allow all of these, but they should be safe to log.
"audience", "resource", "scope", "requested_token_type", "actor_token_type", "subject_token_type",
)
}
func (p parameterAuditorHandler) CanHandleTokenEndpointRequest(ctx context.Context, requester fosite.AccessRequester) bool {
p.auditLogger.Audit(auditevent.HTTPRequestParameters, &plog.AuditParams{
ReqCtx: ctx,
KeysAndValues: auditevent.SanitizeParams(requester.GetRequestForm(), paramsSafeToLogTokenEndpoint()),
})
return false
}

2
internal/federationdomain/endpointsmanager/manager.go
View File

@@ -119,7 +119,6 @@ func (m *Manager) SetFederationDomains(federationDomains ...*federationdomainpro
tokenHMACKeyGetter,
nil,
timeoutsConfiguration,
m.auditLogger,
)
// For all the other endpoints, make another oauth helper with exactly the same settings except use real storage.
@@ -129,7 +128,6 @@ func (m *Manager) SetFederationDomains(federationDomains ...*federationdomainpro
tokenHMACKeyGetter,
m.dynamicJWKSProvider,
timeoutsConfiguration,
m.auditLogger,
)
upstreamStateEncoder := dynamiccodec.New(

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

@@ -22,7 +22,6 @@ import (
"go.pinniped.dev/internal/federationdomain/clientregistry"
"go.pinniped.dev/internal/federationdomain/csrftoken"
"go.pinniped.dev/internal/federationdomain/endpoints/jwks"
"go.pinniped.dev/internal/federationdomain/endpoints/tokenendpointauditor"
"go.pinniped.dev/internal/federationdomain/endpoints/tokenexchange"
"go.pinniped.dev/internal/federationdomain/formposthtml"
"go.pinniped.dev/internal/federationdomain/idtokenlifespan"
@@ -232,7 +231,6 @@ func FositeOauth2Helper(
hmacSecretOfLengthAtLeast32Func func() []byte,
jwksProvider jwks.DynamicJWKSProvider,
timeoutsConfiguration timeouts.Configuration,
auditLogger plog.AuditLogger,
) fosite.OAuth2Provider {
oauthConfig := &fosite.Config{
IDTokenIssuer: issuer,
@@ -273,8 +271,6 @@ func FositeOauth2Helper(
CoreStrategy: strategy.NewDynamicOauth2HMACStrategy(oauthConfig, hmacSecretOfLengthAtLeast32Func),
OpenIDConnectTokenStrategy: strategy.NewDynamicOpenIDConnectECDSAStrategy(oauthConfig, jwksProvider),
},
// Put this before others to make sure it logs params!
tokenendpointauditor.AuditorHandlerFactory(auditLogger),
compose.OAuth2AuthorizeExplicitFactory,
compose.OAuth2RefreshTokenGrantFactory,
// Use a custom factory to allow selective overrides of the ID token lifespan during authcode exchange.