Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
15 changes: 13 additions & 2 deletions lib/services/access_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -792,10 +792,21 @@ func (a *accessChecker) EnumerateEntities(resource AccessCheckable, listFn roleE
wildcardAllowed := false
wildcardDenied := false

// Only append allowed entries and update wildcardAllowed if the role
// allows the resource without any matcher. In the real CheckAccess,
// RoleMatchers(matchers).MatchAll(role, types.Allow) is only run when
// namespace and label matching passes on this resource. Checking
// if the role allows the resource without any matcher confirms
// namespace and label matching has passed.
var resourceAllowedByRole bool
if err := NewRoleSet(role).checkAccess(resource, a.info.Traits, AccessState{MFAVerified: true}); err == nil {
resourceAllowedByRole = true
}

for _, e := range listFn(role, types.Allow) {
if e == types.Wildcard {
wildcardAllowed = true
} else {
} else if resourceAllowedByRole {
entities = append(entities, e)
}
}
Expand All @@ -810,7 +821,7 @@ func (a *accessChecker) EnumerateEntities(resource AccessCheckable, listFn roleE

result.wildcardDenied = result.wildcardDenied || wildcardDenied

if err := NewRoleSet(role).checkAccess(resource, a.info.Traits, AccessState{MFAVerified: true}); err == nil {
if resourceAllowedByRole {
result.wildcardAllowed = result.wildcardAllowed || wildcardAllowed
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/services/role.go
Original file line number Diff line number Diff line change
Expand Up @@ -2586,7 +2586,7 @@ type AccessCheckable interface {

var rbacLogger = logutils.NewPackageLogger(teleport.ComponentKey, teleport.ComponentRBAC)

// resourceRequiresLabelMatching decides if a resource requires lapel matching
// resourceRequiresLabelMatching decides if a resource requires label matching
// when making RBAC access decisions.
func resourceRequiresLabelMatching(r AccessCheckable) bool {
// Some resources do not need label matching when assessing whether the user
Expand Down
31 changes: 29 additions & 2 deletions lib/services/role_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4533,6 +4533,18 @@ func TestRoleSetEnumerateDatabaseUsersAndNames(t *testing.T) {
},
}

roleSales := &types.RoleV6{
Metadata: types.Metadata{Name: "sales-prod", Namespace: apidefaults.Namespace},
Spec: types.RoleSpecV6{
Allow: types.RoleConditions{
Namespaces: []string{apidefaults.Namespace},
DatabaseLabels: types.Labels{"env": []string{"sales"}},
DatabaseUsers: []string{"sales"},
DatabaseNames: []string{"sales"},
},
},
}

roleNoDBAccess := &types.RoleV6{
Metadata: types.Metadata{Name: "no_db_access", Namespace: apidefaults.Namespace},
Spec: types.RoleSpecV6{
Expand Down Expand Up @@ -4603,12 +4615,12 @@ func TestRoleSetEnumerateDatabaseUsersAndNames(t *testing.T) {
roles: RoleSet{roleDevStage, roleDevProd},
server: dbStage,
enumDBUserResult: EnumerationResult{
allowedDeniedMap: map[string]bool{"dev": true, "root": false},
allowedDeniedMap: map[string]bool{"root": false},
wildcardAllowed: true,
wildcardDenied: false,
},
enumDBNameResult: EnumerationResult{
allowedDeniedMap: map[string]bool{"dev": true, "root": false},
allowedDeniedMap: map[string]bool{"root": false},
wildcardAllowed: true,
wildcardDenied: false,
},
Expand Down Expand Up @@ -4658,6 +4670,21 @@ func TestRoleSetEnumerateDatabaseUsersAndNames(t *testing.T) {
wildcardDenied: false,
},
},
{
name: "role sales does not match the resource and should be skipped",
roles: RoleSet{roleSales, roleDevProd},
server: dbProd,
enumDBUserResult: EnumerationResult{
allowedDeniedMap: map[string]bool{"dev": true},
wildcardAllowed: false,
wildcardDenied: false,
},
enumDBNameResult: EnumerationResult{
allowedDeniedMap: map[string]bool{"dev": true},
wildcardAllowed: false,
wildcardDenied: false,
},
},
}
for _, tc := range testCases {
accessChecker := makeAccessCheckerWithRoleSet(tc.roles)
Expand Down
4 changes: 2 additions & 2 deletions tool/tsh/common/tsh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5113,10 +5113,10 @@ func TestListDatabasesWithUsers(t *testing.T) {
roles: services.RoleSet{roleDevStage, roleDevProd},
database: dbStage,
wantUsers: &dbUsers{
Allowed: []string{"*", "dev"},
Allowed: []string{"*"},
Denied: []string{"superuser"},
},
wantText: "[* dev], except: [superuser]",
wantText: "[*], except: [superuser]",
},
{
name: "developer allowed only specific username/database in prod database",
Expand Down
Loading