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

chore: create type for unique role names#13506

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Emyrk merged 16 commits intomainfromstevenmasley/typed_role_name
Jun 11, 2024
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletionscli/server_createadminuser.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -192,7 +192,7 @@ func (r *RootCmd) newCreateAdminUserCommand() *serpent.Command {
HashedPassword: []byte(hashedPassword),
CreatedAt: dbtime.Now(),
UpdatedAt: dbtime.Now(),
RBACRoles: []string{rbac.RoleOwner()},
RBACRoles: []string{rbac.RoleOwner().String()},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

review:string does not implementfmt.Stringer :(

Emyrk reacted with confused emoji
LoginType: database.LoginTypePassword,
})
if err != nil {
Expand DownExpand Up@@ -222,7 +222,7 @@ func (r *RootCmd) newCreateAdminUserCommand() *serpent.Command {
UserID: newUser.ID,
CreatedAt: dbtime.Now(),
UpdatedAt: dbtime.Now(),
Roles: []string{rbac.ScopedRoleOrgAdmin(org.ID)},
Roles: []string{rbac.RoleOrgAdmin()},
})
if err != nil {
return xerrors.Errorf("insert organization member: %w", err)
Expand Down
5 changes: 3 additions & 2 deletionscli/server_createadminuser_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,6 +17,7 @@ import (
"github.com/coder/coder/v2/coderd/database/dbtime"
"github.com/coder/coder/v2/coderd/rbac"
"github.com/coder/coder/v2/coderd/userpassword"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/pty/ptytest"
"github.com/coder/coder/v2/testutil"
)
Expand DownExpand Up@@ -56,7 +57,7 @@ func TestServerCreateAdminUser(t *testing.T) {
require.NoError(t, err)
require.True(t, ok, "password does not match")

require.EqualValues(t, []string{rbac.RoleOwner()}, user.RBACRoles, "user does not have owner role")
require.EqualValues(t, []string{codersdk.RoleOwner}, user.RBACRoles, "user does not have owner role")

// Check that user is admin in every org.
orgs, err := db.GetOrganizations(ctx)
Expand All@@ -71,7 +72,7 @@ func TestServerCreateAdminUser(t *testing.T) {
orgIDs2 := make(map[uuid.UUID]struct{}, len(orgMemberships))
for _, membership := range orgMemberships {
orgIDs2[membership.OrganizationID] = struct{}{}
assert.Equal(t, []string{rbac.ScopedRoleOrgAdmin(membership.OrganizationID)}, membership.Roles, "user is not org admin")
assert.Equal(t, []string{rbac.RoleOrgAdmin()}, membership.Roles, "user is not org admin")
}

require.Equal(t, orgIDs, orgIDs2, "user is not in all orgs")
Expand Down
3 changes: 2 additions & 1 deletioncoderd/audit.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -199,7 +199,8 @@ func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogs
Roles: []codersdk.SlimRole{},
}

for _, roleName := range dblog.UserRoles {
for _, input := range dblog.UserRoles {
roleName, _ := rbac.RoleNameFromString(input)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Should we at least drop an error log if we were unable to covert any of the roles rom the db?

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Since this is in the convert audit log, I think that could get quite noisy. I agree this is not ideal

rbacRole, _ := rbac.RoleByName(roleName)
user.Roles = append(user.Roles, db2sdk.SlimRole(rbacRole))
}
Expand Down
7 changes: 5 additions & 2 deletionscoderd/coderdtest/authorize.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -60,10 +60,13 @@ func AssertRBAC(t *testing.T, api *coderd.API, client *codersdk.Client) RBACAsse
roles, err := api.Database.GetAuthorizationUserRoles(ctx, key.UserID)
require.NoError(t, err, "fetch user roles")

roleNames, err := roles.RoleNames()
require.NoError(t, err)

return RBACAsserter{
Subject: rbac.Subject{
ID: key.UserID.String(),
Roles: rbac.RoleNames(roles.Roles),
Roles: rbac.RoleIdentifiers(roleNames),
Groups: roles.Groups,
Scope: rbac.ScopeName(key.Scope),
},
Expand DownExpand Up@@ -435,7 +438,7 @@ func randomRBACType() string {
func RandomRBACSubject() rbac.Subject {
return rbac.Subject{
ID: uuid.NewString(),
Roles: rbac.RoleNames{rbac.RoleMember()},
Roles: rbac.RoleIdentifiers{rbac.RoleMember()},
Groups: []string{namesgenerator.GetRandomName(1)},
Scope: rbac.ScopeAll,
}
Expand Down
46 changes: 26 additions & 20 deletionscoderd/coderdtest/coderdtest.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -55,6 +55,7 @@ import (
"github.com/coder/coder/v2/coderd/autobuild"
"github.com/coder/coder/v2/coderd/awsidentity"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/db2sdk"
"github.com/coder/coder/v2/coderd/database/dbauthz"
"github.com/coder/coder/v2/coderd/database/dbrollup"
"github.com/coder/coder/v2/coderd/database/dbtestutil"
Expand DownExpand Up@@ -663,21 +664,25 @@ func CreateFirstUser(t testing.TB, client *codersdk.Client) codersdk.CreateFirst

// CreateAnotherUser creates and authenticates a new user.
// Roles can include org scoped roles with 'roleName:<organization_id>'
func CreateAnotherUser(t testing.TB, client *codersdk.Client, organizationID uuid.UUID, roles ...string) (*codersdk.Client, codersdk.User) {
func CreateAnotherUser(t testing.TB, client *codersdk.Client, organizationID uuid.UUID, roles ...rbac.RoleIdentifier) (*codersdk.Client, codersdk.User) {
return createAnotherUserRetry(t, client, organizationID, 5, roles)
}

func CreateAnotherUserMutators(t testing.TB, client *codersdk.Client, organizationID uuid.UUID, roles []string, mutators ...func(r *codersdk.CreateUserRequest)) (*codersdk.Client, codersdk.User) {
func CreateAnotherUserMutators(t testing.TB, client *codersdk.Client, organizationID uuid.UUID, roles []rbac.RoleIdentifier, mutators ...func(r *codersdk.CreateUserRequest)) (*codersdk.Client, codersdk.User) {
return createAnotherUserRetry(t, client, organizationID, 5, roles, mutators...)
}

// AuthzUserSubject does not include the user's groups.
func AuthzUserSubject(user codersdk.User, orgID uuid.UUID) rbac.Subject {
roles := make(rbac.RoleNames, 0, len(user.Roles))
roles := make(rbac.RoleIdentifiers, 0, len(user.Roles))
// Member role is always implied
roles = append(roles, rbac.RoleMember())
for _, r := range user.Roles {
roles = append(roles, r.Name)
orgID, _ := uuid.Parse(r.OrganizationID) // defaults to nil
roles = append(roles, rbac.RoleIdentifier{
Name: r.Name,
OrganizationID: orgID,
})
}
// We assume only 1 org exists
roles = append(roles, rbac.ScopedRoleOrgMember(orgID))
Expand All@@ -690,7 +695,7 @@ func AuthzUserSubject(user codersdk.User, orgID uuid.UUID) rbac.Subject {
}
}

func createAnotherUserRetry(t testing.TB, client *codersdk.Client, organizationID uuid.UUID, retries int, roles []string, mutators ...func(r *codersdk.CreateUserRequest)) (*codersdk.Client, codersdk.User) {
func createAnotherUserRetry(t testing.TB, client *codersdk.Client, organizationID uuid.UUID, retries int, roles []rbac.RoleIdentifier, mutators ...func(r *codersdk.CreateUserRequest)) (*codersdk.Client, codersdk.User) {
req := codersdk.CreateUserRequest{
Email: namesgenerator.GetRandomName(10) + "@coder.com",
Username: RandomUsername(t),
Expand DownExpand Up@@ -748,36 +753,37 @@ func createAnotherUserRetry(t testing.TB, client *codersdk.Client, organizationI

if len(roles) > 0 {
// Find the roles for the org vs the site wide roles
orgRoles := make(map[string][]string)
var siteRoles []string
orgRoles := make(map[uuid.UUID][]rbac.RoleIdentifier)
var siteRoles []rbac.RoleIdentifier

for _, roleName := range roles {
roleName := roleName
orgID, ok := rbac.IsOrgRole(roleName)
roleName, _, err = rbac.RoleSplit(roleName)
require.NoError(t, err, "split org role name")
ok := roleName.IsOrgRole()
if ok {
roleName, _, err = rbac.RoleSplit(roleName)
require.NoError(t, err, "split rolename")
orgRoles[orgID] = append(orgRoles[orgID], roleName)
orgRoles[roleName.OrganizationID] = append(orgRoles[roleName.OrganizationID], roleName)
} else {
siteRoles = append(siteRoles, roleName)
}
}
// Update the roles
for _, r := range user.Roles {
siteRoles = append(siteRoles, r.Name)
orgID, _ := uuid.Parse(r.OrganizationID)
siteRoles = append(siteRoles, rbac.RoleIdentifier{
Name: r.Name,
OrganizationID: orgID,
})
}

onlyName := func(role rbac.RoleIdentifier) string {
return role.Name
}

user, err = client.UpdateUserRoles(context.Background(), user.ID.String(), codersdk.UpdateRoles{Roles: siteRoles})
user, err = client.UpdateUserRoles(context.Background(), user.ID.String(), codersdk.UpdateRoles{Roles:db2sdk.List(siteRoles, onlyName)})
require.NoError(t, err, "update site roles")

// Update org roles
for orgID, roles := range orgRoles {
organizationID, err := uuid.Parse(orgID)
require.NoError(t, err, fmt.Sprintf("parse org id %q", orgID))
_, err = client.UpdateOrganizationMemberRoles(context.Background(), organizationID, user.ID.String(),
codersdk.UpdateRoles{Roles: roles})
_, err = client.UpdateOrganizationMemberRoles(context.Background(), orgID, user.ID.String(),
codersdk.UpdateRoles{Roles: db2sdk.List(roles, onlyName)})
require.NoError(t, err, "update org membership roles")
}
}
Expand Down
30 changes: 16 additions & 14 deletionscoderd/database/db2sdk/db2sdk.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -170,7 +170,12 @@ func User(user database.User, organizationIDs []uuid.UUID) codersdk.User {
}

for _, roleName := range user.RBACRoles {
rbacRole, err := rbac.RoleByName(roleName)
// TODO: Currently the api only returns site wide roles.
// Should it return organization roles?
rbacRole, err := rbac.RoleByName(rbac.RoleIdentifier{
Name: roleName,
OrganizationID: uuid.Nil,
})
Comment on lines +173 to +178
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

follow-up for TODO?

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I'm not going to do this until we get farther in the organization work.

Right now this api is used for the/users page on the UI. If I were to add it, then organization roles would pop up there, which is probably incorrect behavior.

So this is a genuine question if we should expand the roles returned, and on the UI filter out the organization ones. But at present, showing org roles at all is incorrect as orgs do not really exist.

if err == nil {
convertedUser.Roles = append(convertedUser.Roles, SlimRole(rbacRole))
} else {
Expand DownExpand Up@@ -519,29 +524,26 @@ func ProvisionerDaemon(dbDaemon database.ProvisionerDaemon) codersdk.Provisioner
}

func SlimRole(role rbac.Role) codersdk.SlimRole {
roleName, orgIDStr, err:=rbac.RoleSplit(role.Name)
iferr !=nil {
roleName = role.Name
orgID:=""
ifrole.Identifier.OrganizationID !=uuid.Nil {
orgID = role.Identifier.OrganizationID.String()
}

return codersdk.SlimRole{
DisplayName: role.DisplayName,
Name:roleName,
OrganizationID:orgIDStr,
Name:role.Identifier.Name,
OrganizationID:orgID,
}
}

func RBACRole(role rbac.Role) codersdk.Role {
roleName, orgIDStr, err := rbac.RoleSplit(role.Name)
if err != nil {
roleName = role.Name
}
orgPerms := role.Org[orgIDStr]
slim := SlimRole(role)

orgPerms := role.Org[slim.OrganizationID]
return codersdk.Role{
Name:roleName,
OrganizationID:orgIDStr,
DisplayName:role.DisplayName,
Name:slim.Name,
OrganizationID:slim.OrganizationID,
DisplayName:slim.DisplayName,
SitePermissions: List(role.Site, RBACPermission),
OrganizationPermissions: List(orgPerms, RBACPermission),
UserPermissions: List(role.User, RBACPermission),
Expand Down
6 changes: 3 additions & 3 deletionscoderd/database/dbauthz/customroles_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -35,7 +35,7 @@ func TestUpsertCustomRoles(t *testing.T) {
}

canAssignRole := rbac.Role{
Name:"can-assign",
Identifier:rbac.RoleIdentifier{Name:"can-assign"},
DisplayName: "",
Site: rbac.Permissions(map[string][]policy.Action{
rbac.ResourceAssignRole.Type: {policy.ActionRead, policy.ActionCreate},
Expand All@@ -51,7 +51,7 @@ func TestUpsertCustomRoles(t *testing.T) {
all = append(all, t)
case rbac.ExpandableRoles:
all = append(all, must(t.Expand())...)
casestring:
caserbac.RoleIdentifier:
all = append(all, must(rbac.RoleByName(t)))
default:
panic("unknown type")
Expand DownExpand Up@@ -80,7 +80,7 @@ func TestUpsertCustomRoles(t *testing.T) {
{
// No roles, so no assign role
name: "no-roles",
subject: rbac.RoleNames([]string{}),
subject: rbac.RoleIdentifiers{},
errorContains: "forbidden",
},
{
Expand Down
Loading
Loading
[8]ページ先頭
©2009-2025 Movatter.jp