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: audit custom role changes#13479

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

Closed
Emyrk wants to merge3 commits intomainfromstevenmasley/custom_role_audit_rebase
Closed
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
3 changes: 2 additions & 1 deletioncoderd/audit/diff.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -21,7 +21,8 @@ type Auditable interface {
database.AuditOAuthConvertState |
database.HealthSettings |
database.OAuth2ProviderApp |
database.OAuth2ProviderAppSecret
database.OAuth2ProviderAppSecret |
database.CustomRole
}

// Map is a map of changed fields in an audited resource. It maps field names to
Expand Down
8 changes: 8 additions & 0 deletionscoderd/audit/request.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -103,6 +103,8 @@ func ResourceTarget[T Auditable](tgt T) string {
return typed.Name
case database.OAuth2ProviderAppSecret:
return typed.DisplaySecret
case database.CustomRole:
return typed.Name
default:
panic(fmt.Sprintf("unknown resource %T for ResourceTarget", tgt))
}
Expand DownExpand Up@@ -140,6 +142,8 @@ func ResourceID[T Auditable](tgt T) uuid.UUID {
return typed.ID
case database.OAuth2ProviderAppSecret:
return typed.ID
case database.CustomRole:
return typed.ID
default:
panic(fmt.Sprintf("unknown resource %T for ResourceID", tgt))
}
Expand DownExpand Up@@ -175,6 +179,8 @@ func ResourceType[T Auditable](tgt T) database.ResourceType {
return database.ResourceTypeOauth2ProviderApp
case database.OAuth2ProviderAppSecret:
return database.ResourceTypeOauth2ProviderAppSecret
case database.CustomRole:
return database.ResourceTypeCustomRole
default:
panic(fmt.Sprintf("unknown resource %T for ResourceType", typed))
}
Expand DownExpand Up@@ -211,6 +217,8 @@ func ResourceRequiresOrgID[T Auditable]() bool {
return false
case database.OAuth2ProviderAppSecret:
return false
case database.CustomRole:
return true
default:
panic(fmt.Sprintf("unknown resource %T for ResourceRequiresOrgID", tgt))
}
Expand Down
2 changes: 2 additions & 0 deletionscoderd/coderdtest/coderdtest.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -758,6 +758,8 @@ func createAnotherUserRetry(t testing.TB, client *codersdk.Client, organizationI
roleName, _, err = rbac.RoleSplit(roleName)
require.NoError(t, err, "split org role name")
if ok {
roleName, _, err = rbac.RoleSplit(roleName)
require.NoError(t, err, "split rolename")
orgRoles[orgID] = append(orgRoles[orgID], roleName)
} else {
siteRoles = append(siteRoles, roleName)
Expand Down
15 changes: 14 additions & 1 deletioncoderd/database/dbauthz/customroles_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -38,7 +38,7 @@ func TestUpsertCustomRoles(t *testing.T) {
Name: "can-assign",
DisplayName: "",
Site: rbac.Permissions(map[string][]policy.Action{
rbac.ResourceAssignRole.Type: {policy.ActionCreate},
rbac.ResourceAssignRole.Type: {policy.ActionRead, policy.ActionCreate},
}),
}

Expand DownExpand Up@@ -243,6 +243,19 @@ func TestUpsertCustomRoles(t *testing.T) {
require.ErrorContains(t, err, tc.errorContains)
} else {
require.NoError(t, err)

roles, err := az.CustomRoles(ctx, database.CustomRolesParams{
LookupRoles: []database.NameOrganizationPair{
{
Name: "test-role",
OrganizationID: tc.organizationID.UUID,
},
},
ExcludeOrgRoles: false,
OrganizationID: uuid.UUID{},
})
require.NoError(t, err)
require.Len(t, roles, 1)
}
})
}
Expand Down
15 changes: 10 additions & 5 deletionscoderd/database/dbmem/dbmem.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1186,12 +1186,16 @@ func (q *FakeQuerier) CustomRoles(_ context.Context, arg database.CustomRolesPar
for _, role := range q.data.customRoles {
role := role
if len(arg.LookupRoles) > 0 {
if !slices.ContainsFunc(arg.LookupRoles, func(s string) bool {
roleName := rbac.RoleName(role.Name, "")
if role.OrganizationID.UUID != uuid.Nil {
roleName = rbac.RoleName(role.Name, role.OrganizationID.UUID.String())
if !slices.ContainsFunc(arg.LookupRoles, func(s database.NameOrganizationPair) bool {
if !strings.EqualFold(s.Name, role.Name) {
return false
}
return strings.EqualFold(s, roleName)

if s.OrganizationID == uuid.Nil {
return true
}

return s.OrganizationID == arg.OrganizationID
}) {
continue
}
Expand DownExpand Up@@ -8405,6 +8409,7 @@ func (q *FakeQuerier) UpsertCustomRole(_ context.Context, arg database.UpsertCus
}

role := database.CustomRole{
ID: uuid.New(),
Name: arg.Name,
DisplayName: arg.DisplayName,
OrganizationID: arg.OrganizationID,
Expand Down
8 changes: 6 additions & 2 deletionscoderd/database/dump.sql
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
DROP INDEX idx_custom_roles_id;
ALTER TABLE custom_roles DROP COLUMN id;
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
-- A role does not need to belong to an organization
ALTER TABLE custom_roles ALTER COLUMN organization_id DROP NOT NULL;

-- (name) is the primary key, this column is almost exclusively for auditing.
ALTER TABLE custom_roles ADD COLUMN id uuid DEFAULT gen_random_uuid() NOT NULL;

-- Ensure unique uuids.
CREATE INDEX idx_custom_roles_id ON custom_roles (id);
ALTER TYPE resource_type ADD VALUE IF NOT EXISTS 'custom_role';
8 changes: 6 additions & 2 deletionscoderd/database/models.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

2 changes: 1 addition & 1 deletioncoderd/database/querier.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

21 changes: 10 additions & 11 deletionscoderd/database/queries.sql.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

7 changes: 2 additions & 5 deletionscoderd/database/queries/roles.sql
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,11 +7,8 @@ WHERE
true
-- Lookup roles filter expects the role names to be in the rbac package
-- format. Eg: name[:<organization_id>]
AND CASE WHEN array_length(@lookup_roles :: text[], 1) > 0 THEN
-- Case insensitive lookup with org_id appended (if non-null).
-- This will return just the name if org_id is null. It'll append
-- the org_id if not null
concat(name, NULLIF(concat(':', organization_id), ':')) ILIKE ANY(@lookup_roles :: text [])
AND CASE WHEN array_length(@lookup_roles :: name_organization_pair_list[], 1) > 0 THEN
(name, organization_id) ILIKE ANY (@lookup_roles::name_organization_pair_list[])
ELSE true
END
-- Org scoping filter, to only fetch site wide roles
Expand Down
3 changes: 3 additions & 0 deletionscoderd/database/sqlc.yaml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -28,6 +28,9 @@ sql:
emit_enum_valid_method: true
emit_all_enum_values: true
overrides:
- db_type: "name_organization_pair_list"
go_type:
type: "NameOrganizationPair"
- column: "custom_roles.site_permissions"
go_type:
type: "CustomRolePermissions"
Expand Down
20 changes: 20 additions & 0 deletionscoderd/database/types.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -113,6 +113,26 @@ func (m StringMapOfInt) Value() (driver.Value, error) {
return json.Marshal(m)
}

// NameOrganizationPair is used as a lookup tuple for custom role rows.
type NameOrganizationPair struct {
Name string `db:"name" json:"name"`
// OrganizationID if unset will assume a null column value
OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"`
}

func (a *NameOrganizationPair) Scan(src interface{}) error {
return xerrors.Errorf("unexpected type %T", src)
}

func (a NameOrganizationPair) Value() (driver.Value, error) {
var orgID interface{} = a.OrganizationID
if a.OrganizationID == uuid.Nil {
orgID = nil
}

return []interface{}{a.Name, orgID}, nil
}

type CustomRolePermissions []CustomRolePermission

func (a *CustomRolePermissions) Scan(src interface{}) error {
Expand Down
25 changes: 24 additions & 1 deletioncoderd/rbac/rolestore/rolestore.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -69,11 +69,34 @@ func Expand(ctx context.Context, db database.Store, names []string) (rbac.Roles,
}

if len(lookup) > 0 {
// The set of roles coming in are formatted as 'rolename[:<org_id>]'.
// In the database, org roles are scoped with an organization column.
lookupArgs := make([]database.NameOrganizationPair, 0, len(lookup))
for _, name := range lookup {
roleName, orgID, err := rbac.RoleSplit(name)
if err != nil {
continue
}

parsedOrgID := uuid.Nil // Default to no org ID
if orgID != "" {
parsedOrgID, err = uuid.Parse(orgID)
if err != nil {
continue
}
}

lookupArgs = append(lookupArgs, database.NameOrganizationPair{
Name: roleName,
OrganizationID: parsedOrgID,
})
}

// If some roles are missing from the database, they are omitted from
// the expansion. These roles are no-ops. Should we raise some kind of
// warning when this happens?
dbroles, err := db.CustomRoles(ctx, database.CustomRolesParams{
LookupRoles:lookup,
LookupRoles:lookupArgs,
ExcludeOrgRoles: false,
OrganizationID: uuid.Nil,
})
Expand Down
6 changes: 3 additions & 3 deletionscoderd/roles.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -20,12 +20,12 @@ import (
// roles. Ideally only included in the enterprise package, but the routes are
// intermixed with AGPL endpoints.
type CustomRoleHandler interface {
PatchOrganizationRole(ctx context.Context,db database.Store, rwhttp.ResponseWriter, orgID uuid.UUID, role codersdk.Role) (codersdk.Role, bool)
PatchOrganizationRole(ctx context.Context,rw http.ResponseWriter, r *http.Request, orgID uuid.UUID, role codersdk.Role) (codersdk.Role, bool)
}

type agplCustomRoleHandler struct{}

func (agplCustomRoleHandler) PatchOrganizationRole(ctx context.Context,_ database.Store, rwhttp.ResponseWriter, _ uuid.UUID, _ codersdk.Role) (codersdk.Role, bool) {
func (agplCustomRoleHandler) PatchOrganizationRole(ctx context.Context,rw http.ResponseWriter, _ *http.Request, _ uuid.UUID, _ codersdk.Role) (codersdk.Role, bool) {
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
Message: "Creating and updating custom roles is an Enterprise feature. Contact sales!",
})
Expand DownExpand Up@@ -54,7 +54,7 @@ func (api *API) patchOrgRoles(rw http.ResponseWriter, r *http.Request) {
return
}

updated, ok := handler.PatchOrganizationRole(ctx,api.Database, rw, organization.ID, req)
updated, ok := handler.PatchOrganizationRole(ctx,rw, r, organization.ID, req)
if !ok {
return
}
Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp