mirror of
https://github.com/gravitational/teleport
synced 2024-10-22 10:13:21 +00:00
Fix the UI to correctly determine if a user has access to a resource (#9473)
Introduce the GuessIfAccessIsPossible method, so callers may verify if a user has access to a category of resources, instead of access to a specific resource. Fixes a bug where the "Session Recordings" menu doesn't show for users with where-based roles.
This commit is contained in:
parent
30a98d4ccf
commit
f16e9f5c53
|
@ -1799,6 +1799,37 @@ func (set RoleSet) String() string {
|
||||||
return fmt.Sprintf("roles %v", strings.Join(roleNames, ","))
|
return fmt.Sprintf("roles %v", strings.Join(roleNames, ","))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// GuessIfAccessIsPossible guesses if access is possible for an entire category
|
||||||
|
// of resources.
|
||||||
|
// It responds the question: "is it possible that there is a resource of this
|
||||||
|
// kind that the current user can access?".
|
||||||
|
// GuessIfAccessIsPossible is used, mainly, for UI decisions ("should the tab
|
||||||
|
// for resource X appear"?). Most callers should use CheckAccessToRule instead.
|
||||||
|
func (set RoleSet) GuessIfAccessIsPossible(ctx RuleContext, namespace string, resource string, verb string, silent bool) error {
|
||||||
|
// "Where" clause are handled differently by the method:
|
||||||
|
// - "allow" rules have their "where" clause always match, as it's assumed
|
||||||
|
// that there could be a resource that matches it.
|
||||||
|
// - "deny" rules have their "where" clause always fail, as it's assumed that
|
||||||
|
// there could be a resource that passes it.
|
||||||
|
return set.checkAccessToRuleImpl(checkAccessParams{
|
||||||
|
ctx: ctx,
|
||||||
|
namespace: namespace,
|
||||||
|
resource: resource,
|
||||||
|
verb: verb,
|
||||||
|
allowWhere: boolParser(true), // always matches
|
||||||
|
denyWhere: boolParser(false), // never matches
|
||||||
|
silent: silent,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
type boolParser bool
|
||||||
|
|
||||||
|
func (p boolParser) Parse(string) (interface{}, error) {
|
||||||
|
return predicate.BoolPredicate(func() bool {
|
||||||
|
return bool(p)
|
||||||
|
}), nil
|
||||||
|
}
|
||||||
|
|
||||||
// CheckAccessToRule checks if the RoleSet provides access in the given
|
// CheckAccessToRule checks if the RoleSet provides access in the given
|
||||||
// namespace to the specified resource and verb.
|
// namespace to the specified resource and verb.
|
||||||
// silent controls whether the access violations are logged.
|
// silent controls whether the access violations are logged.
|
||||||
|
@ -1807,35 +1838,58 @@ func (set RoleSet) CheckAccessToRule(ctx RuleContext, namespace string, resource
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return trace.Wrap(err)
|
return trace.Wrap(err)
|
||||||
}
|
}
|
||||||
actionsParser, err := NewActionsParser(ctx)
|
|
||||||
|
return set.checkAccessToRuleImpl(checkAccessParams{
|
||||||
|
ctx: ctx,
|
||||||
|
namespace: namespace,
|
||||||
|
resource: resource,
|
||||||
|
verb: verb,
|
||||||
|
allowWhere: whereParser,
|
||||||
|
denyWhere: whereParser,
|
||||||
|
silent: silent,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
type checkAccessParams struct {
|
||||||
|
ctx RuleContext
|
||||||
|
namespace string
|
||||||
|
resource string
|
||||||
|
verb string
|
||||||
|
allowWhere, denyWhere predicate.Parser
|
||||||
|
silent bool
|
||||||
|
}
|
||||||
|
|
||||||
|
func (set RoleSet) checkAccessToRuleImpl(p checkAccessParams) error {
|
||||||
|
actionsParser, err := NewActionsParser(p.ctx)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return trace.Wrap(err)
|
return trace.Wrap(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// check deny: a single match on a deny rule prohibits access
|
// check deny: a single match on a deny rule prohibits access
|
||||||
for _, role := range set {
|
for _, role := range set {
|
||||||
matchNamespace, _ := MatchNamespace(role.GetNamespaces(types.Deny), types.ProcessNamespace(namespace))
|
matchNamespace, _ := MatchNamespace(role.GetNamespaces(types.Deny), types.ProcessNamespace(p.namespace))
|
||||||
if matchNamespace {
|
if matchNamespace {
|
||||||
matched, err := MakeRuleSet(role.GetRules(types.Deny)).Match(whereParser, actionsParser, resource, verb)
|
matched, err := MakeRuleSet(role.GetRules(types.Deny)).Match(p.denyWhere, actionsParser, p.resource, p.verb)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return trace.Wrap(err)
|
return trace.Wrap(err)
|
||||||
}
|
}
|
||||||
if matched {
|
if matched {
|
||||||
if !silent {
|
if !p.silent {
|
||||||
log.WithFields(log.Fields{
|
log.WithFields(log.Fields{
|
||||||
trace.Component: teleport.ComponentRBAC,
|
trace.Component: teleport.ComponentRBAC,
|
||||||
}).Infof("Access to %v %v in namespace %v denied to %v: deny rule matched.",
|
}).Infof("Access to %v %v in namespace %v denied to %v: deny rule matched.",
|
||||||
verb, resource, namespace, role.GetName())
|
p.verb, p.resource, p.namespace, role.GetName())
|
||||||
}
|
}
|
||||||
return trace.AccessDenied("access denied to perform action %q on %q", verb, resource)
|
return trace.AccessDenied("access denied to perform action %q on %q", p.verb, p.resource)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// check allow: if rule matches, grant access to resource
|
// check allow: if rule matches, grant access to resource
|
||||||
for _, role := range set {
|
for _, role := range set {
|
||||||
matchNamespace, _ := MatchNamespace(role.GetNamespaces(types.Allow), types.ProcessNamespace(namespace))
|
matchNamespace, _ := MatchNamespace(role.GetNamespaces(types.Allow), types.ProcessNamespace(p.namespace))
|
||||||
if matchNamespace {
|
if matchNamespace {
|
||||||
match, err := MakeRuleSet(role.GetRules(types.Allow)).Match(whereParser, actionsParser, resource, verb)
|
match, err := MakeRuleSet(role.GetRules(types.Allow)).Match(p.allowWhere, actionsParser, p.resource, p.verb)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return trace.Wrap(err)
|
return trace.Wrap(err)
|
||||||
}
|
}
|
||||||
|
@ -1845,13 +1899,13 @@ func (set RoleSet) CheckAccessToRule(ctx RuleContext, namespace string, resource
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if !silent {
|
if !p.silent {
|
||||||
log.WithFields(log.Fields{
|
log.WithFields(log.Fields{
|
||||||
trace.Component: teleport.ComponentRBAC,
|
trace.Component: teleport.ComponentRBAC,
|
||||||
}).Infof("Access to %v %v in namespace %v denied to %v: no allow rule matched.",
|
}).Infof("Access to %v %v in namespace %v denied to %v: no allow rule matched.",
|
||||||
verb, resource, namespace, set)
|
p.verb, p.resource, p.namespace, set)
|
||||||
}
|
}
|
||||||
return trace.AccessDenied("access denied to perform action %q on %q", verb, resource)
|
return trace.AccessDenied("access denied to perform action %q on %q", p.verb, p.resource)
|
||||||
}
|
}
|
||||||
|
|
||||||
// ExtractConditionForIdentifier returns a restrictive filter expression
|
// ExtractConditionForIdentifier returns a restrictive filter expression
|
||||||
|
|
|
@ -1491,6 +1491,204 @@ func TestCheckRuleAccess(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestGuessIfAccessIsPossible(t *testing.T) {
|
||||||
|
// Examples from https://goteleport.com/docs/access-controls/reference/#rbac-for-sessions.
|
||||||
|
ownSessions, err := types.NewRole("own-sessions", types.RoleSpecV4{
|
||||||
|
Allow: types.RoleConditions{
|
||||||
|
Rules: []types.Rule{
|
||||||
|
{
|
||||||
|
Resources: []string{types.KindSession},
|
||||||
|
Verbs: []string{types.VerbList, types.VerbRead},
|
||||||
|
Where: "contains(session.participants, user.metadata.name)",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
require.NoError(t, err)
|
||||||
|
ownSSHSessions, err := types.NewRole("own-ssh-sessions", types.RoleSpecV4{
|
||||||
|
Allow: types.RoleConditions{
|
||||||
|
Rules: []types.Rule{
|
||||||
|
{
|
||||||
|
Resources: []string{types.KindSSHSession},
|
||||||
|
Verbs: []string{types.Wildcard},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
Deny: types.RoleConditions{
|
||||||
|
Rules: []types.Rule{
|
||||||
|
{
|
||||||
|
Resources: []string{types.KindSSHSession},
|
||||||
|
Verbs: []string{types.VerbList, types.VerbRead, types.VerbUpdate, types.VerbDelete},
|
||||||
|
Where: "!contains(ssh_session.participants, user.metadata.name)",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
// Simple, all-or-nothing roles.
|
||||||
|
readAllSessions, err := types.NewRole("all-sessions", types.RoleSpecV4{
|
||||||
|
Allow: types.RoleConditions{
|
||||||
|
Rules: []types.Rule{
|
||||||
|
{
|
||||||
|
Resources: []string{types.KindSession},
|
||||||
|
Verbs: []string{types.VerbList, types.VerbRead},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
require.NoError(t, err)
|
||||||
|
allowSSHSessions, err := types.NewRole("all-ssh-sessions", types.RoleSpecV4{
|
||||||
|
Allow: types.RoleConditions{
|
||||||
|
Rules: []types.Rule{
|
||||||
|
{
|
||||||
|
Resources: []string{types.KindSSHSession},
|
||||||
|
Verbs: []string{types.Wildcard},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
require.NoError(t, err)
|
||||||
|
denySSHSessions, err := types.NewRole("deny-ssh-sessions", types.RoleSpecV4{
|
||||||
|
Deny: types.RoleConditions{
|
||||||
|
Rules: []types.Rule{
|
||||||
|
{
|
||||||
|
Resources: []string{types.KindSSHSession},
|
||||||
|
Verbs: []string{types.Wildcard},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
type checkAccessParams struct {
|
||||||
|
ctx Context
|
||||||
|
namespace string
|
||||||
|
resource string
|
||||||
|
verbs []string
|
||||||
|
}
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
roles RoleSet
|
||||||
|
params *checkAccessParams
|
||||||
|
// wantRuleAccess fully evaluates where conditions, used to determine access
|
||||||
|
// to specific resources.
|
||||||
|
wantRuleAccess bool
|
||||||
|
// wantGuessAccess doesn't evaluate where conditions, used to determine
|
||||||
|
// access to a category of resources.
|
||||||
|
wantGuessAccess bool
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "global session list/read allowed",
|
||||||
|
roles: RoleSet{readAllSessions},
|
||||||
|
params: &checkAccessParams{
|
||||||
|
namespace: apidefaults.Namespace,
|
||||||
|
resource: types.KindSession,
|
||||||
|
verbs: []string{types.VerbList, types.VerbRead},
|
||||||
|
},
|
||||||
|
wantRuleAccess: true,
|
||||||
|
wantGuessAccess: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "own session list/read allowed",
|
||||||
|
roles: RoleSet{ownSessions}, // allowed despite "where" clause in allow rules
|
||||||
|
params: &checkAccessParams{
|
||||||
|
namespace: apidefaults.Namespace,
|
||||||
|
resource: types.KindSession,
|
||||||
|
verbs: []string{types.VerbList, types.VerbRead},
|
||||||
|
},
|
||||||
|
wantRuleAccess: false, // where condition needs specific resource
|
||||||
|
wantGuessAccess: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "session list/read denied",
|
||||||
|
roles: RoleSet{allowSSHSessions, denySSHSessions}, // none mention "session"
|
||||||
|
params: &checkAccessParams{
|
||||||
|
namespace: apidefaults.Namespace,
|
||||||
|
resource: types.KindSession,
|
||||||
|
verbs: []string{types.VerbList, types.VerbRead},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "session write denied",
|
||||||
|
roles: RoleSet{
|
||||||
|
readAllSessions, // readonly
|
||||||
|
allowSSHSessions, denySSHSessions, // none mention "session"
|
||||||
|
},
|
||||||
|
params: &checkAccessParams{
|
||||||
|
namespace: apidefaults.Namespace,
|
||||||
|
resource: types.KindSession,
|
||||||
|
verbs: []string{types.VerbUpdate, types.VerbDelete},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "global SSH session list/read allowed",
|
||||||
|
roles: RoleSet{allowSSHSessions},
|
||||||
|
params: &checkAccessParams{
|
||||||
|
namespace: apidefaults.Namespace,
|
||||||
|
resource: types.KindSSHSession,
|
||||||
|
verbs: []string{types.VerbList, types.VerbRead},
|
||||||
|
},
|
||||||
|
wantRuleAccess: true,
|
||||||
|
wantGuessAccess: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "own SSH session list/read allowed",
|
||||||
|
roles: RoleSet{ownSSHSessions}, // allowed despite "where" clause in deny rules
|
||||||
|
params: &checkAccessParams{
|
||||||
|
namespace: apidefaults.Namespace,
|
||||||
|
resource: types.KindSSHSession,
|
||||||
|
verbs: []string{types.VerbList, types.VerbRead},
|
||||||
|
},
|
||||||
|
wantRuleAccess: false, // where condition needs specific resource
|
||||||
|
wantGuessAccess: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "SSH session list/read denied",
|
||||||
|
roles: RoleSet{
|
||||||
|
allowSSHSessions, ownSSHSessions,
|
||||||
|
denySSHSessions, // unconditional deny, takes precedence
|
||||||
|
},
|
||||||
|
params: &checkAccessParams{
|
||||||
|
namespace: apidefaults.Namespace,
|
||||||
|
resource: types.KindSSHSession,
|
||||||
|
verbs: []string{types.VerbCreate, types.VerbList, types.VerbRead, types.VerbUpdate, types.VerbDelete},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "SSH session list/read denied - different role ordering",
|
||||||
|
roles: RoleSet{
|
||||||
|
allowSSHSessions,
|
||||||
|
denySSHSessions, // unconditional deny, takes precedence
|
||||||
|
ownSSHSessions,
|
||||||
|
},
|
||||||
|
params: &checkAccessParams{
|
||||||
|
namespace: apidefaults.Namespace,
|
||||||
|
resource: types.KindSSHSession,
|
||||||
|
verbs: []string{types.VerbCreate, types.VerbList, types.VerbRead, types.VerbUpdate, types.VerbDelete},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
for _, test := range tests {
|
||||||
|
t.Run(test.name, func(t *testing.T) {
|
||||||
|
params := test.params
|
||||||
|
const silent = true
|
||||||
|
for _, verb := range params.verbs {
|
||||||
|
err := test.roles.CheckAccessToRule(¶ms.ctx, params.namespace, params.resource, verb, silent)
|
||||||
|
if gotAccess, wantAccess := err == nil, test.wantRuleAccess; gotAccess != wantAccess {
|
||||||
|
t.Errorf("CheckAccessToRule(verb=%q) returned err = %v=q, wantAccess = %v", verb, err, wantAccess)
|
||||||
|
}
|
||||||
|
|
||||||
|
err = test.roles.GuessIfAccessIsPossible(¶ms.ctx, params.namespace, params.resource, verb, silent)
|
||||||
|
if gotAccess, wantAccess := err == nil, test.wantGuessAccess; gotAccess != wantAccess {
|
||||||
|
t.Errorf("GuessIfAccessIsPossible(verb=%q) returned err = %q, wantAccess = %v", verb, err, wantAccess)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestCheckRuleSorting(t *testing.T) {
|
func TestCheckRuleSorting(t *testing.T) {
|
||||||
testCases := []struct {
|
testCases := []struct {
|
||||||
name string
|
name string
|
||||||
|
|
|
@ -149,14 +149,12 @@ func getWindowsDesktopLogins(roleSet services.RoleSet) []string {
|
||||||
|
|
||||||
func hasAccess(roleSet services.RoleSet, ctx *services.Context, kind string, verbs ...string) bool {
|
func hasAccess(roleSet services.RoleSet, ctx *services.Context, kind string, verbs ...string) bool {
|
||||||
for _, verb := range verbs {
|
for _, verb := range verbs {
|
||||||
// Since this check occurs often and it does not imply the caller is trying
|
// Since this check occurs often and does not imply the caller is trying to
|
||||||
// to access any resource, silence any logging done on the proxy.
|
// access any resource, silence any logging done on the proxy.
|
||||||
err := roleSet.CheckAccessToRule(ctx, apidefaults.Namespace, kind, verb, true)
|
if err := roleSet.GuessIfAccessIsPossible(ctx, apidefaults.Namespace, kind, verb, true); err != nil {
|
||||||
if err != nil {
|
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue