From 26c0c4479d36b8778919d3a83efecadc39da565e Mon Sep 17 00:00:00 2001 From: Jim Date: Fri, 9 Apr 2021 10:41:54 -0400 Subject: [PATCH] send MaxAge when set for auth method (#1094) --- internal/auth/oidc/service_start_auth.go | 15 ++++++-- internal/auth/oidc/service_start_auth_test.go | 34 +++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/internal/auth/oidc/service_start_auth.go b/internal/auth/oidc/service_start_auth.go index 3b54d87ba3..9740e5d2c5 100644 --- a/internal/auth/oidc/service_start_auth.go +++ b/internal/auth/oidc/service_start_auth.go @@ -99,14 +99,25 @@ func StartAuth(ctx context.Context, oidcRepoFn OidcRepoFactory, authMethodId str if err != nil { return nil, nil, "", errors.Wrap(err, op) } + oidcOpts := []oidc.Option{ + oidc.WithState(string(encodedEncryptedSt)), + oidc.WithNonce(nonce), + } + switch { + case am.MaxAge == -1: + oidcOpts = append(oidcOpts, oidc.WithMaxAge(0)) + case am.MaxAge > 0: + oidcOpts = append(oidcOpts, oidc.WithMaxAge(uint(am.MaxAge))) + default: + } + // a bare min oidc.Request needed for the provider.AuthURL(...) call. We've intentionally not populated // things like Audiences, because this oidc.Request isn't cached and not intended for use in future legs // of the authen flow. oidcReq, err := oidc.NewRequest( AttemptExpiration, callbackRedirect, - oidc.WithState(string(encodedEncryptedSt)), - oidc.WithNonce(nonce)) + oidcOpts...) if err != nil { return nil, nil, "", errors.New(errors.Unknown, op, "unable to create oidc request", errors.WithWrap(err)) } diff --git a/internal/auth/oidc/service_start_auth_test.go b/internal/auth/oidc/service_start_auth_test.go index abfa89e8e7..e46234867d 100644 --- a/internal/auth/oidc/service_start_auth_test.go +++ b/internal/auth/oidc/service_start_auth_test.go @@ -6,6 +6,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "strconv" "testing" "time" @@ -60,6 +61,16 @@ func Test_StartAuth(t *testing.T) { WithCertificates(tpCert...), ) + testAuthMethodWithMaxAge := TestAuthMethod( + t, conn, databaseWrapper, org.PublicId, ActivePublicState, + "test-rp3", "fido", + WithIssuer(TestConvertToUrls(t, tp.Addr())[0]), + WithApiUrl(TestConvertToUrls(t, testController.URL)[0]), + WithSigningAlgs(Alg(tpAlg)), + WithCertificates(tpCert...), + WithMaxAge(-1), + ) + stdSetup := func(am *AuthMethod, repoFn OidcRepoFactory, apiSrv *httptest.Server) (a *AuthMethod, allowedRedirect string) { // update the allowed redirects for the TestProvider tpAllowedRedirect := fmt.Sprintf(CallbackEndpoint, apiSrv.URL, am.PublicId) @@ -97,6 +108,13 @@ func Test_StartAuth(t *testing.T) { setup: stdSetup, roundTripper: `{"alice": "hi bob", "eve": "chuckles says hi alice"}`, }, + { + name: "simple-with-max-age", + repoFn: repoFn, + apiSrv: testController, + authMethod: testAuthMethodWithMaxAge, + setup: stdSetup, + }, { name: "inactive", repoFn: repoFn, @@ -163,6 +181,22 @@ func Test_StartAuth(t *testing.T) { require.Equal(1, len(authParams["nonce"])) require.Equal(1, len(authParams["state"])) + if tt.authMethod.MaxAge != 0 { + var expected int + switch { + case tt.authMethod.MaxAge == -1: + expected = 0 + default: + expected = int(tt.authMethod.MaxAge) + } + gotSlice, ok := authParams["max_age"] + require.True(ok) + require.Equal(1, len(gotSlice)) + got, err := strconv.Atoi(gotSlice[0]) + require.NoError(err) + assert.Equal(expected, got) + } + // verify the state in the authUrl can be decrypted and it's correct state := authParams["state"][0] wrappedStReq, err := unwrapMessage(ctx, state)