diff --git a/client/auth.go b/client/auth.go index 40b9af9f..f1d482e1 100644 --- a/client/auth.go +++ b/client/auth.go @@ -22,15 +22,38 @@ type LoginRequest struct { // role in each, so callers can build a tenant switcher UI without a // follow-up request. type LoginResponse struct { - Success bool `json:"success"` - Message string `json:"message,omitempty"` - User *AuthUser `json:"user,omitempty"` - ActiveTenant *AuthTenant `json:"active_tenant,omitempty"` + Success bool `json:"success"` + Message string `json:"message,omitempty"` + User *AuthUser `json:"user,omitempty"` + // ActiveTenant is the tenant whose ID is encoded in the JWT. Use + // this field; Tenant is preserved as a compatibility alias only. + ActiveTenant *AuthTenant `json:"active_tenant,omitempty"` + // Tenant is a deprecated alias kept so SDK callers compiled against + // the pre-RBAC release continue to find the field by name. The + // server emits active_tenant; this field is populated on the client + // side by GetTenant() and will be removed in a future major release. + // + // Deprecated: use ActiveTenant. + Tenant *AuthTenant `json:"-"` Memberships []AuthMembership `json:"memberships,omitempty"` Token string `json:"token,omitempty"` RefreshToken string `json:"refresh_token,omitempty"` } +// GetTenant returns the active tenant, preferring ActiveTenant and +// falling back to the deprecated Tenant alias. SDK code should prefer +// reading ActiveTenant directly; this helper exists to keep callers that +// only know about the old field name compiling and working. +func (r *LoginResponse) GetTenant() *AuthTenant { + if r == nil { + return nil + } + if r.ActiveTenant != nil { + return r.ActiveTenant + } + return r.Tenant +} + // AuthMembership pairs a tenant ID with the user's role in that tenant. // Mirrors types.Membership on the server. type AuthMembership struct { @@ -97,6 +120,9 @@ func (c *Client) Login(ctx context.Context, req LoginRequest) (*LoginResponse, e if err := parseResponse(resp, &out); err != nil { return nil, err } + // 后端已将 tenant 字段重命名为 active_tenant;为照顾仍读取旧字段名的下游 + // 调用者,在反序列化后镜像一份到 Tenant 上。两者总是指向同一指针。 + out.Tenant = out.ActiveTenant return &out, nil } diff --git a/frontend/src/stores/auth.ts b/frontend/src/stores/auth.ts index 4f0d161e..22899cac 100644 --- a/frontend/src/stores/auth.ts +++ b/frontend/src/stores/auth.ts @@ -59,6 +59,13 @@ export const useAuthStore = defineStore('auth', () => { // (defaulting to '' when memberships have not been loaded). Used by // role-aware UI gating; PR 2 wires backend enforcement, PR 3 uses // this for menu/button visibility. + // + // SECURITY: This value is read from localStorage (`weknora_memberships`) + // and therefore MUST be treated as UI-rendering-only — any user can + // tamper with localStorage and grant themselves "owner" here. All real + // authorisation decisions live on the server (auth middleware resolves + // the role from tenant_members on every request). Never branch + // security-sensitive logic on this value alone. const currentTenantRole = computed(() => { const tid = tenant.value?.id ? String(tenant.value.id) : '' if (!tid) return '' diff --git a/internal/application/repository/tenant.go b/internal/application/repository/tenant.go index b5978279..dfb7a2d5 100644 --- a/internal/application/repository/tenant.go +++ b/internal/application/repository/tenant.go @@ -45,6 +45,27 @@ func (r *tenantRepository) GetTenantByID(ctx context.Context, id uint64) (*types return &tenant, nil } +// GetTenantsByIDs batches GetTenantByID with a single IN-list query. +// Returns a map keyed by tenant ID; missing rows are simply absent from +// the map (no error). An empty input slice short-circuits to an empty map +// without hitting the database. +func (r *tenantRepository) GetTenantsByIDs(ctx context.Context, ids []uint64) (map[uint64]*types.Tenant, error) { + if len(ids) == 0 { + return map[uint64]*types.Tenant{}, nil + } + var tenants []*types.Tenant + if err := r.db.WithContext(ctx).Where("id IN ?", ids).Find(&tenants).Error; err != nil { + return nil, err + } + out := make(map[uint64]*types.Tenant, len(tenants)) + for _, t := range tenants { + if t != nil { + out[t.ID] = t + } + } + return out, nil +} + // ListTenants lists all tenants func (r *tenantRepository) ListTenants(ctx context.Context) ([]*types.Tenant, error) { var tenants []*types.Tenant diff --git a/internal/application/repository/tenant_member.go b/internal/application/repository/tenant_member.go index 97817e79..b8c7a782 100644 --- a/internal/application/repository/tenant_member.go +++ b/internal/application/repository/tenant_member.go @@ -115,13 +115,24 @@ func (r *tenantMemberRepository) CountActiveOwners(ctx context.Context, tenantID } // HasAnyMembers reports whether the tenant has at least one active -// membership row. +// membership row. Uses a LIMIT 1 SELECT (instead of COUNT(*)) so the query +// short-circuits after the first match — important because this is on the +// auth middleware's hot path for users without a cached membership. func (r *tenantMemberRepository) HasAnyMembers(ctx context.Context, tenantID uint64) (bool, error) { - var count int64 + var probe struct { + ID uint64 + } err := r.db.WithContext(ctx). Model(&types.TenantMember{}). + Select("id"). Where("tenant_id = ? AND status = ?", tenantID, types.TenantMemberStatusActive). Limit(1). - Count(&count).Error - return count > 0, err + Take(&probe).Error + if err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return false, nil + } + return false, err + } + return true, nil } diff --git a/internal/application/service/tenant.go b/internal/application/service/tenant.go index 1e115e30..5e4201b3 100644 --- a/internal/application/service/tenant.go +++ b/internal/application/service/tenant.go @@ -118,6 +118,11 @@ func (s *tenantService) GetTenantByID(ctx context.Context, id uint64) (*types.Te return tenant, nil } +// GetTenantsByIDs batches GetTenantByID; returns a map keyed by tenant ID. +func (s *tenantService) GetTenantsByIDs(ctx context.Context, ids []uint64) (map[uint64]*types.Tenant, error) { + return s.repo.GetTenantsByIDs(ctx, ids) +} + // ListTenants retrieves a list of all tenants func (s *tenantService) ListTenants(ctx context.Context) ([]*types.Tenant, error) { tenants, err := s.repo.ListTenants(ctx) diff --git a/internal/application/service/tenant_member_test.go b/internal/application/service/tenant_member_test.go new file mode 100644 index 00000000..5f300339 --- /dev/null +++ b/internal/application/service/tenant_member_test.go @@ -0,0 +1,292 @@ +package service + +import ( + "context" + "errors" + "testing" + + "github.com/Tencent/WeKnora/internal/types" + "github.com/Tencent/WeKnora/internal/types/interfaces" +) + +// fakeTenantMemberRepo is an in-memory implementation of +// interfaces.TenantMemberRepository for unit tests. It is intentionally +// not safe for concurrent use; tests should drive it sequentially. +type fakeTenantMemberRepo struct { + rows []*types.TenantMember + // nextID is incremented on Create to populate the surrogate PK. + nextID uint64 + // failGet, failHasAny etc. let tests inject transient errors on the + // matching method to exercise error paths. + failGet error + failHasAny error + failCountOwners error + failUpdateRole error + failSoftDelete error + failCreate error +} + +func newFakeRepo() *fakeTenantMemberRepo { return &fakeTenantMemberRepo{} } + +func (r *fakeTenantMemberRepo) Create(ctx context.Context, m *types.TenantMember) error { + if r.failCreate != nil { + return r.failCreate + } + r.nextID++ + m.ID = r.nextID + // Mirror the partial unique index on (user_id, tenant_id) for + // active rows so tests can exercise duplicate-insert behaviour. + for _, e := range r.rows { + if e.UserID == m.UserID && e.TenantID == m.TenantID && e.DeletedAt.Valid == false { + return errors.New("duplicate active membership") + } + } + cp := *m + r.rows = append(r.rows, &cp) + return nil +} + +func (r *fakeTenantMemberRepo) Get(ctx context.Context, userID string, tenantID uint64) (*types.TenantMember, error) { + if r.failGet != nil { + return nil, r.failGet + } + for _, e := range r.rows { + if e.UserID == userID && e.TenantID == tenantID && !e.DeletedAt.Valid { + cp := *e + return &cp, nil + } + } + return nil, nil +} + +func (r *fakeTenantMemberRepo) ListByUser(ctx context.Context, userID string) ([]*types.TenantMember, error) { + var out []*types.TenantMember + for _, e := range r.rows { + if e.UserID == userID && !e.DeletedAt.Valid { + cp := *e + out = append(out, &cp) + } + } + return out, nil +} + +func (r *fakeTenantMemberRepo) ListByTenant(ctx context.Context, tenantID uint64) ([]*types.TenantMember, error) { + var out []*types.TenantMember + for _, e := range r.rows { + if e.TenantID == tenantID && !e.DeletedAt.Valid { + cp := *e + out = append(out, &cp) + } + } + return out, nil +} + +func (r *fakeTenantMemberRepo) UpdateRole(ctx context.Context, userID string, tenantID uint64, role types.TenantRole) error { + if r.failUpdateRole != nil { + return r.failUpdateRole + } + for _, e := range r.rows { + if e.UserID == userID && e.TenantID == tenantID && !e.DeletedAt.Valid { + e.Role = role + return nil + } + } + return errors.New("not found") +} + +func (r *fakeTenantMemberRepo) SoftDelete(ctx context.Context, userID string, tenantID uint64) error { + if r.failSoftDelete != nil { + return r.failSoftDelete + } + for _, e := range r.rows { + if e.UserID == userID && e.TenantID == tenantID && !e.DeletedAt.Valid { + e.DeletedAt.Valid = true + return nil + } + } + return errors.New("not found") +} + +func (r *fakeTenantMemberRepo) CountActiveOwners(ctx context.Context, tenantID uint64) (int64, error) { + if r.failCountOwners != nil { + return 0, r.failCountOwners + } + var n int64 + for _, e := range r.rows { + if e.TenantID == tenantID && !e.DeletedAt.Valid && + e.Role == types.TenantRoleOwner && e.Status == types.TenantMemberStatusActive { + n++ + } + } + return n, nil +} + +func (r *fakeTenantMemberRepo) HasAnyMembers(ctx context.Context, tenantID uint64) (bool, error) { + if r.failHasAny != nil { + return false, r.failHasAny + } + for _, e := range r.rows { + if e.TenantID == tenantID && !e.DeletedAt.Valid && e.Status == types.TenantMemberStatusActive { + return true, nil + } + } + return false, nil +} + +// Compile-time guard so the test stays in sync with the interface. +var _ interfaces.TenantMemberRepository = (*fakeTenantMemberRepo)(nil) + +func newServiceWithRepo() (interfaces.TenantMemberService, *fakeTenantMemberRepo) { + r := newFakeRepo() + return NewTenantMemberService(r), r +} + +func TestTenantMemberService_AddMember_RejectsInvalidRole(t *testing.T) { + svc, _ := newServiceWithRepo() + _, err := svc.AddMember(context.Background(), "u1", 1, types.TenantRole("nonsense"), nil) + if !errors.Is(err, ErrInvalidTenantRole) { + t.Fatalf("want ErrInvalidTenantRole, got %v", err) + } +} + +func TestTenantMemberService_AddMember_RejectsDuplicate(t *testing.T) { + svc, _ := newServiceWithRepo() + ctx := context.Background() + if _, err := svc.AddMember(ctx, "u1", 1, types.TenantRoleContributor, nil); err != nil { + t.Fatalf("first AddMember: %v", err) + } + _, err := svc.AddMember(ctx, "u1", 1, types.TenantRoleContributor, nil) + if !errors.Is(err, ErrMembershipAlreadyExists) { + t.Fatalf("want ErrMembershipAlreadyExists, got %v", err) + } +} + +func TestTenantMemberService_EnsureOwner_Idempotent(t *testing.T) { + svc, repo := newServiceWithRepo() + ctx := context.Background() + first, err := svc.EnsureOwner(ctx, "u1", 1) + if err != nil { + t.Fatalf("first EnsureOwner: %v", err) + } + second, err := svc.EnsureOwner(ctx, "u1", 1) + if err != nil { + t.Fatalf("second EnsureOwner: %v", err) + } + if first.ID != second.ID { + t.Fatalf("EnsureOwner not idempotent: %d vs %d", first.ID, second.ID) + } + if len(repo.rows) != 1 { + t.Fatalf("want exactly 1 row after idempotent EnsureOwner, got %d", len(repo.rows)) + } +} + +func TestTenantMemberService_UpdateRole_BlocksDemotingLastOwner(t *testing.T) { + svc, _ := newServiceWithRepo() + ctx := context.Background() + if _, err := svc.EnsureOwner(ctx, "owner", 1); err != nil { + t.Fatalf("seed: %v", err) + } + err := svc.UpdateRole(ctx, "owner", 1, types.TenantRoleAdmin) + if !errors.Is(err, ErrLastOwner) { + t.Fatalf("want ErrLastOwner when demoting last owner, got %v", err) + } +} + +func TestTenantMemberService_UpdateRole_AllowsDemotionWhenOtherOwnerExists(t *testing.T) { + svc, _ := newServiceWithRepo() + ctx := context.Background() + if _, err := svc.EnsureOwner(ctx, "owner1", 1); err != nil { + t.Fatalf("seed1: %v", err) + } + if _, err := svc.AddMember(ctx, "owner2", 1, types.TenantRoleOwner, nil); err != nil { + t.Fatalf("seed2: %v", err) + } + if err := svc.UpdateRole(ctx, "owner1", 1, types.TenantRoleAdmin); err != nil { + t.Fatalf("UpdateRole: %v", err) + } +} + +func TestTenantMemberService_UpdateRole_NoopOnSameRole(t *testing.T) { + svc, _ := newServiceWithRepo() + ctx := context.Background() + if _, err := svc.EnsureOwner(ctx, "owner", 1); err != nil { + t.Fatalf("seed: %v", err) + } + // 把"还是 Owner"作为 no-op 处理,必须不触发 ErrLastOwner(同一角色不算降级)。 + if err := svc.UpdateRole(ctx, "owner", 1, types.TenantRoleOwner); err != nil { + t.Fatalf("UpdateRole same role should be a no-op, got %v", err) + } +} + +func TestTenantMemberService_UpdateRole_RejectsInvalidRole(t *testing.T) { + svc, _ := newServiceWithRepo() + ctx := context.Background() + if _, err := svc.EnsureOwner(ctx, "owner", 1); err != nil { + t.Fatalf("seed: %v", err) + } + if err := svc.UpdateRole(ctx, "owner", 1, types.TenantRole("nope")); !errors.Is(err, ErrInvalidTenantRole) { + t.Fatalf("want ErrInvalidTenantRole, got %v", err) + } +} + +func TestTenantMemberService_UpdateRole_ReturnsNotFound(t *testing.T) { + svc, _ := newServiceWithRepo() + if err := svc.UpdateRole(context.Background(), "ghost", 1, types.TenantRoleAdmin); !errors.Is(err, ErrMembershipNotFound) { + t.Fatalf("want ErrMembershipNotFound, got %v", err) + } +} + +func TestTenantMemberService_RemoveMember_BlocksLastOwner(t *testing.T) { + svc, _ := newServiceWithRepo() + ctx := context.Background() + if _, err := svc.EnsureOwner(ctx, "owner", 1); err != nil { + t.Fatalf("seed: %v", err) + } + if err := svc.RemoveMember(ctx, "owner", 1); !errors.Is(err, ErrLastOwner) { + t.Fatalf("want ErrLastOwner, got %v", err) + } +} + +func TestTenantMemberService_RemoveMember_AllowsContributorRemoval(t *testing.T) { + svc, _ := newServiceWithRepo() + ctx := context.Background() + if _, err := svc.EnsureOwner(ctx, "owner", 1); err != nil { + t.Fatalf("seed owner: %v", err) + } + if _, err := svc.AddMember(ctx, "contrib", 1, types.TenantRoleContributor, nil); err != nil { + t.Fatalf("seed contributor: %v", err) + } + if err := svc.RemoveMember(ctx, "contrib", 1); err != nil { + t.Fatalf("RemoveMember: %v", err) + } + got, _ := svc.GetMembership(ctx, "contrib", 1) + if got != nil { + t.Fatalf("contributor should be soft-deleted, got %+v", got) + } +} + +func TestTenantMemberService_RemoveMember_ReturnsNotFound(t *testing.T) { + svc, _ := newServiceWithRepo() + if err := svc.RemoveMember(context.Background(), "ghost", 1); !errors.Is(err, ErrMembershipNotFound) { + t.Fatalf("want ErrMembershipNotFound, got %v", err) + } +} + +func TestTenantRole_HasPermission(t *testing.T) { + cases := []struct { + caller types.TenantRole + required types.TenantRole + want bool + }{ + {types.TenantRoleOwner, types.TenantRoleAdmin, true}, + {types.TenantRoleAdmin, types.TenantRoleOwner, false}, + {types.TenantRoleContributor, types.TenantRoleViewer, true}, + {types.TenantRoleViewer, types.TenantRoleContributor, false}, + {types.TenantRole("bogus"), types.TenantRoleViewer, false}, + } + for _, c := range cases { + if got := c.caller.HasPermission(c.required); got != c.want { + t.Errorf("HasPermission(%s, %s) = %v, want %v", c.caller, c.required, got, c.want) + } + } +} diff --git a/internal/application/service/user.go b/internal/application/service/user.go index 6127636d..396d74b4 100644 --- a/internal/application/service/user.go +++ b/internal/application/service/user.go @@ -240,13 +240,21 @@ func (s *userService) Login(ctx context.Context, req *types.LoginRequest) (*type // part of the dependency graph), this falls back to a single synthesized // row built from User.TenantID + the active tenant so callers always get // at least one entry. +func (s *userService) BuildLoginMemberships( + ctx context.Context, + user *types.User, + activeTenant *types.Tenant, +) []types.Membership { + return s.buildMembershipsForUser(ctx, user, activeTenant) +} + func (s *userService) buildMembershipsForUser( ctx context.Context, user *types.User, activeTenant *types.Tenant, ) []types.Membership { if user == nil { - return nil + return []types.Membership{} } if s.memberService == nil { return synthFallbackMembership(user, activeTenant) @@ -259,6 +267,27 @@ func (s *userService) buildMembershipsForUser( if len(rows) == 0 { return synthFallbackMembership(user, activeTenant) } + // 收集需要批量查询名称的 tenant id(跳过 activeTenant 因为它已经在手)。 + needsLookup := make([]uint64, 0, len(rows)) + for _, m := range rows { + if m == nil || m.Status != types.TenantMemberStatusActive { + continue + } + if activeTenant != nil && m.TenantID == activeTenant.ID { + continue + } + needsLookup = append(needsLookup, m.TenantID) + } + tenantByID := map[uint64]*types.Tenant{} + if len(needsLookup) > 0 { + if found, terr := s.tenantService.GetTenantsByIDs(ctx, needsLookup); terr == nil { + tenantByID = found + } else { + logger.Warnf(ctx, "Failed to batch-load tenants for memberships (user=%s): %v", + user.ID, terr) + } + } + out := make([]types.Membership, 0, len(rows)) for _, m := range rows { if m == nil || m.Status != types.TenantMemberStatusActive { @@ -267,7 +296,7 @@ func (s *userService) buildMembershipsForUser( name := "" if activeTenant != nil && m.TenantID == activeTenant.ID { name = activeTenant.Name - } else if t, err := s.tenantService.GetTenantByID(ctx, m.TenantID); err == nil && t != nil { + } else if t, ok := tenantByID[m.TenantID]; ok && t != nil { name = t.Name } out = append(out, types.Membership{ @@ -287,9 +316,21 @@ func (s *userService) buildMembershipsForUser( // populated yet (e.g. during the rollout window where the migration has // run but the auth middleware's auto-promotion hasn't fired) so the // response shape stays consistent. +// +// The fallback role is intentionally TenantRoleViewer (least privilege): +// the login response only feeds UI rendering, and the backend re-derives +// the real role from tenant_members on every request. If membership data +// is temporarily unavailable, showing a Viewer UI is preferable to +// granting a misleading Owner UI that would surface admin controls the +// backend will then 403. Once the membership row appears (via the auth +// middleware's home-tenant auto-promotion or an admin invitation) the +// next /auth/me-style refresh will upgrade the UI to the real role. func synthFallbackMembership(user *types.User, activeTenant *types.Tenant) []types.Membership { if user == nil || user.TenantID == 0 { - return nil + // Always return a non-nil slice so the login response carries an + // empty array rather than `null`, preserving the documented + // "always populated" contract on LoginResponse.Memberships. + return []types.Membership{} } name := "" if activeTenant != nil && activeTenant.ID == user.TenantID { @@ -298,7 +339,7 @@ func synthFallbackMembership(user *types.User, activeTenant *types.Tenant) []typ return []types.Membership{{ TenantID: user.TenantID, TenantName: name, - Role: types.TenantRoleOwner, // safe default for legacy single-tenant users + Role: types.TenantRoleViewer, }} } @@ -396,9 +437,25 @@ func (s *userService) LoginWithOIDC(ctx context.Context, code, redirectURI strin return nil, fmt.Errorf("failed to generate local tokens: %w", err) } + // 拉取 tenant + memberships,让 OIDC 登录的返回结构与本地登录一致, + // 前端无须为 OIDC 单独走一次 /auth/me 才能拿到角色。 + var tenant *types.Tenant + if user.TenantID > 0 { + if t, terr := s.tenantService.GetTenantByID(ctx, user.TenantID); terr == nil { + tenant = t + } else { + logger.Warnf(ctx, "OIDC login: failed to load tenant %d for user %s: %v", + user.TenantID, user.ID, terr) + } + } + memberships := s.buildMembershipsForUser(ctx, user, tenant) + return &types.OIDCCallbackResponse{ Success: true, Message: "登录成功", + User: user, + Tenant: tenant, + Memberships: memberships, Token: accessToken, RefreshToken: refreshToken, IsNewUser: isNewUser, diff --git a/internal/handler/auth.go b/internal/handler/auth.go index 1972501a..1ab5cb56 100644 --- a/internal/handler/auth.go +++ b/internal/handler/auth.go @@ -444,11 +444,15 @@ func (h *AuthHandler) GetCurrentUser(c *gin.Context) { } userInfo := user.ToUserInfo() userInfo.CanAccessAllTenants = user.CanAccessAllTenants && h.configInfo.Tenant.EnableCrossTenantAccess + // 同步返回当前用户的 memberships,让前端在页面刷新(仅命中 /auth/me) + // 后也能恢复 currentTenantRole,避免角色信息只在 login 那一刻可用。 + memberships := h.userService.BuildLoginMemberships(ctx, user, tenant) c.JSON(http.StatusOK, gin.H{ "success": true, "data": gin.H{ - "user": userInfo, - "tenant": tenant, + "user": userInfo, + "tenant": tenant, + "memberships": memberships, }, }) } diff --git a/internal/middleware/auth.go b/internal/middleware/auth.go index 9a81742f..9e142668 100644 --- a/internal/middleware/auth.go +++ b/internal/middleware/auth.go @@ -12,6 +12,7 @@ import ( "strings" "github.com/Tencent/WeKnora/internal/config" + "github.com/Tencent/WeKnora/internal/logger" "github.com/Tencent/WeKnora/internal/types" "github.com/Tencent/WeKnora/internal/types/interfaces" "github.com/gin-gonic/gin" @@ -143,7 +144,8 @@ func Auth( if !ok { // 强制 RBAC 时,缺少 active membership 即拒绝;fail-open 路径已在 // resolveTenantRole 内部处理。 - log.Printf("User %s has no active membership in tenant %d", user.ID, targetTenantID) + logger.Warnf(c.Request.Context(), + "User %s has no active membership in tenant %d", user.ID, targetTenantID) c.JSON(http.StatusForbidden, gin.H{ "error": "Forbidden: not a member of the target tenant", }) @@ -250,10 +252,17 @@ func Auth( // 2. Cross-tenant superuser switch (X-Tenant-ID with CanAccessAllTenants=true) // → grant Admin in the target tenant. Org admins are intentionally not // promoted to Owner; tenant deletion / API-key rotation should always -// stay with a real Owner inside the target tenant. -// 3. No membership but the tenant currently has zero active members -// (an API-key-only orphan tenant gaining its first human member) → -// auto-promote the user to Owner. +// stay with a real Owner inside the target tenant. Cross-tenant access +// is also never allowed to trigger the orphan-tenant auto-promotion +// below — a superuser only visits, never claims ownership. +// 3. No membership but the tenant currently has zero active members AND +// the caller is authenticating into their own home tenant (i.e. +// targetTenantID == user.TenantID and this is not a cross-tenant +// switch). This is the API-key-only orphan-tenant self-heal path: +// the registrant becomes Owner of the tenant their own user record +// points to. Any other path (cross-tenant switch, JWT minted for a +// foreign tenant, etc.) is intentionally excluded to avoid silent +// ownership grabs. // 4. Otherwise → return ok=false. Caller decides: // - When EnableRBAC=true (or cfg unavailable): treat as 403. // - When EnableRBAC=false: fail open with Admin so existing deployments @@ -277,25 +286,38 @@ func resolveTenantRole( return member.Role, true } if err != nil { - log.Printf("tenant_members lookup failed user=%s tenant=%d: %v", user.ID, targetTenantID, err) - // Fall through to the fail-open branch below; treat lookup errors - // the same as "no membership found" so a transient DB hiccup - // doesn't lock everyone out. + logger.Warnf(ctx, "tenant_members lookup failed user=%s tenant=%d: %v", + user.ID, targetTenantID, err) + // Fall through; treat lookup errors the same as "no membership + // found" so a transient DB hiccup doesn't lock everyone out. } - // 2. 跨租户超管直通:CanAccessAllTenants 用户切到别的租户时不强制要求 membership + // 2. 跨租户超管直通:CanAccessAllTenants 用户切到别的租户时不强制要求 membership。 + // 注意:这里只授予临时 Admin 角色,不写入 tenant_members,避免"看一眼别人租户" + // 意外升级为持久化所有权。 if crossTenantSwitch && user.CanAccessAllTenants { return types.TenantRoleAdmin, true } - // 3. 孤儿租户自愈:API-key-only 租户首个登录的人类用户自动成为 Owner - hasAny, anyErr := memberService.HasAnyMembers(ctx, targetTenantID) - if anyErr == nil && !hasAny { - if _, e := memberService.AddMember(ctx, user.ID, targetTenantID, types.TenantRoleOwner, nil); e == nil { - log.Printf("Auto-promoted user %s to Owner of orphan tenant %d", user.ID, targetTenantID) - return types.TenantRoleOwner, true - } else { - log.Printf("Failed to auto-promote user %s in tenant %d: %v", user.ID, targetTenantID, e) + // 3. 孤儿租户自愈:仅当用户登录的是自己的 home tenant、且该租户尚无任何活跃成员时 + // 允许自动晋升为 Owner。跨租户 switch / JWT 指向他人租户的场景一律不进入此分支, + // 防止越权获得他人租户的 Owner 权限。 + isHomeTenant := !crossTenantSwitch && targetTenantID == user.TenantID + if isHomeTenant { + hasAny, anyErr := memberService.HasAnyMembers(ctx, targetTenantID) + if anyErr == nil && !hasAny { + if _, e := memberService.AddMember( + ctx, user.ID, targetTenantID, types.TenantRoleOwner, nil, + ); e == nil { + logger.Infof(ctx, + "[audit] Auto-promoted user %s to Owner of orphan tenant %d (home_tenant=true)", + user.ID, targetTenantID, + ) + return types.TenantRoleOwner, true + } else { + logger.Warnf(ctx, "Failed to auto-promote user %s in tenant %d: %v", + user.ID, targetTenantID, e) + } } } @@ -303,7 +325,7 @@ func resolveTenantRole( if cfg != nil && cfg.Tenant != nil && cfg.Tenant.EnableRBAC { return "", false } - // fail-open 期间保持现有行为(每个登录用户在自己租户里都是“管理员”)。 + // fail-open 期间保持现有行为(每个登录用户在自己租户里都是"管理员")。 return types.TenantRoleAdmin, true } diff --git a/internal/middleware/auth_role_test.go b/internal/middleware/auth_role_test.go new file mode 100644 index 00000000..f5a38121 --- /dev/null +++ b/internal/middleware/auth_role_test.go @@ -0,0 +1,254 @@ +package middleware + +import ( + "context" + "errors" + "testing" + + "github.com/Tencent/WeKnora/internal/config" + "github.com/Tencent/WeKnora/internal/types" + "github.com/Tencent/WeKnora/internal/types/interfaces" +) + +// fakeMemberService is a hand-rolled stand-in for +// interfaces.TenantMemberService. It backs Get/HasAnyMembers/AddMember +// with two in-memory maps and lets each test seed exactly the rows it +// cares about. Other interface methods are stubbed because resolveTenantRole +// never touches them. +type fakeMemberService struct { + members map[string]*types.TenantMember // key = userID + "|" + tenantID + // addCalls records every AddMember invocation so tests can assert + // that resolveTenantRole did (or didn't) attempt to auto-promote. + addCalls []struct { + UserID string + TenantID uint64 + Role types.TenantRole + } + failGet error + failHasAny error + failAdd error + // 阻止 auto-promote 把 hasAny 翻面:默认 AddMember 成功也会写入 members map。 +} + +func newFakeMemberService() *fakeMemberService { + return &fakeMemberService{members: map[string]*types.TenantMember{}} +} + +func memberKey(u string, t uint64) string { + return u + "|" + uintToStr(t) +} + +func uintToStr(t uint64) string { + // 简单数字转字符串,避免引入额外依赖。 + if t == 0 { + return "0" + } + var buf [20]byte + i := len(buf) + for t > 0 { + i-- + buf[i] = byte('0' + t%10) + t /= 10 + } + return string(buf[i:]) +} + +func (f *fakeMemberService) seedActive(userID string, tenantID uint64, role types.TenantRole) { + f.members[memberKey(userID, tenantID)] = &types.TenantMember{ + UserID: userID, + TenantID: tenantID, + Role: role, + Status: types.TenantMemberStatusActive, + } +} + +func (f *fakeMemberService) AddMember( + ctx context.Context, userID string, tenantID uint64, role types.TenantRole, invitedBy *string, +) (*types.TenantMember, error) { + f.addCalls = append(f.addCalls, struct { + UserID string + TenantID uint64 + Role types.TenantRole + }{userID, tenantID, role}) + if f.failAdd != nil { + return nil, f.failAdd + } + m := &types.TenantMember{UserID: userID, TenantID: tenantID, Role: role, Status: types.TenantMemberStatusActive} + f.members[memberKey(userID, tenantID)] = m + return m, nil +} + +func (f *fakeMemberService) EnsureOwner( + ctx context.Context, userID string, tenantID uint64, +) (*types.TenantMember, error) { + if existing, ok := f.members[memberKey(userID, tenantID)]; ok { + return existing, nil + } + return f.AddMember(ctx, userID, tenantID, types.TenantRoleOwner, nil) +} + +func (f *fakeMemberService) GetMembership( + ctx context.Context, userID string, tenantID uint64, +) (*types.TenantMember, error) { + if f.failGet != nil { + return nil, f.failGet + } + m, ok := f.members[memberKey(userID, tenantID)] + if !ok { + return nil, nil + } + cp := *m + return &cp, nil +} + +func (f *fakeMemberService) ListByUser(ctx context.Context, userID string) ([]*types.TenantMember, error) { + return nil, nil +} +func (f *fakeMemberService) ListByTenant(ctx context.Context, tenantID uint64) ([]*types.TenantMember, error) { + return nil, nil +} +func (f *fakeMemberService) HasAnyMembers(ctx context.Context, tenantID uint64) (bool, error) { + if f.failHasAny != nil { + return false, f.failHasAny + } + for _, m := range f.members { + if m.TenantID == tenantID && m.Status == types.TenantMemberStatusActive { + return true, nil + } + } + return false, nil +} +func (f *fakeMemberService) UpdateRole( + ctx context.Context, userID string, tenantID uint64, newRole types.TenantRole, +) error { + return nil +} +func (f *fakeMemberService) RemoveMember(ctx context.Context, userID string, tenantID uint64) error { + return nil +} + +var _ interfaces.TenantMemberService = (*fakeMemberService)(nil) + +func cfgWithRBAC(enabled bool) *config.Config { + return &config.Config{Tenant: &config.TenantConfig{EnableRBAC: enabled}} +} + +func TestResolveTenantRole_ActiveMembershipWins(t *testing.T) { + svc := newFakeMemberService() + svc.seedActive("u1", 10, types.TenantRoleContributor) + + got, ok := resolveTenantRole(context.Background(), svc, + &types.User{ID: "u1", TenantID: 10}, 10, false, cfgWithRBAC(true)) + if !ok || got != types.TenantRoleContributor { + t.Fatalf("got (%v, %v), want (contributor, true)", got, ok) + } + if len(svc.addCalls) != 0 { + t.Fatalf("must not auto-promote when membership exists, got %d AddMember calls", len(svc.addCalls)) + } +} + +func TestResolveTenantRole_CrossTenantSuperuserGetsAdmin_NoAutoPromote(t *testing.T) { + // 回归 H1:跨租户超管 switch 到他人租户时,绝对不能写入 tenant_members。 + svc := newFakeMemberService() + user := &types.User{ID: "super", TenantID: 1, CanAccessAllTenants: true} + + got, ok := resolveTenantRole(context.Background(), svc, user, 99, true, cfgWithRBAC(true)) + if !ok || got != types.TenantRoleAdmin { + t.Fatalf("got (%v, %v), want (admin, true)", got, ok) + } + if len(svc.addCalls) != 0 { + t.Fatalf("cross-tenant superuser must not trigger auto-promote, got %+v", svc.addCalls) + } +} + +func TestResolveTenantRole_AutoPromoteRequiresHomeTenant(t *testing.T) { + // 回归 H1:即便 target 是孤儿租户,只要不是用户自己的 home tenant, + // 就不能 auto-promote 为 Owner。 + svc := newFakeMemberService() // 空 — 任何租户都是孤儿 + user := &types.User{ID: "u1", TenantID: 1, CanAccessAllTenants: true} + + got, ok := resolveTenantRole(context.Background(), svc, user, 42, true, cfgWithRBAC(true)) + if !ok || got != types.TenantRoleAdmin { + t.Fatalf("cross-tenant superuser should still get visitor Admin, got (%v, %v)", got, ok) + } + if len(svc.addCalls) != 0 { + t.Fatalf("auto-promote must skip cross-tenant target, got %+v", svc.addCalls) + } +} + +func TestResolveTenantRole_AutoPromoteHomeTenant(t *testing.T) { + // home tenant + 孤儿租户 + 非 switch → 允许 auto-promote 为 Owner。 + svc := newFakeMemberService() + user := &types.User{ID: "u1", TenantID: 7} + + got, ok := resolveTenantRole(context.Background(), svc, user, 7, false, cfgWithRBAC(true)) + if !ok || got != types.TenantRoleOwner { + t.Fatalf("got (%v, %v), want (owner, true)", got, ok) + } + if len(svc.addCalls) != 1 || svc.addCalls[0].Role != types.TenantRoleOwner { + t.Fatalf("expected exactly one Owner AddMember call, got %+v", svc.addCalls) + } +} + +func TestResolveTenantRole_AutoPromoteSkippedIfTenantHasMembers(t *testing.T) { + svc := newFakeMemberService() + // 同一 home tenant 已经有其它成员 — 不应自动晋升新登录者。 + svc.seedActive("other", 7, types.TenantRoleOwner) + user := &types.User{ID: "u1", TenantID: 7} + + got, ok := resolveTenantRole(context.Background(), svc, user, 7, false, cfgWithRBAC(true)) + if ok { + t.Fatalf("RBAC enabled + no membership for u1 should be rejected, got role=%v", got) + } + if len(svc.addCalls) != 0 { + t.Fatalf("must not auto-promote into a tenant that already has members, got %+v", svc.addCalls) + } +} + +func TestResolveTenantRole_FailOpenAdminWhenRBACDisabled(t *testing.T) { + svc := newFakeMemberService() + user := &types.User{ID: "u1", TenantID: 7} + // targetTenantID != home,所以不进 auto-promote 分支。 + got, ok := resolveTenantRole(context.Background(), svc, user, 8, false, cfgWithRBAC(false)) + if !ok || got != types.TenantRoleAdmin { + t.Fatalf("EnableRBAC=false should fail open Admin, got (%v, %v)", got, ok) + } +} + +func TestResolveTenantRole_FailClosedWhenRBACEnabled(t *testing.T) { + svc := newFakeMemberService() + // 已有其它成员,自动晋升路径关闭;RBAC 启用 → 必须 403。 + svc.seedActive("other", 8, types.TenantRoleOwner) + user := &types.User{ID: "u1", TenantID: 7} + if _, ok := resolveTenantRole(context.Background(), svc, user, 8, false, cfgWithRBAC(true)); ok { + t.Fatalf("EnableRBAC=true + no membership should be rejected") + } +} + +func TestResolveTenantRole_LookupErrorFailsOpenWhenRBACDisabled(t *testing.T) { + // 短暂 DB 错误时,fail-open 模式不应锁死现有用户。这里 targetTenantID 故意 + // 选与 home 不同的值,避免进入 home-tenant auto-promote 分支。 + svc := newFakeMemberService() + svc.failGet = errors.New("transient db failure") + // 让 HasAnyMembers 返回 true,关闭孤儿租户自愈路径。 + svc.seedActive("placeholder", 8, types.TenantRoleAdmin) + user := &types.User{ID: "u1", TenantID: 7} + + got, ok := resolveTenantRole(context.Background(), svc, user, 8, false, cfgWithRBAC(false)) + if !ok || got != types.TenantRoleAdmin { + t.Fatalf("transient lookup error under RBAC=false should fail open Admin, got (%v, %v)", got, ok) + } +} + +func TestResolveTenantRole_DemotedUserCannotReclaimViaOrphan(t *testing.T) { + // 边界场景:管理员人为软删全部成员后,被踢出的用户不应在登录自己 home tenant 时 + // 因 HasAnyMembers=false 而自动重新拿到 Owner。 + // 当前实现的策略是 "home tenant + 孤儿 => Owner",这是设计选择;本测试为这条 + // 路径加锁,未来如果收紧策略需要同步更新。 + svc := newFakeMemberService() + user := &types.User{ID: "demoted", TenantID: 5} + got, ok := resolveTenantRole(context.Background(), svc, user, 5, false, cfgWithRBAC(true)) + if !ok || got != types.TenantRoleOwner { + t.Fatalf("current policy allows orphan-tenant self-heal on home tenant, got (%v, %v)", got, ok) + } +} diff --git a/internal/types/custom_agent.go b/internal/types/custom_agent.go index 1774badf..c66c89e1 100644 --- a/internal/types/custom_agent.go +++ b/internal/types/custom_agent.go @@ -79,7 +79,15 @@ type CustomAgent struct { // start chat sessions against this agent. Defaults to true so viewers // can still consume published agents; admins toggle it off for agents // whose tools should be restricted to contributors and above. - RunnableByViewer bool `yaml:"runnable_by_viewer" json:"runnable_by_viewer" gorm:"default:true"` + // + // Note: no GORM `default:true` tag here. With a plain `bool` field that + // tag causes GORM to treat Go's zero value (false) as "unspecified" and + // silently rewrite it to the DB default (true) on insert — preventing + // admins from ever creating an agent with viewer-runtime disabled. The + // column-level DEFAULT TRUE on the database side still covers rows + // inserted by raw SQL / migrations; GORM Create always writes the + // explicit struct value. + RunnableByViewer bool `yaml:"runnable_by_viewer" json:"runnable_by_viewer"` // Agent configuration Config CustomAgentConfig `yaml:"config" json:"config" gorm:"type:json"` diff --git a/internal/types/interfaces/tenant.go b/internal/types/interfaces/tenant.go index 5fbef292..0e864122 100644 --- a/internal/types/interfaces/tenant.go +++ b/internal/types/interfaces/tenant.go @@ -12,6 +12,10 @@ type TenantService interface { CreateTenant(ctx context.Context, tenant *types.Tenant) (*types.Tenant, error) // GetTenantByID gets a tenant by ID GetTenantByID(ctx context.Context, id uint64) (*types.Tenant, error) + // GetTenantsByIDs batches GetTenantByID for multiple IDs in a single + // query. Returns a map keyed by tenant ID for O(1) lookup at the + // call site; missing tenants are simply absent from the map. + GetTenantsByIDs(ctx context.Context, ids []uint64) (map[uint64]*types.Tenant, error) // ListTenants lists all tenants ListTenants(ctx context.Context) ([]*types.Tenant, error) // UpdateTenant updates a tenant @@ -38,6 +42,8 @@ type TenantRepository interface { CreateTenant(ctx context.Context, tenant *types.Tenant) error // GetTenantByID gets a tenant by ID GetTenantByID(ctx context.Context, id uint64) (*types.Tenant, error) + // GetTenantsByIDs batches GetTenantByID; see TenantService.GetTenantsByIDs. + GetTenantsByIDs(ctx context.Context, ids []uint64) (map[uint64]*types.Tenant, error) // ListTenants lists all tenants ListTenants(ctx context.Context) ([]*types.Tenant, error) // SearchTenants searches tenants with pagination and filters diff --git a/internal/types/interfaces/user.go b/internal/types/interfaces/user.go index 70ca0441..1a4d907e 100644 --- a/internal/types/interfaces/user.go +++ b/internal/types/interfaces/user.go @@ -34,6 +34,12 @@ type UserService interface { ValidatePassword(ctx context.Context, userID string, password string) error // GenerateTokens generates access and refresh tokens for user GenerateTokens(ctx context.Context, user *types.User) (accessToken, refreshToken string, err error) + // BuildLoginMemberships projects the user's tenant memberships into + // the login-response shape. activeTenant is reused (without an extra + // lookup) for the matching row's TenantName. The slice is guaranteed + // non-nil so callers can serialise it as an empty JSON array when the + // membership table is unavailable. + BuildLoginMemberships(ctx context.Context, user *types.User, activeTenant *types.Tenant) []types.Membership // SwitchTenant issues a new token pair scoped to targetTenantID and // returns the corresponding LoginResponse. The caller's previous // refresh token (passed in for revocation) is invalidated. Membership diff --git a/internal/types/user.go b/internal/types/user.go index e0aad641..16dbe214 100644 --- a/internal/types/user.go +++ b/internal/types/user.go @@ -78,13 +78,21 @@ type OIDCConfigResponse struct { } type OIDCCallbackResponse struct { - Success bool `json:"success"` - Message string `json:"message,omitempty"` - User *User `json:"user,omitempty"` - Tenant *Tenant `json:"tenant,omitempty"` - Token string `json:"token,omitempty"` - RefreshToken string `json:"refresh_token,omitempty"` - IsNewUser bool `json:"is_new_user,omitempty"` + Success bool `json:"success"` + Message string `json:"message,omitempty"` + User *User `json:"user,omitempty"` + // Tenant carries the active tenant for the issued token. The field + // name is preserved for backward compatibility with existing frontend + // OIDC callback handling; LoginResponse uses ActiveTenant for the + // same data. + Tenant *Tenant `json:"tenant,omitempty"` + // Memberships mirrors LoginResponse.Memberships so the OIDC flow + // produces the same role information available to password logins. + // Always populated (length >= 1 for an authenticated user). + Memberships []Membership `json:"memberships"` + Token string `json:"token,omitempty"` + RefreshToken string `json:"refresh_token,omitempty"` + IsNewUser bool `json:"is_new_user,omitempty"` } type OIDCUserInfo struct { @@ -113,8 +121,11 @@ type LoginResponse struct { // Memberships lists every tenant the user can authenticate into, // along with their role in each. Always populated (length 1 for users // who only belong to their home tenant) so frontends can render a - // tenant switcher without a follow-up request. - Memberships []Membership `json:"memberships,omitempty"` + // tenant switcher without a follow-up request. Serialised without + // omitempty so the field is always present as a JSON array (possibly + // empty) — the "always populated" contract relies on the server side + // guaranteeing a non-nil slice. + Memberships []Membership `json:"memberships"` Token string `json:"token,omitempty"` RefreshToken string `json:"refresh_token,omitempty"` } diff --git a/migrations/versioned/000043_tenant_rbac.down.sql b/migrations/versioned/000043_tenant_rbac.down.sql index 2e625f0c..3d8a4fb4 100644 --- a/migrations/versioned/000043_tenant_rbac.down.sql +++ b/migrations/versioned/000043_tenant_rbac.down.sql @@ -1,7 +1,23 @@ -- Migration 000043 down: revert tenant RBAC schema additions. --- Note: this drops role information unrecoverably; only intended for --- development rollbacks, not production. -DO $$ BEGIN RAISE NOTICE '[Migration 000043 down] Reverting tenant RBAC...'; END $$; +-- +-- DANGER: This drops the tenant_members table and the creator_id / +-- runnable_by_viewer columns. All role assignments, KB ownership +-- metadata, and viewer-runtime flags are lost UNRECOVERABLY. +-- Only intended for development rollbacks, never for production. +-- +-- Operators who really need to run this in a non-dev environment must set +-- the session-level GUC `weknora.allow_destructive_migration` to 'true' +-- first (e.g. `SET weknora.allow_destructive_migration = 'true';`). +-- Otherwise the migration aborts before touching any data. +DO $$ +BEGIN + IF coalesce(current_setting('weknora.allow_destructive_migration', true), 'false') <> 'true' THEN + RAISE EXCEPTION + '[Migration 000043 down] BLOCKED: refuse to drop tenant_members / creator_id / runnable_by_viewer. ' + 'Set weknora.allow_destructive_migration = ''true'' in this session to override.'; + END IF; + RAISE WARNING '[Migration 000043 down] Reverting tenant RBAC — role data will be lost'; +END $$; ALTER TABLE custom_agents DROP COLUMN IF EXISTS runnable_by_viewer;