fix(auth): wiring Auth/Sessions, нормализация email, GetUserByID для /me, 409 на дубль, timing-guard логина
This commit is contained in:
+8
-1
@@ -5,10 +5,12 @@ import (
|
||||
"log"
|
||||
"net/http"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/jackc/pgx/v5/pgxpool"
|
||||
|
||||
"github.com/vasyakrg/dns-autoresolver/internal/api"
|
||||
"github.com/vasyakrg/dns-autoresolver/internal/auth"
|
||||
"github.com/vasyakrg/dns-autoresolver/internal/config"
|
||||
"github.com/vasyakrg/dns-autoresolver/internal/crypto"
|
||||
"github.com/vasyakrg/dns-autoresolver/internal/provider/registry"
|
||||
@@ -18,6 +20,10 @@ import (
|
||||
"github.com/vasyakrg/dns-autoresolver/internal/web"
|
||||
)
|
||||
|
||||
// sessionTTL is how long a login session cookie remains valid before the
|
||||
// user must re-authenticate.
|
||||
const sessionTTL = 720 * time.Hour
|
||||
|
||||
// isAPIPath reports whether path must be routed to the API router rather
|
||||
// than the SPA. "/api" (no trailing slash) counts as an API path too —
|
||||
// only strings.HasPrefix(path, "/api/") would otherwise miss it and fall
|
||||
@@ -46,12 +52,13 @@ func main() {
|
||||
log.Fatalf("cipher: %v", err)
|
||||
}
|
||||
st := store.New(pool)
|
||||
sessions := auth.NewSessions(st, sessionTTL)
|
||||
|
||||
reg := registry.New()
|
||||
reg.Register(selectel.New())
|
||||
|
||||
svc := service.New(st, st, reg, cipher)
|
||||
a := &api.API{Svc: svc, Store: st, Cipher: cipher, Reg: reg}
|
||||
a := &api.API{Svc: svc, Store: st, Cipher: cipher, Reg: reg, Auth: st, Sessions: sessions}
|
||||
apiRouter := api.NewRouter(a)
|
||||
|
||||
webHandler, err := web.Handler()
|
||||
|
||||
@@ -61,6 +61,7 @@ type ProviderRegistry interface {
|
||||
type AuthStore interface {
|
||||
RegisterUser(ctx context.Context, email, passwordHash string) (store.User, store.Project, error)
|
||||
GetUserByEmail(ctx context.Context, email string) (store.User, error)
|
||||
GetUserByID(ctx context.Context, userID uuid.UUID) (store.User, error)
|
||||
GetUserProject(ctx context.Context, userID uuid.UUID) (store.Project, error)
|
||||
}
|
||||
|
||||
|
||||
@@ -2,17 +2,43 @@ package api
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"log"
|
||||
"net/http"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/google/uuid"
|
||||
|
||||
"github.com/vasyakrg/dns-autoresolver/internal/auth"
|
||||
"github.com/vasyakrg/dns-autoresolver/internal/store"
|
||||
)
|
||||
|
||||
const sessionCookieName = "session"
|
||||
|
||||
// dummyPasswordHash is a valid-format argon2 hash with no real matching
|
||||
// password. handleLogin runs VerifyPassword against it whenever the email
|
||||
// lookup fails, so a login attempt for an unregistered email takes the same
|
||||
// wall-clock time as one for a registered email with a wrong password —
|
||||
// otherwise the timing difference would let an attacker enumerate which
|
||||
// emails are registered.
|
||||
var dummyPasswordHash string
|
||||
|
||||
func init() {
|
||||
h, err := auth.HashPassword("dns-autoresolver-timing-guard-dummy")
|
||||
if err != nil {
|
||||
panic("api: failed to initialize dummy password hash: " + err.Error())
|
||||
}
|
||||
dummyPasswordHash = h
|
||||
}
|
||||
|
||||
// normalizeEmail trims surrounding whitespace and lowercases the email so
|
||||
// storage and lookup are always consistent regardless of how the client
|
||||
// cased or padded the input.
|
||||
func normalizeEmail(email string) string {
|
||||
return strings.ToLower(strings.TrimSpace(email))
|
||||
}
|
||||
|
||||
// ctxKeyUserID is a private context key carrying the authenticated user's ID.
|
||||
// Task 4's RequireAuth middleware sets it after validating the session
|
||||
// cookie; handleMe reads it back.
|
||||
@@ -45,7 +71,8 @@ func (a *API) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
if !decodeBody(w, r, &req) {
|
||||
return
|
||||
}
|
||||
if req.Email == "" || req.Password == "" {
|
||||
email := normalizeEmail(req.Email)
|
||||
if email == "" || req.Password == "" {
|
||||
writeErr(w, http.StatusBadRequest, "email and password are required")
|
||||
return
|
||||
}
|
||||
@@ -57,8 +84,12 @@ func (a *API) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
u, p, err := a.Auth.RegisterUser(r.Context(), req.Email, hash)
|
||||
u, p, err := a.Auth.RegisterUser(r.Context(), email, hash)
|
||||
if err != nil {
|
||||
if errors.Is(err, store.ErrEmailTaken) {
|
||||
writeErr(w, http.StatusConflict, "email already registered")
|
||||
return
|
||||
}
|
||||
log.Printf("api: register user failed: %v", err)
|
||||
writeErr(w, http.StatusInternalServerError, "internal error")
|
||||
return
|
||||
@@ -87,9 +118,14 @@ func (a *API) handleLogin(w http.ResponseWriter, r *http.Request) {
|
||||
if !decodeBody(w, r, &req) {
|
||||
return
|
||||
}
|
||||
email := normalizeEmail(req.Email)
|
||||
|
||||
u, err := a.Auth.GetUserByEmail(r.Context(), req.Email)
|
||||
u, err := a.Auth.GetUserByEmail(r.Context(), email)
|
||||
if err != nil {
|
||||
// No such user: still spend the argon2 verification cost against a
|
||||
// fixed dummy hash (see dummyPasswordHash) so this path isn't
|
||||
// distinguishable by timing from a wrong-password rejection below.
|
||||
_, _ = auth.VerifyPassword(dummyPasswordHash, req.Password)
|
||||
invalidCredentials(w)
|
||||
return
|
||||
}
|
||||
@@ -131,8 +167,7 @@ func (a *API) handleLogout(w http.ResponseWriter, r *http.Request) {
|
||||
// handleMe returns the authenticated caller's identity + default project.
|
||||
// The user ID comes from the request context, set by Task 4's RequireAuth
|
||||
// middleware after validating the session cookie (tests set it directly via
|
||||
// context.WithValue in the interim). AuthStore has no GetUserByID — the
|
||||
// email field is intentionally left empty here; see task-3-report.md.
|
||||
// context.WithValue in the interim).
|
||||
func (a *API) handleMe(w http.ResponseWriter, r *http.Request) {
|
||||
userID, ok := userIDFromContext(r.Context())
|
||||
if !ok {
|
||||
@@ -140,6 +175,13 @@ func (a *API) handleMe(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
u, err := a.Auth.GetUserByID(r.Context(), userID)
|
||||
if err != nil {
|
||||
log.Printf("api: get user by id failed: %v", err)
|
||||
writeErr(w, http.StatusInternalServerError, "internal error")
|
||||
return
|
||||
}
|
||||
|
||||
p, err := a.Auth.GetUserProject(r.Context(), userID)
|
||||
if err != nil {
|
||||
log.Printf("api: get user project failed: %v", err)
|
||||
@@ -147,8 +189,5 @@ func (a *API) handleMe(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
writeJSON(w, http.StatusOK, authResponse{
|
||||
User: userResponse{ID: userID.String()},
|
||||
Project: projectResponse{ID: p.ID.String(), Name: p.Name},
|
||||
})
|
||||
writeJSON(w, http.StatusOK, toAuthResponse(u, p))
|
||||
}
|
||||
|
||||
@@ -20,6 +20,7 @@ import (
|
||||
type mockAuthStore struct {
|
||||
registerUserFn func(ctx context.Context, email, passwordHash string) (store.User, store.Project, error)
|
||||
getUserByEmailFn func(ctx context.Context, email string) (store.User, error)
|
||||
getUserByIDFn func(ctx context.Context, userID uuid.UUID) (store.User, error)
|
||||
getUserProjectFn func(ctx context.Context, userID uuid.UUID) (store.Project, error)
|
||||
}
|
||||
|
||||
@@ -31,6 +32,10 @@ func (m *mockAuthStore) GetUserByEmail(ctx context.Context, email string) (store
|
||||
return m.getUserByEmailFn(ctx, email)
|
||||
}
|
||||
|
||||
func (m *mockAuthStore) GetUserByID(ctx context.Context, userID uuid.UUID) (store.User, error) {
|
||||
return m.getUserByIDFn(ctx, userID)
|
||||
}
|
||||
|
||||
func (m *mockAuthStore) GetUserProject(ctx context.Context, userID uuid.UUID) (store.Project, error) {
|
||||
return m.getUserProjectFn(ctx, userID)
|
||||
}
|
||||
@@ -125,6 +130,59 @@ func TestAuthRegister_Success(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestAuthRegister_NormalizesEmail verifies the fix for the email-consistency
|
||||
// gap: a padded/mixed-case email is trimmed+lowercased before it reaches the
|
||||
// store, so storage and later lookups are always consistent.
|
||||
func TestAuthRegister_NormalizesEmail(t *testing.T) {
|
||||
a, authStore, _ := newTestAuthAPI()
|
||||
userID := uuid.New()
|
||||
var gotEmail string
|
||||
authStore.registerUserFn = func(_ context.Context, email, passwordHash string) (store.User, store.Project, error) {
|
||||
gotEmail = email
|
||||
return store.User{ID: userID, Email: email}, store.Project{ID: uuid.New(), UserID: userID, Name: "default"}, nil
|
||||
}
|
||||
|
||||
router := NewRouter(a)
|
||||
body := `{"email":" Alice@X.com ","password":"correct-horse"}`
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/register", strings.NewReader(body))
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
if gotEmail != "alice@x.com" {
|
||||
t.Fatalf("expected normalized email passed to RegisterUser, got %q", gotEmail)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAuthRegister_DuplicateEmailReturns409 verifies the fix for the
|
||||
// duplicate-registration gap: RegisterUser reporting store.ErrEmailTaken
|
||||
// must surface as 409, not a generic 500.
|
||||
func TestAuthRegister_DuplicateEmailReturns409(t *testing.T) {
|
||||
a, authStore, _ := newTestAuthAPI()
|
||||
authStore.registerUserFn = func(context.Context, string, string) (store.User, store.Project, error) {
|
||||
return store.User{}, store.Project{}, store.ErrEmailTaken
|
||||
}
|
||||
|
||||
router := NewRouter(a)
|
||||
body := `{"email":"dup@example.com","password":"correct-horse"}`
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/register", strings.NewReader(body))
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusConflict {
|
||||
t.Fatalf("status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
var got map[string]string
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got["error"] != "email already registered" {
|
||||
t.Fatalf(`expected error "email already registered", got %q`, got["error"])
|
||||
}
|
||||
}
|
||||
|
||||
// --- login ---
|
||||
|
||||
func TestAuthLogin_CorrectPassword(t *testing.T) {
|
||||
@@ -159,6 +217,40 @@ func TestAuthLogin_CorrectPassword(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestAuthLogin_NormalizesEmail verifies that a login for a padded/mixed-case
|
||||
// email reaches GetUserByEmail already trimmed+lowercased — the same
|
||||
// normalization applied on register, so "Alice@X.com" at registration and
|
||||
// "alice@x.com" at login resolve to the same account.
|
||||
func TestAuthLogin_NormalizesEmail(t *testing.T) {
|
||||
a, authStore, _ := newTestAuthAPI()
|
||||
hash, err := auth.HashPassword("correct-horse")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
userID := uuid.New()
|
||||
var gotEmail string
|
||||
authStore.getUserByEmailFn = func(_ context.Context, email string) (store.User, error) {
|
||||
gotEmail = email
|
||||
return store.User{ID: userID, Email: email, PasswordHash: hash}, nil
|
||||
}
|
||||
authStore.getUserProjectFn = func(_ context.Context, uid uuid.UUID) (store.Project, error) {
|
||||
return store.Project{ID: uuid.New(), UserID: uid, Name: "default"}, nil
|
||||
}
|
||||
|
||||
router := NewRouter(a)
|
||||
body := `{"email":" Alice@X.com ","password":"correct-horse"}`
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/login", strings.NewReader(body))
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
if gotEmail != "alice@x.com" {
|
||||
t.Fatalf("expected normalized email passed to GetUserByEmail, got %q", gotEmail)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthLogin_WrongPassword(t *testing.T) {
|
||||
a, authStore, _ := newTestAuthAPI()
|
||||
hash, err := auth.HashPassword("correct-horse")
|
||||
@@ -240,6 +332,46 @@ func TestAuthLogout_ClearsSessionAndDestroys(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// --- me ---
|
||||
|
||||
// TestAuthMe_ReturnsRealEmail verifies the fix for the /me gap: the handler
|
||||
// now resolves the authenticated user via GetUserByID and returns their real
|
||||
// email, instead of leaving it blank.
|
||||
func TestAuthMe_ReturnsRealEmail(t *testing.T) {
|
||||
a, authStore, _ := newTestAuthAPI()
|
||||
userID := uuid.New()
|
||||
projectID := uuid.New()
|
||||
authStore.getUserByIDFn = func(_ context.Context, id uuid.UUID) (store.User, error) {
|
||||
if id != userID {
|
||||
t.Fatalf("unexpected user id: %s", id)
|
||||
}
|
||||
return store.User{ID: userID, Email: "me@example.com"}, nil
|
||||
}
|
||||
authStore.getUserProjectFn = func(_ context.Context, uid uuid.UUID) (store.Project, error) {
|
||||
return store.Project{ID: projectID, UserID: uid, Name: "default"}, nil
|
||||
}
|
||||
|
||||
router := NewRouter(a)
|
||||
req := httptest.NewRequest(http.MethodGet, "/api/v1/auth/me", nil)
|
||||
req = req.WithContext(context.WithValue(req.Context(), ctxKeyUserID{}, userID))
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
var got authResponse
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got.User.ID != userID.String() || got.User.Email != "me@example.com" {
|
||||
t.Fatalf("unexpected user in /me response: %+v", got.User)
|
||||
}
|
||||
if got.Project.ID != projectID.String() {
|
||||
t.Fatalf("unexpected project in /me response: %+v", got.Project)
|
||||
}
|
||||
}
|
||||
|
||||
// errNoRowsForTest stands in for a "not found" error a real store would
|
||||
// return (e.g. pgx.ErrNoRows) — handlers must not distinguish it from any
|
||||
// other GetUserByEmail failure in the response they send.
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package store
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -57,6 +58,41 @@ func TestGetUserByEmail_FindsRegisteredUser(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestRegisterUser_DuplicateEmailReturnsErrEmailTaken verifies the fix for
|
||||
// the duplicate-registration gap: a second RegisterUser call for an
|
||||
// already-taken email must fail with the ErrEmailTaken sentinel (mapped from
|
||||
// the UNIQUE constraint violation on users.email), not a generic pgx error.
|
||||
func TestRegisterUser_DuplicateEmailReturnsErrEmailTaken(t *testing.T) {
|
||||
s, ctx := newStore(t)
|
||||
|
||||
if _, _, err := s.RegisterUser(ctx, "dup@example.com", "argon2-hash"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if _, _, err := s.RegisterUser(ctx, "dup@example.com", "argon2-hash"); !errors.Is(err, ErrEmailTaken) {
|
||||
t.Fatalf("expected ErrEmailTaken, got %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestGetUserByID_ReturnsUser verifies the fix for the /me gap: GetUserByID
|
||||
// returns the same user created by RegisterUser, including their real email.
|
||||
func TestGetUserByID_ReturnsUser(t *testing.T) {
|
||||
s, ctx := newStore(t)
|
||||
|
||||
u, _, err := s.RegisterUser(ctx, "gina@example.com", "argon2-hash")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
got, err := s.GetUserByID(ctx, u.ID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got.ID != u.ID || got.Email != "gina@example.com" {
|
||||
t.Fatalf("unexpected user: %+v", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSessionLifecycle_CreateGetDelete verifies CreateSession + GetSessionUser
|
||||
// round-trips to the owning user ID, an expired session is excluded from
|
||||
// GetSessionUser, and DeleteSession removes the session.
|
||||
|
||||
@@ -48,3 +48,19 @@ func (q *Queries) GetUserByEmail(ctx context.Context, email string) (User, error
|
||||
)
|
||||
return i, err
|
||||
}
|
||||
|
||||
const getUserByID = `-- name: GetUserByID :one
|
||||
SELECT id, email, created_at, password_hash FROM users WHERE id = $1
|
||||
`
|
||||
|
||||
func (q *Queries) GetUserByID(ctx context.Context, id uuid.UUID) (User, error) {
|
||||
row := q.db.QueryRow(ctx, getUserByID, id)
|
||||
var i User
|
||||
err := row.Scan(
|
||||
&i.ID,
|
||||
&i.Email,
|
||||
&i.CreatedAt,
|
||||
&i.PasswordHash,
|
||||
)
|
||||
return i, err
|
||||
}
|
||||
|
||||
@@ -3,3 +3,6 @@ INSERT INTO users (id, email, password_hash) VALUES ($1, $2, $3) RETURNING *;
|
||||
|
||||
-- name: GetUserByEmail :one
|
||||
SELECT * FROM users WHERE email = $1;
|
||||
|
||||
-- name: GetUserByID :one
|
||||
SELECT * FROM users WHERE id = $1;
|
||||
|
||||
@@ -7,6 +7,7 @@ import (
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/jackc/pgx/v5"
|
||||
"github.com/jackc/pgx/v5/pgconn"
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
|
||||
"github.com/vasyakrg/dns-autoresolver/internal/provider"
|
||||
@@ -14,6 +15,11 @@ import (
|
||||
"github.com/vasyakrg/dns-autoresolver/internal/store/dto"
|
||||
)
|
||||
|
||||
// ErrEmailTaken is returned by RegisterUser when the email is already
|
||||
// registered — a UNIQUE constraint violation (pgerrcode 23505) on
|
||||
// users.email.
|
||||
var ErrEmailTaken = errors.New("store: email already registered")
|
||||
|
||||
// Account/Template/Domain are provider-neutral domain structs returned by the
|
||||
// thin wrappers below, so callers (internal/api) never need to import
|
||||
// internal/store/db directly.
|
||||
@@ -279,6 +285,17 @@ func (s *Store) GetUserByEmail(ctx context.Context, email string) (User, error)
|
||||
return toUser(u), nil
|
||||
}
|
||||
|
||||
// GetUserByID looks up a user by primary key — used by handleMe (Task 3
|
||||
// hardening) to return the authenticated caller's real email instead of
|
||||
// leaving it blank.
|
||||
func (s *Store) GetUserByID(ctx context.Context, id uuid.UUID) (User, error) {
|
||||
u, err := s.q.GetUserByID(ctx, id)
|
||||
if err != nil {
|
||||
return User{}, err
|
||||
}
|
||||
return toUser(u), nil
|
||||
}
|
||||
|
||||
func (s *Store) CreateProjectForUser(ctx context.Context, userID uuid.UUID, name string) (Project, error) {
|
||||
p, err := s.q.CreateProject(ctx, db.CreateProjectParams{ID: uuid.New(), UserID: userID, Name: name})
|
||||
if err != nil {
|
||||
@@ -337,6 +354,10 @@ func (s *Store) RegisterUser(ctx context.Context, email, passwordHash string) (U
|
||||
uid := uuid.New()
|
||||
dbu, err := q.CreateUser(ctx, db.CreateUserParams{ID: uid, Email: email, PasswordHash: ptr(passwordHash)})
|
||||
if err != nil {
|
||||
var pgErr *pgconn.PgError
|
||||
if errors.As(err, &pgErr) && pgErr.Code == "23505" {
|
||||
return User{}, Project{}, ErrEmailTaken
|
||||
}
|
||||
return User{}, Project{}, err
|
||||
}
|
||||
dbp, err := q.CreateProject(ctx, db.CreateProjectParams{ID: uuid.New(), UserID: uid, Name: "default"})
|
||||
|
||||
Reference in New Issue
Block a user