Skip to content

Commit

Permalink
[v17] Adds RBAC support for Identity Center Accounts (#49978)
Browse files Browse the repository at this point in the history
* [v17] Adds RBAC support for Identity Center Accounts

Backports #49808

 - Adds custom RoleMatchers for Identity Center resources
 - Disables label checking for Identity Center Account resources
 - Adds custom action checker that understands the generic
   `KindIdentityCenter` resource kind, and falls back from the
   requested resource kind to the generic one if not explicitly
   denied.

* Update lib/services/role.go

Co-authored-by: Sakshyam Shah <[email protected]>

---------

Co-authored-by: Sakshyam Shah <[email protected]>
  • Loading branch information
tcsc and flyinghermit authored Dec 10, 2024
1 parent 6029741 commit 3b225bf
Show file tree
Hide file tree
Showing 7 changed files with 588 additions and 46 deletions.
41 changes: 37 additions & 4 deletions lib/auth/auth_with_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,20 @@ func (a *ServerWithRoles) authConnectorAction(namespace string, resource string,
return nil
}

// identityCenterAction is a special checker that grants access to Identity Center
// resources. In order to simplify the writing of role condition statements, the
// various Identity Center resources are bundled up under an umbrella
// `KindIdentityCenter` resource kind. This means that if access to the target
// resource is not explicitly denied, then the user has a second chance to get
// access via the generic resource kind.
func (a *ServerWithRoles) identityCenterAction(namespace string, resource string, verbs ...string) error {
err := a.action(namespace, resource, verbs...)
if err == nil || services.IsAccessExplicitlyDenied(err) {
return trace.Wrap(err)
}
return trace.Wrap(a.action(namespace, types.KindIdentityCenter, verbs...))
}

// actionForListWithCondition extracts a restrictive filter condition to be
// added to a list query after a simple resource check fails.
func (a *ServerWithRoles) actionForListWithCondition(namespace, resource, identifier string) (*types.WhereExpr, error) {
Expand Down Expand Up @@ -1321,6 +1335,17 @@ func (c *resourceAccess) checkAccess(resource types.ResourceWithLabels, filter s
return true, nil
}

type actionChecker func(namespace, resourceKind string, verbs ...string) error

func (a *ServerWithRoles) selectActionChecker(resourceKind string) actionChecker {
if resourceKind == types.KindIdentityCenterAccount {
// Identity Center resources can be specified multiple ways in a Role
// Condition statement, so we need a special checker to handle it.
return a.identityCenterAction
}
return a.action
}

// ListUnifiedResources returns a paginated list of unified resources filtered by user access.
func (a *ServerWithRoles) ListUnifiedResources(ctx context.Context, req *proto.ListUnifiedResourcesRequest) (*proto.ListUnifiedResourcesResponse, error) {
filter := services.MatchResourceFilter{
Expand Down Expand Up @@ -1358,7 +1383,8 @@ func (a *ServerWithRoles) ListUnifiedResources(ctx context.Context, req *proto.L
actionVerbs = []string{types.VerbList}
}

resourceAccess.kindAccessMap[kind] = a.action(apidefaults.Namespace, kind, actionVerbs...)
checkAction := a.selectActionChecker(kind)
resourceAccess.kindAccessMap[kind] = checkAction(apidefaults.Namespace, kind, actionVerbs...)
}

// Before doing any listing, verify that the user is allowed to list
Expand Down Expand Up @@ -1702,7 +1728,8 @@ func (a *ServerWithRoles) ListResources(ctx context.Context, req proto.ListResou
return nil, trace.NotImplemented("resource type %s does not support pagination", req.ResourceType)
}

if err := a.action(req.Namespace, req.ResourceType, actionVerbs...); err != nil {
checkAction := a.selectActionChecker(req.ResourceType)
if err := checkAction(req.Namespace, req.ResourceType, actionVerbs...); err != nil {
return nil, trace.Wrap(err)
}

Expand Down Expand Up @@ -1829,9 +1856,14 @@ func (r resourceChecker) CanAccess(resource types.Resource) error {
}
case types.SAMLIdPServiceProvider:
return r.CheckAccess(rr, state)

case types.Resource153Unwrapper:
if checkable, ok := rr.(services.AccessCheckable); ok {
return r.CheckAccess(checkable, state)
}
}

return trace.BadParameter("could not check access to resource type %T", r)
return trace.BadParameter("could not check access to resource type %T", resource)
}

// newResourceAccessChecker creates a resourceAccessChecker for the provided resource type
Expand All @@ -1846,7 +1878,8 @@ func (a *ServerWithRoles) newResourceAccessChecker(resource string) (resourceAcc
types.KindKubeServer,
types.KindUserGroup,
types.KindUnifiedResource,
types.KindSAMLIdPServiceProvider:
types.KindSAMLIdPServiceProvider,
types.KindIdentityCenterAccount:
return &resourceChecker{AccessChecker: a.context.Checker}, nil
default:
return nil, trace.BadParameter("could not check access to resource type %s", resource)
Expand Down
88 changes: 81 additions & 7 deletions lib/auth/auth_with_roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5768,28 +5768,102 @@ func TestUnifiedResources_IdentityCenter(t *testing.T) {
})
require.NoError(t, err)

t.Run("access denied", func(t *testing.T) {
// Asserts that, with no RBAC or matchers in place, acces to IC Accounts
// is denied by default
setAccountAssignment := func(role types.Role) {
r := role.(*types.RoleV6)
r.Spec.Allow.AccountAssignments = []types.IdentityCenterAccountAssignment{
{
Account: "11111111",
PermissionSet: "some:arn",
},
}
}

userNoAccess, _, err := CreateUserAndRole(srv.Auth(), "test", nil, nil)
t.Run("no access", func(t *testing.T) {
userNoAccess, _, err := CreateUserAndRole(srv.Auth(), "no-access", nil, nil)
require.NoError(t, err)

identity := TestUser(userNoAccess.GetName())
clt, err := srv.NewClient(identity)
require.NoError(t, err)
defer clt.Close()

_, err = clt.ListResources(ctx, proto.ListResourcesRequest{
resp, err := clt.ListResources(ctx, proto.ListResourcesRequest{
ResourceType: types.KindIdentityCenterAccount,
Labels: map[string]string{
types.OriginLabel: apicommon.OriginAWSIdentityCenter,
},
})
require.NoError(t, err)
require.Empty(t, resp.Resources)
})

t.Run("access via generic kind", func(t *testing.T) {
user, _, err := CreateUserAndRole(srv.Auth(), "read-generic", nil,
[]types.Rule{
types.NewRule(types.KindIdentityCenter, services.RO()),
},
WithRoleMutator(setAccountAssignment))
require.NoError(t, err)

identity := TestUser(user.GetName())
clt, err := srv.NewClient(identity)
require.NoError(t, err)
defer clt.Close()

resp, err := clt.ListResources(ctx, proto.ListResourcesRequest{
ResourceType: types.KindIdentityCenterAccount,
Labels: map[string]string{
types.OriginLabel: apicommon.OriginAWSIdentityCenter,
},
})
require.True(t, trace.IsAccessDenied(err))
require.NoError(t, err)
require.Len(t, resp.Resources, 1)
})

t.Run("access via specific kind", func(t *testing.T) {
user, _, err := CreateUserAndRole(srv.Auth(), "read-specific", nil,
[]types.Rule{
types.NewRule(types.KindIdentityCenterAccount, services.RO()),
},
WithRoleMutator(setAccountAssignment))
require.NoError(t, err)

identity := TestUser(user.GetName())
clt, err := srv.NewClient(identity)
require.NoError(t, err)
defer clt.Close()

resp, err := clt.ListResources(ctx, proto.ListResourcesRequest{
ResourceType: types.KindIdentityCenterAccount,
})
require.NoError(t, err)
require.Len(t, resp.Resources, 1)
})

// TODO(tcsc): Add other tests one RBAC implemented
t.Run("denied via specific kind beats allow via generic kind", func(t *testing.T) {
user, _, err := CreateUserAndRole(srv.Auth(), "specific-beats-generic", nil,
[]types.Rule{
types.NewRule(types.KindIdentityCenter, services.RO()),
},
WithRoleMutator(func(r types.Role) {
setAccountAssignment(r)
r.SetRules(types.Deny, []types.Rule{
types.NewRule(types.KindIdentityCenterAccount, services.RO()),
})
}))
require.NoError(t, err)

identity := TestUser(user.GetName())
clt, err := srv.NewClient(identity)
require.NoError(t, err)
defer clt.Close()

_, err = clt.ListResources(ctx, proto.ListResourcesRequest{
ResourceType: types.KindIdentityCenterAccount,
})
require.True(t, trace.IsAccessDenied(err),
"Expected Access Denied, got %v", err)
})
}

func BenchmarkListUnifiedResourcesFilter(b *testing.B) {
Expand Down
12 changes: 12 additions & 0 deletions lib/services/access_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,18 @@ func (a *accessChecker) CheckAccess(r AccessCheckable, state AccessState, matche
if err := a.checkAllowedResources(r); err != nil {
return trace.Wrap(err)
}

switch rr := r.(type) {
case types.Resource153Unwrapper:
switch urr := rr.Unwrap().(type) {
case IdentityCenterAccount:
matchers = append(matchers, NewIdentityCenterAccountMatcher(urr))

case IdentityCenterAccountAssignment:
matchers = append(matchers, NewIdentityCenterAccountAssignmentMatcher(urr))
}
}

return trace.Wrap(a.RoleSet.checkAccess(r, a.info.Traits, state, matchers...))
}

Expand Down
34 changes: 19 additions & 15 deletions lib/services/access_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -2191,19 +2191,32 @@ func (m *RequestValidator) pruneResourceRequestRoles(
necessaryRoles := make(map[string]struct{})
for _, resource := range resources {
var (
rolesForResource []types.Role
resourceMatcher *KubeResourcesMatcher
rolesForResource []types.Role
matchers []RoleMatcher
kubeResourceMatcher *KubeResourcesMatcher
)
kubernetesResources, err := getKubeResourcesFromResourceIDs(resourceIDs, resource.GetName())
if err != nil {
return nil, trace.Wrap(err)
}
if len(kubernetesResources) > 0 {
resourceMatcher = NewKubeResourcesMatcher(kubernetesResources)
kubeResourceMatcher = NewKubeResourcesMatcher(kubernetesResources)
matchers = append(matchers, kubeResourceMatcher)
}

switch rr := resource.(type) {
case types.Resource153Unwrapper:
switch urr := rr.Unwrap().(type) {
case IdentityCenterAccount:
matchers = append(matchers, NewIdentityCenterAccountMatcher(urr))

case IdentityCenterAccountAssignment:
matchers = append(matchers, NewIdentityCenterAccountAssignmentMatcher(urr))
}
}

for _, role := range allRoles {
roleAllowsAccess, err := m.roleAllowsResource(role, resource, loginHint, resourceMatcherToMatcherSlice(resourceMatcher)...)
roleAllowsAccess, err := m.roleAllowsResource(role, resource, loginHint, matchers...)
if err != nil {
return nil, trace.Wrap(err)
}
Expand All @@ -2217,7 +2230,7 @@ func (m *RequestValidator) pruneResourceRequestRoles(
// If any of the requested resources didn't match with the provided roles,
// we deny the request because the user is trying to request more access
// than what is allowed by its search_as_roles.
if resourceMatcher != nil && len(resourceMatcher.Unmatched()) > 0 {
if kubeResourceMatcher != nil && len(kubeResourceMatcher.Unmatched()) > 0 {
resourcesStr, err := types.ResourceIDsToString(resourceIDs)
if err != nil {
return nil, trace.Wrap(err)
Expand All @@ -2226,7 +2239,7 @@ func (m *RequestValidator) pruneResourceRequestRoles(
`no roles configured in the "search_as_roles" for this user allow `+
`access to at least one requested resources. `+
`resources: %s roles: %v unmatched resources: %v`,
resourcesStr, roles, resourceMatcher.Unmatched())
resourcesStr, roles, kubeResourceMatcher.Unmatched())
}
if len(loginHint) > 0 {
// If we have a login hint, request the single role with the fewest
Expand Down Expand Up @@ -2335,15 +2348,6 @@ func (m *RequestValidator) roleAllowsResource(
return true, nil
}

// resourceMatcherToMatcherSlice returns the resourceMatcher in a RoleMatcher slice
// if the resourceMatcher is not nil, otherwise returns a nil slice.
func resourceMatcherToMatcherSlice(resourceMatcher *KubeResourcesMatcher) []RoleMatcher {
if resourceMatcher == nil {
return nil
}
return []RoleMatcher{resourceMatcher}
}

// getUnderlyingResourcesByResourceIDs gets the underlying resources the user
// requested access. Except for resource Kinds present in types.KubernetesResourcesKinds,
// the underlying resources are the same as requested. If the resource requested
Expand Down
Loading

0 comments on commit 3b225bf

Please sign in to comment.