Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v17] Adds RBAC support for Identity Center Accounts #49978

Merged
merged 2 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
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
Loading