diff --git a/lib/services/access_checker.go b/lib/services/access_checker.go index fb20b76150a12..a655e5363e571 100644 --- a/lib/services/access_checker.go +++ b/lib/services/access_checker.go @@ -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) } } @@ -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 } } diff --git a/lib/services/role.go b/lib/services/role.go index 2b593e728bca2..51e35ab20a120 100644 --- a/lib/services/role.go +++ b/lib/services/role.go @@ -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 diff --git a/lib/services/role_test.go b/lib/services/role_test.go index 84832a1303d62..d9abd16251af0 100644 --- a/lib/services/role_test.go +++ b/lib/services/role_test.go @@ -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{ @@ -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, }, @@ -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) diff --git a/tool/tsh/common/tsh_test.go b/tool/tsh/common/tsh_test.go index 2976bb24c2b19..5ca01a7c4ad12 100644 --- a/tool/tsh/common/tsh_test.go +++ b/tool/tsh/common/tsh_test.go @@ -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",