fix(auth): reseed standard roles and reject duplicate usernames as 400
Registration always writes role-guest; a deleted standard role became a masked INTERNAL. Username unique violations are now a 400 instead of an infrastructure 500.
This commit is contained in:
@@ -0,0 +1,36 @@
|
||||
-- +goose Up
|
||||
-- 注册与管理员建号硬编码依赖 role-guest / role-engineer / role-director / role-tech-lead。
|
||||
-- 00015 仅把 role-admin 标为 is_system,其余四个标准角色可被角色管理删除,
|
||||
-- 且 goose 不会重跑种子、启动引导也不补角色,缺失后注册写入 user_role_bindings 触发 FK 23503。
|
||||
-- 幂等补种并升为系统角色(禁删、禁改名;权限矩阵仍可编辑)。
|
||||
|
||||
INSERT INTO system_roles (id, code, name, description, is_system) VALUES
|
||||
('role-director', 'DIRECTOR', '研发总监', '负责研发中心整体项目决策、立项审查、权限监督与终审批准。', true),
|
||||
('role-tech-lead', 'TECH_LEAD', '研发技术主管', '负责具体研发管线的进度把控、文档责任人分配与技术审核。', true),
|
||||
('role-engineer', 'ENGINEER', '方案工程师', '负责医疗器械规格设计、受控正文编制、技术测算与推演。', true),
|
||||
('role-guest', 'GUEST', '外部审计/访客', '默认最小权限,仅在被授权范围内查阅。', true)
|
||||
ON CONFLICT (id) DO UPDATE SET is_system = true;
|
||||
|
||||
INSERT INTO system_role_permissions (role_id, permission_id, effect)
|
||||
SELECT 'role-director', p.id, 'allow' FROM system_permissions p
|
||||
WHERE p.perm_key IN ('project:create','project:assign_lead','workflow:config_visibility','doc:override_visibility','doc:export','workflow:view_all')
|
||||
ON CONFLICT DO NOTHING;
|
||||
|
||||
INSERT INTO system_role_permissions (role_id, permission_id, effect)
|
||||
SELECT 'role-tech-lead', p.id, 'allow' FROM system_permissions p
|
||||
WHERE p.perm_key IN ('project:create','project:assign_lead','doc:override_visibility','doc:export','workflow:view_all')
|
||||
ON CONFLICT DO NOTHING;
|
||||
|
||||
INSERT INTO system_role_permissions (role_id, permission_id, effect)
|
||||
SELECT 'role-engineer', p.id, 'allow' FROM system_permissions p
|
||||
WHERE p.perm_key IN ('doc:export','workflow:view_all')
|
||||
ON CONFLICT DO NOTHING;
|
||||
|
||||
INSERT INTO system_role_permissions (role_id, permission_id, effect)
|
||||
SELECT 'role-guest', p.id, 'allow' FROM system_permissions p
|
||||
WHERE p.perm_key IN ('doc:export')
|
||||
ON CONFLICT DO NOTHING;
|
||||
|
||||
-- +goose Down
|
||||
UPDATE system_roles SET is_system = false
|
||||
WHERE id IN ('role-director', 'role-tech-lead', 'role-engineer', 'role-guest');
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
|
||||
"chat-agent-system/internal/config"
|
||||
"github.com/jackc/pgx/v5"
|
||||
"github.com/jackc/pgx/v5/pgconn"
|
||||
"github.com/jackc/pgx/v5/pgxpool"
|
||||
)
|
||||
|
||||
@@ -161,6 +162,9 @@ func (s *PgStore) CreateUser(ctx context.Context, user *UserAccount) (*UserAccou
|
||||
`, user.Username, user.Name, roleLegacy, user.Status, user.MustChangePassword,
|
||||
user.RoleID, user.Position, user.Department, user.EmployeeID, user.Email).Scan(&id)
|
||||
if err != nil {
|
||||
if isUsernameConflict(err) {
|
||||
return nil, ErrUsernameExists
|
||||
}
|
||||
return nil, fmt.Errorf("insert user: %w", err)
|
||||
}
|
||||
|
||||
@@ -188,6 +192,20 @@ func (s *PgStore) CreateUser(ctx context.Context, user *UserAccount) (*UserAccou
|
||||
return s.GetUserByID(ctx, id)
|
||||
}
|
||||
|
||||
func isUsernameConflict(err error) bool {
|
||||
var pgErr *pgconn.PgError
|
||||
if !errors.As(err, &pgErr) || pgErr.Code != "23505" {
|
||||
return false
|
||||
}
|
||||
switch pgErr.ConstraintName {
|
||||
case "uq_users_username", "users_account_id_key", "users_username_key":
|
||||
return true
|
||||
default:
|
||||
name := strings.ToLower(pgErr.ConstraintName)
|
||||
return strings.Contains(name, "username") || strings.Contains(name, "account_id")
|
||||
}
|
||||
}
|
||||
|
||||
func (s *PgStore) UpdateUser(ctx context.Context, user *UserAccount, actorID int64) (*UserAccount, error) {
|
||||
_, err := s.pool.Exec(ctx, `
|
||||
UPDATE users SET
|
||||
|
||||
@@ -2,6 +2,7 @@ package auth_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"os"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -80,6 +81,48 @@ func TestPgStore_SessionSurvivesNewStoreInstance(t *testing.T) {
|
||||
_ = store2.RevokeByTokenHash(ctx, tokenHash)
|
||||
}
|
||||
|
||||
func TestPgStore_CreateUserDuplicateUsername(t *testing.T) {
|
||||
pool := openTestPool(t)
|
||||
if pool == nil {
|
||||
return
|
||||
}
|
||||
defer pool.Close()
|
||||
|
||||
cfg := &config.Config{Env: "dev"}
|
||||
ctx := context.Background()
|
||||
store, err := auth.NewPgStore(pool, cfg)
|
||||
if err != nil {
|
||||
t.Fatalf("NewPgStore: %v", err)
|
||||
}
|
||||
username := "dup_" + time.Now().Format("150405.000000")
|
||||
account := &auth.UserAccount{
|
||||
Username: username,
|
||||
Name: "重复探测",
|
||||
PasswordHash: "hash",
|
||||
Status: auth.StatusActive,
|
||||
RoleID: "role-guest",
|
||||
Position: "访客/待指派",
|
||||
}
|
||||
created, err := store.CreateUser(ctx, account)
|
||||
if err != nil {
|
||||
t.Fatalf("first CreateUser: %v", err)
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
_, _ = pool.Exec(context.Background(), `DELETE FROM users WHERE id = $1`, created.ID)
|
||||
})
|
||||
_, err = store.CreateUser(ctx, &auth.UserAccount{
|
||||
Username: username,
|
||||
Name: "重复探测",
|
||||
PasswordHash: "hash",
|
||||
Status: auth.StatusActive,
|
||||
RoleID: "role-guest",
|
||||
Position: "访客/待指派",
|
||||
})
|
||||
if !errors.Is(err, auth.ErrUsernameExists) {
|
||||
t.Fatalf("duplicate username = %v, want ErrUsernameExists", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPgStore_DisabledUserSessionRejected(t *testing.T) {
|
||||
pool := openTestPool(t)
|
||||
if pool == nil {
|
||||
|
||||
@@ -2,6 +2,7 @@ package auth
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
"sync"
|
||||
@@ -10,6 +11,9 @@ import (
|
||||
"chat-agent-system/internal/config"
|
||||
)
|
||||
|
||||
// ErrUsernameExists 用户名或 account_id 唯一约束冲突。调用方应返回 400,而不是基础设施 500。
|
||||
var ErrUsernameExists = errors.New("用户名已存在")
|
||||
|
||||
// AuthStore 用户与认证配置存储接口
|
||||
type AuthStore interface {
|
||||
GetUserByUsername(ctx context.Context, username string) (*UserAccount, error)
|
||||
@@ -162,7 +166,7 @@ func (s *MemoryStore) CreateUser(ctx context.Context, user *UserAccount) (*UserA
|
||||
|
||||
for _, existing := range s.users {
|
||||
if strings.EqualFold(existing.Username, user.Username) {
|
||||
return nil, fmt.Errorf("username already exists: %s", user.Username)
|
||||
return nil, ErrUsernameExists
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,39 @@
|
||||
package auth
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"testing"
|
||||
|
||||
"github.com/jackc/pgx/v5/pgconn"
|
||||
)
|
||||
|
||||
func TestIsUsernameConflict(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
err error
|
||||
want bool
|
||||
}{
|
||||
{"username index", &pgconn.PgError{Code: "23505", ConstraintName: "uq_users_username"}, true},
|
||||
{"account id", &pgconn.PgError{Code: "23505", ConstraintName: "users_account_id_key"}, true},
|
||||
{"username key", &pgconn.PgError{Code: "23505", ConstraintName: "users_username_key"}, true},
|
||||
{"fk is not username", &pgconn.PgError{Code: "23503", ConstraintName: "user_role_bindings_role_id_fkey"}, false},
|
||||
{"other unique", &pgconn.PgError{Code: "23505", ConstraintName: "user_credentials_pkey"}, false},
|
||||
{"plain error", errors.New("insert user: boom"), false},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if got := isUsernameConflict(tc.err); got != tc.want {
|
||||
t.Fatalf("isUsernameConflict() = %v, want %v", got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestErrUsernameExistsIsNotInfrastructureShaped(t *testing.T) {
|
||||
if !errors.Is(ErrUsernameExists, ErrUsernameExists) {
|
||||
t.Fatal("sentinel identity")
|
||||
}
|
||||
if ErrUsernameExists.Error() != "用户名已存在" {
|
||||
t.Fatalf("message = %q", ErrUsernameExists.Error())
|
||||
}
|
||||
}
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"strconv"
|
||||
"strings"
|
||||
@@ -89,7 +90,12 @@ func (s *Server) handleAuthLogin(w http.ResponseWriter, r *http.Request) {
|
||||
writeSafeError(w, r, http.StatusForbidden, errMsg)
|
||||
return
|
||||
}
|
||||
writeSafeError(w, r, http.StatusUnauthorized, errMsg)
|
||||
status := http.StatusUnauthorized
|
||||
if authIsInfrastructureError(err) {
|
||||
status = http.StatusInternalServerError
|
||||
slog.Error("auth_login_failed", "request_id", httpx.RequestID(r), "err", err)
|
||||
}
|
||||
writeSafeError(w, r, status, errMsg)
|
||||
return
|
||||
}
|
||||
|
||||
@@ -122,6 +128,7 @@ func (s *Server) handleAuthRegister(w http.ResponseWriter, r *http.Request) {
|
||||
status := http.StatusBadRequest
|
||||
if authIsInfrastructureError(err) {
|
||||
status = http.StatusInternalServerError
|
||||
slog.Error("auth_register_failed", "request_id", httpx.RequestID(r), "err", err)
|
||||
}
|
||||
writeSafeError(w, r, status, err.Error())
|
||||
return
|
||||
|
||||
@@ -216,6 +216,48 @@ func TestAuthHandlers_RegisterAndApproval(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthHandlers_RegisterDuplicateUsername(t *testing.T) {
|
||||
ts, _ := setupTestServer(t)
|
||||
body, _ := json.Marshal(map[string]string{
|
||||
"username": "dup_user",
|
||||
"password": "DupPass123",
|
||||
"name": "重复用户",
|
||||
})
|
||||
first, err := http.Post(ts.URL+"/api/v1/auth/register", "application/json", bytes.NewReader(body))
|
||||
if err != nil {
|
||||
t.Fatalf("first register: %v", err)
|
||||
}
|
||||
defer first.Body.Close()
|
||||
if first.StatusCode != http.StatusOK {
|
||||
t.Fatalf("first register status %d", first.StatusCode)
|
||||
}
|
||||
|
||||
secondBody, _ := json.Marshal(map[string]string{
|
||||
"username": "dup_user",
|
||||
"password": "DupPass123",
|
||||
"name": "重复用户",
|
||||
})
|
||||
second, err := http.Post(ts.URL+"/api/v1/auth/register", "application/json", bytes.NewReader(secondBody))
|
||||
if err != nil {
|
||||
t.Fatalf("second register: %v", err)
|
||||
}
|
||||
defer second.Body.Close()
|
||||
if second.StatusCode != http.StatusBadRequest {
|
||||
t.Fatalf("duplicate username must be 400, got %d", second.StatusCode)
|
||||
}
|
||||
var env map[string]any
|
||||
if err := json.NewDecoder(second.Body).Decode(&env); err != nil {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
errText, _ := env["error"].(string)
|
||||
if !bytes.Contains([]byte(errText), []byte("已存在")) {
|
||||
t.Fatalf("expected error to mention 已存在, got %#v", env)
|
||||
}
|
||||
if env["code"] != "VALIDATION" {
|
||||
t.Fatalf("expected VALIDATION, got %#v", env["code"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthHandlers_AdminCreateUser_MustChangePassword(t *testing.T) {
|
||||
ts, _ := setupTestServer(t)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user