From b04a9936c2ec15205218d267e49dc0f82e29e056 Mon Sep 17 00:00:00 2001 From: Ralf Grubenmann Date: Fri, 28 Aug 2026 13:21:02 +0200 Subject: [PATCH] feat: make casbin rule matching consider domain hierarchy --- .../internal/middleware/casbin_model.conf | 3 +- .../backend/internal/middleware/rbac.go | 57 +++ .../middleware/rbac_domain_match_test.go | 358 ++++++++++++++++++ .../backend/internal/middleware/rbac_test.go | 10 + .../internal/service/project_service.go | 20 +- .../backend/internal/service/team_service.go | 18 +- 6 files changed, 436 insertions(+), 30 deletions(-) create mode 100644 components/backend/internal/middleware/rbac_domain_match_test.go diff --git a/components/backend/internal/middleware/casbin_model.conf b/components/backend/internal/middleware/casbin_model.conf index 45fdc95d..b3f6a50a 100644 --- a/components/backend/internal/middleware/casbin_model.conf +++ b/components/backend/internal/middleware/casbin_model.conf @@ -16,6 +16,7 @@ p = sub, domain, obj, act g = _, _, _ # g2 is for global roles, e.g. "g2, adminuser, admin" means user "adminuser" has role admin in ALL domains g2 = _, _ +g3 = _, _ [policy_effect] # this contains rules on how effects of policies are combined. We don't specify effects, so every policy has the default "allow" effect @@ -32,4 +33,4 @@ e = some(where (p.eft == allow)) # this just check that the object and action in the request and policy match # || g2(r.sub, "admin") # this overrides all of the other stuff and allows if a user has the admin role -m = (g(r.sub, p.sub, r.domain) || g2(r.sub, p.sub) || p.sub=="*") && globMatch(r.domain, p.domain) && r.obj == p.obj && r.act == p.act || g2(r.sub, "admin") +m = r.obj == p.obj && r.act == p.act && (g2(r.sub, p.sub) || hasRoleInDomainOrAncestor(r.sub, p.sub, r.domain, p.domain) ) || g2(r.sub, "admin") diff --git a/components/backend/internal/middleware/rbac.go b/components/backend/internal/middleware/rbac.go index a454bd90..71be2f67 100644 --- a/components/backend/internal/middleware/rbac.go +++ b/components/backend/internal/middleware/rbac.go @@ -7,10 +7,13 @@ import ( "fmt" "log/slog" "os" + "slices" + "strings" "github.com/casbin/casbin/v3" "github.com/casbin/casbin/v3/log" "github.com/casbin/casbin/v3/model" + "github.com/casbin/casbin/v3/util" entadapter "github.com/casbin/ent-adapter" _ "github.com/lib/pq" "github.com/swissdatasciencecenter/hackagon/components/backend/internal/config" @@ -163,9 +166,63 @@ func NewRBACEnforcer(cfg *config.Config) (*Enforcer, error) { return nil, err } + // Register hierarchical domain matching for both g role lookups and the + // matcher. A request to /hackathon/h1/team/t1 matches a policy on + // /hackathon/h1 (and any glob pattern under it). + e.AddFunction("hasRoleInDomainOrAncestor", domainMatch(e)) return &Enforcer{enforcer: e}, nil } +func parentDomain(domain string) string { + if domain == "" || domain == "/" { + return "" + } + domain = strings.Trim(domain, "/") + parts := strings.Split(domain, "/") + numPartsPerDomainSection := 2 + if len(parts) <= numPartsPerDomainSection { + return "" + } + return "/" + strings.Join(parts[:len(parts)-numPartsPerDomainSection], "/") +} + +func hasRoleInDomain(e *casbin.Enforcer, sub, role, domain string) bool { + roles := e.GetRolesForUserInDomain(sub, domain) + return slices.Contains(roles, role) +} +func domainMatch(e *casbin.Enforcer) func(args ...interface{}) (interface{}, error) { + return func(args ...interface{}) (interface{}, error) { + sub, ok := args[0].(string) + if !ok { + return nil, fmt.Errorf("could not convert sub to string: %v", sub) + } + role, ok := args[1].(string) + if !ok { + return nil, fmt.Errorf("could not convert role to string: %v", sub) + } + requestDomain, ok := args[2].(string) + if !ok { + return nil, fmt.Errorf("could not convert requestDomain to string: %v", sub) + } + policyDomain, ok := args[3].(string) + if !ok { + return nil, fmt.Errorf("could not convert policyDomain to string: %v", sub) + } + for d := requestDomain; d != ""; d = parentDomain(d) { + matched, err := util.GlobMatch(d, policyDomain) + if err != nil { + return false, err + } + if !matched { + continue + } + if role == "*" || hasRoleInDomain(e, sub, role, d) { + return true, nil + } + } + return false, nil + } +} func defaultPolicies(cfg *config.Config, e *casbin.Enforcer) error { policies := [][]string{ // HackathonOrganizer can create new hackathons diff --git a/components/backend/internal/middleware/rbac_domain_match_test.go b/components/backend/internal/middleware/rbac_domain_match_test.go new file mode 100644 index 00000000..051aa29a --- /dev/null +++ b/components/backend/internal/middleware/rbac_domain_match_test.go @@ -0,0 +1,358 @@ +//go:build test && unittest + +package middleware + +import ( + "fmt" + "testing" + + "github.com/casbin/casbin/v3" + "github.com/casbin/casbin/v3/model" + "github.com/casbin/casbin/v3/util" +) + +func setupTestEnforcer() *casbin.Enforcer { + m, err := model.NewModelFromString(` +[request_definition] +r = sub, domain, obj, act + +[policy_definition] +p = sub, domain, obj, act + +[role_definition] +g = _, _, _ + +[policy_effect] +e = some(where (p.eft == allow)) + +[matchers] +m = g(r.sub, p.sub, r.domain) && hasRoleInDomainOrAncestor(r.sub, p.sub, r.domain, p.domain) && r.obj == p.obj && r.act == p.act +`) + if err != nil { + panic(err) + } + e, err := casbin.NewEnforcer(m) + if err != nil { + panic(err) + } + // Register the domain match function so the matcher can use it + e.AddFunction("hasRoleInDomainOrAncestor", domainMatch(e)) + return e +} + +func TestDomainMatchDebug(t *testing.T) { + e := setupTestEnforcer() + + // Add a grouping policy + _, err := e.AddGroupingPolicy("alice", "owner", "/hackathon/h1") + if err != nil { + t.Fatalf("AddGroupingPolicy failed: %v", err) + } + + // Rebuild role links + _ = e.BuildRoleLinks() + + // Check what GetRolesForUserInDomain returns + roles := e.GetRolesForUserInDomain("alice", "/hackathon/h1") + fmt.Printf("Roles for alice in /hackathon/h1: %v\n", roles) + + // Test domainMatch directly + dm := domainMatch(e) + result, err := dm("alice", "owner", "/hackathon/h1", "/hackathon/h1") + fmt.Printf("domainMatch result: %v, err: %v\n", result, err) + + // Test parentDomain + fmt.Printf("parentDomain(/hackathon/h1) = %q\n", parentDomain("/hackathon/h1")) + fmt.Printf("parentDomain(/hackathon/h1/team/t1) = %q\n", parentDomain("/hackathon/h1/team/t1")) +} + +func TestDomainMatch(t *testing.T) { + tests := []struct { + name string + sub string + role string + requestDomain string + policyDomain string + policies []string // g policies to add: "user,role,domain" + want bool + }{ + // Exact match — user has role at exact domain + { + name: "exact match owner", + sub: "alice", + role: "owner", + requestDomain: "/hackathon/h1", + policyDomain: "/hackathon/h1", + policies: []string{"alice", "owner", "/hackathon/h1"}, + want: true, + }, + { + name: "exact match member", + sub: "bob", + role: "member", + requestDomain: "/hackathon/h1/team/t1", + policyDomain: "/hackathon/h1/team/t1", + policies: []string{"bob", "member", "/hackathon/h1/team/t1"}, + want: true, + }, + + // Hierarchical — user has role at parent domain + { + name: "parent domain owner", + sub: "alice", + role: "owner", + requestDomain: "/hackathon/h1/team/t1", + policyDomain: "/hackathon/h1", + policies: []string{"alice", "owner", "/hackathon/h1"}, + want: true, + }, + { + name: "grandparent domain owner", + sub: "alice", + role: "owner", + requestDomain: "/hackathon/h1/team/t1/submission/s1", + policyDomain: "/hackathon/h1", + policies: []string{"alice", "owner", "/hackathon/h1"}, + want: true, + }, + { + name: "parent domain member", + sub: "bob", + role: "member", + requestDomain: "/hackathon/h1/team/t1", + policyDomain: "/hackathon/h1", + policies: []string{"bob", "member", "/hackathon/h1"}, + want: true, + }, + + // Role mismatch — user has different role at ancestor + { + name: "different role at parent", + sub: "bob", + role: "owner", + requestDomain: "/hackathon/h1/team/t1", + policyDomain: "/hackathon/h1", + policies: []string{"bob", "member", "/hackathon/h1"}, + want: false, + }, + + // No role assigned + { + name: "no role assigned", + sub: "charlie", + role: "owner", + requestDomain: "/hackathon/h1", + policyDomain: "/hackathon/h1", + policies: nil, + want: false, + }, + + // Cross-hackathon rejection — user has role in different hackathon + { + name: "different hackathon", + sub: "alice", + role: "owner", + requestDomain: "/hackathon/h2", + policyDomain: "/hackathon/h1", + policies: []string{"alice", "owner", "/hackathon/h2"}, + want: false, + }, + { + name: "different hackathon subdomain", + sub: "alice", + role: "owner", + requestDomain: "/hackathon/h2/team/t1", + policyDomain: "/hackathon/h1", + policies: []string{"alice", "owner", "/hackathon/h2"}, + want: false, + }, + + // Glob pattern — user has role at matching glob domain + { + name: "glob wildcard match", + sub: "bob", + role: "member", + requestDomain: "/hackathon/h1/team/t1", + policyDomain: "/hackathon/*/team/*", + policies: []string{"bob", "member", "/hackathon/*/team/*"}, + want: true, + }, + { + name: "glob wildcard deep match", + sub: "bob", + role: "member", + requestDomain: "/hackathon/h1/team/t1/submission/s1", + policyDomain: "/hackathon/*/team/*", + policies: []string{"bob", "member", "/hackathon/*/team/*"}, + want: true, + }, + { + name: "glob wildcard project", + sub: "alice", + role: "owner", + requestDomain: "/hackathon/h1/project/p1", + policyDomain: "/hackathon/*/project/*", + policies: []string{"alice", "owner", "/hackathon/*/project/*"}, + want: true, + }, + + // Glob pattern — no match + { + name: "glob wildcard no match", + sub: "bob", + role: "member", + requestDomain: "/hackathon/h1", + policyDomain: "/hackathon/*/team/*", + policies: []string{"bob", "member", "/hackathon/*/team/*"}, + want: false, + }, + + // Empty strings + { + name: "both empty", + sub: "alice", + role: "owner", + requestDomain: "", + policyDomain: "", + policies: []string{"alice", "owner", ""}, + want: true, + }, + { + name: "request empty", + sub: "alice", + role: "owner", + requestDomain: "", + policyDomain: "/hackathon/h1", + policies: []string{"alice", "owner", "/hackathon/h1"}, + want: false, + }, + { + name: "policy empty", + sub: "alice", + role: "owner", + requestDomain: "/hackathon/h1", + policyDomain: "", + policies: []string{"alice", "owner", "/hackathon/h1"}, + want: false, + }, + + // Partial pair boundaries must NOT match + { + name: "partial pair no match", + sub: "alice", + role: "owner", + requestDomain: "/hackathon/h1/team/t1", + policyDomain: "/hackathon/h1/team", + policies: []string{"alice", "owner", "/hackathon/h1/team"}, + want: false, + }, + { + name: "partial pair entity no match", + sub: "alice", + role: "owner", + requestDomain: "/hackathon/h1", + policyDomain: "/hackathon", + policies: []string{"alice", "owner", "/hackathon"}, + want: false, + }, + { + name: "partial pair entity glob no match", + sub: "bob", + role: "member", + requestDomain: "/hackathon/h1/team/t1", + policyDomain: "/hackathon/*/team", + policies: []string{"bob", "member", "/hackathon/*/team"}, + want: false, + }, + + // Role at team level + { + name: "role at team level", + sub: "bob", + role: "member", + requestDomain: "/hackathon/h1/team/t1", + policyDomain: "/hackathon/h1/team/t1", + policies: []string{"bob", "member", "/hackathon/h1/team/t1"}, + want: true, + }, + + // Role at ancestor but checking at descendant — should match + { + name: "ancestor role checks descendant", + sub: "alice", + role: "owner", + requestDomain: "/hackathon/h1/project/p1", + policyDomain: "/hackathon/h1", + policies: []string{"alice", "owner", "/hackathon/h1"}, + want: true, + }, + { + name: "submission read for owner", + sub: "alice", + role: "owner", + requestDomain: "/hackathon/h1/team/t1", + policyDomain: "/hackathon/*", + policies: []string{"alice", "owner", "/hackathon/h1"}, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + e := setupTestEnforcer() + if len(tt.policies) > 0 { + parts := make([]interface{}, 0, len(tt.policies)) + for _, pol := range tt.policies { + parts = append(parts, pol) + } + _, _ = e.AddGroupingPolicy(parts...) + } + // Rebuild role links to pick up newly added grouping policies + _ = e.BuildRoleLinks() + + dm := domainMatch(e) + got, err := dm(tt.sub, tt.role, tt.requestDomain, tt.policyDomain) + if err != nil { + t.Errorf("domainMatch() error = %v", err) + return + } + if got != tt.want { + // Debug: check roles + roles := e.GetRolesForUserInDomain(tt.sub, tt.requestDomain) + t.Logf("domainMatch(%q, %q, %q, %q) = %v, want %v", + tt.sub, tt.role, tt.requestDomain, tt.policyDomain, got, tt.want) + t.Logf(" roles in request domain: %v", roles) + // Check parent domains + for d := tt.requestDomain; d != ""; d = parentDomain(d) { + matched, _ := util.GlobMatch(d, tt.policyDomain) + roles := e.GetRolesForUserInDomain(tt.sub, d) + t.Logf(" domain %q: globMatch=%v, roles=%v", d, matched, roles) + } + } + }) + } +} + +func splitCSV(s string) []string { + // Simple CSV split for test data + result := []string{} + current := "" + inQuote := false + for _, r := range s { + switch r { + case '"': + inQuote = !inQuote + case ',': + if !inQuote { + result = append(result, current) + current = "" + continue + } + fallthrough + default: + current += string(r) + } + } + result = append(result, current) + return result +} diff --git a/components/backend/internal/middleware/rbac_test.go b/components/backend/internal/middleware/rbac_test.go index f580b887..439b728e 100644 --- a/components/backend/internal/middleware/rbac_test.go +++ b/components/backend/internal/middleware/rbac_test.go @@ -62,6 +62,16 @@ var _ = Describe("RBAC Enforcer", func() { }, Entry("alice owner reads h1", "alice", "h1", Hackathon, Read, true), Entry("alice owner writes h1", "alice", "h1", Hackathon, Write, true), + Entry("alice owner reads h1 submissions", "alice", "h1", Submission, Read, true), + Entry("alice owner cannot read h2 submissions", "alice", "h2", Submission, Read, false), + Entry( + "alice owner cannot write h1 submissions", + "alice", + "h1", + Submission, + Write, + false, + ), Entry("alice cannot read h2", "alice", "h2", Hackathon, Read, false), Entry("bob member reads h1", "bob", "h1", Hackathon, Read, true), Entry("bob member cannot write h1", "bob", "h1", Hackathon, Write, false), diff --git a/components/backend/internal/service/project_service.go b/components/backend/internal/service/project_service.go index 39e1ade6..f7832952 100644 --- a/components/backend/internal/service/project_service.go +++ b/components/backend/internal/service/project_service.go @@ -634,18 +634,14 @@ func (s *ProjectService) Edit( hackathonID := project.Edges.Hackathon.ID - err = s.enforcer.RequirePermission( + if err := s.enforcer.RequirePermission( ctx, hackathonID.String(), mw.Project, mw.Write, mw.WithProject(projectID.String()), - ) - if err != nil { - err = s.enforcer.RequirePermission(ctx, hackathonID.String(), mw.Project, mw.Write) - if err != nil { - return nil, status.Error(codes.PermissionDenied, "permission denied") - } + ); err != nil { + return nil, status.Error(codes.PermissionDenied, "permission denied") } // Build the update query with only provided fields @@ -746,18 +742,14 @@ func (s *ProjectService) Delete( hackathonID := project.Edges.Hackathon.ID - err = s.enforcer.RequirePermission( + if err := s.enforcer.RequirePermission( ctx, hackathonID.String(), mw.Project, mw.Write, mw.WithProject(projectID.String()), - ) - if err != nil { - err = s.enforcer.RequirePermission(ctx, hackathonID.String(), mw.Project, mw.Write) - if err != nil { - return nil, status.Error(codes.PermissionDenied, "permission denied") - } + ); err != nil { + return nil, status.Error(codes.PermissionDenied, "permission denied") } // Delete the project diff --git a/components/backend/internal/service/team_service.go b/components/backend/internal/service/team_service.go index c8f7edea..1c9c4883 100644 --- a/components/backend/internal/service/team_service.go +++ b/components/backend/internal/service/team_service.go @@ -209,11 +209,7 @@ func (s *TeamService) Edit( ctx, hackathonID, m.Team, m.Write, m.WithTeam(t.ID.String()), ); err != nil { - if err := s.enforcer.RequirePermission( - ctx, hackathonID, m.Team, m.Write, - ); err != nil { - return nil, err - } + return nil, err } u, err := s.dbClient.User.Query(). @@ -1040,11 +1036,7 @@ func (s *TeamService) GetSubmission( ctx, hackathonID, m.Submission, m.Read, m.WithTeam(t.ID.String()), ); err != nil { - if err := s.enforcer.RequirePermission( - ctx, hackathonID, m.Submission, m.Read, - ); err != nil { - return nil, err - } + return nil, err } // Find the latest submission for this team (highest version). @@ -1093,11 +1085,7 @@ func (s *TeamService) ListSubmissions( ctx, hackathonID, m.Submission, m.Read, m.WithTeam(t.ID.String()), ); err != nil { - if err := s.enforcer.RequirePermission( - ctx, hackathonID, m.Submission, m.Read, - ); err != nil { - return nil, err - } + return nil, err } submissions, err := s.dbClient.Submission.Query().