feat(PROJ-46): E-Mail als primärer Login-Identifier für Tenant-User
Tenant-User (tenant_id IS NOT NULL) melden sich künftig per E-Mail an statt per Username — behebt Verwechslungen wie im Support-Fall vom 2026-06-13 (Login schlug trotz Passwort-Reset fehl, weil E-Mail statt Username verwendet wurde). Nicht-Tenant-User (Superadmin/System) können weiterhin Username ODER E-Mail nutzen. Neue Store.VerifyLogin() prüft erst per E-Mail (alle User), fällt dann auf Username zurück (nur tenant_id IS NULL). VerifyPassword() bleibt für den IMAP-Server-Login-Pfad (PROJ-26) unverändert. Bewusster Breaking Change für Tenant-User, Datenqualität vorab geprüft (0 Kollisionen). Security-Nachtrag: bcrypt-Dummy-Compare im "user not found"-Pfad ergänzt, um Timing-basierte Identifier-Enumeration zu verhindern. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
804cd62201
commit
767373b206
@@ -20,6 +20,13 @@ const (
|
||||
RoleSuperAdmin = "superadmin"
|
||||
|
||||
bcryptCost = 12
|
||||
|
||||
// dummyBcryptHash is used by VerifyLogin to burn bcrypt time when no user
|
||||
// matched, closing the timing side-channel that would otherwise let an
|
||||
// attacker distinguish "unknown identifier" from "wrong password" (PROJ-46
|
||||
// security review). Precomputed hash of a fixed placeholder string — the
|
||||
// plaintext is never used or compared meaningfully, only the cost matters.
|
||||
dummyBcryptHash = "$2a$12$C6UzMDM.H6dfI/f/IKcEeO4TW/OZ/6PdTdSU0/eV1JCJXo.0DGvTa"
|
||||
)
|
||||
|
||||
// User represents a user account in the system.
|
||||
@@ -284,6 +291,59 @@ func (s *Store) VerifyPassword(username, password string) (*User, error) {
|
||||
return &u, nil
|
||||
}
|
||||
|
||||
// VerifyLogin checks credentials for the web-login path (PROJ-46) and returns
|
||||
// the user on success. Lookup semantics:
|
||||
// 1. Match by email (`email = $1`) — valid for ALL users (tenant users AND
|
||||
// non-tenant users like superadmin/system).
|
||||
// 2. If no email match, fall back to username (`username = $1`) — but ONLY
|
||||
// accept the match when tenant_id IS NULL (superadmin/system users).
|
||||
// Tenant users (tenant_id IS NOT NULL) can therefore no longer log in via
|
||||
// their username; they must use their email address.
|
||||
//
|
||||
// Note: VerifyPassword (username-only) is intentionally left untouched — it is
|
||||
// used by the IMAP server login path (PROJ-26).
|
||||
func (s *Store) VerifyLogin(ctx context.Context, identifier, password string) (*User, error) {
|
||||
// 1. Email lookup — matches any user.
|
||||
row := s.pool.QueryRow(ctx,
|
||||
`SELECT id, username, email, role, source, active, created_at, tenant_id, totp_enabled, totp_reset_at, totp_reset_by, list_page_size, password_hash
|
||||
FROM users WHERE email = $1`,
|
||||
identifier,
|
||||
)
|
||||
u, hash, err := scanUserWithHash(row)
|
||||
if err != nil {
|
||||
if !errors.Is(err, pgx.ErrNoRows) {
|
||||
return nil, fmt.Errorf("userstore: verify login (email): %w", err)
|
||||
}
|
||||
// 2. Username lookup — only accepted for non-tenant users (tenant_id IS NULL).
|
||||
row = s.pool.QueryRow(ctx,
|
||||
`SELECT id, username, email, role, source, active, created_at, tenant_id, totp_enabled, totp_reset_at, totp_reset_by, list_page_size, password_hash
|
||||
FROM users WHERE username = $1 AND tenant_id IS NULL`,
|
||||
identifier,
|
||||
)
|
||||
u, hash, err = scanUserWithHash(row)
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
// PROJ-46 security review: run bcrypt against a dummy hash even when no
|
||||
// user was found, so "unknown identifier" and "wrong password" take
|
||||
// comparable time. Without this, the missing bcrypt call (~150-300ms
|
||||
// cheaper) lets an attacker enumerate valid identifiers by timing the
|
||||
// login endpoint.
|
||||
_ = bcrypt.CompareHashAndPassword([]byte(dummyBcryptHash), []byte(password))
|
||||
return nil, errors.New("userstore: user not found")
|
||||
}
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("userstore: verify login (username): %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
if !u.Active {
|
||||
return nil, errors.New("userstore: account disabled")
|
||||
}
|
||||
if err := bcrypt.CompareHashAndPassword([]byte(hash), []byte(password)); err != nil {
|
||||
return nil, errors.New("userstore: wrong password")
|
||||
}
|
||||
return u, nil
|
||||
}
|
||||
|
||||
// Update applies a partial update to a user record.
|
||||
func (s *Store) Update(id int64, req UpdateUserRequest) (*User, error) {
|
||||
ctx := context.Background()
|
||||
@@ -517,6 +577,19 @@ func scanUser(row pgx.Row) (*User, error) {
|
||||
return &u, nil
|
||||
}
|
||||
|
||||
// scanUserWithHash scans a full user row that includes the password_hash column
|
||||
// (used by the login verification paths). The pgx.ErrNoRows sentinel is passed
|
||||
// through unwrapped so callers can distinguish "not found" from other errors.
|
||||
func scanUserWithHash(row pgx.Row) (*User, string, error) {
|
||||
var u User
|
||||
var hash string
|
||||
err := row.Scan(&u.ID, &u.Username, &u.Email, &u.Role, &u.Source, &u.Active, &u.CreatedAt, &u.TenantID, &u.TOTPEnabled, &u.TOTPResetAt, &u.TOTPResetBy, &u.ListPageSize, &hash)
|
||||
if err != nil {
|
||||
return nil, "", err
|
||||
}
|
||||
return &u, hash, nil
|
||||
}
|
||||
|
||||
func scanUserRow(rows pgx.Rows) (*User, error) {
|
||||
var u User
|
||||
if err := rows.Scan(&u.ID, &u.Username, &u.Email, &u.Role, &u.Source, &u.Active, &u.CreatedAt, &u.TenantID, &u.TOTPEnabled, &u.TOTPResetAt, &u.TOTPResetBy, &u.ListPageSize); err != nil {
|
||||
|
||||
@@ -116,6 +116,74 @@ func TestVerifyPassword(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestVerifyLogin covers the full PROJ-46 login-identifier matrix.
|
||||
func TestVerifyLogin(t *testing.T) {
|
||||
s := newTestStore(t)
|
||||
ctx := context.Background()
|
||||
|
||||
tenantID := int64(1)
|
||||
|
||||
// Tenant user: username "patrick" != email
|
||||
if _, err := s.Create(userstore.CreateUserRequest{
|
||||
Username: "patrick", Email: "patrick@perlbach24.de",
|
||||
Password: "pw-tenant", Role: userstore.RoleUser, TenantID: &tenantID,
|
||||
}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// Non-tenant user (superadmin/system), tenant_id IS NULL
|
||||
if _, err := s.Create(userstore.CreateUserRequest{
|
||||
Username: "superadmin", Email: "superadmin@localhost",
|
||||
Password: "pw-super", Role: userstore.RoleSuperAdmin, TenantID: nil,
|
||||
}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// 1. Tenant user via email → success
|
||||
u, err := s.VerifyLogin(ctx, "patrick@perlbach24.de", "pw-tenant")
|
||||
if err != nil {
|
||||
t.Fatalf("tenant user via email should succeed: %v", err)
|
||||
}
|
||||
if u.Username != "patrick" {
|
||||
t.Errorf("Username = %q, want patrick", u.Username)
|
||||
}
|
||||
|
||||
// 2. Tenant user via username (≠ email) → invalid_credentials
|
||||
if _, err := s.VerifyLogin(ctx, "patrick", "pw-tenant"); err == nil {
|
||||
t.Error("tenant user via username should be rejected")
|
||||
}
|
||||
|
||||
// 3. Non-tenant user via username → success
|
||||
u, err = s.VerifyLogin(ctx, "superadmin", "pw-super")
|
||||
if err != nil {
|
||||
t.Fatalf("non-tenant user via username should succeed: %v", err)
|
||||
}
|
||||
if u.Username != "superadmin" {
|
||||
t.Errorf("Username = %q, want superadmin", u.Username)
|
||||
}
|
||||
|
||||
// 4. Non-tenant user via email → success
|
||||
u, err = s.VerifyLogin(ctx, "superadmin@localhost", "pw-super")
|
||||
if err != nil {
|
||||
t.Fatalf("non-tenant user via email should succeed: %v", err)
|
||||
}
|
||||
if u.Username != "superadmin" {
|
||||
t.Errorf("Username = %q, want superadmin", u.Username)
|
||||
}
|
||||
|
||||
// 5. Unknown identifier → invalid_credentials
|
||||
if _, err := s.VerifyLogin(ctx, "ghost@nowhere.tld", "x"); err == nil {
|
||||
t.Error("unknown identifier should be rejected")
|
||||
}
|
||||
if _, err := s.VerifyLogin(ctx, "ghost", "x"); err == nil {
|
||||
t.Error("unknown username should be rejected")
|
||||
}
|
||||
|
||||
// Wrong password for valid identifier → rejected
|
||||
if _, err := s.VerifyLogin(ctx, "patrick@perlbach24.de", "wrong"); err == nil {
|
||||
t.Error("wrong password should be rejected")
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdateUser(t *testing.T) {
|
||||
s := newTestStore(t)
|
||||
u, _ := s.Create(userstore.CreateUserRequest{
|
||||
|
||||
Reference in New Issue
Block a user