diff --git a/Server/api/diagnostics_handler_test.go b/Server/api/diagnostics_handler_test.go index 9cb8b6a8..d7f73b68 100644 --- a/Server/api/diagnostics_handler_test.go +++ b/Server/api/diagnostics_handler_test.go @@ -10,11 +10,12 @@ import ( "github.com/owncord/server/auth" "github.com/owncord/server/config" "github.com/owncord/server/db" + "github.com/owncord/server/permissions" ) // setupDiagnosticsRouter creates a full router with an authenticated user for // diagnostics testing. -func setupDiagnosticsRouter(t *testing.T) (http.Handler, string) { +func setupDiagnosticsRouter(t *testing.T) (http.Handler, string, *db.DB) { t.Helper() database, err := db.Open(":memory:") @@ -46,11 +47,11 @@ func setupDiagnosticsRouter(t *testing.T) (http.Handler, string) { uid, hash, ) - return handler, token + return handler, token, database } func TestDiagnosticsConnectivity_ReturnsData(t *testing.T) { - router, token := setupDiagnosticsRouter(t) + router, token, _ := setupDiagnosticsRouter(t) req := httptest.NewRequest(http.MethodGet, "/api/v1/diagnostics/connectivity", nil) req.Header.Set("Authorization", "Bearer "+token) @@ -82,7 +83,7 @@ func TestDiagnosticsConnectivity_ReturnsData(t *testing.T) { } func TestDiagnosticsConnectivity_Unauthenticated(t *testing.T) { - router, _ := setupDiagnosticsRouter(t) + router, _, _ := setupDiagnosticsRouter(t) req := httptest.NewRequest(http.MethodGet, "/api/v1/diagnostics/connectivity", nil) req.RemoteAddr = "127.0.0.1:9999" @@ -94,6 +95,37 @@ func TestDiagnosticsConnectivity_Unauthenticated(t *testing.T) { } } +// TestDiagnosticsConnectivity_MemberForbidden locks the RequirePermission gate +// on the route. Without it, only 200-for-owner and 401-unauthenticated were +// covered, so deleting the ADMINISTRATOR gate broke no test while exposing the +// server's network topology to every member. +func TestDiagnosticsConnectivity_MemberForbidden(t *testing.T) { + router, _, database := setupDiagnosticsRouter(t) + + uid, err := database.CreateUser("diagmember", "$2a$12$fake", int(permissions.MemberRoleID)) + if err != nil { + t.Fatalf("CreateUser: %v", err) + } + token := "diagtest-member-token" + if _, err := database.Exec( + `INSERT INTO sessions (user_id, token, device, ip_address, expires_at) + VALUES (?, ?, 'test', '127.0.0.1', '2099-01-01T00:00:00Z')`, + uid, auth.HashToken(token), + ); err != nil { + t.Fatalf("insert session: %v", err) + } + + req := httptest.NewRequest(http.MethodGet, "/api/v1/diagnostics/connectivity", nil) + req.Header.Set("Authorization", "Bearer "+token) + req.RemoteAddr = "127.0.0.1:9999" + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + if rr.Code != http.StatusForbidden { + t.Errorf("status = %d, want 403; body: %s", rr.Code, rr.Body.String()) + } +} + // ─── isPrivateIP tests ────────────────────────────────────────────────────── func TestIsPrivateIP(t *testing.T) { diff --git a/Server/api/invite_handler_test.go b/Server/api/invite_handler_test.go index 789de56d..8cea1344 100644 --- a/Server/api/invite_handler_test.go +++ b/Server/api/invite_handler_test.go @@ -10,6 +10,7 @@ import ( "github.com/owncord/server/api" "github.com/owncord/server/auth" "github.com/owncord/server/db" + "github.com/owncord/server/permissions" "github.com/owncord/server/service" ) @@ -89,6 +90,40 @@ func TestCreateInvite_MemberForbidden(t *testing.T) { } } +// TestCreateInvite_ChannelAllowOverrideDoesNotGrant pins the scope boundary of +// RequirePermission: it gates on SERVER-WIDE bits, so a per-channel allow must +// never open it. The state is reachable — the admin channel-permission handler +// masks override input with permissions.AllPerms, which includes ManageInvites. +// This kills the plausible-looking "just route RequirePermission through +// Checker.HasChannelPerm" refactor, which would pass naive review because +// GetChannelPermissions returns (0, 0, nil) when no override row exists. +func TestCreateInvite_ChannelAllowOverrideDoesNotGrant(t *testing.T) { + database := newAuthTestDB(t) + limiter := auth.NewRateLimiter() + router := buildInviteRouter(database, limiter) + + token := loginAndGetToken(t, router, database, "overrideuser", 4) + + if _, err := database.Exec( + `INSERT INTO channels (id, name, type) VALUES (1, 'general', 'text')`); err != nil { + t.Fatalf("insert channel: %v", err) + } + if _, err := database.Exec( + `INSERT INTO channel_overrides (channel_id, role_id, allow, deny) VALUES (1, 4, ?, 0)`, + permissions.ManageInvites, + ); err != nil { + t.Fatalf("insert channel override: %v", err) + } + + rr := postJSONWithToken(t, router, "/api/v1/invites", token, map[string]any{ + "max_uses": 1, + }) + + if rr.Code != http.StatusForbidden { + t.Errorf("CreateInvite with channel allow override status = %d, want 403", rr.Code) + } +} + func TestCreateInvite_Unlimited(t *testing.T) { database := newAuthTestDB(t) limiter := auth.NewRateLimiter() diff --git a/Server/api/middleware.go b/Server/api/middleware.go index b270d4d5..1e218112 100644 --- a/Server/api/middleware.go +++ b/Server/api/middleware.go @@ -84,8 +84,11 @@ func AuthMiddleware(database *db.DB) func(http.Handler) http.Handler { } // Load role for permission checks. + // A dangling role_id returns (nil, nil) from GetRoleByID, so the nil + // check is load-bearing: without it a nil role reaches the context + // and every downstream permission check has to re-guard it. role, err := database.GetRoleByID(user.RoleID) - if err != nil { + if err != nil || role == nil { writeJSON(w, http.StatusUnauthorized, errorResponse{ Error: "UNAUTHORIZED", Message: "role not found", @@ -106,9 +109,20 @@ func AuthMiddleware(database *db.DB) func(http.Handler) http.Handler { } } -// RequirePermission returns middleware that checks the authenticated user's -// role permissions. Returns 403 if the user lacks the required permission. -// The ADMINISTRATOR bit (0x40000000) bypasses all checks. +// RequirePermission returns middleware gating a route on SERVER-WIDE role +// permissions. Returns 403 if the user lacks them. +// +// Scope contract — this is the whole reason the middleware and the service +// layer look like two permission systems: +// - It consults the role bitfield only. Channel overrides are NOT applied, +// because a route reaching this middleware has no channel id to resolve +// them against, and a per-channel allow must never open a server-wide gate. +// - Anything channel-scoped belongs in the service layer behind +// permissions.Checker (via svc.Permissions), which resolves overrides. +// - ADMINISTRATOR bypasses; multi-bit masks require ALL bits. +// +// The rule itself lives in permissions.HasServerPerm so no call site can +// re-derive it. func RequirePermission(perm int64) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -121,13 +135,7 @@ func RequirePermission(perm int64) func(http.Handler) http.Handler { return } - // ADMINISTRATOR bypasses all permission checks. - if permissions.HasAdmin(role.Permissions) { - next.ServeHTTP(w, r) - return - } - - if role.Permissions&perm == 0 { + if !permissions.HasServerPerm(role.Permissions, perm) { writeJSON(w, http.StatusForbidden, errorResponse{ Error: "FORBIDDEN", Message: "insufficient permissions", diff --git a/Server/api/middleware_test.go b/Server/api/middleware_test.go index 5366e142..68ea2c56 100644 --- a/Server/api/middleware_test.go +++ b/Server/api/middleware_test.go @@ -14,6 +14,7 @@ import ( "github.com/owncord/server/api" "github.com/owncord/server/auth" "github.com/owncord/server/db" + "github.com/owncord/server/permissions" ) // ─── Helpers ───────────────────────────────────────────────────────────────── @@ -143,6 +144,51 @@ func TestAuthMiddleware_MalformedAuthHeader(t *testing.T) { } } +// TestAuthMiddleware_DanglingRoleUnauthorized pins the `role == nil` guard: +// GetRoleByID returns (nil, nil) for a role_id with no roles row, so without +// the guard a nil role reached the request context and the request only died +// later, at RequirePermission's own nil check (403) — or not at all on routes +// that have no RequirePermission. +func TestAuthMiddleware_DanglingRoleUnauthorized(t *testing.T) { + database := newAPITestDB(t) + + // users.role_id has a FK to roles(id), so the dangling row can only be + // created with FK enforcement momentarily off (db.Open pins the pool to a + // single connection, so the pragma applies to the inserts that follow). + if _, err := database.Exec(`PRAGMA foreign_keys=OFF`); err != nil { + t.Fatalf("disable foreign keys: %v", err) + } + res, err := database.Exec( + `INSERT INTO users (username, password, role_id) VALUES ('dangling', '$2a$12$fake', 999)`) + if err != nil { + t.Fatalf("insert dangling user: %v", err) + } + uid, _ := res.LastInsertId() + if _, err := database.Exec(`PRAGMA foreign_keys=ON`); err != nil { + t.Fatalf("re-enable foreign keys: %v", err) + } + + token, _ := auth.GenerateToken() + if _, err := database.Exec( + `INSERT INTO sessions (user_id, token, device, ip_address, expires_at) + VALUES (?, ?, 'test', '127.0.0.1', '2099-01-01T00:00:00Z')`, + uid, auth.HashToken(token), + ); err != nil { + t.Fatalf("insert session: %v", err) + } + + h := api.AuthMiddleware(database)(http.HandlerFunc(ok)) + req := httptest.NewRequest(http.MethodGet, "/", nil) + withBearer(req, token) + rr := httptest.NewRecorder() + + h.ServeHTTP(rr, req) + + if rr.Code != http.StatusUnauthorized { + t.Errorf("AuthMiddleware dangling role status = %d, want 401", rr.Code) + } +} + // ─── RequirePermission tests ────────────────────────────────────────────────── func TestRequirePermission_Allowed(t *testing.T) { @@ -152,9 +198,8 @@ func TestRequirePermission_Allowed(t *testing.T) { hash := auth.HashToken(token) _, _ = database.CreateSession(uid, hash, "test", "127.0.0.1") - // SEND_MESSAGES = 0x1 — Member role has this bit h := api.AuthMiddleware(database)( - api.RequirePermission(0x1)(http.HandlerFunc(ok)), + api.RequirePermission(permissions.SendMessages)(http.HandlerFunc(ok)), ) req := httptest.NewRequest(http.MethodGet, "/", nil) withBearer(req, token) @@ -174,9 +219,8 @@ func TestRequirePermission_Forbidden(t *testing.T) { hash := auth.HashToken(token) _, _ = database.CreateSession(uid, hash, "test", "127.0.0.1") - // MANAGE_ROLES = 0x1000000 — Member does not have this h := api.AuthMiddleware(database)( - api.RequirePermission(0x1000000)(http.HandlerFunc(ok)), + api.RequirePermission(permissions.ManageRoles)(http.HandlerFunc(ok)), ) req := httptest.NewRequest(http.MethodGet, "/", nil) withBearer(req, token) @@ -199,7 +243,7 @@ func TestRequirePermission_Administrator_Bypass(t *testing.T) { // Any permission should pass for ADMINISTRATOR h := api.AuthMiddleware(database)( - api.RequirePermission(0x1000000)(http.HandlerFunc(ok)), + api.RequirePermission(permissions.ManageRoles)(http.HandlerFunc(ok)), ) req := httptest.NewRequest(http.MethodGet, "/", nil) withBearer(req, token) @@ -212,6 +256,31 @@ func TestRequirePermission_Administrator_Bypass(t *testing.T) { } } +// TestRequirePermission_MultiBitRequiresAllBits pins the one behaviour the +// HasServerPerm consolidation changed: a multi-bit mask is ALL-of, not any-of. +// The previous raw `role.Permissions&perm == 0` test returned 200 here because +// Member holds SendMessages, which was enough to make the mask non-zero. +func TestRequirePermission_MultiBitRequiresAllBits(t *testing.T) { + database := newAPITestDB(t) + uid, _ := database.CreateUser("multibit", "hash", 4) // Member role = 1635, has SendMessages, not ManageRoles + token, _ := auth.GenerateToken() + hash := auth.HashToken(token) + _, _ = database.CreateSession(uid, hash, "test", "127.0.0.1") + + h := api.AuthMiddleware(database)( + api.RequirePermission(permissions.SendMessages | permissions.ManageRoles)(http.HandlerFunc(ok)), + ) + req := httptest.NewRequest(http.MethodGet, "/", nil) + withBearer(req, token) + rr := httptest.NewRecorder() + + h.ServeHTTP(rr, req) + + if rr.Code != http.StatusForbidden { + t.Errorf("RequirePermission partial multi-bit mask status = %d, want 403", rr.Code) + } +} + // ─── RateLimitMiddleware tests ──────────────────────────────────────────────── func TestRateLimitMiddleware_UnderLimit(t *testing.T) { @@ -940,6 +1009,15 @@ CREATE TABLE IF NOT EXISTS channels ( voice_max_video INTEGER NOT NULL DEFAULT 0 ); +CREATE TABLE IF NOT EXISTS channel_overrides ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + channel_id INTEGER NOT NULL REFERENCES channels(id) ON DELETE CASCADE, + role_id INTEGER NOT NULL REFERENCES roles(id) ON DELETE CASCADE, + allow INTEGER NOT NULL DEFAULT 0, + deny INTEGER NOT NULL DEFAULT 0, + UNIQUE(channel_id, role_id) +); + CREATE TABLE IF NOT EXISTS messages ( id INTEGER PRIMARY KEY AUTOINCREMENT, channel_id INTEGER NOT NULL REFERENCES channels(id) ON DELETE CASCADE, diff --git a/Server/permissions/permissions.go b/Server/permissions/permissions.go index 1068d532..502bf7c6 100644 --- a/Server/permissions/permissions.go +++ b/Server/permissions/permissions.go @@ -71,6 +71,14 @@ func HasAdmin(rolePerms int64) bool { return rolePerms&Administrator != 0 } +// HasServerPerm reports whether a role holds a SERVER-WIDE permission. +// Administrator bypasses. Channel overrides are deliberately NOT consulted — +// use Checker.HasChannelPerm/HasChannelPermBatch whenever a channel id exists. +// Multi-bit masks are ALL-of (every bit must be present), matching HasPerm. +func HasServerPerm(rolePerms, perm int64) bool { + return HasAdmin(rolePerms) || HasPerm(rolePerms, perm) +} + // EffectivePerms computes the resolved permission set for a channel override. // The formula matches Discord's channel override semantics: // diff --git a/Server/permissions/permissions_test.go b/Server/permissions/permissions_test.go index f1e8cf54..3a610056 100644 --- a/Server/permissions/permissions_test.go +++ b/Server/permissions/permissions_test.go @@ -172,6 +172,38 @@ func TestHasAdmin_OwnerRolePermsHasBit(t *testing.T) { } } +// ─── HasServerPerm tests ────────────────────────────────────────────────────── + +// TestHasServerPerm locks the contract api.RequirePermission inherits: admin +// bypass, and ALL-of semantics for multi-bit masks (a raw `perms&mask != 0` +// test would make them any-of). +func TestHasServerPerm(t *testing.T) { + tests := []struct { + name string + rolePerms int64 + perm int64 + want bool + }{ + {"admin bypasses a bit it lacks", permissions.Administrator, permissions.ManageInvites, true}, + // Deliberately UNLIKE HasPerm(Administrator, 0) == false: the admin + // bypass short-circuits before the zero-mask guard. + {"admin with zero mask", permissions.Administrator, 0, true}, + {"exact bit held", permissions.ManageInvites, permissions.ManageInvites, true}, + {"bit not held", permissions.SendMessages, permissions.ManageInvites, false}, + {"non-admin with zero mask", permissions.SendMessages, 0, false}, + {"multi-bit mask partially held", permissions.SendMessages, permissions.SendMessages | permissions.ManageRoles, false}, + {"multi-bit mask fully held", permissions.SendMessages | permissions.ManageRoles, permissions.SendMessages | permissions.ManageRoles, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := permissions.HasServerPerm(tt.rolePerms, tt.perm); got != tt.want { + t.Errorf("HasServerPerm() = %v, want %v", got, tt.want) + } + }) + } +} + // ─── EffectivePerms tests ───────────────────────────────────────────────────── // EffectivePerms(rolePerm, allow, deny) = (rolePerm & ^deny) | allow diff --git a/Server/service/channel.go b/Server/service/channel.go index 1cc6b082..5692091e 100644 --- a/Server/service/channel.go +++ b/Server/service/channel.go @@ -57,7 +57,9 @@ func (s *ChannelService) ListVisibleChannels(ctx context.Context, userID int64) if !permissions.HasAdmin(role.Permissions) { overrides, err = s.st.GetAllChannelPermissionsForRole(role.ID) if err != nil { - overrides = make(map[int64]db.ChannelOverride) + // Fail closed — an empty map would return every denied channel. + slog.Error("ChannelService.ListVisibleChannels GetAllChannelPermissionsForRole", "err", err, "user_id", userID, "role_id", role.ID) + return nil, fmt.Errorf("%w: failed to fetch channel overrides", ErrInternal) } } diff --git a/Server/service/channel_test.go b/Server/service/channel_test.go new file mode 100644 index 00000000..e135297c --- /dev/null +++ b/Server/service/channel_test.go @@ -0,0 +1,42 @@ +package service + +import ( + "context" + "errors" + "testing" + + "github.com/owncord/server/db" + "github.com/owncord/server/permissions" +) + +// TestListVisibleChannels_OverrideFetchErrorFailsClosed is the uncached half of +// the same fail-open bug as TestHasChannelPerm_OverrideFetchErrorDenies: an +// empty override map here would list every channel the role is explicitly +// denied. The listing must error instead of leaking the denied channel. +func TestListVisibleChannels_OverrideFetchErrorFailsClosed(t *testing.T) { + database := newTestDB(t) + seedRole(t, database, &db.Role{ + ID: permissions.MemberRoleID, + Name: "member", + Permissions: permissions.SendMessages | permissions.ReadMessages | permissions.AddReactions, + Position: 1, + }) + seedUserRole(t, database, 1, permissions.MemberRoleID) + seedChannel(t, database, &db.Channel{ID: 10, Name: "secret", Type: "text"}) + seedChannelOverride(t, database, permissions.MemberRoleID, 10, 0, permissions.ReadMessages) + + st := errOverrideStore{DB: database} + permSvc := NewPermissionService(st, permissions.NewChecker(database)) + svc := NewChannelService(st, permSvc) + + // Either failing path is acceptable and both are ErrInternal: the permission + // cache may short-circuit on its own fail-closed nil, or ListVisibleChannels' + // own override branch may error. What must never happen is a 200 listing. + got, err := svc.ListVisibleChannels(context.Background(), 1) + if !errors.Is(err, ErrInternal) { + t.Fatalf("ListVisibleChannels err = %v, want ErrInternal", err) + } + if got != nil { + t.Fatalf("ListVisibleChannels returned %d channels on override fetch failure, want none", len(got)) + } +} diff --git a/Server/service/message.go b/Server/service/message.go index b45b562d..443beb11 100644 --- a/Server/service/message.go +++ b/Server/service/message.go @@ -636,31 +636,21 @@ func (s *MessageService) GetAccessibleChannelIDs(userID int64) ([]int64, error) return nil, fmt.Errorf("%w: failed to get role", ErrInternal) } - isAdmin := permissions.HasAdmin(role.Permissions) var overrides map[int64]db.ChannelOverride - if !isAdmin { + if !permissions.HasAdmin(role.Permissions) { var overrideErr error overrides, overrideErr = s.st.GetAllChannelPermissionsForRole(role.ID) if overrideErr != nil { return nil, fmt.Errorf("%w: failed to fetch channel overrides", ErrInternal) } - if overrides == nil { - overrides = make(map[int64]db.ChannelOverride) - } } + // Single visibility predicate shared with REST ListVisibleChannels and the + // ws ready payload, so no site can drift. + visibleIDs := s.perms.Checker().VisibleChannelIDs(role.Permissions, channelRefs(channels), permOverrides(overrides)) var ids []int64 for i := range channels { - if channels[i].Type == "dm" { - continue - } - if isAdmin { - ids = append(ids, channels[i].ID) - continue - } - o := overrides[channels[i].ID] - effective := permissions.EffectivePerms(role.Permissions, o.Allow, o.Deny) - if effective&permissions.ReadMessages == permissions.ReadMessages { + if visibleIDs[channels[i].ID] { ids = append(ids, channels[i].ID) } } diff --git a/Server/service/moderation.go b/Server/service/moderation.go index 1c7f44e0..e6cce0c9 100644 --- a/Server/service/moderation.go +++ b/Server/service/moderation.go @@ -35,8 +35,7 @@ func (s *ModerationService) requireBanPermission(actorID int64) error { if err != nil || actorRole == nil { return fmt.Errorf("%w: failed to load actor role", ErrForbidden) } - if !permissions.HasAdmin(actorRole.Permissions) && - !permissions.HasPerm(actorRole.Permissions, permissions.BanMembers) { + if !permissions.HasServerPerm(actorRole.Permissions, permissions.BanMembers) { return fmt.Errorf("%w: missing BAN_MEMBERS permission", ErrForbidden) } return nil diff --git a/Server/service/permission.go b/Server/service/permission.go index 778dce06..498e1a9d 100644 --- a/Server/service/permission.go +++ b/Server/service/permission.go @@ -2,6 +2,7 @@ package service import ( "context" + "log/slog" "sync" "time" @@ -14,7 +15,7 @@ import ( type cachedPerms struct { roleID int64 rolePerms int64 - overrides map[int64]db.ChannelOverride + overrides map[int64]permissions.ChannelOverride populatedAt time.Time } @@ -62,12 +63,7 @@ func (s *PermissionService) HasChannelPerm(userID, channelID, perm int64) bool { if cp == nil { return false } - if permissions.HasAdmin(cp.rolePerms) { - return true - } - o := cp.overrides[channelID] // zero-value (0,0) when no override exists - effective := permissions.EffectivePerms(cp.rolePerms, o.Allow, o.Deny) - return effective&perm == perm + return s.checker.HasChannelPermBatch(cp.rolePerms, cp.overrides, channelID, perm) } // RequireChannelAccess checks whether the user can access the channel with @@ -150,10 +146,18 @@ func (s *PermissionService) getOrPopulate(userID int64) *cachedPerms { if err != nil || role == nil { return nil } - overrides, err := s.st.GetAllChannelPermissionsForRole(role.ID) - if err != nil { - // Fall back to uncached if override fetch fails. - overrides = make(map[int64]db.ChannelOverride) + // Admins bypass every channel check, so skip the fetch entirely (mirrors + // ChannelService.ListVisibleChannels and ws.buildReady). + var overrides map[int64]permissions.ChannelOverride + if !permissions.HasAdmin(role.Permissions) { + raw, oErr := s.st.GetAllChannelPermissionsForRole(role.ID) + if oErr != nil { + // Fail closed: an empty map would silently drop every deny bit, + // and caching it would keep doing so for permCacheTTL. + slog.Error("PermissionService.getOrPopulate override fetch failed, denying", "err", oErr, "user_id", userID, "role_id", role.ID) + return nil + } + overrides = permOverrides(raw) } cp = &cachedPerms{ diff --git a/Server/service/permission_test.go b/Server/service/permission_test.go index 021c7753..f584a3b8 100644 --- a/Server/service/permission_test.go +++ b/Server/service/permission_test.go @@ -1,6 +1,7 @@ package service import ( + "errors" "testing" "time" @@ -8,6 +9,41 @@ import ( "github.com/owncord/server/permissions" ) +// errOverrideStore wraps a real *db.DB but always fails the channel-override +// fetch, so the fail-closed contract (A-2026-07-16) is testable. Embedding +// *db.DB satisfies the service Store interface; only the one overridden method +// diverges, every other call still hits the real database. +type errOverrideStore struct { + *db.DB +} + +func (errOverrideStore) GetAllChannelPermissionsForRole(int64) (map[int64]db.ChannelOverride, error) { + return nil, errors.New("boom") +} + +// TestHasChannelPerm_OverrideFetchErrorDenies locks the fail-closed rule: when +// the override fetch errors we must NOT substitute an empty map, because that +// restores every bit a channel-level deny had stripped — and PermissionService +// would then cache that degraded snapshot for permCacheTTL. +func TestHasChannelPerm_OverrideFetchErrorDenies(t *testing.T) { + database := newTestDB(t) + seedRole(t, database, &db.Role{ + ID: permissions.MemberRoleID, + Name: "member", + Permissions: permissions.SendMessages | permissions.ReadMessages | permissions.AddReactions, + Position: 1, + }) + seedUserRole(t, database, 1, permissions.MemberRoleID) + seedChannel(t, database, &db.Channel{ID: 10, Name: "readonly", Type: "text"}) + seedChannelOverride(t, database, permissions.MemberRoleID, 10, 0, permissions.ReadMessages) + + svc := NewPermissionService(errOverrideStore{DB: database}, permissions.NewChecker(database)) + + if svc.HasChannelPerm(1, 10, permissions.ReadMessages) { + t.Fatal("override fetch failure must deny, not fall back to the base role bits") + } +} + // newTestPermService creates a PermissionService backed by a real in-memory DB // pre-populated with a single role and user. func newTestPermService(t *testing.T) (*PermissionService, *db.DB) { @@ -100,6 +136,28 @@ func TestHasChannelPerm_AdminBypass(t *testing.T) { } } +// TestHasChannelPerm_AdminSkipsOverrideFetch locks the admin skip in +// getOrPopulate: fail-closed must not extend to admins, who bypass every +// channel check anyway. Without the skip, an override-fetch outage would +// deny admins everything instead of nothing. +func TestHasChannelPerm_AdminSkipsOverrideFetch(t *testing.T) { + database := newTestDB(t) + seedRole(t, database, &db.Role{ + ID: permissions.AdminRoleID, + Name: "admin", + Permissions: permissions.Administrator, + Position: 90, + }) + seedUserRole(t, database, 1, permissions.AdminRoleID) + seedChannel(t, database, &db.Channel{ID: 10, Name: "general", Type: "text"}) + + svc := NewPermissionService(errOverrideStore{DB: database}, permissions.NewChecker(database)) + + if !svc.HasChannelPerm(1, 10, permissions.ManageMessages) { + t.Fatal("admin must not be denied by an override-fetch outage; the fetch is skipped for admins") + } +} + func TestInvalidateUser_ClearsCacheForUser(t *testing.T) { svc, database := newTestPermService(t) seedChannel(t, database, &db.Channel{ID: 10, Name: "general", Type: "text"})