refactor(rbac): address PR 1 review feedback

Tighten correctness, security and observability of the tenant RBAC
foundation introduced in 3c4cdd70 in response to internal review.

Bugfixes
- custom_agents.runnable_by_viewer: drop the GORM `default:true` tag.
  GORM treats a plain bool's zero value as "unspecified" and silently
  rewrites it to the column default on insert, which would prevent
  admins from ever creating an agent with viewer-runtime disabled. The
  column-level DEFAULT TRUE on the database side still backstops rows
  inserted by raw SQL / migrations.
- TenantMemberRepository.HasAnyMembers: replace `Limit(1).Count(&n)`
  with `Take(&probe)`. GORM's Count ignores Limit and scans the whole
  table, which matters because this runs on the auth middleware's
  hot path for users without a cached membership.
- service/user.LoginWithOIDC: include User, Tenant and Memberships in
  the OIDCCallbackResponse so the OIDC flow returns the same shape as
  password login (previously User and friends were left nil).

Security
- middleware/auth.resolveTenantRole: restrict orphan-tenant
  auto-promotion to the caller's home tenant. Cross-tenant superuser
  visits and JWTs minted for foreign tenants no longer trigger silent
  Owner grants. Two regression tests pin the new behaviour.
- service/user.synthFallbackMembership: default the synthesised role
  to Viewer instead of Owner. The login response only feeds UI; the
  backend re-derives the real role from tenant_members on every
  request, so showing a Viewer UI is preferable to a misleading Owner
  UI that would surface admin controls the backend will then 403.
- migrations/versioned/000043_tenant_rbac.down.sql: gate the destructive
  drop behind a `weknora.allow_destructive_migration` GUC so an
  accidental `migrate down` cannot wipe role data in production.
- frontend/src/stores/auth.ts: comment currentTenantRole as
  rendering-only so future PRs cannot mistake the localStorage-backed
  value for an authorisation signal.

API contract
- Memberships in LoginResponse and OIDCCallbackResponse now serialises
  without omitempty, and buildMembershipsForUser / synthFallbackMembership
  always return a non-nil slice — frontend can rely on JSON `[]`.
- handler/auth.GetCurrentUser includes memberships so /auth/me refreshes
  recover currentTenantRole after a page reload.

Performance
- Add TenantService.GetTenantsByIDs (with the matching repo method) and
  use it inside buildMembershipsForUser to batch tenant-name lookups
  instead of issuing one query per membership.

Observability
- Replace the remaining `log.Printf` calls in middleware/auth with
  contextual logger.Warnf / logger.Infof so role resolution events ride
  the standard request-id / Langfuse trace plumbing. The successful
  auto-promote line is tagged `[audit]` for log mining.

Tests
- internal/application/service/tenant_member_test.go: hand-rolled
  fakeTenantMemberRepo plus 11 cases covering AddMember validation,
  EnsureOwner idempotence, "cannot demote/remove the last Owner",
  same-role no-op, ErrInvalidTenantRole / ErrMembershipNotFound, and
  the TenantRole.HasPermission lattice.
- internal/middleware/auth_role_test.go: fakeMemberService plus 8 cases
  pinning every branch of resolveTenantRole, including the two H1
  regressions (cross-tenant superuser must not auto-promote; orphan
  auto-promote requires the home tenant).
This commit is contained in:
wizardchen
2026-05-13 20:24:55 +08:00
committed by lyingbug
parent 7f7a48880f
commit f3d1210577
15 changed files with 792 additions and 46 deletions
+30 -4
View File
@@ -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
}
+7
View File
@@ -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 ''
+21
View File
@@ -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
@@ -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
}
+5
View File
@@ -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)
@@ -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)
}
}
}
+61 -4
View File
@@ -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,
+6 -2
View File
@@ -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,
},
})
}
+41 -19
View File
@@ -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
}
+254
View File
@@ -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)
}
}
+9 -1
View File
@@ -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"`
+6
View File
@@ -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
+6
View File
@@ -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
+20 -9
View File
@@ -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"`
}
@@ -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;