diff --git a/core/authenticate/service.go b/core/authenticate/service.go index 02c451845..c50e88ce1 100644 --- a/core/authenticate/service.go +++ b/core/authenticate/service.go @@ -236,7 +236,8 @@ func (s Service) StartFlow(ctx context.Context, request RegistrationStartRequest } // the consent gate runs first: a check on the request alone, costing no // lookup, and it has to fail before anything is sent or redirected - if err := s.gateFlowConsent(request.Intent, request.AcceptedDocumentIDs); err != nil { + consented, err := s.gateFlowConsent(request.Intent, request.AcceptedDocumentIDs) + if err != nil { return nil, err } // both mail strategies know the address before anything is sent, and share @@ -262,7 +263,7 @@ func (s Service) StartFlow(ctx context.Context, request RegistrationStartRequest if request.Intent != FlowIntentUnspecified { flow.Metadata[flowIntentKey] = request.Intent.String() } - if len(request.AcceptedDocumentIDs) > 0 { + if len(consented) > 0 { flow.Metadata[flowConsentKey] = map[string]any{ consentDocumentIDsKey: request.AcceptedDocumentIDs, consentIPAddressKey: request.IPAddress, @@ -473,22 +474,23 @@ func (s Service) gateFlowStart(ctx context.Context, intent FlowIntent, email str // or redirected — true for OIDC too, where the email is unknown but the intent // is not. Without an intent only the unknown-id rule runs, and completeness // waits for user creation. A login checks nothing, because it writes no record. -func (s Service) gateFlowConsent(intent FlowIntent, ids []string) error { +func (s Service) gateFlowConsent(intent FlowIntent, ids []string) ([]consent.Document, error) { if s.consentService == nil || intent == FlowIntentLogin { - return nil + return nil, nil } + var documents []consent.Document var err error if intent == FlowIntentSignup { - _, err = s.consentService.ResolveAll(ids) + documents, err = s.consentService.ResolveAll(ids) } else { - _, err = s.consentService.Resolve(ids) + documents, err = s.consentService.Resolve(ids) } if err != nil { // the wrapped error names what is missing, for the log not the response - return fmt.Errorf("%w: %w", ErrConsentRequired, err) + return nil, fmt.Errorf("%w: %w", ErrConsentRequired, err) } - return nil + return documents, nil } // applyMailOTP actions when user submitted otp from the email diff --git a/core/authenticate/service_test.go b/core/authenticate/service_test.go index 0a1f82441..71c3d7c87 100644 --- a/core/authenticate/service_test.go +++ b/core/authenticate/service_test.go @@ -1301,10 +1301,16 @@ func TestService_StartFlow_WritesIntentAndConsent(t *testing.T) { storedFlow = flow }).Return(nil) + enabled := consent.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), + consent.Config{Enabled: true, Documents: map[string]consent.DocumentConfig{ + "terms_of_service": {Title: "Terms & Conditions", Version: "v2", URL: "https://example.org/t"}, + "privacy_policy": {Title: "Privacy Policy", Version: "v1", URL: "https://example.org/p"}, + }}, nil, nil) + srv := authenticate.NewService(nil, authenticate.Config{ MailOTP: authenticate.MailOTPConfig{Validity: 10 * time.Minute}, TestUsers: testusers.Config{Enabled: true, OTP: "111111", Domain: "example.com"}, - }, mockFlowRepo, mailer.NewMockDialer(), nil, nil, mockUserService, nil, nil, nil, nil, nil) + }, mockFlowRepo, mailer.NewMockDialer(), nil, nil, mockUserService, nil, nil, nil, enabled, nil) srv.Now = func() time.Time { return timeNow } _, err := srv.StartFlow(ctx, request) @@ -1464,14 +1470,18 @@ func TestService_StartFlow_Consent(t *testing.T) { assert.ErrorIs(t, err, consent.ErrUnknownDocuments) }) - t.Run("a login checks nothing, because it writes no record", func(t *testing.T) { + t.Run("a login checks nothing, and persists nothing, because it writes no record", func(t *testing.T) { // an unexpected call fails the test rather than passing silently mockConsent := mocks.NewConsentService(t) mockFlowRepo, mockUserService, _, _, _ := createMocks(t) ctx := context.Background() mockUserService.EXPECT().GetByID(ctx, email).Return(user.User{ID: "user-id", Email: email}, nil) - mockFlowRepo.EXPECT().Set(ctx, mock.Anything).Return(nil) + + var storedFlow *authenticate.Flow + mockFlowRepo.EXPECT().Set(ctx, mock.Anything).Run(func(_ context.Context, flow *authenticate.Flow) { + storedFlow = flow + }).Return(nil) srv := authenticate.NewService(nil, authenticate.Config{ MailOTP: authenticate.MailOTPConfig{Validity: 10 * time.Minute}, @@ -1479,11 +1489,17 @@ func TestService_StartFlow_Consent(t *testing.T) { }, mockFlowRepo, mailer.NewMockDialer(), nil, nil, mockUserService, nil, nil, nil, mockConsent, nil) _, err := srv.StartFlow(ctx, authenticate.RegistrationStartRequest{ - Method: authenticate.MailOTPAuthMethod.String(), - Email: email, - Intent: authenticate.FlowIntentLogin, + Method: authenticate.MailOTPAuthMethod.String(), + Email: email, + Intent: authenticate.FlowIntentLogin, + AcceptedDocumentIDs: acceptedIDs, + IPAddress: "10.0.0.1", }) require.NoError(t, err) + require.NotNil(t, storedFlow) + + _, ok := storedFlow.Consent() + assert.False(t, ok, "a login must persist no consent block") }) t.Run("a deployment with consent disabled ignores the ids rather than rejecting them", func(t *testing.T) { @@ -1500,6 +1516,40 @@ func TestService_StartFlow_Consent(t *testing.T) { require.NoError(t, err) require.NotNil(t, got) }) + + t.Run("a deployment with consent disabled persists no consent onto the flow", func(t *testing.T) { + disabled := consent.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), + consent.Config{Enabled: false}, nil, nil) + + ctx := context.Background() + mockFlowRepo, mockUserService, _, _, _ := createMocks(t) + mockUserService.EXPECT().GetByID(ctx, email). + Return(user.User{}, errors.New("user not found")).Maybe() + + var storedFlow *authenticate.Flow + mockFlowRepo.EXPECT().Set(ctx, mock.Anything).Run(func(_ context.Context, flow *authenticate.Flow) { + storedFlow = flow + }).Return(nil) + + srv := authenticate.NewService(nil, authenticate.Config{ + MailOTP: authenticate.MailOTPConfig{Validity: 10 * time.Minute}, + TestUsers: testusers.Config{Enabled: true, OTP: "111111", Domain: "example.com"}, + }, mockFlowRepo, mailer.NewMockDialer(), nil, nil, mockUserService, nil, nil, nil, disabled, nil) + + _, err := srv.StartFlow(ctx, authenticate.RegistrationStartRequest{ + Method: authenticate.MailOTPAuthMethod.String(), + Email: email, + Intent: authenticate.FlowIntentSignup, + AcceptedDocumentIDs: acceptedIDs, + IPAddress: "10.0.0.1", + }) + require.NoError(t, err) + require.NotNil(t, storedFlow) + + _, ok := storedFlow.Consent() + assert.False(t, ok, "a disabled deployment must persist no consent block") + assert.Equal(t, authenticate.FlowIntentSignup, storedFlow.Intent()) + }) } // TestFlow_IntentAndConsent covers the accessors directly: the JSON round trip