Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #268 from dmage/sar-reason
Better log message when action is not allowed
  • Loading branch information
openshift-merge-robot committed Mar 21, 2021
2 parents d2768b8 + dc909f3 commit 1b129a6
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 2 deletions.
29 changes: 27 additions & 2 deletions pkg/dockerregistry/server/auth.go
Expand Up @@ -387,6 +387,31 @@ func verifyOpenShiftUser(ctx context.Context, c client.UsersInterfacer) (string,
return userInfo.GetName(), string(userInfo.GetUID()), nil
}

func sarStatus(sar *authorizationapi.SelfSubjectAccessReview) string {
var b strings.Builder

if sar.Status.Allowed {
b.WriteString("allowed")
} else if sar.Status.Denied {
b.WriteString("denied")
} else {
b.WriteString("no opinion")
}

if sar.Status.Reason != "" {
b.WriteString(" (")
b.WriteString(sar.Status.Reason)
b.WriteString(")")
}

if sar.Status.EvaluationError != "" {
b.WriteString(": ")
b.WriteString(sar.Status.EvaluationError)
}

return b.String()
}

func verifyWithSAR(ctx context.Context, resource, namespace, name, verb string, c client.SelfSubjectAccessReviewsNamespacer) error {
sar := authorizationapi.SelfSubjectAccessReview{
Spec: authorizationapi.SelfSubjectAccessReviewSpec{
Expand All @@ -409,7 +434,7 @@ func verifyWithSAR(ctx context.Context, resource, namespace, name, verb string,
}

if !response.Status.Allowed {
dcontext.GetLogger(ctx).Errorf("OpenShift access denied: %s", response.Status.Reason)
dcontext.GetLogger(ctx).Errorf("OpenShift access denied: %s", sarStatus(response))
return ErrOpenShiftAccessDenied
}

Expand All @@ -436,7 +461,7 @@ func verifyWithGlobalSAR(ctx context.Context, resource, subresource, verb string
return err
}
if !response.Status.Allowed {
dcontext.GetLogger(ctx).Errorf("OpenShift access denied: %s", response.Status.Reason)
dcontext.GetLogger(ctx).Errorf("OpenShift access denied: %s", sarStatus(response))
return ErrOpenShiftAccessDenied
}
return nil
Expand Down
54 changes: 54 additions & 0 deletions pkg/dockerregistry/server/auth_test.go
Expand Up @@ -519,3 +519,57 @@ func simulateOpenShiftMaster(responses []response) (*httptest.Server, *[]string)
}))
return server, &actions
}

func TestSARStatus(t *testing.T) {
testCases := []struct {
sar *authorizationapi.SelfSubjectAccessReview
output string
}{
{
sar: &authorizationapi.SelfSubjectAccessReview{
Status: authorizationapi.SubjectAccessReviewStatus{},
},
output: "no opinion",
},
{
sar: &authorizationapi.SelfSubjectAccessReview{
Status: authorizationapi.SubjectAccessReviewStatus{
Allowed: true,
},
},
output: "allowed",
},
{
sar: &authorizationapi.SelfSubjectAccessReview{
Status: authorizationapi.SubjectAccessReviewStatus{
Denied: true,
},
},
output: "denied",
},
{
sar: &authorizationapi.SelfSubjectAccessReview{
Status: authorizationapi.SubjectAccessReviewStatus{
Denied: true,
EvaluationError: "webhook failed closed",
},
},
output: "denied: webhook failed closed",
},
{
sar: &authorizationapi.SelfSubjectAccessReview{
Status: authorizationapi.SubjectAccessReviewStatus{
Reason: "Error",
EvaluationError: "no user on request.",
},
},
output: "no opinion (Error): no user on request.",
},
}
for _, tc := range testCases {
s := sarStatus(tc.sar)
if s != tc.output {
t.Errorf("got %q, want %q", s, tc.output)
}
}
}

0 comments on commit 1b129a6

Please sign in to comment.