From fac50ad138cc39c18442b474ed95b408e58ed20c Mon Sep 17 00:00:00 2001 From: Rohan Chakraborty Date: Sun, 30 Aug 2026 19:37:36 +0530 Subject: [PATCH] feat(authenticate): separate login from signup with an explicit intent Frontier cannot tell a signup from a login: every strategy ends at getOrCreateUser, which returns the existing user or creates one, so a login with an unknown address silently creates the account. Adds a FlowIntent carried on flow metadata and both gates from the RFC's intent by strategy table. StartFlow is the fast path, rejecting a login with no account or a signup with one before an OTP is sent, for the strategies that know the email that early. User creation is the gate that matters, since every strategy reaches it and OIDC has no email until the callback. The intent also replaces the passkey guess: signup picks the register ceremony, login picks the login ceremony. An unspecified intent keeps today's create-or-get and today's guess, so existing clients and deployments are unaffected. Flow.Metadata is existing JSONB, so no migration. The accessors parse rather than assert, since JSONB does not return the types it was given, and are nil-receiver-safe so the caller without a flow needs no branch. Error mapping at the handlers and all consent work follow separately. Refs docs/rfcs/0002-explicit-consent-at-signup.md Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VW3nysiE4H83VQk6BroMYc --- core/authenticate/authenticate.go | 111 +++++++ core/authenticate/authenticators.go | 3 +- core/authenticate/service.go | 106 ++++++- core/authenticate/service_test.go | 477 ++++++++++++++++++++++++++++ 4 files changed, 680 insertions(+), 17 deletions(-) diff --git a/core/authenticate/authenticate.go b/core/authenticate/authenticate.go index d722b9345f..3d37e8bc7b 100644 --- a/core/authenticate/authenticate.go +++ b/core/authenticate/authenticate.go @@ -63,6 +63,40 @@ var APIAssertions = []ClientAssertion{ PassthroughHeaderClientAssertion, } +// FlowIntent says whether the caller wants to log an existing user in or create +// a new one. It rides on the flow, the only thing that survives an OIDC redirect. +type FlowIntent string + +const ( + // FlowIntentUnspecified keeps the old create-or-get behaviour, so clients + // that send no intent are unaffected. + FlowIntentUnspecified FlowIntent = "" + FlowIntentLogin FlowIntent = "login" + FlowIntentSignup FlowIntent = "signup" +) + +func (i FlowIntent) String() string { + return string(i) +} + +// keys under Flow.Metadata. +const ( + flowIntentKey = "intent" + flowConsentKey = "consent" + + consentDocumentIDsKey = "accepted_document_ids" + consentIPAddressKey = "ip_address" + consentAtKey = "at" +) + +// FlowConsent is what the user accepted at flow start, read back after the +// redirect. The IP and timestamp are from the acceptance, not the callback. +type FlowConsent struct { + AcceptedDocumentIDs []string + IPAddress string + At time.Time +} + // Flow is a temporary state used to finish login/registration flows type Flow struct { ID uuid.UUID @@ -94,6 +128,74 @@ func (f Flow) IsValid(currentTime time.Time) bool { return f.ExpiresAt.After(currentTime) } +// Intent reads the flow intent from metadata. A nil flow is allowed, so callers +// with no flow need no branch, and reads as unspecified. +func (f *Flow) Intent() FlowIntent { + if f == nil { + return FlowIntentUnspecified + } + intent, ok := f.Metadata[flowIntentKey].(string) + if !ok { + return FlowIntentUnspecified + } + return FlowIntent(intent) +} + +// Consent reads what the user accepted from metadata, reporting whether the flow +// carries one at all. A nil flow is allowed. +// +// Metadata is JSONB and does not return the types it was given — ids come back +// as []any and the timestamp as a string — so this parses rather than asserts, +// and treats an unparseable key as no consent. +func (f *Flow) Consent() (FlowConsent, bool) { + if f == nil { + return FlowConsent{}, false + } + raw, ok := f.Metadata[flowConsentKey].(map[string]any) + if !ok { + return FlowConsent{}, false + } + + consent := FlowConsent{ + AcceptedDocumentIDs: parseStringSlice(raw[consentDocumentIDsKey]), + } + if ip, ok := raw[consentIPAddressKey].(string); ok { + consent.IPAddress = ip + } + switch at := raw[consentAtKey].(type) { + case time.Time: + consent.At = at + case string: + if parsed, err := time.Parse(time.RFC3339Nano, at); err == nil { + consent.At = parsed + } + } + if len(consent.AcceptedDocumentIDs) == 0 { + // a consent that names no document is not a consent + return FlowConsent{}, false + } + return consent, true +} + +// parseStringSlice reads a string list that may have been through a JSON round +// trip, where it comes back as []any. Non-string entries are dropped. +func parseStringSlice(value any) []string { + switch list := value.(type) { + case []string: + return list + case []any: + parsed := make([]string, 0, len(list)) + for _, item := range list { + if str, ok := item.(string); ok { + parsed = append(parsed, str) + } + } + return parsed + default: + return nil + } +} + type RegistrationStartRequest struct { Method string // ReturnToURL is where flow should end to after successful verification @@ -106,6 +208,15 @@ type RegistrationStartRequest struct { // For most cases it could be host of frontier but in case of proxies, this will be proxy public endpoint. // callback_url should be one of the allowed urls configured at instance level CallbackUrl string + + // Intent says whether this is a login or a signup. Unset keeps create-or-get. + Intent FlowIntent + // AcceptedDocumentIDs are stored on the flow so they survive the redirect to + // an identity provider, and are checked when the user is created. + AcceptedDocumentIDs []string + // IPAddress is where the acceptance came from. Authenticate is skip-listed, + // so the handler extracts it rather than reading it off the context. + IPAddress string } type RegistrationFinishRequest struct { diff --git a/core/authenticate/authenticators.go b/core/authenticate/authenticators.go index 92c8dfae4d..321936100b 100644 --- a/core/authenticate/authenticators.go +++ b/core/authenticate/authenticators.go @@ -277,7 +277,8 @@ func authenticateWithPassthroughHeader(ctx context.Context, s *Service) (Princip return Principal{}, errSkip } - currentUser, err := s.getOrCreateUser(ctx, strings.TrimSpace(val), strings.Split(val, "@")[0]) + // no flow on this path; the nil-safe accessors make that a nil argument + currentUser, err := s.getOrCreateUser(ctx, nil, strings.TrimSpace(val), strings.Split(val, "@")[0]) if err != nil { s.log.DebugContext(ctx, "failed to get user", "err", err) return Principal{}, err diff --git a/core/authenticate/service.go b/core/authenticate/service.go index 7c8bc75b16..da8c1cf88e 100644 --- a/core/authenticate/service.go +++ b/core/authenticate/service.go @@ -59,6 +59,8 @@ var ( ErrInvalidOIDCState = errors.New("invalid auth state") ErrFlowInvalid = errors.New("invalid flow or expired") ErrOIDCTokenExchange = errors.New("failed to exchange oidc authorization code") + ErrLoginUserNotFound = errors.New("no account for this email") + ErrSignupUserExists = errors.New("an account already exists for this email") ) type UserService interface { @@ -203,6 +205,14 @@ func (s Service) StartFlow(ctx context.Context, request RegistrationStartRequest if !utils.Contains(s.SupportedStrategies(), request.Method) { return nil, ErrUnsupportedMethod } + // both mail strategies know the address before anything is sent, and share + // applyMailOTP at the other end, so they share the gate here. Passkey gates + // in its own branch, where it already looks the user up to pick a ceremony. + if request.Method == MailOTPAuthMethod.String() || request.Method == MailLinkAuthMethod.String() { + if err := s.gateFlowStart(ctx, request.Intent, request.Email); err != nil { + return nil, err + } + } flow := &Flow{ ID: uuid.New(), Method: request.Method, @@ -214,19 +224,37 @@ func (s Service) StartFlow(ctx context.Context, request RegistrationStartRequest "callback_url": request.CallbackUrl, }, } + // only write what the caller sent, so today's clients produce today's flow row + if request.Intent != FlowIntentUnspecified { + flow.Metadata[flowIntentKey] = request.Intent.String() + } + if len(request.AcceptedDocumentIDs) > 0 { + flow.Metadata[flowConsentKey] = map[string]any{ + consentDocumentIDsKey: request.AcceptedDocumentIDs, + consentIPAddressKey: request.IPAddress, + consentAtKey: s.Now(), + } + } if request.Method == PassKeyAuthMethod.String() { - needRegistration := false - loggedInUser, err := s.userService.GetByID(ctx, request.Email) - if err != nil { - needRegistration = true - } else { - storedPasskey, passKeyExists := loggedInUser.Metadata["passkey_credentials"] - if !passKeyExists { - needRegistration = true - } - if _, ok := storedPasskey.(string); !ok { + loggedInUser, userErr := s.userService.GetByID(ctx, request.Email) + if err := checkIntent(request.Intent, userErr == nil); err != nil { + return nil, err + } + + // the intent picks the ceremony; without one, fall back to the old guess + needRegistration := request.Intent == FlowIntentSignup + if request.Intent == FlowIntentUnspecified { + if userErr != nil { needRegistration = true + } else { + storedPasskey, passKeyExists := loggedInUser.Metadata["passkey_credentials"] + if !passKeyExists { + needRegistration = true + } + if _, ok := storedPasskey.(string); !ok { + needRegistration = true + } } } @@ -377,6 +405,34 @@ func otpAttempts(md metadata.Metadata) int { } } +// checkIntent applies the login and signup gates. An unspecified intent checks +// nothing, which is what keeps existing clients working. +func checkIntent(intent FlowIntent, userExists bool) error { + switch intent { + case FlowIntentLogin: + if !userExists { + return ErrLoginUserNotFound + } + case FlowIntentSignup: + if userExists { + return ErrSignupUserExists + } + } + return nil +} + +// gateFlowStart is the fast path of the login gate, for the strategies that know +// the email up front: it fails before anything is mailed. The gate that matters +// is at user creation, which every strategy reaches including OIDC. +func (s Service) gateFlowStart(ctx context.Context, intent FlowIntent, email string) error { + if intent == FlowIntentUnspecified { + // nothing to check, and no reason to spend a lookup + return nil + } + _, err := s.userService.GetByID(ctx, email) + return checkIntent(intent, err == nil) +} + // applyMailOTP actions when user submitted otp from the email // user can be considered as verified if code is valid // create a new user if required @@ -417,7 +473,7 @@ func (s Service) applyMailOTP(ctx context.Context, request RegistrationFinishReq return nil, fmt.Errorf("failed to successfully register via otp: %w", err) } - newUser, err := s.getOrCreateUser(ctx, flow.Email, "") + newUser, err := s.getOrCreateUser(ctx, flow, flow.Email, "") if err != nil { return nil, err } @@ -460,7 +516,12 @@ func (s Service) startPassKeyRegisterMethod(ctx context.Context, flow *Flow) (*R } func (s Service) startPassKeyLoginMethod(ctx context.Context, loggedInUser user.User, flow *Flow) (*RegistrationStartResponse, error) { - decodedCredBytes, err := base64.StdEncoding.DecodeString(loggedInUser.Metadata["passkey_credentials"].(string)) + // a login intent reaches this unchecked, so read rather than assert + storedPasskey, ok := loggedInUser.Metadata["passkey_credentials"].(string) + if !ok { + return nil, errors.New("no passkey registered for this account") + } + decodedCredBytes, err := base64.StdEncoding.DecodeString(storedPasskey) if err != nil { return nil, err } @@ -537,7 +598,7 @@ func (s Service) finishPassKeyRegisterMethod(ctx context.Context, request Regist if err != nil { return nil, err } - newUser, err := s.getOrCreateUser(ctx, flow.Email, "") + newUser, err := s.getOrCreateUser(ctx, flow, flow.Email, "") if err != nil { return nil, err } @@ -598,7 +659,7 @@ func (s Service) finishPassKeyLoginMethod(ctx context.Context, request Registrat return nil, err } - existingUser, err := s.getOrCreateUser(ctx, flow.Email, "") + existingUser, err := s.getOrCreateUser(ctx, flow, flow.Email, "") if err != nil { return nil, err } @@ -723,7 +784,7 @@ func (s Service) applyOIDC(ctx context.Context, request RegistrationFinishReques } // register a new user - newUser, err := s.getOrCreateUser(ctx, oauthProfile.Email, oauthProfile.Name) + newUser, err := s.getOrCreateUser(ctx, flow, oauthProfile.Email, oauthProfile.Name) if err != nil { return nil, err } @@ -781,10 +842,17 @@ func (s Service) consumeFlow(ctx context.Context, id uuid.UUID) error { return s.flowRepo.Delete(ctx, id) } -func (s Service) getOrCreateUser(ctx context.Context, email, title string) (user.User, error) { +// getOrCreateUser returns the user for the email, creating one if there is none. +// A nil flow means a caller that has none, and gets the old create-or-get. +func (s Service) getOrCreateUser(ctx context.Context, flow *Flow, email, title string) (user.User, error) { + intent := flow.Intent() + // create a new user based on email if it doesn't exist existingUser, err := s.userService.GetByID(ctx, email) if err == nil { + if intent == FlowIntentSignup { + return user.User{}, ErrSignupUserExists + } // user is already registered // TODO(kushsharma): should we update metadata like profile picture from social logins @@ -792,6 +860,12 @@ func (s Service) getOrCreateUser(ctx context.Context, email, title string) (user return existingUser, nil } + // the gate that matters: every strategy ends here, the last point before an + // account would be created for someone trying to log in + if intent == FlowIntentLogin { + return user.User{}, ErrLoginUserNotFound + } + // register a new user newUser, err := s.userService.Create(ctx, user.User{ Title: title, diff --git a/core/authenticate/service_test.go b/core/authenticate/service_test.go index 826228ba81..747991bde9 100644 --- a/core/authenticate/service_test.go +++ b/core/authenticate/service_test.go @@ -12,6 +12,7 @@ import ( "testing" "time" + "github.com/go-webauthn/webauthn/webauthn" "golang.org/x/crypto/bcrypt" "github.com/raystack/frontier/core/authenticate/strategy" @@ -1026,3 +1027,479 @@ func TestService_BuildToken(t *testing.T) { }) } } + +// passkeyUserMetadata returns metadata holding one stored passkey credential, +// base64 encoded the way startPassKeyLoginMethod expects it. +func passkeyUserMetadata(t *testing.T) pkgMetadata.Metadata { + t.Helper() + + credBytes, err := json.Marshal([]webauthn.Credential{{ + ID: []byte("credential-id"), + PublicKey: []byte("public-key"), + }}) + require.NoError(t, err) + + return pkgMetadata.Metadata{ + "passkey_credentials": base64.StdEncoding.EncodeToString(credBytes), + } +} + +func testWebAuthn(t *testing.T) *webauthn.WebAuthn { + t.Helper() + + webAuth, err := webauthn.New(&webauthn.Config{ + RPDisplayName: "frontier test", + RPID: "example.com", + RPOrigins: []string{"https://example.com"}, + }) + require.NoError(t, err) + return webAuth +} + +// TestService_StartFlow_Intent walks the intent-by-strategy table. For passkey +// the intent also picks the ceremony, which used to be guessed. +func TestService_StartFlow_Intent(t *testing.T) { + defaultHashCost := authenticate.OTPHashCost + authenticate.OTPHashCost = bcrypt.MinCost + t.Cleanup(func() { authenticate.OTPHashCost = defaultHashCost }) + + const email = "test@example.com" + + tests := []struct { + name string + + method string + intent authenticate.FlowIntent + userExists bool + userHasPasskey bool + + wantErr error + wantErrContains string + wantPasskeyCeremony string + }{ + { + name: "mail otp login starts the flow when the address has an account", + method: authenticate.MailOTPAuthMethod.String(), + intent: authenticate.FlowIntentLogin, + userExists: true, + }, + { + name: "mail otp login is rejected before the code is sent when it does not", + method: authenticate.MailOTPAuthMethod.String(), + intent: authenticate.FlowIntentLogin, + wantErr: authenticate.ErrLoginUserNotFound, + }, + { + name: "mail otp signup starts the flow when the address has no account", + method: authenticate.MailOTPAuthMethod.String(), + intent: authenticate.FlowIntentSignup, + }, + { + name: "mail otp signup is rejected when the address already has one", + method: authenticate.MailOTPAuthMethod.String(), + intent: authenticate.FlowIntentSignup, + userExists: true, + wantErr: authenticate.ErrSignupUserExists, + }, + { + name: "mail otp without an intent starts the flow for a known address", + method: authenticate.MailOTPAuthMethod.String(), + intent: authenticate.FlowIntentUnspecified, + userExists: true, + }, + { + name: "mail otp without an intent starts the flow for an unknown address", + method: authenticate.MailOTPAuthMethod.String(), + intent: authenticate.FlowIntentUnspecified, + }, + // mail link knows the address as early as mail otp and shares its finish + // path, so the gate has to behave the same for both + { + name: "mail link login starts the flow when the address has an account", + method: authenticate.MailLinkAuthMethod.String(), + intent: authenticate.FlowIntentLogin, + userExists: true, + }, + { + name: "mail link login is rejected before the link is sent when it does not", + method: authenticate.MailLinkAuthMethod.String(), + intent: authenticate.FlowIntentLogin, + wantErr: authenticate.ErrLoginUserNotFound, + }, + { + name: "mail link signup starts the flow when the address has no account", + method: authenticate.MailLinkAuthMethod.String(), + intent: authenticate.FlowIntentSignup, + }, + { + name: "mail link signup is rejected when the address already has one", + method: authenticate.MailLinkAuthMethod.String(), + intent: authenticate.FlowIntentSignup, + userExists: true, + wantErr: authenticate.ErrSignupUserExists, + }, + { + name: "mail link without an intent starts the flow for a known address", + method: authenticate.MailLinkAuthMethod.String(), + intent: authenticate.FlowIntentUnspecified, + userExists: true, + }, + { + name: "mail link without an intent starts the flow for an unknown address", + method: authenticate.MailLinkAuthMethod.String(), + intent: authenticate.FlowIntentUnspecified, + }, + { + name: "passkey login runs the login ceremony when the address has an account", + method: authenticate.PassKeyAuthMethod.String(), + intent: authenticate.FlowIntentLogin, + userExists: true, + userHasPasskey: true, + wantPasskeyCeremony: strategy.PasskeyLoginType, + }, + { + name: "passkey login is rejected when the address has no account", + method: authenticate.PassKeyAuthMethod.String(), + intent: authenticate.FlowIntentLogin, + wantErr: authenticate.ErrLoginUserNotFound, + }, + { + name: "passkey login fails when the account has no registered passkey", + method: authenticate.PassKeyAuthMethod.String(), + intent: authenticate.FlowIntentLogin, + userExists: true, + wantErrContains: "no passkey registered", + }, + { + name: "passkey signup runs the register ceremony when the address has no account", + method: authenticate.PassKeyAuthMethod.String(), + intent: authenticate.FlowIntentSignup, + wantPasskeyCeremony: strategy.PasskeyRegisterType, + }, + { + name: "passkey signup is rejected when the address already has an account", + method: authenticate.PassKeyAuthMethod.String(), + intent: authenticate.FlowIntentSignup, + userExists: true, + userHasPasskey: true, + wantErr: authenticate.ErrSignupUserExists, + }, + { + name: "passkey without an intent still guesses register for an unknown address", + method: authenticate.PassKeyAuthMethod.String(), + intent: authenticate.FlowIntentUnspecified, + wantPasskeyCeremony: strategy.PasskeyRegisterType, + }, + { + name: "passkey without an intent still guesses login for a registered passkey", + method: authenticate.PassKeyAuthMethod.String(), + intent: authenticate.FlowIntentUnspecified, + userExists: true, + userHasPasskey: true, + wantPasskeyCeremony: strategy.PasskeyLoginType, + }, + { + name: "passkey without an intent still guesses register when the account has no passkey", + method: authenticate.PassKeyAuthMethod.String(), + intent: authenticate.FlowIntentUnspecified, + userExists: true, + wantPasskeyCeremony: strategy.PasskeyRegisterType, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + isPasskey := tt.method == authenticate.PassKeyAuthMethod.String() + + mockFlowRepo, mockUserService, _, _, _ := createMocks(t) + + // passkey always looks the address up; the mail strategies only do + // when there is an intent to check + if isPasskey || tt.intent != authenticate.FlowIntentUnspecified { + if tt.userExists { + existing := user.User{ID: uuid.New().String(), Email: email} + if tt.userHasPasskey { + existing.Metadata = passkeyUserMetadata(t) + } + mockUserService.EXPECT().GetByID(ctx, email).Return(existing, nil) + } else { + mockUserService.EXPECT().GetByID(ctx, email).Return(user.User{}, errors.New("user not found")) + } + } + + var storedFlow *authenticate.Flow + if tt.wantErr == nil && tt.wantErrContains == "" { + mockFlowRepo.EXPECT().Set(ctx, mock.Anything).Run(func(_ context.Context, flow *authenticate.Flow) { + storedFlow = flow + }).Return(nil) + } + + var webAuth *webauthn.WebAuthn + var mockDialer mailer.Dialer + if isPasskey { + webAuth = testWebAuthn(t) + } else { + mockDialer = mailer.NewMockDialer() + } + + srv := authenticate.NewService(nil, authenticate.Config{ + MailOTP: authenticate.MailOTPConfig{Validity: 10 * time.Minute}, + MailLink: authenticate.MailLinkConfig{Validity: 10 * time.Minute}, + TestUsers: testusers.Config{Enabled: true, OTP: "111111", Domain: "example.com"}, + }, mockFlowRepo, mockDialer, nil, nil, mockUserService, nil, webAuth, nil) + + got, err := srv.StartFlow(ctx, authenticate.RegistrationStartRequest{ + Method: tt.method, + Email: email, + Intent: tt.intent, + // mail link embeds the callback host in the link it sends + CallbackUrl: "http://localhost:7400/v1beta1/auth/callback", + }) + + switch { + case tt.wantErr != nil: + assert.ErrorIs(t, err, tt.wantErr) + assert.Nil(t, got) + case tt.wantErrContains != "": + require.Error(t, err) + assert.ErrorContains(t, err, tt.wantErrContains) + assert.Nil(t, got) + default: + require.NoError(t, err) + require.NotNil(t, got) + require.NotNil(t, storedFlow) + if tt.wantPasskeyCeremony != "" { + assert.Equal(t, tt.wantPasskeyCeremony, storedFlow.Metadata["passkey_type"]) + } + } + }) + } +} + +// TestService_StartFlow_WritesIntentAndConsent covers what StartFlow puts on the +// flow, which is where both fields have to live to survive an OIDC redirect. +func TestService_StartFlow_WritesIntentAndConsent(t *testing.T) { + defaultHashCost := authenticate.OTPHashCost + authenticate.OTPHashCost = bcrypt.MinCost + t.Cleanup(func() { authenticate.OTPHashCost = defaultHashCost }) + + const email = "test@example.com" + timeNow := time.Now().UTC() + + startFlow := func(t *testing.T, request authenticate.RegistrationStartRequest) *authenticate.Flow { + t.Helper() + + ctx := context.Background() + mockFlowRepo, mockUserService, _, _, _ := createMocks(t) + if request.Intent != authenticate.FlowIntentUnspecified { + mockUserService.EXPECT().GetByID(ctx, email).Return(user.User{}, errors.New("user not found")) + } + + 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) + srv.Now = func() time.Time { return timeNow } + + _, err := srv.StartFlow(ctx, request) + require.NoError(t, err) + require.NotNil(t, storedFlow) + return storedFlow + } + + t.Run("writes the intent and the consent when the caller sends them", func(t *testing.T) { + flow := startFlow(t, authenticate.RegistrationStartRequest{ + Method: authenticate.MailOTPAuthMethod.String(), + Email: email, + Intent: authenticate.FlowIntentSignup, + AcceptedDocumentIDs: []string{"terms_of_service", "privacy_policy"}, + IPAddress: "10.0.0.1", + }) + + assert.Equal(t, authenticate.FlowIntentSignup, flow.Intent()) + + consent, ok := flow.Consent() + require.True(t, ok) + assert.Equal(t, []string{"terms_of_service", "privacy_policy"}, consent.AcceptedDocumentIDs) + assert.Equal(t, "10.0.0.1", consent.IPAddress) + // the timestamp is when the user accepted, not when the flow finishes + assert.Equal(t, timeNow, consent.At) + }) + + t.Run("leaves the flow untouched when the caller sends neither", func(t *testing.T) { + flow := startFlow(t, authenticate.RegistrationStartRequest{ + Method: authenticate.MailOTPAuthMethod.String(), + Email: email, + }) + + // a client that sends no intent writes the same flow row it does today + assert.Equal(t, pkgMetadata.Metadata{"callback_url": ""}, flow.Metadata) + assert.Equal(t, authenticate.FlowIntentUnspecified, flow.Intent()) + _, ok := flow.Consent() + assert.False(t, ok) + }) +} + +// TestFlow_IntentAndConsent covers the accessors directly: the JSON round trip +// the database puts metadata through, and the nil receiver callers rely on. +func TestFlow_IntentAndConsent(t *testing.T) { + acceptedAt := time.Now().UTC().Truncate(time.Second) + + t.Run("a nil flow reads as no intent and no consent", func(t *testing.T) { + var flow *authenticate.Flow + + assert.Equal(t, authenticate.FlowIntentUnspecified, flow.Intent()) + _, ok := flow.Consent() + assert.False(t, ok) + }) + + t.Run("survives the round trip the database puts metadata through", func(t *testing.T) { + ctx := context.Background() + flowRepo := &jsonFlowRepository{flows: map[uuid.UUID]jsonStoredFlow{}} + flowID := uuid.New() + + require.NoError(t, flowRepo.Set(ctx, mailOTPFlow(flowID, acceptedAt, "nonce", pkgMetadata.Metadata{ + "callback_url": "", + "intent": authenticate.FlowIntentSignup.String(), + "consent": map[string]any{ + "accepted_document_ids": []string{"terms_of_service"}, + "ip_address": "10.0.0.1", + "at": acceptedAt, + }, + }))) + + stored, err := flowRepo.Get(ctx, flowID) + require.NoError(t, err) + + // JSON gives the ids back as []any and the timestamp as a string, so the + // accessors parse rather than assert + assert.Equal(t, authenticate.FlowIntentSignup, stored.Intent()) + consent, ok := stored.Consent() + require.True(t, ok) + assert.Equal(t, []string{"terms_of_service"}, consent.AcceptedDocumentIDs) + assert.Equal(t, "10.0.0.1", consent.IPAddress) + assert.True(t, acceptedAt.Equal(consent.At)) + }) + + t.Run("an unparseable or empty consent is no consent", func(t *testing.T) { + for name, md := range map[string]pkgMetadata.Metadata{ + "missing": {"callback_url": ""}, + "wrong type": {"consent": "yes"}, + "no documents": {"consent": map[string]any{"ip_address": "10.0.0.1"}}, + "unknown types": {"consent": map[string]any{"accepted_document_ids": []any{1, 2}}}, + } { + t.Run(name, func(t *testing.T) { + flow := &authenticate.Flow{Metadata: md} + _, ok := flow.Consent() + assert.False(t, ok) + }) + } + }) + + t.Run("an intent of the wrong type reads as unspecified", func(t *testing.T) { + flow := &authenticate.Flow{Metadata: pkgMetadata.Metadata{"intent": 2}} + assert.Equal(t, authenticate.FlowIntentUnspecified, flow.Intent()) + }) +} + +// TestService_FinishFlow_Intent covers the second gate, at user creation, which +// is the only point OIDC can be gated at since it has no email before then. +func TestService_FinishFlow_Intent(t *testing.T) { + timeNow := time.Now() + otpHash, err := bcrypt.GenerateFromPassword([]byte("111111"), bcrypt.MinCost) + require.NoError(t, err) + + const email = "test@example.com" + existingUser := user.User{ID: "user-id", Email: email} + + tests := []struct { + name string + intent authenticate.FlowIntent + userExists bool + + wantErr error + wantUserCreate bool + }{ + { + name: "login logs the existing user in", + intent: authenticate.FlowIntentLogin, + userExists: true, + }, + { + name: "login never creates the account", + intent: authenticate.FlowIntentLogin, + wantErr: authenticate.ErrLoginUserNotFound, + }, + { + name: "signup creates the account", + intent: authenticate.FlowIntentSignup, + wantUserCreate: true, + }, + { + name: "signup never logs the existing user in", + intent: authenticate.FlowIntentSignup, + userExists: true, + wantErr: authenticate.ErrSignupUserExists, + }, + { + name: "without an intent an existing user is logged in", + intent: authenticate.FlowIntentUnspecified, + userExists: true, + }, + { + name: "without an intent an unknown address is created, as before", + intent: authenticate.FlowIntentUnspecified, + wantUserCreate: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + flowID := uuid.New() + + md := pkgMetadata.Metadata{"callback_url": ""} + if tt.intent != authenticate.FlowIntentUnspecified { + md["intent"] = tt.intent.String() + } + + mockFlowRepo, mockUserService, _, _, _ := createMocks(t) + mockFlowRepo.EXPECT().Get(ctx, flowID).Return(mailOTPFlow(flowID, timeNow, string(otpHash), md), nil) + mockFlowRepo.EXPECT().Delete(ctx, flowID).Return(nil) + + if tt.userExists { + mockUserService.EXPECT().GetByID(ctx, email).Return(existingUser, nil) + } else { + mockUserService.EXPECT().GetByID(ctx, email).Return(user.User{}, errors.New("user not found")) + } + if tt.wantUserCreate { + mockUserService.EXPECT().Create(ctx, mock.Anything).Return(existingUser, nil) + } + + srv := authenticate.NewService(nil, authenticate.Config{}, mockFlowRepo, nil, + nil, nil, mockUserService, nil, nil, nil) + srv.Now = func() time.Time { return timeNow } + + got, err := srv.FinishFlow(ctx, authenticate.RegistrationFinishRequest{ + Method: authenticate.MailOTPAuthMethod.String(), + State: flowID.String(), + Code: "111111", + }) + + if tt.wantErr != nil { + assert.ErrorIs(t, err, tt.wantErr) + assert.Nil(t, got) + return + } + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, existingUser, got.User) + }) + } +}