diff --git a/internal/handler/auth.go b/internal/handler/auth.go index 2944c8f..b8d09c3 100644 --- a/internal/handler/auth.go +++ b/internal/handler/auth.go @@ -53,6 +53,15 @@ type verifyReq struct { Token string `json:"token"` } +type setEmailReq struct { + Email string `json:"email"` +} + +type changePasswordReq struct { + CurrentPassword string `json:"current_password"` + NewPassword string `json:"new_password"` +} + // --- handlers --- // SignUp creates a user, plants the session cookie, returns the user. @@ -172,6 +181,39 @@ func (h *AuthHandler) ResetPassword(w http.ResponseWriter, r *http.Request) { response.JSON(w, http.StatusOK, map[string]bool{"ok": true}) } +// ChangePassword — protected — replaces the caller's password after +// verifying the current one. The new password bumps token_version, which +// invalidates every previously-issued JWT for this user. We immediately +// re-issue a fresh session cookie so the caller's current device stays +// logged in; every other device gets signed out on next request. +func (h *AuthHandler) ChangePassword(w http.ResponseWriter, r *http.Request) { + uid := middleware.UserID(r.Context()) + + var req changePasswordReq + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + response.Error(w, http.StatusBadRequest, "invalid JSON body") + return + } + // current_password is optional — accounts with no password on file + // (OAuth-only signups setting their first one) skip it. The service + // enforces verification when a hash exists. + v := validator.New() + validator.Required(v, "new_password", req.NewPassword) + validator.MinLength(v, "new_password", req.NewPassword, 8) + if !v.Valid() { + response.Error(w, http.StatusBadRequest, v.Error()) + return + } + + res, err := h.svc.ChangePassword(r.Context(), uid, req.CurrentPassword, req.NewPassword) + if err != nil { + h.renderAuthErr(w, err) + return + } + auth.Set(w, res.Token, res.Expires, h.cookies) + response.JSON(w, http.StatusOK, res.User) +} + // VerifyEmail flips email_verified_at after the user clicks the link. func (h *AuthHandler) VerifyEmail(w http.ResponseWriter, r *http.Request) { var req verifyReq @@ -190,6 +232,34 @@ func (h *AuthHandler) VerifyEmail(w http.ResponseWriter, r *http.Request) { response.JSON(w, http.StatusOK, map[string]bool{"ok": true}) } +// SetEmail — protected — sets (or replaces) the caller's email address. +// Used by OAuth-signup users who landed without one, and by anyone wanting +// to change their address. The new value is stored as unverified and a +// verification link is mailed out immediately. +func (h *AuthHandler) SetEmail(w http.ResponseWriter, r *http.Request) { + uid := middleware.UserID(r.Context()) + + var req setEmailReq + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + response.Error(w, http.StatusBadRequest, "invalid JSON body") + return + } + v := validator.New() + validator.Required(v, "email", req.Email) + validator.Email(v, "email", req.Email) + if !v.Valid() { + response.Error(w, http.StatusBadRequest, v.Error()) + return + } + + u, err := h.svc.SetEmail(r.Context(), uid, req.Email) + if err != nil { + h.renderAuthErr(w, err) + return + } + response.JSON(w, http.StatusOK, u) +} + // ResendVerification — protected — refires the verification email for the // currently-logged-in user. func (h *AuthHandler) ResendVerification(w http.ResponseWriter, r *http.Request) { diff --git a/internal/handler/oauth.go b/internal/handler/oauth.go index bc94a73..455d873 100644 --- a/internal/handler/oauth.go +++ b/internal/handler/oauth.go @@ -171,8 +171,6 @@ func (h *OAuthHandler) Callback(w http.ResponseWriter, r *http.Request) { }) if err != nil { switch { - case errors.Is(err, service.ErrOAuthEmailMissing): - h.redirectErr(w, r, "/login?oauth_error=email_required") case errors.Is(err, service.ErrOAuthUnavailable): h.redirectErr(w, r, "/login?oauth_error=unavailable") default: diff --git a/internal/model/user.go b/internal/model/user.go index 6555081..43930b3 100644 --- a/internal/model/user.go +++ b/internal/model/user.go @@ -1,6 +1,9 @@ package model -import "time" +import ( + "encoding/json" + "time" +) // User represents a row in the `users` table. // @@ -14,7 +17,10 @@ import "time" // frontend uses this to decide whether to show the "verify your email" ribbon. type User struct { ID string `json:"id"` - Email string `json:"email"` + // Email is nullable: OAuth users (typically Facebook accounts with no + // verified email on file) sign up without one and can fill it in later + // from their profile. + Email *string `json:"email"` Handle string `json:"handle"` Name string `json:"name"` AvatarURL *string `json:"avatar_url"` @@ -27,6 +33,20 @@ type User struct { UpdatedAt time.Time `json:"updated_at"` } +// MarshalJSON adds a derived `has_password` flag so the frontend can pick +// "Change password" vs "Set a password" for OAuth-only accounts, without +// the password hash itself ever leaving the server. +func (u User) MarshalJSON() ([]byte, error) { + type alias User + return json.Marshal(&struct { + alias + HasPassword bool `json:"has_password"` + }{ + alias: alias(u), + HasPassword: u.PasswordHash != nil, + }) +} + // AuthToken represents a row in the `auth_tokens` table. // Used for both password reset and email verification flows. type AuthToken struct { diff --git a/internal/repository/user_repo.go b/internal/repository/user_repo.go index c756f92..2137500 100644 --- a/internal/repository/user_repo.go +++ b/internal/repository/user_repo.go @@ -63,17 +63,26 @@ func (r *UserRepo) FindByEmail(ctx context.Context, email string) (*model.User, } // CreateOAuth inserts a user with a NULL password_hash — used by the OAuth -// sign-in flow when there's no matching email yet. Same handle-collision -// retry contract as Create. -func (r *UserRepo) CreateOAuth(ctx context.Context, email, name, handle string, avatarURL *string, emailVerified bool) (*model.User, error) { +// sign-in flow when there's no matching email yet. Email is nullable here: +// providers that don't return one (e.g. phone-only Facebook accounts) end +// up as users with email=NULL until they fill it in via profile settings. +// Same handle-collision retry contract as Create. +func (r *UserRepo) CreateOAuth(ctx context.Context, email *string, name, handle string, avatarURL *string, emailVerified bool) (*model.User, error) { const q = ` INSERT INTO users (email, password_hash, name, handle, avatar_url, email_verified_at) VALUES ($1, NULL, $2, $3, $4, CASE WHEN $5::boolean THEN now() ELSE NULL END) RETURNING id, email, handle, password_hash, name, avatar_url, role, plan, email_verified_at, token_version, created_at, updated_at` + var normEmail *string + if email != nil { + v := strings.ToLower(strings.TrimSpace(*email)) + if v != "" { + normEmail = &v + } + } u := &model.User{} err := r.pg.Pool.QueryRow(ctx, q, - strings.ToLower(strings.TrimSpace(email)), name, handle, avatarURL, emailVerified, + normEmail, name, handle, avatarURL, emailVerified, ).Scan(&u.ID, &u.Email, &u.Handle, &u.PasswordHash, &u.Name, &u.AvatarURL, &u.Role, &u.Plan, &u.EmailVerifiedAt, &u.TokenVersion, &u.CreatedAt, &u.UpdatedAt) if err != nil { @@ -118,6 +127,39 @@ func (r *UserRepo) findOne(ctx context.Context, where string, args ...any) (*mod return u, nil } +// SetEmail fills in (or replaces) a user's email and resets email_verified_at +// to NULL so a fresh verification flow can run. Returns ErrEmailTaken if +// another row already owns this address. +// +// Used by the "add your email later" flow for OAuth users who signed up +// without one. The caller (auth service) is expected to follow up by +// minting and sending a verification token. +func (r *UserRepo) SetEmail(ctx context.Context, id, email string) (*model.User, error) { + const q = ` + UPDATE users + SET email = $2, + email_verified_at = NULL, + updated_at = now() + WHERE id = $1 + RETURNING id, email, handle, password_hash, name, avatar_url, role, plan, + email_verified_at, token_version, created_at, updated_at` + + u := &model.User{} + err := r.pg.Pool.QueryRow(ctx, q, id, strings.ToLower(strings.TrimSpace(email))). + Scan(&u.ID, &u.Email, &u.Handle, &u.PasswordHash, &u.Name, &u.AvatarURL, &u.Role, &u.Plan, + &u.EmailVerifiedAt, &u.TokenVersion, &u.CreatedAt, &u.UpdatedAt) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return nil, ErrUserNotFound + } + if isUniqueViolation(err, "users_email_key") { + return nil, ErrEmailTaken + } + return nil, fmt.Errorf("set email: %w", err) + } + return u, nil +} + // UpdateProfile updates the user's name, avatar, and/or handle. Pass nil to // leave a field unchanged. Returns the updated user. // diff --git a/internal/server/server.go b/internal/server/server.go index d00961c..79d811f 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -520,6 +520,8 @@ func (s *Server) setupRouter() { r.Group(func(r chi.Router) { r.Use(middleware.RequireAuth(s.authIssuer, s.userRepo)) r.Get("/me", s.authH.Me) + r.Post("/email", s.authH.SetEmail) + r.Post("/change-password", s.authH.ChangePassword) r.Post("/resend-verification", s.authH.ResendVerification) }) }) diff --git a/internal/service/auth_service.go b/internal/service/auth_service.go index 89cafc5..3f104c8 100644 --- a/internal/service/auth_service.go +++ b/internal/service/auth_service.go @@ -164,12 +164,6 @@ type OAuthProfile struct { AvatarURL string } -// ErrOAuthEmailMissing is returned by SignInWithOAuth when the provider did -// not give us an email and the account has no linkage we could match on. -// Without an email we can't safely create a new user (since later sign-ins -// via email/password would have no way to converge with this account). -var ErrOAuthEmailMissing = errors.New("oauth provider did not return an email") - // ErrOAuthUnavailable means the OAuth repo wasn't wired (no Postgres at // startup). Handler maps to 503. var ErrOAuthUnavailable = errors.New("oauth service unavailable") @@ -177,13 +171,19 @@ var ErrOAuthUnavailable = errors.New("oauth service unavailable") // SignInWithOAuth turns a provider profile into a session. Three-step lookup: // // 1. Existing linkage on (provider, provider_user_id) → log that user in. -// 2. Existing user with same email → link this provider to them, log in. -// 3. Otherwise create a new user (no password, email_verified) and link. +// 2. Provider gave us an email AND an existing user has the same email → +// link this provider to them, log in. +// 3. Otherwise create a new user (no password, email_verified iff we have +// an email) and link. // -// Every successful OAuth round-trip is treated as proof the user controls -// the email address — Google + Facebook both verify email ownership before -// returning it to us, so we always mark the email as verified on sign-in -// regardless of the provider's `email_verified` flag. +// When the provider returns no email (Facebook accounts with no verified +// email on file, accounts that declined the email scope, etc.) we skip +// the merge step and create the user with email=NULL. They can add an +// email later via PATCH /users/me — the frontend prompts them. +// +// When we DO have an email, every successful OAuth round-trip is treated +// as proof the user controls it — Google + Facebook both verify email +// ownership before returning it, so we mark the email as verified. // // In every case the resulting user gets a fresh JWT just like LogIn. func (s *AuthService) SignInWithOAuth(ctx context.Context, prof OAuthProfile) (*AuthResult, error) { @@ -201,36 +201,47 @@ func (s *AuthService) SignInWithOAuth(ctx context.Context, prof OAuthProfile) (* // avatar may have changed at the provider). _, _ = s.oauth.Link(ctx, u.ID, prof.Provider, prof.ProviderUserID, optStr(prof.Email), optStr(prof.Name), optStr(prof.AvatarURL)) - s.ensureEmailVerified(ctx, u) + if prof.Email != "" { + s.ensureEmailVerified(ctx, u) + } return s.issueSession(u) } else if !errors.Is(err, repository.ErrOAuthAccountNotFound) { return nil, err } - // No linkage yet — we need at least an email to safely converge with an - // existing account or create a new one. - if prof.Email == "" { - return nil, ErrOAuthEmailMissing - } - // 2. Existing user with the same email → link. - if u, err := s.users.FindByEmail(ctx, prof.Email); err == nil { - if _, err := s.oauth.Link(ctx, u.ID, prof.Provider, prof.ProviderUserID, - optStr(prof.Email), optStr(prof.Name), optStr(prof.AvatarURL)); err != nil { + // Only attempt this when the provider actually gave us an email. With + // no email there's no merge key — we just create a fresh user below. + if prof.Email != "" { + if u, err := s.users.FindByEmail(ctx, prof.Email); err == nil { + if _, err := s.oauth.Link(ctx, u.ID, prof.Provider, prof.ProviderUserID, + optStr(prof.Email), optStr(prof.Name), optStr(prof.AvatarURL)); err != nil { + return nil, err + } + s.ensureEmailVerified(ctx, u) + return s.issueSession(u) + } else if !errors.Is(err, repository.ErrUserNotFound) { return nil, err } - s.ensureEmailVerified(ctx, u) - return s.issueSession(u) - } else if !errors.Is(err, repository.ErrUserNotFound) { - return nil, err } // 3. Brand new user. Reuse the handle-collision retry loop. displayName := prof.Name - if displayName == "" { + if displayName == "" && prof.Email != "" { displayName = strings.SplitN(prof.Email, "@", 2)[0] } + if displayName == "" { + // No name AND no email — fall back to the provider name so the user + // at least has a non-empty display name they can rename later. + displayName = strings.Title(prof.Provider) + " user" //nolint:staticcheck // legacy Title is fine for ASCII labels + } base := slugForHandle(prof.Email) + if base == "user" && prof.ProviderUserID != "" { + // slugForHandle returns "user" for empty input — prefer something + // less collision-prone for OAuth-no-email signups. + base = "user-" + safeSlugSuffix(prof.ProviderUserID) + } + emailVerified := prof.Email != "" // can only be verified if we have an email var u *model.User var err error for attempt := 0; attempt < 8; attempt++ { @@ -238,13 +249,13 @@ func (s *AuthService) SignInWithOAuth(ctx context.Context, prof OAuthProfile) (* if attempt > 0 { handle = fmt.Sprintf("%s-%s", base, randomHandleSuffix()) } - // Always create as verified — successful OAuth = email ownership proof. - u, err = s.users.CreateOAuth(ctx, prof.Email, displayName, handle, optStr(prof.AvatarURL), true) + u, err = s.users.CreateOAuth(ctx, optStr(prof.Email), displayName, handle, optStr(prof.AvatarURL), emailVerified) if err == nil { break } if errors.Is(err, repository.ErrEmailTaken) { // Race: someone else just claimed this email. Fall back to step 2. + // Only reachable when prof.Email != "" (NULL emails never collide). if u2, err2 := s.users.FindByEmail(ctx, prof.Email); err2 == nil { _, _ = s.oauth.Link(ctx, u2.ID, prof.Provider, prof.ProviderUserID, optStr(prof.Email), optStr(prof.Name), optStr(prof.AvatarURL)) @@ -268,6 +279,30 @@ func (s *AuthService) SignInWithOAuth(ctx context.Context, prof OAuthProfile) (* return s.issueSession(u) } +// safeSlugSuffix reduces an arbitrary provider id to a short ASCII tail +// suitable as part of a handle. Provider ids are typically all-digit +// (Facebook) or short ASCII (Google sub), so a trailing 8-char window is +// plenty and avoids handles getting silly-long. +func safeSlugSuffix(s string) string { + out := strings.Map(func(r rune) rune { + switch { + case r >= 'a' && r <= 'z', r >= '0' && r <= '9': + return r + case r >= 'A' && r <= 'Z': + return r + ('a' - 'A') + default: + return -1 + } + }, s) + if len(out) > 8 { + out = out[len(out)-8:] + } + if out == "" { + return randomHandleSuffix() + } + return out +} + // optStr returns nil for empty input, &s otherwise — used to populate // nullable text columns from possibly-empty profile fields. func optStr(s string) *string { @@ -277,6 +312,78 @@ func optStr(s string) *string { return &s } +// ChangePassword sets the caller's password, in two modes: +// +// 1. Account HAS a password — caller must supply the current one. We +// verify with bcrypt before replacing, defending against an attacker +// who got hold of the session cookie but doesn't know the password. +// 2. Account has NO password yet (OAuth-only signup) — caller can set +// one without proving anything beyond their existing session. There's +// no current password to verify against, and the session already +// grants them everything the new password would, so requiring an +// email round-trip here would only be friction. +// +// In both cases token_version is bumped (invalidating every JWT ever +// issued to this user) and a fresh session is re-issued so the calling +// device stays logged in while every other device is signed out. +func (s *AuthService) ChangePassword(ctx context.Context, userID, currentPassword, newPassword string) (*AuthResult, error) { + if len(newPassword) < 8 { + return nil, ErrWeakPassword + } + u, err := s.users.FindByID(ctx, userID) + if err != nil { + return nil, ErrUserNotFound + } + if u.PasswordHash != nil { + // Account has a password — current must match before we'll replace it. + if err := bcrypt.CompareHashAndPassword([]byte(*u.PasswordHash), []byte(currentPassword)); err != nil { + return nil, ErrInvalidCredentials + } + } + // If u.PasswordHash == nil we skip the verification: setting an initial + // password for an OAuth-only account doesn't require proving anything + // beyond the active session. + + hash, err := bcrypt.GenerateFromPassword([]byte(newPassword), bcrypt.DefaultCost) + if err != nil { + return nil, fmt.Errorf("hash password: %w", err) + } + if err := s.users.UpdatePassword(ctx, userID, string(hash)); err != nil { + return nil, err + } + // Re-fetch to pick up the new token_version so the freshly-issued JWT + // has the right "tv" claim and the cookie set after this call keeps + // the user logged in on this device. + u2, err := s.users.FindByID(ctx, userID) + if err != nil { + return nil, err + } + return s.issueSession(u2) +} + +// SetEmail fills in (or replaces) the caller's email address. Used by the +// "add your email" flow for OAuth users who signed up without one, and by +// any user who later wants to change their address. +// +// The address is stored as unverified — a verification link is sent to it +// immediately. Returns ErrEmailAlreadyTaken if another row already owns it. +func (s *AuthService) SetEmail(ctx context.Context, userID, email string) (*model.User, error) { + u, err := s.users.SetEmail(ctx, userID, email) + if err != nil { + if errors.Is(err, repository.ErrEmailTaken) { + return nil, ErrEmailAlreadyTaken + } + return nil, err + } + if err := s.sendVerificationEmail(ctx, u); err != nil { + // Verification email failure shouldn't block the set — the user can + // always trigger a resend. + s.logger.Warn("verification email failed after SetEmail", + zap.String("user", u.ID), zap.Error(err)) + } + return u, nil +} + // ensureEmailVerified flips email_verified_at to now() on the given user // if it's not already set. Called from every SignInWithOAuth branch so a // successful OAuth round-trip immediately satisfies email verification @@ -331,9 +438,15 @@ func (s *AuthService) RequestPasswordReset(ctx context.Context, email string) er return err } + // OAuth-only users may have no email on file — skip the send silently + // (the caller already cannot tell whether the email was registered, so + // "no email to send to" surfaces the same way). + if u.Email == nil || *u.Email == "" { + return nil + } link := fmt.Sprintf("%s/reset-password?token=%s", s.frontend, raw) body := passwordResetEmail(u.Name, link) - if err := s.mail.Send(u.Email, "Reset your Prabhupāda Says password", body); err != nil { + if err := s.mail.Send(*u.Email, "Reset your Prabhupāda Says password", body); err != nil { s.logger.Warn("password reset email failed", zap.String("user", u.ID), zap.Error(err)) } return nil @@ -408,6 +521,12 @@ func (s *AuthService) issueSession(u *model.User) (*AuthResult, error) { } func (s *AuthService) sendVerificationEmail(ctx context.Context, u *model.User) error { + // Users with no email on file (OAuth signups where the provider didn't + // return one) have nothing to send to. The caller is expected to gate + // on this, but defensively no-op here too. + if u.Email == nil || *u.Email == "" { + return nil + } _ = s.tokens.InvalidatePending(ctx, u.ID, model.PurposeEmailVerification) raw, hash, err := newToken() @@ -420,7 +539,7 @@ func (s *AuthService) sendVerificationEmail(ctx context.Context, u *model.User) link := fmt.Sprintf("%s/verify-email?token=%s", s.frontend, raw) body := verifyEmailMessage(u.Name, link) - return s.mail.Send(u.Email, "Verify your email — Prabhupāda Says", body) + return s.mail.Send(*u.Email, "Verify your email — Prabhupāda Says", body) } // newToken returns (rawHex, sha256HexHash). The raw value goes in the diff --git a/migrations/0013_make_user_email_nullable.down.sql b/migrations/0013_make_user_email_nullable.down.sql new file mode 100644 index 0000000..be190b5 --- /dev/null +++ b/migrations/0013_make_user_email_nullable.down.sql @@ -0,0 +1,3 @@ +-- Reverting this requires backfilling NULL emails with synthetic values +-- first; left as a manual step (see operational runbook). +ALTER TABLE users ALTER COLUMN email SET NOT NULL; diff --git a/migrations/0013_make_user_email_nullable.up.sql b/migrations/0013_make_user_email_nullable.up.sql new file mode 100644 index 0000000..3042d24 --- /dev/null +++ b/migrations/0013_make_user_email_nullable.up.sql @@ -0,0 +1,10 @@ +-- Some auth providers (Facebook in particular) let users sign up without +-- linking a verified email — phone-only accounts and accounts whose owner +-- declined the `email` permission both fall under this. To accept those +-- users we need to allow NULL on users.email. +-- +-- The UNIQUE constraint stays put: Postgres treats NULLs as distinct under +-- a regular UNIQUE index (multiple rows with NULL email are allowed) so +-- nothing breaks for OAuth-only users with no address on file. Real email +-- addresses are still de-duplicated, exactly as before. +ALTER TABLE users ALTER COLUMN email DROP NOT NULL;