Skip to content

Commit 3209c86

Browse files
EmyrkEmyrk
authored
chore: authz 'any_org' to return if at least 1 org has perms (coder#14009)
* chore: authz 'any_org' to return if at least 1 org has perms Allows checking if a user can do an action in any organization, rather than a specific one. Allows asking general questions on the UI to determine which elements to show. * more strict, add comments to policy * add unit tests and extend to /authcheck api * make field optional
1 parent b7102b3 commit 3209c86

14 files changed

+196
-14
lines changed

‎coderd/apidoc/docs.go

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/authorize.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,10 @@ func (api *API) checkAuthorization(rw http.ResponseWriter, r *http.Request) {
167167
}
168168

169169
obj := rbac.Object{
170-
Owner: v.Object.OwnerID,
171-
OrgID: v.Object.OrganizationID,
172-
Type: string(v.Object.ResourceType),
170+
Owner: v.Object.OwnerID,
171+
OrgID: v.Object.OrganizationID,
172+
Type: string(v.Object.ResourceType),
173+
AnyOrgOwner: v.Object.AnyOrgOwner,
173174
}
174175
if obj.Owner == "me" {
175176
obj.Owner = auth.ID

‎coderd/rbac/astvalue.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,10 @@ func (z Object) regoValue() ast.Value {
124124
ast.StringTerm("org_owner"),
125125
ast.StringTerm(z.OrgID),
126126
},
127+
[2]*ast.Term{
128+
ast.StringTerm("any_org"),
129+
ast.BooleanTerm(z.AnyOrgOwner),
130+
},
127131
[2]*ast.Term{
128132
ast.StringTerm("type"),
129133
ast.StringTerm(z.Type),

‎coderd/rbac/authz.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ func Filter[O Objecter](ctx context.Context, auth Authorizer, subject Subject, a
181181
for _, o := range objects {
182182
rbacObj := o.RBACObject()
183183
if rbacObj.Type != objectType {
184-
return nil, xerrors.Errorf("object types must be uniform across the set (%s), found %s", objectType, rbacObj)
184+
return nil, xerrors.Errorf("object types must be uniform across the set (%s), found %s", objectType, rbacObj.Type)
185185
}
186186
err := auth.Authorize(ctx, subject, action, o.RBACObject())
187187
if err == nil {
@@ -387,6 +387,13 @@ func (a RegoAuthorizer) authorize(ctx context.Context, subject Subject, action p
387387
return xerrors.Errorf("subject must have a scope")
388388
}
389389

390+
// The caller should use either 1 or the other (or none).
391+
// Using "AnyOrgOwner" and an OrgID is a contradiction.
392+
// An empty uuid or a nil uuid means "no org owner".
393+
if object.AnyOrgOwner && !(object.OrgID == "" || object.OrgID == "00000000-0000-0000-0000-000000000000") {
394+
return xerrors.Errorf("object cannot have 'any_org' and an 'org_id' specified, values are mutually exclusive")
395+
}
396+
390397
astV, err := regoInputValue(subject, action, object)
391398
if err != nil {
392399
return xerrors.Errorf("convert input to value: %w", err)

‎coderd/rbac/authz_internal_test.go

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,22 @@ func TestAuthorizeDomain(t *testing.T) {
291291
unuseID := uuid.New()
292292
allUsersGroup := "Everyone"
293293

294+
// orphanedUser has no organization
295+
orphanedUser := Subject{
296+
ID: "me",
297+
Scope: must(ExpandScope(ScopeAll)),
298+
Groups: []string{},
299+
Roles: Roles{
300+
must(RoleByName(RoleMember())),
301+
},
302+
}
303+
testAuthorize(t, "OrphanedUser", orphanedUser, []authTestCase{
304+
{resource: ResourceWorkspace.InOrg(defOrg).WithOwner(orphanedUser.ID), actions: ResourceWorkspace.AvailableActions(), allow: false},
305+
306+
// Orphaned user cannot create workspaces in any organization
307+
{resource: ResourceWorkspace.AnyOrganization().WithOwner(orphanedUser.ID), actions: []policy.Action{policy.ActionCreate}, allow: false},
308+
})
309+
294310
user := Subject{
295311
ID: "me",
296312
Scope: must(ExpandScope(ScopeAll)),
@@ -370,6 +386,10 @@ func TestAuthorizeDomain(t *testing.T) {
370386
{resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), actions: ResourceWorkspace.AvailableActions(), allow: true},
371387
{resource: ResourceWorkspace.InOrg(defOrg), actions: ResourceWorkspace.AvailableActions(), allow: false},
372388

389+
// AnyOrganization using a user scoped permission
390+
{resource: ResourceWorkspace.AnyOrganization().WithOwner(user.ID), actions: ResourceWorkspace.AvailableActions(), allow: true},
391+
{resource: ResourceTemplate.AnyOrganization(), actions: []policy.Action{policy.ActionCreate}, allow: false},
392+
373393
{resource: ResourceWorkspace.WithOwner(user.ID), actions: ResourceWorkspace.AvailableActions(), allow: true},
374394

375395
{resource: ResourceWorkspace.All(), actions: ResourceWorkspace.AvailableActions(), allow: false},
@@ -443,6 +463,8 @@ func TestAuthorizeDomain(t *testing.T) {
443463
workspaceExceptConnect := slice.Omit(ResourceWorkspace.AvailableActions(), policy.ActionApplicationConnect, policy.ActionSSH)
444464
workspaceConnect := []policy.Action{policy.ActionApplicationConnect, policy.ActionSSH}
445465
testAuthorize(t, "OrgAdmin", user, []authTestCase{
466+
{resource: ResourceTemplate.AnyOrganization(), actions: []policy.Action{policy.ActionCreate}, allow: true},
467+
446468
// Org + me
447469
{resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), actions: ResourceWorkspace.AvailableActions(), allow: true},
448470
{resource: ResourceWorkspace.InOrg(defOrg), actions: workspaceExceptConnect, allow: true},
@@ -479,6 +501,9 @@ func TestAuthorizeDomain(t *testing.T) {
479501
}
480502

481503
testAuthorize(t, "SiteAdmin", user, []authTestCase{
504+
// Similar to an orphaned user, but has site level perms
505+
{resource: ResourceTemplate.AnyOrganization(), actions: []policy.Action{policy.ActionCreate}, allow: true},
506+
482507
// Org + me
483508
{resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), actions: ResourceWorkspace.AvailableActions(), allow: true},
484509
{resource: ResourceWorkspace.InOrg(defOrg), actions: ResourceWorkspace.AvailableActions(), allow: true},
@@ -1078,9 +1103,10 @@ func testAuthorize(t *testing.T, name string, subject Subject, sets ...[]authTes
10781103
t.Logf("input: %s", string(d))
10791104
if authError != nil {
10801105
var uerr *UnauthorizedError
1081-
xerrors.As(authError, &uerr)
1082-
t.Logf("internal error: %+v", uerr.Internal().Error())
1083-
t.Logf("output: %+v", uerr.Output())
1106+
if xerrors.As(authError, &uerr) {
1107+
t.Logf("internal error: %+v", uerr.Internal().Error())
1108+
t.Logf("output: %+v", uerr.Output())
1109+
}
10841110
}
10851111

10861112
if c.allow {
@@ -1115,10 +1141,15 @@ func testAuthorize(t *testing.T, name string, subject Subject, sets ...[]authTes
11151141
require.Equal(t, 0, len(partialAuthz.partialQueries.Support), "expected 0 support rules in scope authorizer")
11161142

11171143
partialErr := partialAuthz.Authorize(ctx, c.resource)
1118-
if authError != nil {
1119-
assert.Error(t, partialErr, "partial allowed invalid request (false positive)")
1120-
} else {
1121-
assert.NoError(t, partialErr, "partial error blocked valid request (false negative)")
1144+
// If 'AnyOrgOwner' is true, a partial eval does not make sense.
1145+
// Run the partial eval to ensure no panics, but the actual authz
1146+
// response does not matter.
1147+
if !c.resource.AnyOrgOwner {
1148+
if authError != nil {
1149+
assert.Error(t, partialErr, "partial allowed invalid request (false positive)")
1150+
} else {
1151+
assert.NoError(t, partialErr, "partial error blocked valid request (false negative)")
1152+
}
11221153
}
11231154
}
11241155
})

‎coderd/rbac/authz_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ func BenchmarkCacher(b *testing.B) {
314314
}
315315
}
316316

317-
func TestCacher(t *testing.T) {
317+
func TestCache(t *testing.T) {
318318
t.Parallel()
319319

320320
t.Run("NoCache", func(t *testing.T) {

‎coderd/rbac/object.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ type Object struct {
2323
Owner string `json:"owner"`
2424
// OrgID specifies which org the object is a part of.
2525
OrgID string `json:"org_owner"`
26+
// AnyOrgOwner will disregard the org_owner when checking for permissions
27+
// Use this to ask, "Can the actor do this action on any org?" when
28+
// the exact organization is not important or known.
29+
// E.g: The UI should show a "create template" button if the user
30+
// can create a template in any org.
31+
AnyOrgOwner bool `json:"any_org"`
2632

2733
// Type is "workspace", "project", "app", etc
2834
Type string `json:"type"`
@@ -115,6 +121,7 @@ func (z Object) All() Object {
115121
Type: z.Type,
116122
ACLUserList: map[string][]policy.Action{},
117123
ACLGroupList: map[string][]policy.Action{},
124+
AnyOrgOwner: z.AnyOrgOwner,
118125
}
119126
}
120127

@@ -126,6 +133,7 @@ func (z Object) WithIDString(id string) Object {
126133
Type: z.Type,
127134
ACLUserList: z.ACLUserList,
128135
ACLGroupList: z.ACLGroupList,
136+
AnyOrgOwner: z.AnyOrgOwner,
129137
}
130138
}
131139

@@ -137,6 +145,7 @@ func (z Object) WithID(id uuid.UUID) Object {
137145
Type: z.Type,
138146
ACLUserList: z.ACLUserList,
139147
ACLGroupList: z.ACLGroupList,
148+
AnyOrgOwner: z.AnyOrgOwner,
140149
}
141150
}
142151

@@ -149,6 +158,21 @@ func (z Object) InOrg(orgID uuid.UUID) Object {
149158
Type: z.Type,
150159
ACLUserList: z.ACLUserList,
151160
ACLGroupList: z.ACLGroupList,
161+
// InOrg implies AnyOrgOwner is false
162+
AnyOrgOwner: false,
163+
}
164+
}
165+
166+
func (z Object) AnyOrganization() Object {
167+
return Object{
168+
ID: z.ID,
169+
Owner: z.Owner,
170+
// AnyOrgOwner cannot have an org owner also set.
171+
OrgID: "",
172+
Type: z.Type,
173+
ACLUserList: z.ACLUserList,
174+
ACLGroupList: z.ACLGroupList,
175+
AnyOrgOwner: true,
152176
}
153177
}
154178

@@ -161,6 +185,7 @@ func (z Object) WithOwner(ownerID string) Object {
161185
Type: z.Type,
162186
ACLUserList: z.ACLUserList,
163187
ACLGroupList: z.ACLGroupList,
188+
AnyOrgOwner: z.AnyOrgOwner,
164189
}
165190
}
166191

@@ -173,6 +198,7 @@ func (z Object) WithACLUserList(acl map[string][]policy.Action) Object {
173198
Type: z.Type,
174199
ACLUserList: acl,
175200
ACLGroupList: z.ACLGroupList,
201+
AnyOrgOwner: z.AnyOrgOwner,
176202
}
177203
}
178204

@@ -184,5 +210,6 @@ func (z Object) WithGroupACL(groups map[string][]policy.Action) Object {
184210
Type: z.Type,
185211
ACLUserList: z.ACLUserList,
186212
ACLGroupList: groups,
213+
AnyOrgOwner: z.AnyOrgOwner,
187214
}
188215
}

‎coderd/rbac/policy.rego

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,18 @@ org := org_allow(input.subject.roles)
9292
default scope_org := 0
9393
scope_org := org_allow([input.scope])
9494

95-
org_allow(roles) := num {
96-
allow := { id: num |
95+
# org_allow_set is a helper function that iterates over all orgs that the actor
96+
# is a member of. For each organization it sets the numerical allow value
97+
# for the given object + action if the object is in the organization.
98+
# The resulting value is a map that looks something like:
99+
# {"10d03e62-7703-4df5-a358-4f76577d4e2f": 1, "5750d635-82e0-4681-bd44-815b18669d65": 1}
100+
# The caller can use this output[<object.org_owner>] to get the final allow value.
101+
#
102+
# The reason we calculate this for all orgs, and not just the input.object.org_owner
103+
# is that sometimes the input.object.org_owner is unknown. In those cases
104+
# we have a list of org_ids that can we use in a SQL 'WHERE' clause.
105+
org_allow_set(roles) := allow_set {
106+
allow_set := { id: num |
97107
id := org_members[_]
98108
set := { x |
99109
perm := roles[_].org[id][_]
@@ -103,6 +113,13 @@ org_allow(roles) := num {
103113
}
104114
num := number(set)
105115
}
116+
}
117+
118+
org_allow(roles) := num {
119+
# If the object has "any_org" set to true, then use the other
120+
# org_allow block.
121+
not input.object.any_org
122+
allow := org_allow_set(roles)
106123

107124
# Return only the org value of the input's org.
108125
# The reason why we do not do this up front, is that we need to make sure
@@ -112,12 +129,47 @@ org_allow(roles) := num {
112129
num := allow[input.object.org_owner]
113130
}
114131

132+
# This block states if "object.any_org" is set to true, then disregard the
133+
# organization id the object is associated with. Instead, we check if the user
134+
# can do the action on any organization.
135+
# This is useful for UI elements when we want to conclude, "Can the user create
136+
# a new template in any organization?"
137+
# It is easier than iterating over every organization the user is apart of.
138+
org_allow(roles) := num {
139+
input.object.any_org # if this is false, this code block is not used
140+
allow := org_allow_set(roles)
141+
142+
143+
# allow is a map of {"<org_id>": <number>}. We only care about values
144+
# that are 1, and ignore the rest.
145+
num := number([
146+
keep |
147+
# for every value in the mapping
148+
value := allow[_]
149+
# only keep values > 0.
150+
# 1 = allow, 0 = abstain, -1 = deny
151+
# We only need 1 explicit allow to allow the action.
152+
# deny's and abstains are intentionally ignored.
153+
value > 0
154+
# result set is a set of [true,false,...]
155+
# which "number()" will convert to a number.
156+
keep := true
157+
])
158+
}
159+
115160
# 'org_mem' is set to true if the user is an org member
161+
# If 'any_org' is set to true, use the other block to determine org membership.
116162
org_mem := true {
163+
not input.object.any_org
117164
input.object.org_owner != ""
118165
input.object.org_owner in org_members
119166
}
120167

168+
org_mem := true {
169+
input.object.any_org
170+
count(org_members) > 0
171+
}
172+
121173
org_ok {
122174
org_mem
123175
}
@@ -126,6 +178,7 @@ org_ok {
126178
# the non-existent org.
127179
org_ok {
128180
input.object.org_owner == ""
181+
not input.object.any_org
129182
}
130183

131184
# User is the same as the site, except it only applies if the user owns the object and

‎coderd/rbac/roles_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,46 @@ func TestRolePermissions(t *testing.T) {
590590
false: {},
591591
},
592592
},
593+
// AnyOrganization tests
594+
{
595+
Name: "CreateOrgMember",
596+
Actions: []policy.Action{policy.ActionCreate},
597+
Resource: rbac.ResourceOrganizationMember.AnyOrganization(),
598+
AuthorizeMap: map[bool][]hasAuthSubjects{
599+
true: {owner, userAdmin, orgAdmin, otherOrgAdmin, orgUserAdmin, otherOrgUserAdmin},
600+
false: {
601+
memberMe, templateAdmin,
602+
orgTemplateAdmin, orgMemberMe, orgAuditor,
603+
otherOrgMember, otherOrgAuditor, otherOrgTemplateAdmin,
604+
},
605+
},
606+
},
607+
{
608+
Name: "CreateTemplateAnyOrg",
609+
Actions: []policy.Action{policy.ActionCreate},
610+
Resource: rbac.ResourceTemplate.AnyOrganization(),
611+
AuthorizeMap: map[bool][]hasAuthSubjects{
612+
true: {owner, templateAdmin, orgTemplateAdmin, otherOrgTemplateAdmin, orgAdmin, otherOrgAdmin},
613+
false: {
614+
userAdmin, memberMe,
615+
orgMemberMe, orgAuditor, orgUserAdmin,
616+
otherOrgMember, otherOrgAuditor, otherOrgUserAdmin,
617+
},
618+
},
619+
},
620+
{
621+
Name: "CreateWorkspaceAnyOrg",
622+
Actions: []policy.Action{policy.ActionCreate},
623+
Resource: rbac.ResourceWorkspace.AnyOrganization().WithOwner(currentUser.String()),
624+
AuthorizeMap: map[bool][]hasAuthSubjects{
625+
true: {owner, orgAdmin, otherOrgAdmin, orgMemberMe},
626+
false: {
627+
memberMe, userAdmin, templateAdmin,
628+
orgAuditor, orgUserAdmin, orgTemplateAdmin,
629+
otherOrgMember, otherOrgAuditor, otherOrgUserAdmin, otherOrgTemplateAdmin,
630+
},
631+
},
632+
},
593633
}
594634

595635
// We expect every permission to be tested above.

‎codersdk/authorization.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ type AuthorizationObject struct {
5454
// are using this option, you should also set the owner ID and organization ID
5555
// if possible. Be as specific as possible using all the fields relevant.
5656
ResourceID string `json:"resource_id,omitempty"`
57+
// AnyOrgOwner (optional) will disregard the org_owner when checking for permissions.
58+
// This cannot be set to true if the OrganizationID is set.
59+
AnyOrgOwner bool `json:"any_org,omitempty"`
5760
}
5861

5962
// AuthCheck allows the authenticated user to check if they have the given permissions

0 commit comments

Comments
 (0)