Skip to content

Commit

Permalink
refactor: Export Action fields (#65)
Browse files Browse the repository at this point in the history 
This changes all the fields of Action to be exported so that an Action
object can be created from an external package.

Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
Co-authored-by: Daniel Pacak <pacak.daniel@gmail.com>
  • Loading branch information
@zubron @danielpacak
zubron and danielpacak authored Feb 19, 2020
1 parent 922e198 commit f549a4c
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 110 deletions.
88 changes: 43 additions & 45 deletions pkg/cmd/list.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,15 @@ NONRESOURCEURL is a partial URL that starts with "/".`

// Action represents an action a subject can be given permission to.
type Action struct {
verb string
resource string
resourceName string
subResource string
Verb string
Resource string
ResourceName string
SubResource string

nonResourceURL string
NonResourceURL string

namespace string
allNamespaces bool
Namespace string
AllNamespaces bool
}

type resolvedAction struct {
Expand Down Expand Up @@ -173,8 +173,8 @@ func NewWhoCanCommand(streams clioptions.IOStreams) (*cobra.Command, error) {
},
}

cmd.Flags().String("subresource", "", "SubResource such as pod/log or deployment/scale")
cmd.Flags().BoolP("all-namespaces", "A", false, "If true, check for users that can do the specified action in any of the available namespaces")
cmd.Flags().String(subResourceFlag, "", "SubResource such as pod/log or deployment/scale")
cmd.Flags().BoolP(allNamespacesFlag, "A", false, "If true, check for users that can do the specified action in any of the available namespaces")

flag.CommandLine.VisitAll(func(gf *flag.Flag) {
cmd.Flags().AddGoFlag(gf)
Expand All @@ -192,61 +192,61 @@ func ActionFrom(clientConfig clientcmd.ClientConfig, flags *pflag.FlagSet, args
return
}

action.verb = args[0]
action.Verb = args[0]
if strings.HasPrefix(args[1], "/") {
action.nonResourceURL = args[1]
glog.V(3).Infof("Resolved nonResourceURL `%s`", action.nonResourceURL)
action.NonResourceURL = args[1]
glog.V(3).Infof("Resolved nonResourceURL `%s`", action.NonResourceURL)
} else {
resourceTokens := strings.SplitN(args[1], "/", 2)
action.resource = resourceTokens[0]
action.Resource = resourceTokens[0]
if len(resourceTokens) > 1 {
action.resourceName = resourceTokens[1]
glog.V(3).Infof("Resolved resourceName `%s`", action.resourceName)
action.ResourceName = resourceTokens[1]
glog.V(3).Infof("Resolved resourceName `%s`", action.ResourceName)
}
}

action.subResource, err = flags.GetString(subResourceFlag)
action.SubResource, err = flags.GetString(subResourceFlag)
if err != nil {
return
}

action.allNamespaces, err = flags.GetBool(allNamespacesFlag)
action.AllNamespaces, err = flags.GetBool(allNamespacesFlag)
if err != nil {
return
}

if action.allNamespaces {
action.namespace = core.NamespaceAll
glog.V(3).Infof("Resolved namespace `%s` from --all-namespaces flag", action.namespace)
if action.AllNamespaces {
action.Namespace = core.NamespaceAll
glog.V(3).Infof("Resolved namespace `%s` from --all-namespaces flag", action.Namespace)
return
}

action.namespace, err = flags.GetString(namespaceFlag)
action.Namespace, err = flags.GetString(namespaceFlag)
if err != nil {
return
}

if action.namespace != "" {
glog.V(3).Infof("Resolved namespace `%s` from --namespace flag", action.namespace)
if action.Namespace != "" {
glog.V(3).Infof("Resolved namespace `%s` from --namespace flag", action.Namespace)
return
}

// Neither --all-namespaces nor --namespace flag was specified
action.namespace, _, err = clientConfig.Namespace()
action.Namespace, _, err = clientConfig.Namespace()
if err != nil {
err = fmt.Errorf("getting namespace from current context: %v", err)
}
glog.V(3).Infof("Resolved namespace `%s` from current context", action.namespace)
glog.V(3).Infof("Resolved namespace `%s` from current context", action.Namespace)
return
}

// Validate makes sure that the specified action is valid.
func (w *WhoCan) validate(action Action) error {
if action.nonResourceURL != "" && action.subResource != "" {
if action.NonResourceURL != "" && action.SubResource != "" {
return fmt.Errorf("--subresource cannot be used with NONRESOURCEURL")
}

err := w.namespaceValidator.Validate(action.namespace)
err := w.namespaceValidator.Validate(action.Namespace)
if err != nil {
return fmt.Errorf("validating namespace: %v", err)
}
Expand All @@ -265,8 +265,8 @@ func (w *WhoCan) Check(action Action) (roleBindings []rbac.RoleBinding, clusterR

resolvedAction := resolvedAction{Action: action}

if action.resource != "" {
resolvedAction.gr, err = w.resourceResolver.Resolve(action.verb, action.resource, action.subResource)
if action.Resource != "" {
resolvedAction.gr, err = w.resourceResolver.Resolve(action.Verb, action.Resource, action.SubResource)
if err != nil {
err = fmt.Errorf("resolving resource: %v", err)
return
Expand Down Expand Up @@ -316,7 +316,7 @@ func (w *WhoCan) CheckAPIAccess(action Action) ([]string, error) {
var warnings []string

// Determine which checks need to be executed.
if action.namespace == "" {
if action.Namespace == "" {
checks = append(checks, check{"list", "namespaces", ""})

nsList, err := w.clientNamespace.List(meta.ListOptions{})
Expand All @@ -328,8 +328,8 @@ func (w *WhoCan) CheckAPIAccess(action Action) ([]string, error) {
checks = append(checks, check{"list", "rolebindings", ns.Name})
}
} else {
checks = append(checks, check{"list", "roles", action.namespace})
checks = append(checks, check{"list", "rolebindings", action.namespace})
checks = append(checks, check{"list", "roles", action.Namespace})
checks = append(checks, check{"list", "rolebindings", action.Namespace})
}

// Actually run the checks and collect warnings.
Expand All @@ -356,7 +356,7 @@ func (w *WhoCan) CheckAPIAccess(action Action) ([]string, error) {

// GetRolesFor returns a set of names of Roles matching the specified Action.
func (w *WhoCan) getRolesFor(action resolvedAction) (roles, error) {
rl, err := w.clientRBAC.Roles(action.namespace).List(meta.ListOptions{})
rl, err := w.clientRBAC.Roles(action.Namespace).List(meta.ListOptions{})
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -396,11 +396,11 @@ func (w *WhoCan) getClusterRolesFor(action resolvedAction) (clusterRoles, error)
// GetRoleBindings returns the RoleBindings that refer to the given set of Role names or ClusterRole names.
func (w *WhoCan) getRoleBindings(action resolvedAction, roleNames roles, clusterRoleNames clusterRoles) (roleBindings []rbac.RoleBinding, err error) {
// TODO I'm wondering if GetRoleBindings should be invoked at all when the --all-namespaces flag is specified?
if action.namespace == core.NamespaceAll {
if action.Namespace == core.NamespaceAll {
return
}

list, err := w.clientRBAC.RoleBindings(action.namespace).List(meta.ListOptions{})
list, err := w.clientRBAC.RoleBindings(action.Namespace).List(meta.ListOptions{})
if err != nil {
return
}
Expand Down Expand Up @@ -452,12 +452,10 @@ func PrintChecks(out io.Writer, action Action, roleBindings []rbac.RoleBinding,
wr := new(tabwriter.Writer)
wr.Init(out, 0, 8, 2, ' ', 0)

actionStr := action.PrettyPrint()

if action.resource != "" {
if action.Resource != "" {
// NonResourceURL permissions can only be granted through ClusterRoles. Hence no point in printing RoleBindings section.
if len(roleBindings) == 0 {
_, _ = fmt.Fprintf(out, "No subjects found with permissions to %s assigned through RoleBindings\n", actionStr)
_, _ = fmt.Fprintf(out, "No subjects found with permissions to %s assigned through RoleBindings\n", action)
} else {
_, _ = fmt.Fprintln(wr, "ROLEBINDING\tNAMESPACE\tSUBJECT\tTYPE\tSA-NAMESPACE")
for _, rb := range roleBindings {
Expand All @@ -471,7 +469,7 @@ func PrintChecks(out io.Writer, action Action, roleBindings []rbac.RoleBinding,
}

if len(clusterRoleBindings) == 0 {
_, _ = fmt.Fprintf(out, "No subjects found with permissions to %s assigned through ClusterRoleBindings\n", actionStr)
_, _ = fmt.Fprintf(out, "No subjects found with permissions to %s assigned through ClusterRoleBindings\n", action)
} else {
_, _ = fmt.Fprintln(wr, "CLUSTERROLEBINDING\tSUBJECT\tTYPE\tSA-NAMESPACE")
for _, rb := range clusterRoleBindings {
Expand All @@ -483,13 +481,13 @@ func PrintChecks(out io.Writer, action Action, roleBindings []rbac.RoleBinding,
_ = wr.Flush()
}

func (w Action) PrettyPrint() string {
if w.nonResourceURL != "" {
return fmt.Sprintf("%s %s", w.verb, w.nonResourceURL)
func (w Action) String() string {
if w.NonResourceURL != "" {
return fmt.Sprintf("%s %s", w.Verb, w.NonResourceURL)
}
name := w.resourceName
name := w.ResourceName
if name != "" {
name = "/" + name
}
return fmt.Sprintf("%s %s%s", w.verb, w.resource, name)
return fmt.Sprintf("%s %s%s", w.Verb, w.Resource, name)
}
60 changes: 30 additions & 30 deletions pkg/cmd/list_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,10 @@ func TestActionFrom(t *testing.T) {
args: []string{"list", "pods"},
flags: flags{namespace: "", allNamespaces: false},
expectedAction: Action{
namespace: "foo",
verb: "list",
resource: "pods",
resourceName: "",
Namespace: "foo",
Verb: "list",
Resource: "pods",
ResourceName: "",
},
},
{
Expand All @@ -112,10 +112,10 @@ func TestActionFrom(t *testing.T) {
flags: flags{namespace: "", allNamespaces: false},
args: []string{"list", "pods"},
expectedAction: Action{
namespace: "",
verb: "list",
resource: "pods",
resourceName: "",
Namespace: "",
Verb: "list",
Resource: "pods",
ResourceName: "",
},
expectedError: errors.New("getting namespace from current context: cannot open context"),
},
Expand All @@ -124,31 +124,31 @@ func TestActionFrom(t *testing.T) {
flags: flags{namespace: "", allNamespaces: true},
args: []string{"get", "service/mongodb"},
expectedAction: Action{
allNamespaces: true,
namespace: core.NamespaceAll,
verb: "get",
resource: "service",
resourceName: "mongodb",
AllNamespaces: true,
Namespace: core.NamespaceAll,
Verb: "get",
Resource: "service",
ResourceName: "mongodb",
},
},
{
name: "D",
flags: flags{namespace: "bar", allNamespaces: false},
args: []string{"delete", "pv"},
expectedAction: Action{
namespace: "bar",
verb: "delete",
resource: "pv",
Namespace: "bar",
Verb: "delete",
Resource: "pv",
},
},
{
name: "F",
flags: flags{namespace: "foo"},
args: []string{"get", "/logs"},
expectedAction: Action{
namespace: "foo",
verb: "get",
nonResourceURL: "/logs",
Namespace: "foo",
Verb: "get",
NonResourceURL: "/logs",
},
},
{
Expand Down Expand Up @@ -236,9 +236,9 @@ func TestValidate(t *testing.T) {
}

action := Action{
nonResourceURL: tt.nonResourceURL,
subResource: tt.subResource,
namespace: tt.namespace,
NonResourceURL: tt.nonResourceURL,
SubResource: tt.subResource,
Namespace: tt.namespace,
}

// when
Expand Down Expand Up @@ -345,7 +345,7 @@ func TestWhoCan_CheckAPIAccess(t *testing.T) {
policyRuleMatcher: policyRuleMatcher,
}
action := Action{
namespace: tt.namespace,
Namespace: tt.namespace,
}

// when
Expand All @@ -366,7 +366,7 @@ func TestWhoCan_GetRolesFor(t *testing.T) {
policyRuleMatcher := new(policyRuleMatcherMock)
client := fake.NewSimpleClientset()

action := resolvedAction{Action: Action{verb: "list", resource: "services"}}
action := resolvedAction{Action: Action{Verb: "list", Resource: "services"}}

viewServicesRole := rbac.Role{
ObjectMeta: meta.ObjectMeta{
Expand Down Expand Up @@ -425,7 +425,7 @@ func TestWhoCan_GetClusterRolesFor(t *testing.T) {
policyRuleMatcher := new(policyRuleMatcherMock)
client := fake.NewSimpleClientset()

action := resolvedAction{Action: Action{verb: "get", resource: "/logs"}}
action := resolvedAction{Action: Action{Verb: "get", Resource: "/logs"}}

getLogsRole := rbac.ClusterRole{
ObjectMeta: meta.ObjectMeta{
Expand Down Expand Up @@ -521,7 +521,7 @@ func TestWhoCan_GetRoleBindings(t *testing.T) {
wc := WhoCan{
clientRBAC: client.RbacV1(),
}
action := resolvedAction{Action: Action{namespace: namespace}}
action := resolvedAction{Action: Action{Namespace: namespace}}

// when
bindings, err := wc.getRoleBindings(action, roleNames, clusterRoleNames)
Expand Down Expand Up @@ -690,10 +690,10 @@ Bob-and-Eve-can-view-pods Eve User
// given
var buf bytes.Buffer
action := Action{
verb: tt.verb,
resource: tt.resource,
nonResourceURL: tt.nonResourceURL,
resourceName: tt.resourceName,
Verb: tt.verb,
Resource: tt.resource,
NonResourceURL: tt.nonResourceURL,
ResourceName: tt.resourceName,
}

// when
Expand Down
14 changes: 7 additions & 7 deletions pkg/cmd/policy_rule_matcher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,20 @@ func (m *matcher) MatchesClusterRole(role rbac.ClusterRole, action resolvedActio

// matches returns `true` if the given PolicyRule matches the specified Action, `false` otherwise.
func (m *matcher) matches(rule rbac.PolicyRule, action resolvedAction) bool {
if action.nonResourceURL != "" {
return m.matchesVerb(rule, action.verb) &&
m.matchesNonResourceURL(rule, action.nonResourceURL)
if action.NonResourceURL != "" {
return m.matchesVerb(rule, action.Verb) &&
m.matchesNonResourceURL(rule, action.NonResourceURL)
}

resource := action.gr.Resource
if action.subResource != "" {
resource += "/" + action.subResource
if action.SubResource != "" {
resource += "/" + action.SubResource
}

return m.matchesVerb(rule, action.verb) &&
return m.matchesVerb(rule, action.Verb) &&
m.matchesResource(rule, resource) &&
m.matchesAPIGroup(rule, action.gr.Group) &&
m.matchesResourceName(rule, action.resourceName)
m.matchesResourceName(rule, action.ResourceName)
}

func (m *matcher) matchesAPIGroup(rule rbac.PolicyRule, actionGroup string) bool {
Expand Down
Loading

0 comments on commit f549a4c

Please sign in to comment.