diff --git a/.wolf/buglog.json b/.wolf/buglog.json index f2c095b8..d65436c7 100644 --- a/.wolf/buglog.json +++ b/.wolf/buglog.json @@ -17761,10 +17761,10 @@ "timestamp": "2026-10-10T04:31:25Z", "last_seen": "2026-10-10T04:31:25Z", "occurrences": 1, - "error_message": "apiserver treats a job:view grant scoped only to self_ug as all: user sees every user's jobs\n\n**Observed:** (source reading, not yet run) a role whose `job:view` grant is\nscoped only to `self_ug` can see every user's jobs. The job-list handler\n(`src/frontend/apiserver/controllers/jobs.go`) only distinguishes\n`DomainType == self`:\n```go\nif permission.DomainType != models.DomainTypeSelf {\n submitUser = filter.Value // jobs.go:178, caller-chosen user\n}\n...\nif !specify_user || permission.DomainType == models.DomainTypeSelf { // :187\n...\nif jobPermission.DomainType == models.DomainTypeSelf { // :762, per-row owner filter\n```\nso `self_ug` falls through every check like `all`: a `submit_user` filter\nnaming any user (or `all`) is honoured and no row is removed.\n`GetRequireResourcePermission()` (`controllers/rbac_check.go:323-346`)\nreturns only the first grant matching the permission, so for the default\n`basic_user` (`config/rbac.yaml:37-40`, `job:view` with `self` then\n`self_ug`) the `self` entry wins and `self_ug` is silently ignored.\n`self_ug` is accepted by role validation (`controllers/roles.go:697-700`)\nand listed as a supported job scope (`models/roles.go:70`), but nothing\nresolves the user's user groups for it.\n\n**Expected:** a `self_ug` job grant covers exactly the jobs whose owner shares\na user group with the requester; a grant list with several scopes covers the\nunion of them; anything else is rejected or filtered out.\n**Impact:** security (authorization bypass). Any role an admin creates with\n`job:view` (or another job permission, unchecked) scoped to `self_ug` grants\nvisibility of all users' jobs. The default `basic_user` is not exposed (its\n`self` grant is matched first) but never gets the group visibility its\nconfig promises. Other job permissions using the same single-grant lookup\nwere not checked yet.\n**Found by:** review, during `/kiro-spec-requirements project-config`\n(2026-10-10, RBAC survey for project-scoped job permissions).", + "error_message": "apiserver treats a job:view grant scoped only to self_ug as all: user sees every user's jobs\n\n**Observed:** (source reading, not yet run) a role whose `job:view` grant is\nscoped only to `self_ug` can see every user's jobs. The job-list handler\n(`src/frontend/apiserver/controllers/jobs.go`) only distinguishes\n`DomainType == self`:\n```go\nif permission.DomainType != models.DomainTypeSelf {\n submitUser = filter.Value // jobs.go:178, caller-chosen user\n}\n...\nif !specify_user || permission.DomainType == models.DomainTypeSelf { // :187\n...\nif jobPermission.DomainType == models.DomainTypeSelf { // :762, per-row owner filter\n```\nso `self_ug` falls through every check like `all`: a `submit_user` filter\nnaming any user (or `all`) is honoured and no row is removed.\n`GetRequireResourcePermission()` (`controllers/rbac_check.go:323-346`)\nreturns only the first grant matching the permission, so for the default\n`basic_user` (`config/rbac.yaml:37-40`, `job:view` with `self` then\n`self_ug`) the `self` entry wins and `self_ug` is silently ignored.\n`self_ug` is accepted by role validation (`controllers/roles.go:697-700`)\nand listed as a supported job scope (`models/roles.go:70`), but nothing\nresolves the user's user groups for it.\n\n**Expected:** a `self_ug` job grant covers exactly the jobs whose owner shares\na user group with the requester; a grant list with several scopes covers the\nunion of them; anything else is rejected or filtered out.\n**Impact:** security (authorization bypass). Any role an admin creates with\n`job:view` (or another job permission, unchecked) scoped to `self_ug` grants\nvisibility of all users' jobs. The default `basic_user` is not exposed (its\n`self` grant is matched first) but never gets the group visibility its\nconfig promises. Other job permissions using the same single-grant lookup\nwere not checked yet.\n**Found by:** review, during `/kiro-spec-requirements project-config`\n(2026-10-10, RBAC survey for project-scoped job permissions).\nUPDATE (2026-10-10): confirmed by the new tests in\n`src/frontend/apiserver/controllers/job_scope_test.go` against the unfixed\nhandlers: with only `self_ug` grants, alice listed carol's job\n(`submit_user carol: rows [..907], want []`), read it by ID (200), and\nkilled it (`Job control accepted`, dispatched). `job:control` is\naffected as well as `job:view`. A user holding `self` and `self_ug` together\ngot `self` only (the second scope was dropped).", "file": "src/frontend/apiserver/controllers/jobs.go, src/frontend/apiserver/controllers/rbac_check.go, src/frontend/apiserver/models/roles.go, src/frontend/apiserver/config/rbac.yaml", - "root_cause": "TODO: required for fixed/partial", - "fix": "TODO: required for fixed/partial\n\nVerification:\n- Test: TODO: name + file; RED before the fix, GREEN after\n- Suites: TODO: suite results and build profile\n- Live: TODO: soak/deploy run, or n/a and why", + "root_cause": "Three defects in `src/frontend/apiserver`:\n- Every job-permission check read one scope: `GetRequireResourcePermission()`\n (`controllers/rbac_check.go`) returns the first grant of the permission,\n and `DeduplicatePermissions()` keeps one entry per permission name, so a\n `self` + `self_ug` grant became one scope.\n- The checks compared that scope only with `self`\n (`controllers/jobs.go` job list ~178/187, by-ID ~762, both control paths\n ~2190 and ~2457; `output.go` compared with `all`), so any other scope,\n `self_ug` included, fell through as unrestricted in the list, by-ID and\n control paths.\n- The apiserver had no user-group data: `handleConfigPush()`\n (`backend/clib.go`) ignored `cluster_config_t.ugroups`, so `self_ug` could\n not have been resolved anyway.\nAlso in the by-ID path: rows were removed from the slice being ranged over,\nwhich skipped the row after each removal.\nSince: pre-existing; `self_ug` was added as a job scope without enforcement\n(origin not bisected).", + "fix": "- `controllers/job_scope.go` (new): `userJobScope()` collects every grant\n of a job permission before deduplication (`collectUserPermissionsWithType()`,\n split out of `GetUserAllPermissionsWithType()` in `rbac_check.go`) and adds\n them up: `all` covers every job, `self` the user's own, `self_ug` the user\n and everyone sharing a user group with them; other scope types cover\n nothing. `covers(owner)` and `ownerFilter()` (a `submit_user in/notin`\n condition for job queries) apply it.\n- `controllers/jobs.go`: the job list appends the owner filter for narrower\n scopes (a `submit_user` the scope does not cover now yields no rows); the\n by-ID path keeps covered rows in a new slice and answers 403 (was 500)\n when nothing is visible; both control paths reject an uncovered job with\n 403. `controllers/output.go`: the owner may always read; others need a\n covering `job:view` grant.\n- `store/usergroups.go` (new) holds the user groups and resolves membership\n (users, nested groups, `all`, exclusions, cycle-safe);\n `models/usergroups.go` (new) the model; `backend`: `cClusterConfUserGroups()`\n converts `cluster_config_t.ugroups`, and `handleConfigPush()` calls the new\n `NotificationHandler.OnUserGroupsUpdated()`, which replaces the store.\n- Behaviour change: the default `basic_user` (`job:view` with `self` and\n `self_ug`) now also sees the jobs of users sharing a user group with them,\n as its configuration says; before, it saw only its own.\n\nVerification:\n- Test: `src/frontend/apiserver/controllers/job_scope_test.go`:\n `TestJobScope_SelfUGListCoversGroupOnly`, `TestJobScope_SelfUGGetByID`,\n `TestJobScope_SelfUGControl`, `TestJobScope_SelfUGOutput`,\n `TestJobScope_GrantsAddUp`: all RED before the fix (carol's job listed, read,\n killed; group mates' output refused; self+self_ug collapsed to self), GREEN\n after. `store/usergroups_test.go` `TestUserGroups_Membership` and\n `backend/handler_test.go` `TestOnUserGroupsUpdated_ReplacesUserGroups`\n (new code; RED as undefined symbols before).\n- Suites: `xce-build.sh apiserver asan xce.build --incremental`: apiserver Go\n packages (store, utils, controllers, backend, backend/auth) and all xcecli\n packages ok; cgo `xce_apiserver` built; meson `apiserver` suite 1/1 OK;\n 2026-10-10.\n- Live: n/a: handler-level tests drive the real gin handlers, RBAC loading\n and stores; the only untested seam is the cgo conversion of\n `cluster_config_t.ugroups`, which compiled into `xce_apiserver`; not run\n against a live controller.", "tags": [ "apiserver", "rbac", @@ -17787,17 +17787,19 @@ "timestamp": "2026-10-10T04:31:27Z", "last_seen": "2026-10-10T04:31:27Z", "occurrences": 1, - "error_message": "xcecli jobs list -P fails: apiserver rejects project_name filter (unsupported filter)\n\n**Observed:** `xcecli jobs list -P ` (`-P/--project`,\n`src/frontend/xcecli/cmd/jobs/list.go:356`) sends the filter\n`{project_name:eq:}` (`xcecli/filters/job_filter.go:108-109`), but\n`project_name` is commented out of the apiserver's `JOB_SUPPORT_FILTERS`\n(`src/frontend/apiserver/models/jobs.go:178`), so `validateJobQueryFilters()`\n(`controllers/jobs.go:470-483`) rejects the request:\n```\nunsupported filter: project_name\n```\n(HTTP 400, INVALID_PARAMS). The same holds for `license_project`\n(`models/jobs.go:179`, `job_filter.go:112-113`). Found by source reading; not\nyet run against a live apiserver.\n\n**Expected:** `xcecli jobs list -P eng` lists the caller's visible jobs whose\nproject is `eng`.\n**Impact:** wrong result: the documented `-P` (and license-project) filter of\n`xcecli jobs list` always fails with an error; users cannot list jobs by\nproject. No data is affected.\n**Found by:** review, during `/kiro-spec-requirements project-config`\n(2026-10-10); `project-config` Requirement 7.5 needs this filter.", + "error_message": "xcecli jobs list -P fails: apiserver rejects project_name filter (unsupported filter)\n\n**Observed:** `xcecli jobs list -P ` (`-P/--project`,\n`src/frontend/xcecli/cmd/jobs/list.go:356`) sends the filter\n`{project_name:eq:}` (`xcecli/filters/job_filter.go:108-109`), but\n`project_name` is commented out of the apiserver's `JOB_SUPPORT_FILTERS`\n(`src/frontend/apiserver/models/jobs.go:178`), so `validateJobQueryFilters()`\n(`controllers/jobs.go:470-483`) rejects the request:\n```\nunsupported filter: project_name\n```\n(HTTP 400, INVALID_PARAMS). The same holds for `license_project`\n(`models/jobs.go:179`, `job_filter.go:112-113`). Found by source reading; not\nyet run against a live apiserver.\n\n**Expected:** `xcecli jobs list -P eng` lists the caller's visible jobs whose\nproject is `eng`.\n**Impact:** wrong result: the documented `-P` (and license-project) filter of\n`xcecli jobs list` always fails with an error; users cannot list jobs by\nproject. No data is affected.\n**Found by:** review, during `/kiro-spec-requirements project-config`\n(2026-10-10); `project-config` Requirement 7.5 needs this filter.\nUPDATE (2026-10-10): confirmed by `TestJobHandler_ProjectFilters`\n(`src/frontend/apiserver/controllers/job_scope_test.go`) before the fix:\n`status = 400 ... unsupported filter: project_name`.", "file": "src/frontend/apiserver/models/jobs.go, src/frontend/apiserver/controllers/jobs.go, src/frontend/xcecli/filters/job_filter.go", - "root_cause": "TODO: required for fixed/partial", - "fix": "TODO: required for fixed/partial\n\nVerification:\n- Test: TODO: name + file; RED before the fix, GREEN after\n- Suites: TODO: suite results and build profile\n- Live: TODO: soak/deploy run, or n/a and why", + "root_cause": "`JOB_SUPPORT_FILTERS` (`src/frontend/apiserver/models/jobs.go`) had\n`project_name` and `license_project` commented out, and\n`validateJobQueryFilters()` (`controllers/jobs.go`) rejects every field not on\nthat list, while `xcecli` (`filters/job_filter.go:108-113`) sends exactly\nthose fields for `-P` and the license-project option. The job store already\ncarries both fields (`JobInfo.ProjectName`, `LicenseProject`). Since:\npre-existing, origin unknown.", + "fix": "`models/jobs.go`: `project_name` and `license_project` are now in\n`JOB_SUPPORT_FILTERS`. No other change was needed: the store query filters\nby the JSON field name. `ARRAY_JOB_SUPPORT_FILTERS` (array statistics) is\nunchanged.\n\nVerification:\n- Test: `TestJobHandler_ProjectFilters`\n (`src/frontend/apiserver/controllers/job_scope_test.go`): RED before (400\n unsupported filter), GREEN after (`{project_name:eq:eng}` and\n `{license_project:eq:lic2}` select only the matching job).\n- Suites: same run as bug-1934: apiserver and xcecli Go tests ok, meson\n `apiserver` suite 1/1 OK, asan, 2026-10-10.\n- Live: n/a: the change is a filter whitelist entry exercised through the\n real handler and store in the test.", "tags": [ "apiserver", "xcecli", "filter", "project" ], - "related_bugs": [], + "related_bugs": [ + "bug-1934" + ], "fix_commits": [], "record": ".wolf/bugs/bug-1935.md" } diff --git a/.wolf/bugs/bug-1934.md b/.wolf/bugs/bug-1934.md index 7573375d..10bfb1af 100644 --- a/.wolf/bugs/bug-1934.md +++ b/.wolf/bugs/bug-1934.md @@ -50,6 +50,13 @@ config promises. Other job permissions using the same single-grant lookup were not checked yet. **Found by:** review, during `/kiro-spec-requirements project-config` (2026-10-10, RBAC survey for project-scoped job permissions). +UPDATE (2026-10-10): confirmed by the new tests in +`src/frontend/apiserver/controllers/job_scope_test.go` against the unfixed +handlers: with only `self_ug` grants, alice listed carol's job +(`submit_user carol: rows [..907], want []`), read it by ID (200), and +killed it (`Job control accepted`, dispatched). `job:control` is +affected as well as `job:view`. A user holding `self` and `self_ug` together +got `self` only (the second scope was dropped). ## Environment - OS: no /etc/os-release; Docker container; NixOS kernel; kernel Linux 6.18.33 x86_64 @@ -71,15 +78,64 @@ were not checked yet. - Evidence: the code lines quoted in Description; no logs. ## Root cause -TODO: required for fixed/partial - +Three defects in `src/frontend/apiserver`: +- Every job-permission check read one scope: `GetRequireResourcePermission()` + (`controllers/rbac_check.go`) returns the first grant of the permission, + and `DeduplicatePermissions()` keeps one entry per permission name, so a + `self` + `self_ug` grant became one scope. +- The checks compared that scope only with `self` + (`controllers/jobs.go` job list ~178/187, by-ID ~762, both control paths + ~2190 and ~2457; `output.go` compared with `all`), so any other scope, + `self_ug` included, fell through as unrestricted in the list, by-ID and + control paths. +- The apiserver had no user-group data: `handleConfigPush()` + (`backend/clib.go`) ignored `cluster_config_t.ugroups`, so `self_ug` could + not have been resolved anyway. +Also in the by-ID path: rows were removed from the slice being ranged over, +which skipped the row after each removal. +Since: pre-existing; `self_ug` was added as a job scope without enforcement +(origin not bisected). ## Fix -TODO: required for fixed/partial - +- `controllers/job_scope.go` (new): `userJobScope()` collects every grant + of a job permission before deduplication (`collectUserPermissionsWithType()`, + split out of `GetUserAllPermissionsWithType()` in `rbac_check.go`) and adds + them up: `all` covers every job, `self` the user's own, `self_ug` the user + and everyone sharing a user group with them; other scope types cover + nothing. `covers(owner)` and `ownerFilter()` (a `submit_user in/notin` + condition for job queries) apply it. +- `controllers/jobs.go`: the job list appends the owner filter for narrower + scopes (a `submit_user` the scope does not cover now yields no rows); the + by-ID path keeps covered rows in a new slice and answers 403 (was 500) + when nothing is visible; both control paths reject an uncovered job with + 403. `controllers/output.go`: the owner may always read; others need a + covering `job:view` grant. +- `store/usergroups.go` (new) holds the user groups and resolves membership + (users, nested groups, `all`, exclusions, cycle-safe); + `models/usergroups.go` (new) the model; `backend`: `cClusterConfUserGroups()` + converts `cluster_config_t.ugroups`, and `handleConfigPush()` calls the new + `NotificationHandler.OnUserGroupsUpdated()`, which replaces the store. +- Behaviour change: the default `basic_user` (`job:view` with `self` and + `self_ug`) now also sees the jobs of users sharing a user group with them, + as its configuration says; before, it saw only its own. ## Verification -- Test: TODO: name + file; RED before the fix, GREEN after -- Suites: TODO: suite results and build profile -- Live: TODO: soak/deploy run, or n/a and why +- Test: `src/frontend/apiserver/controllers/job_scope_test.go`: + `TestJobScope_SelfUGListCoversGroupOnly`, `TestJobScope_SelfUGGetByID`, + `TestJobScope_SelfUGControl`, `TestJobScope_SelfUGOutput`, + `TestJobScope_GrantsAddUp`: all RED before the fix (carol's job listed, read, + killed; group mates' output refused; self+self_ug collapsed to self), GREEN + after. `store/usergroups_test.go` `TestUserGroups_Membership` and + `backend/handler_test.go` `TestOnUserGroupsUpdated_ReplacesUserGroups` + (new code; RED as undefined symbols before). +- Suites: `xce-build.sh apiserver asan xce.build --incremental`: apiserver Go + packages (store, utils, controllers, backend, backend/auth) and all xcecli + packages ok; cgo `xce_apiserver` built; meson `apiserver` suite 1/1 OK; + 2026-10-10. +- Live: n/a: handler-level tests drive the real gin handlers, RBAC loading + and stores; the only untested seam is the cgo conversion of + `cluster_config_t.ugroups`, which compiled into `xce_apiserver`; not run + against a live controller. ## References -- Related: none +- Related: bug-1935 (same session, same files) +- Spec: `.kiro/specs/project-config/requirements.md` Requirement 8 (project + job scope builds on `userJobScope()`) diff --git a/.wolf/bugs/bug-1935.md b/.wolf/bugs/bug-1935.md index ed61f40d..072228ca 100644 --- a/.wolf/bugs/bug-1935.md +++ b/.wolf/bugs/bug-1935.md @@ -10,7 +10,7 @@ last_seen: "2026-10-10T04:31:27Z" occurrences: 1 files: ["src/frontend/apiserver/models/jobs.go", "src/frontend/apiserver/controllers/jobs.go", "src/frontend/xcecli/filters/job_filter.go"] tags: ["apiserver", "xcecli", "filter", "project"] -related: [] +related: ["bug-1934"] --- # bug-1935: xcecli jobs list -P fails: apiserver rejects project_name filter (unsupported filter) @@ -36,6 +36,9 @@ project is `eng`. project. No data is affected. **Found by:** review, during `/kiro-spec-requirements project-config` (2026-10-10); `project-config` Requirement 7.5 needs this filter. +UPDATE (2026-10-10): confirmed by `TestJobHandler_ProjectFilters` +(`src/frontend/apiserver/controllers/job_scope_test.go`) before the fix: +`status = 400 ... unsupported filter: project_name`. ## Environment - OS: no /etc/os-release; Docker container; NixOS kernel; kernel Linux 6.18.33 x86_64 @@ -55,15 +58,28 @@ project. No data is affected. - Evidence: the code lines quoted in Description; no logs. ## Root cause -TODO: required for fixed/partial - +`JOB_SUPPORT_FILTERS` (`src/frontend/apiserver/models/jobs.go`) had +`project_name` and `license_project` commented out, and +`validateJobQueryFilters()` (`controllers/jobs.go`) rejects every field not on +that list, while `xcecli` (`filters/job_filter.go:108-113`) sends exactly +those fields for `-P` and the license-project option. The job store already +carries both fields (`JobInfo.ProjectName`, `LicenseProject`). Since: +pre-existing, origin unknown. ## Fix -TODO: required for fixed/partial - +`models/jobs.go`: `project_name` and `license_project` are now in +`JOB_SUPPORT_FILTERS`. No other change was needed: the store query filters +by the JSON field name. `ARRAY_JOB_SUPPORT_FILTERS` (array statistics) is +unchanged. ## Verification -- Test: TODO: name + file; RED before the fix, GREEN after -- Suites: TODO: suite results and build profile -- Live: TODO: soak/deploy run, or n/a and why +- Test: `TestJobHandler_ProjectFilters` + (`src/frontend/apiserver/controllers/job_scope_test.go`): RED before (400 + unsupported filter), GREEN after (`{project_name:eq:eng}` and + `{license_project:eq:lic2}` select only the matching job). +- Suites: same run as bug-1934: apiserver and xcecli Go tests ok, meson + `apiserver` suite 1/1 OK, asan, 2026-10-10. +- Live: n/a: the change is a filter whitelist entry exercised through the + real handler and store in the test. ## References -- Related: none +- Related: bug-1934 +- Spec: `.kiro/specs/project-config/requirements.md` Requirement 7.5 diff --git a/src/frontend/apiserver/backend/clib.go b/src/frontend/apiserver/backend/clib.go index 50b1265b..9f5040dc 100644 --- a/src/frontend/apiserver/backend/clib.go +++ b/src/frontend/apiserver/backend/clib.go @@ -520,6 +520,7 @@ func handleConfigPush(cfgdata *C.xce_configdata_t) C.int { cClusterConfHostConfigs(conf), cluster, ) + gHandler.OnUserGroupsUpdated(cClusterConfUserGroups(conf)) return 0 } diff --git a/src/frontend/apiserver/backend/convert.go b/src/frontend/apiserver/backend/convert.go index 9862c85a..3782d16e 100644 --- a/src/frontend/apiserver/backend/convert.go +++ b/src/frontend/apiserver/backend/convert.go @@ -250,6 +250,42 @@ func cHostConfigToModel(hc *C.xce_hostconfig_t) *models.HostInfo { }) } +// cStrList converts an xce_strlist_t to a Go slice. +func cStrList(l *C.xce_strlist_t) []string { + n := int(l.count) + if n == 0 || l.data == nil { + return nil + } + out := make([]string, 0, n) + for _, p := range unsafe.Slice(l.data, n) { + if p != nil { + out = append(out, C.GoString(p)) + } + } + return out +} + +// cClusterConfUserGroups extracts the user groups from a cluster_config_t +// (bug-1934: the self_ug job scope resolves them). +func cClusterConfUserGroups(conf *C.cluster_config_t) []models.UserGroupInfo { + n := int(conf.nugroups) + if n == 0 || conf.ugroups == nil { + return nil + } + out := make([]models.UserGroupInfo, 0, n) + for _, gp := range unsafe.Slice(conf.ugroups, n) { + if gp == nil || gp.name == nil { + continue + } + out = append(out, models.UserGroupInfo{ + Name: C.GoString(gp.name), + Members: cStrList(&gp.members), + ExMembers: cStrList(&gp.ex_members), + }) + } + return out +} + // cClusterConfHostConfigs extracts host config models from a cluster_config_t. func cClusterConfHostConfigs(conf *C.cluster_config_t) []models.HostInfo { n := int(conf.nhconfigs) diff --git a/src/frontend/apiserver/backend/handler.go b/src/frontend/apiserver/backend/handler.go index 8b7577b8..53f2a2d6 100644 --- a/src/frontend/apiserver/backend/handler.go +++ b/src/frontend/apiserver/backend/handler.go @@ -192,6 +192,11 @@ func (h *DefaultHandler) OnClusterUpdated(incoming *models.ClusterInfo) { SetHeartbeatInterval(incoming.HeartbeatInterval) } +// OnUserGroupsUpdated replaces the stored user groups as a whole. +func (h *DefaultHandler) OnUserGroupsUpdated(groups []models.UserGroupInfo) { + store.SetUserGroups(groups) +} + func (h *DefaultHandler) OnResourceUpdated(res *models.ResourceDefinitionInfo) { store.Resources.Put(res.ResourceName, *res) } diff --git a/src/frontend/apiserver/backend/handler_test.go b/src/frontend/apiserver/backend/handler_test.go index de636691..cfd5713b 100644 --- a/src/frontend/apiserver/backend/handler_test.go +++ b/src/frontend/apiserver/backend/handler_test.go @@ -672,3 +672,20 @@ func TestDispatchJobArray_LogsArrayIdentity(t *testing.T) { t.Errorf("log still carries the raw base_jobid: %v", fields) } } + +// bug-1934: a configuration push replaces the user groups the self_ug job +// scope reads. +func TestOnUserGroupsUpdated_ReplacesUserGroups(t *testing.T) { + orig := store.UserGroupsSnapshot() + t.Cleanup(func() { store.SetUserGroups(orig) }) + h := &DefaultHandler{} + + h.OnUserGroupsUpdated([]models.UserGroupInfo{{Name: "team", Members: []string{"alice", "bob"}}}) + if !store.UsersShareGroup("alice", "bob") { + t.Fatal("alice and bob should share team after the push") + } + h.OnUserGroupsUpdated(nil) + if store.UsersShareGroup("alice", "bob") { + t.Error("a push without user groups should remove team") + } +} diff --git a/src/frontend/apiserver/backend/notification.go b/src/frontend/apiserver/backend/notification.go index f26bc6ff..dc82dc84 100644 --- a/src/frontend/apiserver/backend/notification.go +++ b/src/frontend/apiserver/backend/notification.go @@ -18,4 +18,7 @@ type NotificationHandler interface { OnClusterUpdated(cluster *models.ClusterInfo) OnResourceUpdated(res *models.ResourceDefinitionInfo) OnResourceLocationUpdated(res *models.ResourceDistributionInfo) + // OnUserGroupsUpdated replaces the user groups (self_ug job scope, + // bug-1934) with those of a configuration push. + OnUserGroupsUpdated(groups []models.UserGroupInfo) } diff --git a/src/frontend/apiserver/controllers/job_scope.go b/src/frontend/apiserver/controllers/job_scope.go new file mode 100644 index 00000000..47c69bc7 --- /dev/null +++ b/src/frontend/apiserver/controllers/job_scope.go @@ -0,0 +1,77 @@ +package controllers + +import ( + "errors" + "sort" + "strings" + + "apiserver/models" + "apiserver/store" + "apiserver/utils" +) + +// jobScope is what all of a user's grants of one job permission cover +// (bug-1934): every job (scope all), or the jobs of a set of owners (scope +// self: the user; scope self_ug: the user and everyone sharing a user group +// with them). Grants add up; a scope type the job object does not support +// covers nothing. +type jobScope struct { + all bool + owners store.UserSet +} + +// userJobScope collects every grant of perm the user holds. It reads the +// grants before DeduplicatePermissions, which keeps one scope per +// permission and would drop the others. +func userJobScope(user, perm string) (jobScope, error) { + s := jobScope{owners: store.NewUserSet()} + found := false + for _, g := range collectUserPermissionsWithType(user) { + if g.Permission != perm { + continue + } + found = true + switch strings.ToLower(g.DomainType) { + case models.DomainTypeAll: + s.all = true + case models.DomainTypeSelf: + s.owners = store.UnionUserSets(s.owners, store.NewUserSet(user)) + case models.DomainTypeSelfUG: + s.owners = store.UnionUserSets(s.owners, store.UserGroupPeers(user)) + } + } + if !found { + return s, errors.New("no permission") + } + return s, nil +} + +// covers reports whether the scope covers a job owned by owner. +func (s jobScope) covers(owner string) bool { + return s.all || s.owners.Has(owner) +} + +// ownerFilter returns the job-query condition that keeps only covered +// owners, or ok == false when the scope covers no one. +func (s jobScope) ownerFilter() (f utils.FilterCondition, needed, ok bool) { + if s.all { + return f, false, true + } + names := make([]string, 0, len(s.owners.Names)) + for n := range s.owners.Names { + names = append(names, n) + } + sort.Strings(names) + if s.owners.All { + if len(names) == 0 { + return f, false, true + } + return utils.FilterCondition{Field: "submit_user", Operator: "notin", + Value: "(" + strings.Join(names, ",") + ")"}, true, true + } + if len(names) == 0 { + return f, false, false + } + return utils.FilterCondition{Field: "submit_user", Operator: "in", + Value: "(" + strings.Join(names, ",") + ")"}, true, true +} diff --git a/src/frontend/apiserver/controllers/job_scope_test.go b/src/frontend/apiserver/controllers/job_scope_test.go new file mode 100644 index 00000000..3e7dc29d --- /dev/null +++ b/src/frontend/apiserver/controllers/job_scope_test.go @@ -0,0 +1,179 @@ +package controllers + +import ( + "net/http" + "testing" + + "apiserver/models" + "apiserver/store" +) + +// jobScopeTestRBAC: alice's job permissions reach only her user groups' +// jobs (self_ug); dave holds self and self_ug together (bug-1934). +const jobScopeTestRBAC = ` +permissions: + - job:view + - job:control + - queue:submit + - queue:control + - queue:view +roles: + group_member: + permissions: + - action: job:view + domain: + - type: self_ug + - action: job:control + domain: + - type: self_ug + - action: queue:control + domain: + - type: all + - action: queue:view + domain: + - type: all + self_and_group: + permissions: + - action: job:view + domain: + - type: self + - type: self_ug + - action: job:control + domain: + - type: self + - type: self_ug + - action: queue:control + domain: + - type: all + - action: queue:view + domain: + - type: all +users: + alice: + roles: + - name: group_member + domain: + - type: all + dave: + roles: + - name: self_and_group + domain: + - type: all +` + +// jobScopeJobs are one RUN job per owner; alice and bob share user group +// "team", carol and dave are in no group with alice, dave and erin share +// "pair". +type jobScopeJobs struct{ alice, bob, carol, dave, erin uint64 } + +func seedJobScope(f *handlerFixture) jobScopeJobs { + f.t.Helper() + store.SetUserGroups([]models.UserGroupInfo{ + {Name: "team", Members: []string{"alice", "bob"}}, + {Name: "pair", Members: []string{"dave", "erin"}}, + }) + base := uint64(3)<<40 | uint64(1)<<20 + j := jobScopeJobs{alice: base + 1, bob: base + 2, carol: base + 3, dave: base + 4, erin: base + 5} + for id, owner := range map[uint64]string{j.alice: "alice", j.bob: "bob", + j.carol: "carol", j.dave: "dave", j.erin: "erin"} { + f.seedJob(models.JobInfo{Unique_JobID: int64(id), JobID: int64(id), + JobName: owner + "_job", JobStatus: models.JOBSTAT["RUN"], SubmitUser: owner}) + } + return j +} + +// A self_ug grant lists the jobs of the requester's user groups and no +// one else's, whatever submit_user the request names. +func TestJobScope_SelfUGListCoversGroupOnly(t *testing.T) { + f := newHandlerFixtureAs(t, jobScopeTestRBAC, "alice", "group_member") + j := seedJobScope(f) + + assertIDs(t, "submit_user all", rowIDs(t, f.listJobs("{submit_user:eq:all}")), + []uint64{j.alice, j.bob}) + assertIDs(t, "submit_user bob (group mate)", rowIDs(t, f.listJobs("{submit_user:eq:bob}")), + []uint64{j.bob}) + assertIDs(t, "submit_user carol (outside the group)", + rowIDs(t, f.listJobs("{submit_user:eq:carol}")), nil) + assertIDs(t, "no submit_user: own jobs", rowIDs(t, f.listJobs("{job_status:eq:RUN}")), + []uint64{j.alice}) +} + +// GET /jobs/{ids} returns only covered jobs. +func TestJobScope_SelfUGGetByID(t *testing.T) { + f := newHandlerFixtureAs(t, jobScopeTestRBAC, "alice", "group_member") + j := seedJobScope(f) + + if w := f.getJobs(jobIDString(j.carol)); w.Code != http.StatusForbidden { + t.Errorf("carol's job: status = %d, want 403; body: %s", w.Code, w.Body.String()) + } + assertIDs(t, "bob's job", rowIDs(t, f.getJobs(jobIDString(j.bob))), []uint64{j.bob}) + assertIDs(t, "bob's and carol's jobs", + rowIDs(t, f.getJobs(jobIDString(j.bob)+","+jobIDString(j.carol))), []uint64{j.bob}) +} + +// Job output follows the same scope: no group, no output. +func TestJobScope_SelfUGOutput(t *testing.T) { + f := newHandlerFixtureAs(t, jobScopeTestRBAC, "alice", "group_member") + j := seedJobScope(f) + + if w := f.do(http.MethodGet, "/xce/v1/jobs/"+jobIDString(j.carol)+"/output", nil); w.Code != http.StatusForbidden { + t.Errorf("carol's output: status = %d, want 403; body: %s", w.Code, w.Body.String()) + } + if w := f.do(http.MethodGet, "/xce/v1/jobs/"+jobIDString(j.bob)+"/output", nil); w.Code == http.StatusForbidden { + t.Errorf("bob's output: status = 403, want access; body: %s", w.Body.String()) + } +} + +// job:control with self_ug controls group mates' jobs only. +func TestJobScope_SelfUGControl(t *testing.T) { + f := newHandlerFixtureAs(t, jobScopeTestRBAC, "alice", "group_member") + j := seedJobScope(f) + + if w := f.controlByID(jobIDString(j.carol), "kill"); w.Code != http.StatusForbidden { + t.Errorf("kill carol's job: status = %d, want 403; body: %s", w.Code, w.Body.String()) + } + if got := f.dispatchedIDs(); len(got) != 0 { + t.Fatalf("carol's job was dispatched: %v", got) + } + if w := f.controlByID(jobIDString(j.bob), "kill"); w.Code != http.StatusOK { + t.Fatalf("kill bob's job: status = %d, want 200; body: %s", w.Code, w.Body.String()) + } + if got := f.dispatchedIDs(); len(got) != 1 || got[0] != j.bob { + t.Errorf("dispatched = %v, want [%d]", got, j.bob) + } + + if w := f.controlByFilter(models.JobControlRequest{Action: "kill", + Filter: models.JobControlFilter{Users: "carol"}}); w.Code != http.StatusForbidden { + t.Errorf("filter kill of carol's jobs: status = %d, want 403; body: %s", w.Code, w.Body.String()) + } + if got := f.dispatchedIDs(); len(got) != 1 { + t.Errorf("filter control dispatched carol's job: %v", got) + } +} + +// Several scopes of one permission add up: self and self_ug together cover +// the requester's own jobs and the group's. +func TestJobScope_GrantsAddUp(t *testing.T) { + f := newHandlerFixtureAs(t, jobScopeTestRBAC, "dave", "self_and_group") + j := seedJobScope(f) + + assertIDs(t, "submit_user all", rowIDs(t, f.listJobs("{submit_user:eq:all}")), + []uint64{j.dave, j.erin}) + assertIDs(t, "erin's job by id", rowIDs(t, f.getJobs(jobIDString(j.erin))), []uint64{j.erin}) +} + +// bug-1935: the project and license-project filters xcecli sends are +// accepted and select by the job's project. +func TestJobHandler_ProjectFilters(t *testing.T) { + f := newHandlerFixture(t) + base := uint64(4)<<40 | uint64(1)<<20 + f.seedJob(models.JobInfo{Unique_JobID: int64(base + 1), JobID: int64(base + 1), + JobStatus: models.JOBSTAT["RUN"], ProjectName: "eng", LicenseProject: "lic1"}) + f.seedJob(models.JobInfo{Unique_JobID: int64(base + 2), JobID: int64(base + 2), + JobStatus: models.JOBSTAT["RUN"], ProjectName: "ops", LicenseProject: "lic2"}) + + assertIDs(t, "project_name eng", rowIDs(t, f.listJobs("{project_name:eq:eng}")), + []uint64{base + 1}) + assertIDs(t, "license_project lic2", rowIDs(t, f.listJobs("{license_project:eq:lic2}")), + []uint64{base + 2}) +} diff --git a/src/frontend/apiserver/controllers/jobs.go b/src/frontend/apiserver/controllers/jobs.go index 05e6b06a..3f65f67f 100644 --- a/src/frontend/apiserver/controllers/jobs.go +++ b/src/frontend/apiserver/controllers/jobs.go @@ -157,13 +157,18 @@ func (j JobController) GetJobsInfo(ctx *gin.Context) { } logger.Debug("get job info accessQueues", zap.Any("accessQueues", accessQueues), zap.Any("permission", permission)) - // check job:view permission - permission, err = GetRequireResourcePermission(submitUser, "job:view") - logger.Debug("get job info for job:view permission:", zap.Any("permission", permission)) + // check job:view permission: every grant counts (bug-1934) + scope, err := userJobScope(submitUser, "job:view") + logger.Debug("get job info for job:view scope:", zap.Any("scope", scope)) if err != nil || len(accessQueues) == 0 { ReturnError(ctx, common.ERROR_CODE, common.ERROR_ACCESS_FORBIDDEN_MSG, nil) return } + ownerFilter, needOwnerFilter, anyOwner := scope.ownerFilter() + if !anyOwner { + ReturnError(ctx, common.ERROR_CODE, common.ERROR_ACCESS_FORBIDDEN_MSG, nil) + return + } // if submit_user is not specified, use query request submission user var specify_user bool = false @@ -175,16 +180,17 @@ func (j JobController) GetJobsInfo(ctx *gin.Context) { if filter.Value == models.DomainTypeAll { delindex = i } - if permission.DomainType != models.DomainTypeSelf { - submitUser = filter.Value - } break } } if delindex != -1 { filters = append(filters[:delindex], filters[delindex+1:]...) } - if !specify_user || permission.DomainType == models.DomainTypeSelf { + // a narrower scope keeps only the jobs of the owners it covers + if needOwnerFilter { + filters = append(filters, ownerFilter) + } + if !specify_user { logger.Debug("submit_user is not specified in query filter, add submit_user into filter: filter=[{submit_user:eq:" + submitUser) filters = append(filters, utils.FilterCondition{ Field: "submit_user", @@ -743,27 +749,29 @@ func (j JobController) GetJobByIDs(ctx *gin.Context) { } // end for jobid logger.Debug("the specific jobid query reply after merge:", zap.Any("jqueryReply", mergeJqueryReply)) - // check rbac + // check rbac: every job:view grant counts (bug-1934); rows are kept in + // a new slice, since removing them from the one being ranged over + // skipped the row after each removal removeFlag := false - for i, row := range mergeJqueryReply { - jobPermission, err := GetRequireResourcePermission(logonUser, "job:view") - if err != nil { - logger.Error(err.Error()) - ReturnError(ctx, common.ERROR_AUTH_CODE, common.ERROR_ACCESS_FORBIDDEN_MSG, nil) - return - } - queuePermission, err := GetRequireResourcePermission(logonUser, "queue:view") - if err != nil { - logger.Error(err.Error()) - ReturnError(ctx, common.ERROR_AUTH_CODE, common.ERROR_ACCESS_FORBIDDEN_MSG, nil) - return - } + jobScope, err := userJobScope(logonUser, "job:view") + if err != nil { + logger.Error(err.Error()) + ReturnError(ctx, common.ERROR_AUTH_CODE, common.ERROR_ACCESS_FORBIDDEN_MSG, nil) + return + } + queuePermission, err := GetRequireResourcePermission(logonUser, "queue:view") + if err != nil { + logger.Error(err.Error()) + ReturnError(ctx, common.ERROR_AUTH_CODE, common.ERROR_ACCESS_FORBIDDEN_MSG, nil) + return + } + visible := mergeJqueryReply[:0:0] + for _, row := range mergeJqueryReply { if job_owner, exist := row.Data["submit_user"]; exist { - if jobPermission.DomainType == models.DomainTypeSelf { - if logonUser != job_owner.(string) { - removeFlag = true - mergeJqueryReply = append(mergeJqueryReply[:i], mergeJqueryReply[i+1:]...) - } + owner, _ := job_owner.(string) + if !jobScope.covers(owner) { + removeFlag = true + continue } } if typeValue != "array_stats" { @@ -773,15 +781,16 @@ func (j JobController) GetJobByIDs(ctx *gin.Context) { permissionQueueValues := queuePermission.DomainValues if !utils.ContainsField(permissionQueueValues, queueName) { removeFlag = true - mergeJqueryReply = append(mergeJqueryReply[:i], mergeJqueryReply[i+1:]...) + continue } } } } - + visible = append(visible, row) } + mergeJqueryReply = visible if removeFlag && len(mergeJqueryReply) == 0 { - ReturnError(ctx, common.ERROR_CODE, common.ERROR_ACCESS_FORBIDDEN_MSG, nil) + ReturnError(ctx, common.ERROR_AUTH_CODE, common.ERROR_ACCESS_FORBIDDEN_MSG, nil) return } @@ -2181,18 +2190,17 @@ func (j JobController) ControlJobByID(ctx *gin.Context) { // check rbac if job_owner, exist := jobInfo.Data["submit_user"]; exist { - jobPermission, err := GetRequireResourcePermission(submitter, "job:control") + jobScope, err := userJobScope(submitter, "job:control") if err != nil { logger.Error(err.Error()) ReturnError(ctx, common.ERROR_AUTH_CODE, common.ERROR_ACCESS_FORBIDDEN_MSG, nil) return } - if jobPermission.DomainType == models.DomainTypeSelf { - if job_owner.(string) != submitter { - logger.Error("Permission denied", zap.String("perm", "job:control"), zap.String("logonUser", submitter)) - ReturnError(ctx, common.ERROR_AUTH_CODE, common.ERROR_ACCESS_FORBIDDEN_MSG, nil) - return - } + // bug-1934: every job:control grant counts + if owner, _ := job_owner.(string); !jobScope.covers(owner) { + logger.Error("Permission denied", zap.String("perm", "job:control"), zap.String("logonUser", submitter)) + ReturnError(ctx, common.ERROR_AUTH_CODE, common.ERROR_ACCESS_FORBIDDEN_MSG, nil) + return } } @@ -2448,18 +2456,17 @@ func (j JobController) ControlJob(ctx *gin.Context) { // check rbac begin if job_owner, exist := job.Data["submit_user"]; exist { - jobPermission, err := GetRequireResourcePermission(submitter, "job:control") + jobScope, err := userJobScope(submitter, "job:control") if err != nil { logger.Error(err.Error()) ReturnError(ctx, common.ERROR_AUTH_CODE, common.ERROR_ACCESS_FORBIDDEN_MSG, nil) return } - if jobPermission.DomainType == models.DomainTypeSelf { - if job_owner.(string) != submitter { - logger.Error("Permission denied", zap.String("perm", "job:control"), zap.String("logonUser", submitter)) - ReturnError(ctx, common.ERROR_AUTH_CODE, common.ERROR_ACCESS_FORBIDDEN_MSG, nil) - return - } + // bug-1934: every job:control grant counts + if owner, _ := job_owner.(string); !jobScope.covers(owner) { + logger.Error("Permission denied", zap.String("perm", "job:control"), zap.String("logonUser", submitter)) + ReturnError(ctx, common.ERROR_AUTH_CODE, common.ERROR_ACCESS_FORBIDDEN_MSG, nil) + return } } diff --git a/src/frontend/apiserver/controllers/jobs_handler_test.go b/src/frontend/apiserver/controllers/jobs_handler_test.go index ec62a2fd..3d550704 100644 --- a/src/frontend/apiserver/controllers/jobs_handler_test.go +++ b/src/frontend/apiserver/controllers/jobs_handler_test.go @@ -125,6 +125,13 @@ type handlerFixture struct { // restored in t.Cleanup. func newHandlerFixture(t *testing.T) *handlerFixture { t.Helper() + return newHandlerFixtureAs(t, handlerTestRBAC, handlerTestUser, "handler_tester") +} + +// newHandlerFixtureAs is newHandlerFixture with the RBAC config and the +// signed user and role given (bug-1934: tests of narrower job scopes). +func newHandlerFixtureAs(t *testing.T, rbac, user, role string) *handlerFixture { + t.Helper() // Save every global this fixture (or setupRBACTest) replaces. origJobs, origArrayJobs, origQueues, origHosts := store.Jobs, store.ArrayJobs, store.Queues, store.Hosts @@ -134,6 +141,7 @@ func newHandlerFixture(t *testing.T) *handlerFixture { origSubmit, origControl := submitJob, controlJob origJobCfg, origLog, origAuthCfg := utils.JobYamlConfig, utils.Log, utils.AuthenticationYamlConfig origRBAC := rbacConfig + origUserGroups := store.UserGroupsSnapshot() t.Cleanup(func() { store.Jobs, store.ArrayJobs, store.Queues, store.Hosts = origJobs, origArrayJobs, origQueues, origHosts @@ -143,15 +151,16 @@ func newHandlerFixture(t *testing.T) *handlerFixture { submitJob, controlJob = origSubmit, origControl utils.JobYamlConfig, utils.Log, utils.AuthenticationYamlConfig = origJobCfg, origLog, origAuthCfg rbacConfig = origRBAC + store.SetUserGroups(origUserGroups) }) // Auth/RBAC (replaces the stores with empty ones), then config. - setupRBACTest(t, handlerTestRBAC) + setupRBACTest(t, rbac) utils.JobYamlConfig = &utils.JobConfig{MaxArraySize: 1000} f := &handlerFixture{ t: t, - token: signTestJWT(handlerTestUser, "handler_tester", time.Now().Add(time.Hour)), + token: signTestJWT(user, role, time.Now().Add(time.Hour)), nextID: handlerTestOrdinaryBase, nextArray: handlerTestArrayBase, } @@ -202,6 +211,7 @@ func newHandlerFixture(t *testing.T) *handlerFixture { g := r.Group("/xce/v1/jobs") g.GET("", jc.GetJobsInfo) g.GET("/:jobids", jc.GetJobByIDs) + g.GET("/:jobids/output", jc.GetJobOutput) g.POST("/create", jc.CreateJob) g.POST("/:jobids/control", jc.ControlJobByID) g.POST("/:jobids/restart", jc.RestartJobByID) diff --git a/src/frontend/apiserver/controllers/output.go b/src/frontend/apiserver/controllers/output.go index ab9eedd7..d4c2722b 100644 --- a/src/frontend/apiserver/controllers/output.go +++ b/src/frontend/apiserver/controllers/output.go @@ -88,10 +88,11 @@ func (j JobController) GetJobOutput(ctx *gin.Context) { return } - // Check ownership: users with domain_type=all can view any job - permission, perr := GetRequireResourcePermission(submitUser, "job:view") - if perr != nil || permission.DomainType != models.DomainTypeAll { - if job.SubmitUser != submitUser { + // Check ownership: the owner may always read; anyone else needs a + // job:view grant covering the job (bug-1934) + if job.SubmitUser != submitUser { + scope, perr := userJobScope(submitUser, "job:view") + if perr != nil || !scope.covers(job.SubmitUser) { ReturnError(ctx, common.ERROR_AUTH_CODE, common.ERROR_ACCESS_FORBIDDEN_MSG, nil) return } diff --git a/src/frontend/apiserver/controllers/rbac_check.go b/src/frontend/apiserver/controllers/rbac_check.go index 120983b4..ecc107e2 100644 --- a/src/frontend/apiserver/controllers/rbac_check.go +++ b/src/frontend/apiserver/controllers/rbac_check.go @@ -168,6 +168,25 @@ func GetBuiltinRolePermissions(roleName string) []Permission { // get all user permission, // if user not exist in store, use basic_user permission func GetUserAllPermissionsWithType(userName string) []models.RolePermission { + perms, fromRoles := collectUserPermissions(userName) + if !fromRoles { + return perms + } + permissionListsType := DeduplicatePermissions(perms) + logger.Debug("case 4 DeduplicatePermissions", zap.Any("permissionListsType", permissionListsType)) + return permissionListsType +} + +// collectUserPermissionsWithType returns every grant of the user, one entry +// per scope, before DeduplicatePermissions (bug-1934: job scopes add up). +func collectUserPermissionsWithType(userName string) []models.RolePermission { + perms, _ := collectUserPermissions(userName) + return perms +} + +// collectUserPermissions returns the user's grants; fromRoles is false when +// the user has no roles and gets the builtin basic_user grants. +func collectUserPermissions(userName string) ([]models.RolePermission, bool) { userRoles := store.GetUserRoleNames(userName) if len(userRoles) == 0 { // case 1: user not in store, use basic_user @@ -184,7 +203,7 @@ func GetUserAllPermissionsWithType(userName string) []models.RolePermission { } } logger.Debug("case 1, assign basic_user permission to unregistered user") - return permissionTransform + return permissionTransform, false } builtInRoles := utils.IncludeBuiltinRoles(userRoles) @@ -212,9 +231,7 @@ func GetUserAllPermissionsWithType(userName string) []models.RolePermission { permissionListsType = append(permissionListsType, GetRoleAllPermissionsWithType(userRole)...) } logger.Debug("case 3, before remove duplicated permissions", zap.Any("permissionListsType", permissionListsType)) - permissionListsType = DeduplicatePermissions(permissionListsType) - logger.Debug("case 4 DeduplicatePermissions", zap.Any("permissionListsType", permissionListsType)) - return permissionListsType + return permissionListsType, true } func GetRoleAllPermissionsWithType(roleName string) []models.RolePermission { diff --git a/src/frontend/apiserver/models/jobs.go b/src/frontend/apiserver/models/jobs.go index b3bb0617..29a4423b 100644 --- a/src/frontend/apiserver/models/jobs.go +++ b/src/frontend/apiserver/models/jobs.go @@ -175,8 +175,8 @@ var JOB_SUPPORT_FILTERS = []string{ "job_id", //"user_group", //"job_group", - //"project_name", - //"license_project", + "project_name", // bug-1935: xcecli jobs list -P + "license_project", // bug-1935: xcecli jobs list --license-project //"sla", "submit_time", "queue_time", diff --git a/src/frontend/apiserver/models/usergroups.go b/src/frontend/apiserver/models/usergroups.go new file mode 100644 index 00000000..d9741822 --- /dev/null +++ b/src/frontend/apiserver/models/usergroups.go @@ -0,0 +1,10 @@ +package models + +// UserGroupInfo is a user group from the cluster configuration +// (users.yaml usergroups). A member is a user name, another group's name or +// "all"; ExMembers are removed from the group (bug-1934). +type UserGroupInfo struct { + Name string `json:"name"` + Members []string `json:"members"` + ExMembers []string `json:"exclude_members"` +} diff --git a/src/frontend/apiserver/store/usergroups.go b/src/frontend/apiserver/store/usergroups.go new file mode 100644 index 00000000..38f85123 --- /dev/null +++ b/src/frontend/apiserver/store/usergroups.go @@ -0,0 +1,179 @@ +package store + +import ( + "sync" + + "apiserver/models" +) + +// User groups from the cluster configuration, for the self_ug job scope +// (bug-1934). Replaced as a whole on every configuration push. +var ( + userGroupsMu sync.RWMutex + userGroups []models.UserGroupInfo +) + +// SetUserGroups replaces the user groups. +func SetUserGroups(groups []models.UserGroupInfo) { + cp := append([]models.UserGroupInfo(nil), groups...) + userGroupsMu.Lock() + userGroups = cp + userGroupsMu.Unlock() +} + +// UserGroupsSnapshot returns a copy of the user groups. +func UserGroupsSnapshot() []models.UserGroupInfo { + userGroupsMu.RLock() + defer userGroupsMu.RUnlock() + return append([]models.UserGroupInfo(nil), userGroups...) +} + +// UserSet is a set of user names; with All set it is every user except +// those in Names. +type UserSet struct { + All bool + Names map[string]bool +} + +// Has reports whether user is in the set. +func (s UserSet) Has(user string) bool { + if s.All { + return !s.Names[user] + } + return s.Names[user] +} + +func newUserSet(names ...string) UserSet { + s := UserSet{Names: map[string]bool{}} + for _, n := range names { + s.Names[n] = true + } + return s +} + +func union(a, b UserSet) UserSet { + switch { + case !a.All && !b.All: + r := newUserSet() + for n := range a.Names { + r.Names[n] = true + } + for n := range b.Names { + r.Names[n] = true + } + return r + case a.All && b.All: // excluded from both + r := UserSet{All: true, Names: map[string]bool{}} + for n := range a.Names { + if b.Names[n] { + r.Names[n] = true + } + } + return r + case b.All: + a, b = b, a + } + // a is every user except a.Names; b is finite + r := UserSet{All: true, Names: map[string]bool{}} + for n := range a.Names { + if !b.Names[n] { + r.Names[n] = true + } + } + return r +} + +func minus(a, b UserSet) UserSet { + r := newUserSet() + switch { + case !a.All && !b.All: + for n := range a.Names { + if !b.Names[n] { + r.Names[n] = true + } + } + case !a.All && b.All: // keep a's names b does not cover... b covers all but b.Names + for n := range a.Names { + if b.Names[n] { + r.Names[n] = true + } + } + case a.All && !b.All: + r.All = true + for n := range a.Names { + r.Names[n] = true + } + for n := range b.Names { + r.Names[n] = true + } + default: // users b excludes that a does not + for n := range b.Names { + if !a.Names[n] { + r.Names[n] = true + } + } + } + return r +} + +// expandGroup returns the users of group name; a name in `visiting` is a +// cycle and contributes nothing. +func expandGroup(byName map[string]models.UserGroupInfo, name string, + visiting map[string]bool) UserSet { + g, ok := byName[name] + if !ok || visiting[name] { + return newUserSet() + } + visiting[name] = true + defer delete(visiting, name) + + member := func(m string) UserSet { + if m == "all" { + return UserSet{All: true, Names: map[string]bool{}} + } + if _, isGroup := byName[m]; isGroup { + return expandGroup(byName, m, visiting) + } + return newUserSet(m) + } + r := newUserSet() + for _, m := range g.Members { + r = union(r, member(m)) + } + for _, m := range g.ExMembers { + r = minus(r, member(m)) + } + return r +} + +// UserGroupPeers returns user itself and every user who shares a user group +// with it. +func UserGroupPeers(user string) UserSet { + userGroupsMu.RLock() + defer userGroupsMu.RUnlock() + + byName := make(map[string]models.UserGroupInfo, len(userGroups)) + for _, g := range userGroups { + byName[g.Name] = g + } + r := newUserSet(user) + for _, g := range userGroups { + users := expandGroup(byName, g.Name, map[string]bool{}) + if users.Has(user) { + r = union(r, users) + } + } + return r +} + +// UsersShareGroup reports whether a and b are the same user or share a user +// group. +func UsersShareGroup(a, b string) bool { + return UserGroupPeers(a).Has(b) +} + +// NewUserSet returns the finite set of names. +func NewUserSet(names ...string) UserSet { return newUserSet(names...) } + +// UnionUserSets returns the users in a or b. +func UnionUserSets(a, b UserSet) UserSet { return union(a, b) } diff --git a/src/frontend/apiserver/store/usergroups_test.go b/src/frontend/apiserver/store/usergroups_test.go new file mode 100644 index 00000000..91a987de --- /dev/null +++ b/src/frontend/apiserver/store/usergroups_test.go @@ -0,0 +1,55 @@ +package store + +import ( + "testing" + + "apiserver/models" +) + +// bug-1934: user-group membership as the cluster config defines it +// (members may be users, other groups or "all"; ex_members are removed). +func TestUserGroups_Membership(t *testing.T) { + orig := UserGroupsSnapshot() + t.Cleanup(func() { SetUserGroups(orig) }) + + SetUserGroups([]models.UserGroupInfo{ + {Name: "team", Members: []string{"alice", "bob"}}, + {Name: "dept", Members: []string{"team", "carol"}, ExMembers: []string{"bob"}}, + {Name: "everyone", Members: []string{"all"}, ExMembers: []string{"mallory"}}, + {Name: "loop1", Members: []string{"loop2", "x"}}, + {Name: "loop2", Members: []string{"loop1"}}, + }) + + cases := []struct { + a, b string + want bool + }{ + {"alice", "bob", true}, // team + {"alice", "carol", true}, // dept (via nested team) + {"bob", "carol", true}, // everyone + {"alice", "mallory", false}, // mallory is excluded from everyone + {"mallory", "mallory", true}, + {"alice", "zed", true}, // everyone + {"x", "alice", true}, // everyone + } + for _, c := range cases { + if got := UsersShareGroup(c.a, c.b); got != c.want { + t.Errorf("UsersShareGroup(%q, %q) = %v, want %v", c.a, c.b, got, c.want) + } + } + + SetUserGroups([]models.UserGroupInfo{ + {Name: "team", Members: []string{"alice", "bob"}}, + {Name: "dept", Members: []string{"team", "carol"}, ExMembers: []string{"bob"}}, + }) + if UsersShareGroup("bob", "carol") { + t.Error("bob is excluded from dept and shares no group with carol") + } + peers := UserGroupPeers("alice") + if peers.All || !peers.Has("bob") || !peers.Has("carol") || !peers.Has("alice") || peers.Has("zed") { + t.Errorf("UserGroupPeers(alice) = %+v, want alice, bob, carol", peers) + } + if p := UserGroupPeers("zed"); p.All || !p.Has("zed") || p.Has("alice") { + t.Errorf("UserGroupPeers(zed) = %+v, want zed only", p) + } +}