diff --git a/docs/QA-03-PRUEFPROTOKOLL.md b/docs/QA-03-PRUEFPROTOKOLL.md new file mode 100644 index 0000000..6a9ddac --- /dev/null +++ b/docs/QA-03-PRUEFPROTOKOLL.md @@ -0,0 +1,68 @@ +# QA-03 – Prüfprotokoll: Prüfgate Rechte & Policy + +Stand: 2026-08-29. Branch `feature/qa-03-pruefgate-rechte-policy` (RBAC-05 + RBAC-04 gemergt). + +## 1. Akzeptanzkriterien RBAC-01 bis RBAC-05 — Testabdeckung + +| Ticket | Titel | Abdeckende Tests | +|---|---|---| +| RBAC-01 | Rollenmodell & Grundrechte | `internal/rbac/role_test.go`: `TestEffectivePermissions_Inheritance`, `TestHasPermission` | +| RBAC-01 | Rollenzuweisung | `internal/rbac/store_test.go`: `TestStore_AssignAndGet`, `TestStore_RejectsSuperadminOutsideAllowedMatrix`, `TestStore_RejectsUnknownRole`, `TestStore_HistoryTracksWhoAndWhen` | +| RBAC-02 | Policy-Enforcement-Schicht (zentral) | `internal/policy` — kein eigenes `*_test.go` in diesem Merge gefunden für `enforcer.go`/`store.go` direkt (siehe Abweichungen unten); Verhalten indirekt über `TestBypass_PolicyEnforcerItselfRespectsRevocation` (dieser Branch) nachgewiesen | +| RBAC-03 | Gruppen & Abteilungen | `internal/rbac/group_test.go`: `TestGroup_CreateAndAddMember`, `TestGroup_RoleAffectsAllCurrentMembers`, `TestGroup_RemoveMemberRevokesRightsImmediately`, `TestGroup_DeleteGroupRevokesRightsWithoutDeletingUser`, `TestGroup_TenantIsolation` | +| RBAC-04 | Modul-scoped Berechtigungen | `internal/policy/module_scope_test.go`: `TestAuthorizeForTenant_DeniesWhenModuleNotActivated`, `TestAuthorizeForTenant_BecomesActiveWithoutRestart`, `TestAuthorizeForTenant_CombinationsOfRoleAndModuleScope` | +| RBAC-05 | Rechte-Administrationsoberfläche | `internal/rbac/handler_test.go`: `TestListRoles`, `TestAssignRole_RejectsSelfEscalation`, `TestAssignRole_AdminCanPromoteOtherUser`, `TestAssignRole_AdminCanDemoteOtherUser` (neu, dieser Branch), `TestRoleHistory_TracksAssignments`, `TestGroupWorkflow`, `TestRequireManageUsers_RejectsPlainUser` | + +**Ergebnis Abschnitt 1:** alle 5 Tickets haben automatisierte Tests, die ihre dokumentierten Akzeptanzkriterien abdecken. RBAC-02 selbst hat keine eigene Testdatei im gemergten Stand — abgedeckt nur indirekt über den in diesem Branch neu geschriebenen `TestBypass_PolicyEnforcerItselfRespectsRevocation`. Als Abweichung festgehalten (Abschnitt 4). + +## 2. Umgehungsversuch der zentralen Policy-Schicht (Akzeptanzkriterium 2 / Prüfung 1) + +Getestet in `internal/rbac/bypass_test.go`: + +- **`TestBypass_NoDirectWriteAPIOutsideStore`**: bestanden. `role_assignments` hat keine Schreib-API außerhalb von `Store.Assign` — Umgehungsversuch scheitert strukturell (Typsystem, kein exportierter DB-Pool). +- **`TestBypass_PolicyEnforcerItselfRespectsRevocation`**: bestanden. RBAC-02s eigentliche Policy-Tabelle (`policy_rules`) reagiert sofort auf `Revoke` — kein Cache, keine verzögerte Wirkung. +- **`TestBypass_HandlerIgnoresCentralPolicyRevocation`**: **deckt einen echten Fund auf**, siehe Abschnitt 4. + +## 3. Rollenwechsel-Szenario (Akzeptanzkriterium 2 / Prüfung 2) + +- Hochstufung (user → tenant_admin): `TestAssignRole_AdminCanPromoteOtherUser` — bestanden. +- Rückstufung (tenant_admin → user): `TestAssignRole_AdminCanDemoteOtherUser` (neu, dieser Branch) — bestanden, inklusive Prüfung, dass `role_assignment_history` beide Richtungen (erst `tenant_admin`, dann `user`) korrekt in chronologischer Reihenfolge festhält. +- Selbst-Eskalation bleibt weiterhin gesperrt (`TestAssignRole_RejectsSelfEscalation`, aus RBAC-05). + +**Ergebnis Abschnitt 3:** bestanden, beide Richtungen automatisiert nachgewiesen. + +## 4. Abweichungen (Akzeptanzkriterium 3: nicht stillschweigend ignoriert) + +### 4.1 RBAC-05-Handler prüfen nicht gegen die zentrale Policy-Schicht (RBAC-02) — Schweregrad: Mittel + +**Fund:** `internal/rbac/handler.go` (`requireManageUsers`) entscheidet Zugriff über `HasPermission(role, PermManageUsers)` — die **statische**, hartcodierte Rollenhierarchie aus `role.go`. Es ruft nirgends `internal/policy.Enforcer.Authorize`/`Guard` auf, die eigentliche zentrale, DB-gestützte Durchsetzungsschicht aus RBAC-02 (`policy_rules`-Tabelle, per `Store.Grant`/`Revoke` administrierbar, versioniert in `policy_rule_changes`). + +**Konsequenz:** ein Administrator, der über die RBAC-02-Policy-Schicht das Recht `tenant.manage_users` von `tenant_admin` entzieht (`policy.Store.Revoke`), sperrt die RBAC-05-Endpunkte **nicht** aus — sie fragen `policy_rules` nie ab. Zwei parallele Enforcement-Pfade statt einer zentralen Schicht, verletzt die Ticket-Produkt-DNA "Rechte werden zentral entschieden, nicht in jedem Handler neu erfunden" (RBAC-05-Ticket) UND RBAC-02s eigenen Anspruch ("keine Tenant- oder Rechteprüfung verstreut in einzelnen Handlern"). + +**Nachweis:** `TestBypass_HandlerIgnoresCentralPolicyRevocation` in `internal/rbac/bypass_test.go`. + +**Nicht in dieser Kachel behoben** (QA-03-Arbeitsweise: kein Umbau angrenzender Bereiche, RBAC-05 ist nicht Vorbedingung von QA-03) — Empfehlung: eigenes Folgeticket, das `requireManageUsers` auf `internal/policy.Guard`/`Enforcer.Authorize` umstellt. + +### 4.2 RBAC-02 hat keine eigene Testdatei im gemergten Stand — Schweregrad: Niedrig + +`internal/policy/enforcer.go` und `store.go` (RBAC-02 selbst) haben keine `enforcer_test.go`/`store_test.go` im Merge-Ergebnis dieses Branches — nur `module_scope_test.go` (RBAC-04) prüft sie indirekt über `AuthorizeForTenant`. Die in diesem Branch neu geschriebenen Bypass-Tests schließen die Lücke teilweise, ersetzen aber keine dedizierten RBAC-02-Unit-Tests. Empfehlung: bei Gelegenheit nachziehen, kein blockierender Fund. + +## 5. RBAC-04-Zusammenspiel mit Lizenz-/Flag-Zustand (Akzeptanzkriterium 3) + +`internal/flag` (aus RBAC-04-Merge) ist vorhanden. `internal/policy.Enforcer.AuthorizeForTenant` verknüpft eine Policy-Regel optional mit einem `flag.Service`-Eintrag (`ModuleScope.FlagKey`): eine sonst erlaubte Regel greift nicht, wenn das zugehörige Modul für den Tenant nicht aktiviert ist. `TestAuthorizeForTenant_DeniesWhenModuleNotActivated` und `TestAuthorizeForTenant_BecomesActiveWithoutRestart` beweisen das bereits (aus RBAC-04, unverändert übernommen). + +**Ergebnis Abschnitt 5:** bestanden, Zusammenspiel vorhanden und getestet. + +## 6. Gesamtergebnis + +Bestanden mit einem dokumentierten Mittel-Schweregrad-Fund (4.1) und einem Niedrig-Schweregrad-Hinweis (4.2). Build-/Test-Ergebnis auf dem Testhost: siehe Abschnitt 7. + +## 7. Build/Test-Ergebnis auf 131 + +Durchgeführt 2026-08-29 auf root@192.168.1.131 (`/root/nexarch-code-qa03`, isolierter Sync, kein Konflikt mit parallelem QA-02-Testlauf): + +- `go mod tidy`, `go build ./...`, `go vet ./...` — alle sauber, keine Fehler. +- `go test ./... -v -p 1` gegen frisch zurückgesetzte Testumgebung — **alle 30 Tests grün**, über alle betroffenen Pakete (`internal/auth`, `internal/policy`, `internal/rbac`, `internal/tenant`, `internal/user`), inklusive `TestBypass_HandlerIgnoresCentralPolicyRevocation` (bestätigt den Fund aus Abschnitt 4.1 als reproduzierbar, nicht nur behauptet) und `TestAssignRole_AdminCanDemoteOtherUser` (neuer Rückstufungs-Test aus Abschnitt 3). +- Keine Regressionen in RBAC-01/02/03/04/05 durch den Merge. + +**QA-03 Gesamtergebnis: bestanden**, mit einem dokumentierten Mittel-Schweregrad-Fund (4.1, Empfehlung: Folgeticket) und einem Niedrig-Schweregrad-Hinweis (4.2). diff --git a/internal/policy/module_scope.go b/internal/policy/module_scope.go new file mode 100644 index 0000000..1f9a5b0 --- /dev/null +++ b/internal/policy/module_scope.go @@ -0,0 +1,90 @@ +package policy + +import ( + "context" + "errors" + "fmt" + + "github.com/jackc/pgx/v5" + + "gitea.perlbach24.de/scripte/nexarch/internal/flag" + "gitea.perlbach24.de/scripte/nexarch/internal/rbac" +) + +// ModuleScope verknuepft eine Policy-Regel mit einem Feature-Flag: existiert +// ein ModuleScope fuer (role, permission), gilt die Regel nur zusaetzlich zur +// Grundberechtigung, wenn FlagKey fuer den jeweiligen Tenant aktiv ist +// (Akzeptanzkriterium 1: Rechte folgen der Lizenz). +type ModuleScope struct { + Role rbac.Role + Permission rbac.Permission + Module string + FlagKey string +} + +// SetModuleScope verknuepft eine bestehende Policy-Regel mit einem Modul/ +// Feature-Flag. Die Regel selbst (Store.Grant) muss unabhaengig davon +// existieren — ModuleScope schraenkt sie nur zusaetzlich ein. +func (s *Store) SetModuleScope(ctx context.Context, role rbac.Role, perm rbac.Permission, module, flagKey string) error { + _, err := s.pool.Exec(ctx, ` + INSERT INTO policy_module_scopes (role, permission, module, flag_key) + VALUES ($1, $2, $3, $4) + ON CONFLICT (role, permission) DO UPDATE SET module = $3, flag_key = $4 + `, string(role), string(perm), module, flagKey) + if err != nil { + return fmt.Errorf("modul-scope setzen: %w", err) + } + return nil +} + +func (s *Store) GetModuleScope(ctx context.Context, role rbac.Role, perm rbac.Permission) (ModuleScope, bool, error) { + var ms ModuleScope + ms.Role, ms.Permission = role, perm + err := s.pool.QueryRow(ctx, ` + SELECT module, flag_key FROM policy_module_scopes WHERE role = $1 AND permission = $2 + `, string(role), string(perm)).Scan(&ms.Module, &ms.FlagKey) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return ModuleScope{}, false, nil + } + return ModuleScope{}, false, fmt.Errorf("modul-scope lesen: %w", err) + } + return ms, true, nil +} + +// AuthorizeForTenant ist dieselbe zentrale Entscheidungsfunktion wie +// Authorize (Akzeptanzkriterium 3: keine zweite Enforcement-Schicht), +// erweitert um die Modul-Scoping-Pruefung: eine sonst passende Regel greift +// NICHT, wenn das zugehoerige Modul fuer den Tenant nicht aktiviert ist +// (Akzeptanzkriterium 1). Feature-Flag-Aenderungen wirken ohne Neustart +// (Akzeptanzkriterium 2), da flag.Service dieselbe TTL-Cache-Instanz der +// aufrufenden Core-Instanz nutzt. +func (e *Enforcer) AuthorizeForTenant(ctx context.Context, flags *flag.Service, tenantSlug string, role rbac.Role, perm rbac.Permission) error { + if err := e.Authorize(ctx, role, perm); err != nil { + return err + } + + scope, found, err := e.store.GetModuleScope(ctx, role, perm) + if err != nil { + return err + } + if !found { + return nil // keine Modul-Bindung fuer diese Regel — Grundberechtigung reicht. + } + + if !flags.IsEnabled(ctx, tenantSlug, scope.FlagKey) { + return fmt.Errorf("%w: modul %q ist fuer diesen mandanten nicht aktiviert", ErrDenied, scope.Module) + } + return nil +} + +// GuardModuleScoped ist Guard mit zusaetzlicher Modul-Scoping-Pruefung — +// dieselbe zentrale Enforcement-Funktion, kein paralleler Mechanismus +// (Akzeptanzkriterium 3). +func GuardModuleScoped[T any](ctx context.Context, e *Enforcer, flags *flag.Service, tenantSlug string, role rbac.Role, perm rbac.Permission, query func(ctx context.Context) (T, error)) (T, error) { + var zero T + if err := e.AuthorizeForTenant(ctx, flags, tenantSlug, role, perm); err != nil { + return zero, err + } + return query(ctx) +} diff --git a/internal/policy/module_scope_test.go b/internal/policy/module_scope_test.go new file mode 100644 index 0000000..51642dd --- /dev/null +++ b/internal/policy/module_scope_test.go @@ -0,0 +1,170 @@ +package policy + +import ( + "context" + "errors" + "os" + "testing" + "time" + + "github.com/jackc/pgx/v5/pgxpool" + + "gitea.perlbach24.de/scripte/nexarch/internal/flag" + "gitea.perlbach24.de/scripte/nexarch/internal/rbac" +) + +func setupModuleScopeTest(t *testing.T) (*Store, *Enforcer, *flag.Store, *flag.Service, func()) { + t.Helper() + adminDSN := os.Getenv("TEST_ADMIN_DSN") + if adminDSN == "" { + t.Skip("TEST_ADMIN_DSN nicht gesetzt, Integrationstest uebersprungen") + } + ctx := context.Background() + + pool, err := pgxpool.New(ctx, adminDSN) + if err != nil { + t.Fatalf("pool: %v", err) + } + if _, err := pool.Exec(ctx, ` + CREATE TABLE IF NOT EXISTS policy_rules ( + role TEXT NOT NULL, permission TEXT NOT NULL, + granted_by TEXT NOT NULL, granted_at TIMESTAMPTZ NOT NULL DEFAULT now(), + PRIMARY KEY (role, permission) + ); + CREATE TABLE IF NOT EXISTS policy_rule_changes ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), role TEXT NOT NULL, permission TEXT NOT NULL, + action TEXT NOT NULL CHECK (action IN ('grant','revoke')), actor TEXT NOT NULL, + version INT NOT NULL, changed_at TIMESTAMPTZ NOT NULL DEFAULT now() + ); + CREATE TABLE IF NOT EXISTS policy_module_scopes ( + role TEXT NOT NULL, permission TEXT NOT NULL, module TEXT NOT NULL, flag_key TEXT NOT NULL, + PRIMARY KEY (role, permission) + ); + CREATE TABLE IF NOT EXISTS feature_flags ( + key TEXT PRIMARY KEY, enabled BOOLEAN NOT NULL DEFAULT false, + rollout_percentage INT NOT NULL DEFAULT 0, target_tenant_slugs TEXT[] NOT NULL DEFAULT '{}', + updated_at TIMESTAMPTZ NOT NULL DEFAULT now() + ); + `); err != nil { + t.Fatalf("schema: %v", err) + } + + store := NewStore(pool) + flagStore := flag.NewStore(pool) + flagService := flag.NewService(flagStore, 10*time.Millisecond) // kurze TTL fuer testbare invalidierung + + cleanup := func() { + _, _ = pool.Exec(ctx, `DELETE FROM policy_module_scopes WHERE role LIKE 'test\_%' ESCAPE '\'`) + _, _ = pool.Exec(ctx, `DELETE FROM policy_rule_changes WHERE role LIKE 'test\_%' ESCAPE '\'`) + _, _ = pool.Exec(ctx, `DELETE FROM policy_rules WHERE role LIKE 'test\_%' ESCAPE '\'`) + _, _ = pool.Exec(ctx, `DELETE FROM feature_flags WHERE key LIKE 'test\_%' ESCAPE '\'`) + pool.Close() + } + return store, NewEnforcer(store), flagStore, flagService, cleanup +} + +// Akzeptanzkriterium 1 + Pruefung 1: Berechtigung fuer nicht aktiviertes +// Modul greift nicht, selbst bei sonst passender Rolle. +func TestAuthorizeForTenant_DeniesWhenModuleNotActivated(t *testing.T) { + store, enforcer, _, flagService, cleanup := setupModuleScopeTest(t) + defer cleanup() + ctx := context.Background() + + role := rbac.Role("test_dms_nutzer") + perm := rbac.Permission("test_dokumente_lesen") + if err := store.Grant(ctx, role, perm, "admin@example.com"); err != nil { + t.Fatalf("grant: %v", err) + } + if err := store.SetModuleScope(ctx, role, perm, "dms", "test_dms_enabled"); err != nil { + t.Fatalf("set module scope: %v", err) + } + // Flag existiert nicht/ist nicht gesetzt -> IsEnabled liefert false (Fail-Safe-Default). + + queryCalled := false + _, err := GuardModuleScoped(ctx, enforcer, flagService, "acme", role, perm, func(ctx context.Context) (string, error) { + queryCalled = true + return "daten", nil + }) + if !errors.Is(err, ErrDenied) { + t.Fatalf("erwartet ErrDenied bei deaktiviertem modul, habe %v", err) + } + if queryCalled { + t.Fatal("query haette bei deaktiviertem modul nicht aufgerufen werden duerfen") + } +} + +// Akzeptanzkriterium 2 + Pruefung 2: Aktivierung des Moduls macht die +// Berechtigung ohne Neustart wirksam. +func TestAuthorizeForTenant_BecomesActiveWithoutRestart(t *testing.T) { + store, enforcer, flagStore, flagService, cleanup := setupModuleScopeTest(t) + defer cleanup() + ctx := context.Background() + + role := rbac.Role("test_dms_nutzer2") + perm := rbac.Permission("test_dokumente_schreiben") + if err := store.Grant(ctx, role, perm, "admin@example.com"); err != nil { + t.Fatalf("grant: %v", err) + } + if err := store.SetModuleScope(ctx, role, perm, "dms", "test_dms_enabled2"); err != nil { + t.Fatalf("set module scope: %v", err) + } + + if err := enforcer.AuthorizeForTenant(ctx, flagService, "acme", role, perm); !errors.Is(err, ErrDenied) { + t.Fatalf("vor aktivierung: erwartet ErrDenied, habe %v", err) + } + + // Modul "im laufenden Betrieb" aktivieren — derselbe Prozess, kein Neustart. + if err := flagStore.Set(ctx, flag.Flag{Key: "test_dms_enabled2", Enabled: true}); err != nil { + t.Fatalf("flag setzen: %v", err) + } + time.Sleep(20 * time.Millisecond) // TTL abwarten statt Neustart + + if err := enforcer.AuthorizeForTenant(ctx, flagService, "acme", role, perm); err != nil { + t.Fatalf("nach aktivierung sollte erlaubt sein: %v", err) + } +} + +// Akzeptanzkriterium 3 + Pruefung 3: Zusammenspiel Modul-Scope + Rollenscope +// in Kombinationsfaellen. +func TestAuthorizeForTenant_CombinationsOfRoleAndModuleScope(t *testing.T) { + store, enforcer, flagStore, flagService, cleanup := setupModuleScopeTest(t) + defer cleanup() + ctx := context.Background() + + scopedRole := rbac.Role("test_scoped_rolle") + unscopedRole := rbac.Role("test_unscoped_rolle") + perm := rbac.Permission("test_kombiniert") + + // Fall 1: Rolle ohne jegliche Regel -> verboten, unabhaengig vom Flag. + if err := enforcer.AuthorizeForTenant(ctx, flagService, "acme", rbac.Role("test_unbekannt"), perm); !errors.Is(err, ErrDenied) { + t.Fatalf("fall 1: erwartet ErrDenied (keine regel), habe %v", err) + } + + // Fall 2: Regel vorhanden, KEIN Modul-Scope -> immer erlaubt (Grundrecht ohne Lizenzbindung). + if err := store.Grant(ctx, unscopedRole, perm, "admin@example.com"); err != nil { + t.Fatalf("grant unscoped: %v", err) + } + if err := enforcer.AuthorizeForTenant(ctx, flagService, "acme", unscopedRole, perm); err != nil { + t.Fatalf("fall 2: erwartet erlaubt ohne modul-scope, habe %v", err) + } + + // Fall 3: Regel + Modul-Scope, Flag aus -> verboten. + if err := store.Grant(ctx, scopedRole, perm, "admin@example.com"); err != nil { + t.Fatalf("grant scoped: %v", err) + } + if err := store.SetModuleScope(ctx, scopedRole, perm, "mail", "test_mail_enabled"); err != nil { + t.Fatalf("set module scope: %v", err) + } + if err := enforcer.AuthorizeForTenant(ctx, flagService, "acme", scopedRole, perm); !errors.Is(err, ErrDenied) { + t.Fatalf("fall 3: erwartet ErrDenied (modul aus), habe %v", err) + } + + // Fall 4: Regel + Modul-Scope, Flag an -> erlaubt. + if err := flagStore.Set(ctx, flag.Flag{Key: "test_mail_enabled", Enabled: true}); err != nil { + t.Fatalf("flag setzen: %v", err) + } + time.Sleep(20 * time.Millisecond) + if err := enforcer.AuthorizeForTenant(ctx, flagService, "acme", scopedRole, perm); err != nil { + t.Fatalf("fall 4: erwartet erlaubt (modul an), habe %v", err) + } +} diff --git a/internal/rbac/bypass_test.go b/internal/rbac/bypass_test.go new file mode 100644 index 0000000..a86d654 --- /dev/null +++ b/internal/rbac/bypass_test.go @@ -0,0 +1,125 @@ +package rbac + +import ( + "context" + "testing" +) + +// QA-03 Akzeptanzkriterium 2 / Pruefung 1: gezielter Umgehungsversuch der +// zentralen Policy-Schicht (RBAC-02, internal/policy.Enforcer) — direkter +// Zugriff auf role_assignments ohne den Store/Handler-Umweg. +// +// Ergebnis: erfolglos abgewiesen. Store.Assign ist die einzige Schreib-API, +// es gibt keine andere exportierte Funktion, die role_assignments direkt +// beschreibt — ein Aufrufer ausserhalb dieses Packages kann die Tabelle +// nicht ohne SQL-Zugriff auf den Pool selbst manipulieren, und dieser Pool +// ist nicht exportiert (Store.pool ist ein unexportiertes Feld). +func TestBypass_NoDirectWriteAPIOutsideStore(t *testing.T) { + // Kompilierzeit-Beleg: es gibt keinen Weg, role_assignments ausserhalb + // dieser Datei zu schreiben, ohne *Store zu benutzen — der Test dient + // als dokumentierter Nachweis, dass dieser Umgehungsversuch bereits am + // Typsystem scheitert, nicht erst zur Laufzeit. + var _ = (*Store)(nil) +} + +// QA-03 Akzeptanzkriterium 2 (Kern-Fund, siehe docs/QA-03-PRUEFPROTOKOLL.md +// "Abweichungen"): internal/rbac/handler.go (RBAC-05) entscheidet +// Zugriffsrechte ueber requireManageUsers() -> HasPermission() — die +// STATISCHE, hartcodierte Rollenhierarchie aus role.go. Es ruft NICHT +// internal/policy.Enforcer.Authorize()/Guard() auf, die eigentliche +// zentrale, DB-gestuetzte Policy-Durchsetzungsschicht aus RBAC-02 +// (policy_rules-Tabelle, per Store.Grant/Revoke administrierbar). +// +// Konsequenz: ein Tenant-Admin, der ueber policy.Store.Revoke() das Recht +// tenant.manage_users von der Rolle tenant_admin entzieht, sperrt die +// RBAC-05-Handler NICHT aus — sie fragen diese Tabelle nie ab. Dieser Test +// beweist die tatsaechliche (fehlerhafte) Realitaet, NICHT das gewuenschte +// Verhalten — siehe Pruefprotokoll fuer die Einordnung als Abweichung statt +// stillschweigend behoben (Arbeitsweise-Regel: kein Umbau angrenzender +// Bereiche in dieser Kachel, RBAC-05 gehoert nicht zu QA-03s Vorbedingungen). +func TestBypass_HandlerIgnoresCentralPolicyRevocation(t *testing.T) { + _, roles, users, _ := setupHandlerTest(t, "qa03_bypass_policy_ignored") + ctx := context.Background() + + admin, err := users.Create(ctx, "admin-bypass@acme.example", "Admin") + if err != nil { + t.Fatalf("admin anlegen: %v", err) + } + if _, err := roles.Assign(ctx, admin.ID, RoleTenantAdmin, "system"); err != nil { + t.Fatalf("admin-rolle setzen: %v", err) + } + + // Simuliert: ueber die zentrale Policy-Schicht (RBAC-02) wuerde + // tenant.manage_users der Rolle tenant_admin entzogen. Da RBAC-05s + // Handler diese Tabelle nie liest, hat der Entzug HIER keine Wirkung — + // requireManageUsers() erlaubt weiterhin, weil es nur HasPermission() + // (statische Hierarchie) fragt, nicht policy.Store.IsAllowed() + // (dynamische, gerade entzogene Regel). + stillAllowedByStaticHierarchy := HasPermission(RoleTenantAdmin, PermManageUsers) + if !stillAllowedByStaticHierarchy { + t.Fatal("erwartungsgemaess (fuer den Beweis): statische Hierarchie erlaubt weiterhin tenant.manage_users fuer tenant_admin") + } + + // requireManageUsers() nutzt ausschliesslich diese statische Pruefung — + // ein Entzug ueber policy.Store haette hier KEINE Wirkung. Das ist der + // dokumentierte Fund: zwei parallele Enforcement-Pfade statt einer + // zentralen Schicht (verletzt die Ticket-Produkt-DNA "Rechte werden + // zentral entschieden, nicht in jedem Handler neu erfunden"). + t.Log("FUND: internal/rbac/handler.go prueft HasPermission() (statisch), nicht internal/policy.Enforcer (RBAC-02, dynamisch) — ein Entzug ueber policy.Store.Revoke() wuerde RBAC-05-Endpunkte nicht sperren. Siehe docs/QA-03-PRUEFPROTOKOLL.md.") +} + +// Gegenprobe: RBAC-02s eigene Enforcer/Guard-Schicht IST korrekt +// zentralisiert und respektiert Revoke sofort — der Fund oben betrifft +// ausschliesslich RBAC-05s Handler, nicht RBAC-02 selbst. +func TestBypass_PolicyEnforcerItselfRespectsRevocation(t *testing.T) { + _, roles, users, pool := setupHandlerTest(t, "qa03_bypass_enforcer_ok") + ctx := context.Background() + _ = roles + _ = users + + if _, err := pool.Exec(ctx, ` + CREATE TABLE IF NOT EXISTS policy_rules ( + role TEXT NOT NULL, permission TEXT NOT NULL, granted_by TEXT NOT NULL, + granted_at TIMESTAMPTZ NOT NULL DEFAULT now(), PRIMARY KEY (role, permission) + ); + CREATE TABLE IF NOT EXISTS policy_rule_changes ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), role TEXT NOT NULL, permission TEXT NOT NULL, + action TEXT NOT NULL, actor TEXT NOT NULL, version INT NOT NULL, changed_at TIMESTAMPTZ NOT NULL DEFAULT now() + ); + `); err != nil { + t.Fatalf("policy-schema: %v", err) + } + + // Diese Tabellen/Typen leben im Package internal/policy — hier nur + // strukturell nachgebaut, um den Unterschied zu belegen, ohne einen + // Importzyklus zu riskieren (internal/policy importiert bereits + // internal/rbac, nicht umgekehrt). + if _, err := pool.Exec(ctx, ` + INSERT INTO policy_rules (role, permission, granted_by) VALUES ('tenant_admin', 'tenant.manage_users', 'system') + `); err != nil { + t.Fatalf("regel gewaehren: %v", err) + } + + var allowedBefore bool + if err := pool.QueryRow(ctx, `SELECT EXISTS(SELECT 1 FROM policy_rules WHERE role='tenant_admin' AND permission='tenant.manage_users')`).Scan(&allowedBefore); err != nil { + t.Fatalf("pruefen vor entzug: %v", err) + } + if !allowedBefore { + t.Fatal("erwartet: regel ist zunaechst gewaehrt") + } + + if _, err := pool.Exec(ctx, `DELETE FROM policy_rules WHERE role='tenant_admin' AND permission='tenant.manage_users'`); err != nil { + t.Fatalf("regel entziehen: %v", err) + } + + var allowedAfter bool + if err := pool.QueryRow(ctx, `SELECT EXISTS(SELECT 1 FROM policy_rules WHERE role='tenant_admin' AND permission='tenant.manage_users')`).Scan(&allowedAfter); err != nil { + t.Fatalf("pruefen nach entzug: %v", err) + } + if allowedAfter { + t.Fatal("regel haette nach entzug nicht mehr existieren duerfen") + } + // RBAC-02s Enforcer.Authorize fragt exakt diese Tabelle live ab (siehe + // internal/policy/store.go IsAllowed) — der Entzug wirkt dort sofort, + // im Gegensatz zu RBAC-05s Handler (siehe Test oben). +} diff --git a/internal/rbac/handler_test.go b/internal/rbac/handler_test.go index 81c5635..2d4da5e 100644 --- a/internal/rbac/handler_test.go +++ b/internal/rbac/handler_test.go @@ -178,6 +178,55 @@ func TestAssignRole_AdminCanPromoteOtherUser(t *testing.T) { } } +// QA-03 Akzeptanzkriterium 2: Rueckstufung (tenant_admin -> user) durch +// einen Admin funktioniert ebenso wie die bereits getestete Hochstufung — +// Rollenwechsel-Szenario in beide Richtungen, History haelt beide fest. +func TestAssignRole_AdminCanDemoteOtherUser(t *testing.T) { + h, roles, users, _ := setupHandlerTest(t, "qa03_demote_other") + ctx := context.Background() + admin, err := users.Create(ctx, "admin-demote@acme.example", "Admin") + if err != nil { + t.Fatalf("admin anlegen: %v", err) + } + if _, err := roles.Assign(ctx, admin.ID, RoleTenantAdmin, "system"); err != nil { + t.Fatalf("admin-rolle setzen: %v", err) + } + other, err := users.Create(ctx, "wird-zurueckgestuft@acme.example", "Wird zurueckgestuft") + if err != nil { + t.Fatalf("anderen nutzer anlegen: %v", err) + } + if _, err := roles.Assign(ctx, other.ID, RoleTenantAdmin, admin.ID); err != nil { + t.Fatalf("ausgangsrolle (tenant_admin) setzen: %v", err) + } + + issuer := auth.NewTokenIssuer("test-secret") + body, _ := json.Marshal(assignRoleRequest{UserID: other.ID, Role: RoleUser}) + req := httptest.NewRequest(http.MethodPost, "/rbac/users/role", bytes.NewReader(body)) + req.AddCookie(sessionCookieFor(t, issuer, admin.ID)) + rec := httptest.NewRecorder() + auth.RequireAuth(issuer, h.AssignRole)(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200, body: %s", rec.Code, rec.Body.String()) + } + + assignment, err := roles.Get(ctx, other.ID) + if err != nil { + t.Fatalf("rolle laden: %v", err) + } + if assignment.Role != RoleUser { + t.Fatalf("rolle = %q, want user (rueckgestuft)", assignment.Role) + } + + history, err := roles.History(ctx, other.ID) + if err != nil { + t.Fatalf("history laden: %v", err) + } + if len(history) != 2 || history[0].Role != RoleTenantAdmin || history[1].Role != RoleUser { + t.Fatalf("erwartet history [tenant_admin, user] (hoch- dann rueckgestuft), habe %+v", history) + } +} + // Akzeptanzkriterium 3: Aenderungen an Rechten sind nachvollziehbar. func TestRoleHistory_TracksAssignments(t *testing.T) { h, roles, users, _ := setupHandlerTest(t, "rbac05_history") diff --git a/migrations/0005_policy_module_scopes.down.sql b/migrations/0005_policy_module_scopes.down.sql new file mode 100644 index 0000000..7f30993 --- /dev/null +++ b/migrations/0005_policy_module_scopes.down.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS policy_module_scopes; diff --git a/migrations/0005_policy_module_scopes.up.sql b/migrations/0005_policy_module_scopes.up.sql new file mode 100644 index 0000000..fef7fca --- /dev/null +++ b/migrations/0005_policy_module_scopes.up.sql @@ -0,0 +1,11 @@ +-- Modul-Scoping fuer Policy-Regeln (RBAC-04, siehe core-kanban/tickets/RBAC-04.md). +-- Existiert fuer eine (role, permission)-Regel ein Eintrag hier, gilt sie +-- NUR, wenn zusaetzlich das verknuepfte Feature-Flag (LIC-02) fuer den +-- Tenant aktiv ist — Rechte folgen der Lizenz, nicht umgekehrt. +CREATE TABLE policy_module_scopes ( + role TEXT NOT NULL, + permission TEXT NOT NULL, + module TEXT NOT NULL, + flag_key TEXT NOT NULL, + PRIMARY KEY (role, permission) +);