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) + }) + } +}