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

fix: improve RBAC scope allow list handling for create actions#20008

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

Open
ThomasK33 wants to merge1 commit intothomask33/09-26-add_token_scope_support_in_cli
base:thomask33/09-26-add_token_scope_support_in_cli
Choose a base branch
Loading
fromthomask33/09-29-feat_typed_rbac_allow_list
Open
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
2 changes: 1 addition & 1 deletioncoderd/database/modelmethods_internal_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -93,7 +93,7 @@ func TestAPIKeyScopesExpand(t *testing.T) {
expanded,err:=effective.Expand()
require.NoError(t,err)
require.Len(t,expanded.AllowIDList,1)
require.Equal(t,"workspace",expanded.AllowIDList[0].Type)
require.Equal(t,rbac.ResourceWorkspace.Type,expanded.AllowIDList[0].Type)
require.Equal(t,workspaceID.String(),expanded.AllowIDList[0].ID)
})

Expand Down
2 changes: 1 addition & 1 deletioncoderd/database/querier_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -4928,7 +4928,7 @@ func createPrebuiltWorkspace(
dbgen.WorkspaceBuild(t,db, database.WorkspaceBuild{
CreatedAt:createdAt,
WorkspaceID:workspace.ID,
TemplateVersionID:extTmplVersion.ID,
TemplateVersionID:extTmplVersion.TemplateVersion.ID,
BuildNumber:1,
Transition:database.WorkspaceTransitionStart,
InitiatorID:tmpl.CreatedBy,
Expand Down
179 changes: 170 additions & 9 deletionscoderd/rbac/authz_internal_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1000,6 +1000,8 @@ func TestAuthorizeScope(t *testing.T) {
},
)

createScopeWorkspaceID := uuid.New()

// This scope can only create workspaces
user = Subject{
ID: "me",
Expand All@@ -1012,15 +1014,15 @@ func TestAuthorizeScope(t *testing.T) {
Identifier: RoleIdentifier{Name: "create_workspace"},
DisplayName: "Create Workspace",
Site: Permissions(map[string][]policy.Action{
// Onlyread access for workspaces.
// Onlycreate access for workspaces.
ResourceWorkspace.Type: {policy.ActionCreate},
}),
Org: map[string][]Permission{},
User: []Permission{},
},
//Empty string allow_list is allowed for actions like 'create'
//Specific IDs still allow creation; reads require matching IDs.
AllowIDList: []AllowListElement{{
Type: ResourceWorkspace.Type, ID:"",
Type: ResourceWorkspace.Type, ID:createScopeWorkspaceID.String(),
}},
},
}
Expand DownExpand Up@@ -1190,8 +1192,7 @@ func TestScopeAllowList(t *testing.T) {
Site: allPermsExcept(ResourceUser),
},
AllowIDList: []AllowListElement{
{Type: ResourceWorkspace.Type, ID: wid.String()},
{Type: ResourceWorkspace.Type, ID: ""}, // Allow to create
{Type: ResourceWorkspace.Type, ID: wid.String()}, // create bypass handled by policy
{Type: ResourceTemplate.Type, ID: policy.WildcardSymbol},
{Type: ResourceGroup.Type, ID: gid.String()},

Expand DownExpand Up@@ -1219,6 +1220,7 @@ func TestScopeAllowList(t *testing.T) {

// Group
{resource: ResourceGroup.InOrg(defOrg).WithID(gid), actions: []policy.Action{policy.ActionRead}},
{resource: ResourceGroup.InOrg(defOrg), actions: []policy.Action{policy.ActionCreate}},
},
),

Expand All@@ -1233,13 +1235,40 @@ func TestScopeAllowList(t *testing.T) {

// `wid` matches on the uuid, but not the type
{resource: ResourceGroup.WithID(wid), actions: []policy.Action{policy.ActionRead}},

// no empty id for the create action
{resource: ResourceGroup.InOrg(defOrg), actions: []policy.Action{policy.ActionCreate}},
},
),
)

t.Run("create requires matching type entry", func(t *testing.T) {
t.Parallel()

subject := Subject{
ID: "me",
Roles: Roles{must(RoleByName(RoleOwner()))},
Scope: Scope{
Role: Role{
Identifier: RoleIdentifier{Name: "AllowListNoWorkspace"},
Site: Permissions(map[string][]policy.Action{
ResourceWorkspace.Type: {policy.ActionCreate},
}),
},
AllowIDList: []AllowListElement{{
Type: ResourceTemplate.Type, ID: uuid.NewString(),
}},
},
}

testAuthorize(t, "CreateRequiresMatchingType", subject,
[]authTestCase{
{
resource: ResourceWorkspace.InOrg(defOrg).WithOwner(subject.ID),
actions: []policy.Action{policy.ActionCreate},
allow: false,
},
},
)
})

// Wildcard type
user = Subject{
ID: "me",
Expand DownExpand Up@@ -1271,6 +1300,7 @@ func TestScopeAllowList(t *testing.T) {
[]authTestCase{
// anything with the id is ok
{resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID).WithID(wid), actions: []policy.Action{policy.ActionRead}},
{resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), actions: []policy.Action{policy.ActionCreate}},
{resource: ResourceGroup.InOrg(defOrg).WithID(wid), actions: []policy.Action{policy.ActionRead}},
{resource: ResourceTemplate.InOrg(defOrg).WithID(wid), actions: []policy.Action{policy.ActionRead}},
},
Expand All@@ -1283,13 +1313,144 @@ func TestScopeAllowList(t *testing.T) {
},
[]authTestCase{
// Anything without the id is not allowed
{resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID), actions: []policy.Action{policy.ActionCreate}},
{resource: ResourceWorkspace.InOrg(defOrg).WithOwner(user.ID).WithID(uuid.New()), actions: []policy.Action{policy.ActionRead}},
},
),
)
}

func TestScopeAllowListFilter(t *testing.T) {
t.Parallel()

workspaceID := uuid.New()
otherWorkspace := uuid.New()
defOrg := uuid.New()

subject := Subject{
ID: "me",
Roles: Roles{must(RoleByName(RoleOwner()))},
Scope: Scope{
Role: Role{
Identifier: RoleIdentifier{Name: "AllowListFilter"},
Site: Permissions(map[string][]policy.Action{
ResourceWorkspace.Type: {policy.ActionRead},
}),
},
AllowIDList: []AllowListElement{
{Type: ResourceWorkspace.Type, ID: workspaceID.String()},
},
},
}

allowed := ResourceWorkspace.WithID(workspaceID).InOrg(defOrg).WithOwner(subject.ID)
denied := ResourceWorkspace.WithID(otherWorkspace).InOrg(defOrg).WithOwner(subject.ID)

ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
defer cancel()

auth := NewAuthorizer(prometheus.NewRegistry())
filtered, err := Filter(ctx, auth, subject, policy.ActionRead, []Object{allowed, denied})
require.NoError(t, err)
require.Len(t, filtered, 1)
require.Equal(t, workspaceID.String(), filtered[0].ID)
}

func TestAuthorizeRequiresScope(t *testing.T) {
t.Parallel()

subject := Subject{
ID: "me",
Roles: Roles{must(RoleByName(RoleOwner()))},
}

auth := NewAuthorizer(prometheus.NewRegistry())
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
defer cancel()

err := auth.Authorize(ctx, subject, policy.ActionRead, ResourceWorkspace.WithID(uuid.New()))
require.Error(t, err)
}

func TestScopeMetricsCounters(t *testing.T) {
t.Parallel()

createSubject := func(allowList []AllowListElement) Subject {
role := Role{
Identifier: RoleIdentifier{Name: "metrics-role"},
Site: []Permission{{
ResourceType: ResourceWorkspace.Type,
Action: policy.ActionRead,
}},
}
scope := Scope{
Role: Role{
Identifier: RoleIdentifier{Name: "scope-metrics"},
Site: []Permission{{
ResourceType: ResourceWorkspace.Type,
Action: policy.ActionRead,
}},
},
AllowIDList: allowList,
}
return Subject{
ID: uuid.NewString(),
Roles: Roles{role},
Groups: []string{},
Scope: scope,
}
}

t.Run("scope allow", func(t *testing.T) {
t.Parallel()

reg := prometheus.NewRegistry()
auth := NewAuthorizer(reg)

subject := createSubject([]AllowListElement{AllowListAll()})
obj := ResourceWorkspace.WithID(uuid.New())

require.NoError(t, auth.Authorize(context.Background(), subject, policy.ActionRead, obj))

metrics, err := reg.Gather()
require.NoError(t, err)

require.True(t, testutil.PromCounterHasValue(t, metrics, 1,
"coderd_authz_scope_enforcement_total",
"scope_allow", "scope_metrics", ResourceWorkspace.Type, "allow",
))
require.False(t, testutil.PromCounterGathered(t, metrics,
"coderd_authz_scope_allowlist_miss_total",
"scope_metrics", ResourceWorkspace.Type,
))
})

t.Run("allow-list miss", func(t *testing.T) {
t.Parallel()

reg := prometheus.NewRegistry()
auth := NewAuthorizer(reg)

allowedID := uuid.NewString()
subject := createSubject([]AllowListElement{{Type: ResourceWorkspace.Type, ID: allowedID}})
obj := ResourceWorkspace.WithID(uuid.New())

err := auth.Authorize(context.Background(), subject, policy.ActionRead, obj)
require.Error(t, err)

metrics, gatherErr := reg.Gather()
require.NoError(t, gatherErr)

require.True(t, testutil.PromCounterHasValue(t, metrics, 1,
"coderd_authz_scope_enforcement_total",
"allow_list_deny", "scope_metrics", ResourceWorkspace.Type, "deny",
))
require.True(t, testutil.PromCounterHasValue(t, metrics, 1,
"coderd_authz_scope_allowlist_miss_total",
"scope_metrics", ResourceWorkspace.Type,
))
})
}

// cases applies a given function to all test cases. This makes generalities easier to create.
func cases(opt func(c authTestCase) authTestCase, cases []authTestCase) []authTestCase {
if opt == nil {
Expand Down
20 changes: 20 additions & 0 deletionscoderd/rbac/policy.rego
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -257,9 +257,29 @@ scope_allow_list if {
input.subject.scope.allow_list[_] == {"type": input.object.type, "id": "*"}
}

# Allow create operations when the allow_list grants the resource type (or global wildcard)
# even if the object ID is not yet known.
scope_allow_list if {
input.action == "create"
not {"type": "*", "id": "*"} in input.subject.scope.allow_list
not {"type": input.object.type, "id": "*"} in input.subject.scope.allow_list

input.object.id == ""

allowed_ids := {allowed_id |
# Iterate over all allow list elements matching the object type
ele := input.subject.scope.allow_list[_]
ele.type in [input.object.type, "*"]
allowed_id := ele.id
}

allowed_ids[_]
}

# A comprehension that iterates over the allow_list and checks if the
# (object.type, object.id) is in the allowed ids.
scope_allow_list if {
input.action != "create"
# If the wildcard is listed in the allow_list, we do not care about the
# object.id. This line is included to prevent partial compilations from
# ever needing to include the object.id.
Expand Down
24 changes: 24 additions & 0 deletionscoderd/rbac/scopes_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,6 +3,7 @@ package rbac_test
import (
"testing"

"github.com/google/uuid"
"github.com/stretchr/testify/require"

"github.com/coder/coder/v2/coderd/rbac"
Expand DownExpand Up@@ -61,3 +62,26 @@ func TestExpandScope(t *testing.T) {
}
})
}

funcTestWorkspaceAgentScopeAllowList(t*testing.T) {
t.Parallel()

workspaceID:=uuid.New()
ownerID:=uuid.New()
templateID:=uuid.New()
versionID:=uuid.New()

scope:=rbac.WorkspaceAgentScope(rbac.WorkspaceAgentScopeParams{
WorkspaceID:workspaceID,
OwnerID:ownerID,
TemplateID:templateID,
VersionID:versionID,
})

require.ElementsMatch(t, []rbac.AllowListElement{
{Type:rbac.ResourceWorkspace.Type,ID:workspaceID.String()},
{Type:rbac.ResourceTemplate.Type,ID:templateID.String()},
{Type:rbac.ResourceTemplate.Type,ID:versionID.String()},
{Type:rbac.ResourceUser.Type,ID:ownerID.String()},
},scope.AllowIDList)
}
Loading
[8]ページ先頭
©2009-2025 Movatter.jp