Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitcc87a0c

Browse files
authored
feat: Implied 'member' roles for site and organization (#1917)
* feat: Member roles are implied and never exlpicitly added* Rename "GetAllUserRoles" to "GetAuthorizationRoles"* feat: Add migration to remove implied roles* rename user auth role middleware
1 parent2878346 commitcc87a0c

21 files changed

+131
-115
lines changed

‎coderd/authorize.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,12 @@ import (
1313
)
1414

1515
funcAuthorizeFilter[O rbac.Objecter](api*API,r*http.Request,action rbac.Action,objects []O) []O {
16-
roles:=httpmw.UserRoles(r)
16+
roles:=httpmw.AuthorizationUserRoles(r)
1717
returnrbac.Filter(r.Context(),api.Authorizer,roles.ID.String(),roles.Roles,action,objects)
1818
}
1919

2020
func (api*API)Authorize(rw http.ResponseWriter,r*http.Request,action rbac.Action,object rbac.Objecter)bool {
21-
roles:=httpmw.UserRoles(r)
21+
roles:=httpmw.AuthorizationUserRoles(r)
2222
err:=api.Authorizer.ByRoleName(r.Context(),roles.ID.String(),roles.Roles,action,object.RBACObject())
2323
iferr!=nil {
2424
httpapi.Write(rw,http.StatusForbidden, httpapi.Response{

‎coderd/coderdtest/coderdtest.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ func CreateAnotherUser(t *testing.T, client *codersdk.Client, organizationID uui
281281
organizationID,err:=uuid.Parse(orgID)
282282
require.NoError(t,err,fmt.Sprintf("parse org id %q",orgID))
283283
_,err=client.UpdateOrganizationMemberRoles(context.Background(),organizationID,user.ID.String(),
284-
codersdk.UpdateRoles{Roles:append(roles,rbac.RoleOrgMember(organizationID))})
284+
codersdk.UpdateRoles{Roles:roles})
285285
require.NoError(t,err,"update org membership roles")
286286
}
287287
}

‎coderd/database/databasefake/databasefake.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ func (q *fakeQuerier) GetUsersByIDs(_ context.Context, ids []uuid.UUID) ([]datab
276276
returnusers,nil
277277
}
278278

279-
func (q*fakeQuerier)GetAllUserRoles(_ context.Context,userID uuid.UUID) (database.GetAllUserRolesRow,error) {
279+
func (q*fakeQuerier)GetAuthorizationUserRoles(_ context.Context,userID uuid.UUID) (database.GetAuthorizationUserRolesRow,error) {
280280
q.mutex.RLock()
281281
deferq.mutex.RUnlock()
282282

@@ -286,6 +286,7 @@ func (q *fakeQuerier) GetAllUserRoles(_ context.Context, userID uuid.UUID) (data
286286
ifu.ID==userID {
287287
u:=u
288288
roles=append(roles,u.RBACRoles...)
289+
roles=append(roles,"member")
289290
user=&u
290291
break
291292
}
@@ -294,14 +295,15 @@ func (q *fakeQuerier) GetAllUserRoles(_ context.Context, userID uuid.UUID) (data
294295
for_,mem:=rangeq.organizationMembers {
295296
ifmem.UserID==userID {
296297
roles=append(roles,mem.Roles...)
298+
roles=append(roles,"organization-member:"+mem.OrganizationID.String())
297299
}
298300
}
299301

300302
ifuser==nil {
301-
return database.GetAllUserRolesRow{},sql.ErrNoRows
303+
return database.GetAuthorizationUserRolesRow{},sql.ErrNoRows
302304
}
303305

304-
return database.GetAllUserRolesRow{
306+
return database.GetAuthorizationUserRolesRow{
305307
ID:userID,
306308
Username:user.Username,
307309
Status:user.Status,
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
--- Remove the now implied 'member' role.
2+
UPDATE
3+
users
4+
SET
5+
rbac_roles= array_append(rbac_roles,'member');
6+
7+
--- Remove the now implied 'organization-member' role.
8+
UPDATE
9+
organization_members
10+
SET
11+
roles= array_append(roles,'organization-member:'||organization_id::text);
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
--- Remove the now implied 'member' role.
2+
UPDATE
3+
users
4+
SET
5+
rbac_roles= array_remove(rbac_roles,'member');
6+
7+
--- Remove the now implied 'organization-member' role.
8+
UPDATE
9+
organization_members
10+
SET
11+
roles= array_remove(roles,'organization-member:'||organization_id::text);

‎coderd/database/querier.go

Lines changed: 3 additions & 1 deletion
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 17 additions & 9 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/database/queries/users.sql

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -134,12 +134,20 @@ WHERE
134134
id= $1 RETURNING*;
135135

136136

137-
-- name: GetAllUserRoles :one
137+
-- name: GetAuthorizationUserRoles :one
138+
-- This function returns roles for authorization purposes. Implied member roles
139+
-- are included.
138140
SELECT
139-
-- username is returned just to help for logging purposes
140-
-- status is used to enforce 'suspended' users, as all roles are ignored
141-
--when suspended.
142-
id, username, status, array_cat(users.rbac_roles,organization_members.roles) ::text[]AS roles
141+
-- username is returned just to help for logging purposes
142+
-- status is used to enforce 'suspended' users, as all roles are ignored
143+
--when suspended.
144+
id, username, status,
145+
array_cat(
146+
-- All users are members
147+
array_append(users.rbac_roles,'member'),
148+
-- All org_members get the org-member role for their orgs
149+
array_append(organization_members.roles,'organization-member:'||organization_members.organization_id::text)) ::text[]
150+
AS roles
143151
FROM
144152
users
145153
LEFT JOIN organization_members

‎coderd/httpmw/apikey.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,19 @@ func APIKey(r *http.Request) database.APIKey {
3131
returnapiKey
3232
}
3333

34+
// User roles are the 'subject' field of Authorize()
35+
typeuserRolesKeystruct{}
36+
37+
// AuthorizationUserRoles returns the roles used for authorization.
38+
// Comes from the ExtractAPIKey handler.
39+
funcAuthorizationUserRoles(r*http.Request) database.GetAuthorizationUserRolesRow {
40+
apiKey,ok:=r.Context().Value(userRolesKey{}).(database.GetAuthorizationUserRolesRow)
41+
if!ok {
42+
panic("developer error: user roles middleware not provided")
43+
}
44+
returnapiKey
45+
}
46+
3447
// OAuth2Configs is a collection of configurations for OAuth-based authentication.
3548
// This should be extended to support other authentication types in the future.
3649
typeOAuth2Configsstruct {
@@ -178,7 +191,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
178191
// If the key is valid, we also fetch the user roles and status.
179192
// The roles are used for RBAC authorize checks, and the status
180193
// is to block 'suspended' users from accessing the platform.
181-
roles,err:=db.GetAllUserRoles(r.Context(),key.UserID)
194+
roles,err:=db.GetAuthorizationUserRoles(r.Context(),key.UserID)
182195
iferr!=nil {
183196
httpapi.Write(rw,http.StatusUnauthorized, httpapi.Response{
184197
Message:"roles not found",

‎coderd/httpmw/authorize.go

Lines changed: 0 additions & 40 deletions
This file was deleted.

‎coderd/httpmw/authorize_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,23 +31,23 @@ func TestExtractUserRoles(t *testing.T) {
3131
{
3232
Name:"Member",
3333
AddUser:func(db database.Store) (database.User, []string,string) {
34-
roles:= []string{rbac.RoleMember()}
34+
roles:= []string{}
3535
user,token:=addUser(t,db,roles...)
36-
returnuser,roles,token
36+
returnuser,append(roles,rbac.RoleMember()),token
3737
},
3838
},
3939
{
4040
Name:"Admin",
4141
AddUser:func(db database.Store) (database.User, []string,string) {
42-
roles:= []string{rbac.RoleMember(),rbac.RoleAdmin()}
42+
roles:= []string{rbac.RoleAdmin()}
4343
user,token:=addUser(t,db,roles...)
44-
returnuser,roles,token
44+
returnuser,append(roles,rbac.RoleMember()),token
4545
},
4646
},
4747
{
4848
Name:"OrgMember",
4949
AddUser:func(db database.Store) (database.User, []string,string) {
50-
roles:= []string{rbac.RoleMember()}
50+
roles:= []string{}
5151
user,token:=addUser(t,db,roles...)
5252
org,err:=db.InsertOrganization(context.Background(), database.InsertOrganizationParams{
5353
ID:uuid.New(),
@@ -58,7 +58,7 @@ func TestExtractUserRoles(t *testing.T) {
5858
})
5959
require.NoError(t,err)
6060

61-
orgRoles:= []string{rbac.RoleOrgMember(org.ID)}
61+
orgRoles:= []string{}
6262
_,err=db.InsertOrganizationMember(context.Background(), database.InsertOrganizationMemberParams{
6363
OrganizationID:org.ID,
6464
UserID:user.ID,
@@ -67,7 +67,7 @@ func TestExtractUserRoles(t *testing.T) {
6767
Roles:orgRoles,
6868
})
6969
require.NoError(t,err)
70-
returnuser,append(roles,orgRoles...),token
70+
returnuser,append(roles,append(orgRoles,rbac.RoleMember(),rbac.RoleOrgMember(org.ID))...),token
7171
},
7272
},
7373
}
@@ -86,7 +86,7 @@ func TestExtractUserRoles(t *testing.T) {
8686
httpmw.ExtractAPIKey(db,&httpmw.OAuth2Configs{}),
8787
)
8888
rtr.Get("/",func(_ http.ResponseWriter,r*http.Request) {
89-
roles:=httpmw.UserRoles(r)
89+
roles:=httpmw.AuthorizationUserRoles(r)
9090
require.ElementsMatch(t,user.ID,roles.ID)
9191
require.ElementsMatch(t,expRoles,roles.Roles)
9292
})

‎coderd/members.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@ func (api *API) putMemberRoles(rw http.ResponseWriter, r *http.Request) {
3434
return
3535
}
3636

37-
added,removed:=rbac.ChangeRoleSet(member.Roles,params.Roles)
37+
// The org-member role is always implied.
38+
impliedTypes:=append(params.Roles,rbac.RoleOrgMember(organization.ID))
39+
added,removed:=rbac.ChangeRoleSet(member.Roles,impliedTypes)
3840
for_,roleName:=rangeadded {
3941
// Assigning a role requires the create permission.
4042
if!api.Authorize(rw,r,rbac.ActionCreate,rbac.ResourceOrgRoleAssignment.WithID(roleName).InOrg(organization.ID)) {

‎coderd/organizations.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,11 @@ func (api *API) postOrganizations(rw http.ResponseWriter, r *http.Request) {
7373
CreatedAt:database.Now(),
7474
UpdatedAt:database.Now(),
7575
Roles: []string{
76-
// Also assign member role incase they get demoted from admin
77-
rbac.RoleOrgMember(organization.ID),
7876
rbac.RoleOrgAdmin(organization.ID),
7977
},
8078
})
8179
iferr!=nil {
82-
returnxerrors.Errorf("create organizationmember: %w",err)
80+
returnxerrors.Errorf("create organizationadmin: %w",err)
8381
}
8482
returnnil
8583
})

‎coderd/rbac/builtin.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ var (
6363
member:func(_string)Role {
6464
returnRole{
6565
Name:member,
66-
DisplayName:"Member",
66+
DisplayName:"",
6767
Site:permissions(map[Object][]Action{
6868
// All users can read all other users and know they exist.
6969
ResourceUser: {ActionRead},
@@ -116,7 +116,7 @@ var (
116116
orgMember:func(organizationIDstring)Role {
117117
returnRole{
118118
Name:roleName(orgMember,organizationID),
119-
DisplayName:"Organization Member",
119+
DisplayName:"",
120120
Org:map[string][]Permission{
121121
organizationID: {
122122
{

‎coderd/rbac/role.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ type Permission struct {
1717
// Users of this package should instead **only** use the role names, and
1818
// this package will expand the role names into their json payloads.
1919
typeRolestruct {
20-
Namestring`json:"name"`
20+
Namestring`json:"name"`
21+
// DisplayName is used for UI purposes. If the role has no display name,
22+
// that means the UI should never display it.
2123
DisplayNamestring`json:"display_name"`
2224
Site []Permission`json:"site"`
2325
// Org is a map of orgid to permissions. We represent orgid as a string.

‎coderd/roles.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ func (api *API) checkPermissions(rw http.ResponseWriter, r *http.Request) {
4545
}
4646

4747
// use the roles of the user specified, not the person making the request.
48-
roles,err:=api.Database.GetAllUserRoles(r.Context(),user.ID)
48+
roles,err:=api.Database.GetAuthorizationUserRoles(r.Context(),user.ID)
4949
iferr!=nil {
5050
httpapi.Forbidden(rw)
5151
return
@@ -91,6 +91,10 @@ func convertRole(role rbac.Role) codersdk.Role {
9191
funcconvertRoles(roles []rbac.Role) []codersdk.Role {
9292
converted:=make([]codersdk.Role,0,len(roles))
9393
for_,role:=rangeroles {
94+
// Roles without display names should never be shown to the ui.
95+
ifrole.DisplayName=="" {
96+
continue
97+
}
9498
converted=append(converted,convertRole(role))
9599
}
96100
returnconverted

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp