-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
rebase 1.9.0 beta.1 #17576
rebase 1.9.0 beta.1 #17576
Changes from 1 commit
f0bb2b6
c671103
07c6b99
710998e
c8626b7
f3769c7
123246b
534c679
ca1b85f
9ff7f3f
d34b354
df449cc
f046a0b
3c7a135
3f45cdc
ee0f726
de36874
64974bc
bf64f2c
cf235c2
4bc612e
66d94ff
fc9b4e2
5b3859b
d1b5fe8
42a1e2c
d49083e
07e5313
ab033d4
32a0c9a
27ad23a
f490d38
2f11419
b3fa18d
22f0b91
0aedd29
7f86e08
99c59c6
afdcb87
2abedd5
5d40a0f
4383cd7
7ffc267
244afd3
30fe89a
de21f14
80cef2d
a92560d
a1cef1f
8454d7d
9344e48
062ffb1
93bd84a
0089bbb
ad7d2fc
60d3fa9
b15c3b6
1d18e82
31f33b0
2cf15a3
ee8266b
bb2ecf0
2db374f
a4d2794
ebc468c
7256949
6c58566
d4ee63c
9e3ca9a
6b39f30
2fa3a79
d826d91
a783419
5c42201
b80c0d2
088b81d
cdc12ca
b374cbd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,17 +27,17 @@ func NewSubjectLocator(delegate authorizerrbac.SubjectLocator) SubjectLocator { | |
return &openshiftSubjectLocator{delegate: delegate} | ||
} | ||
|
||
func (a *openshiftAuthorizer) Authorize(attributes authorizer.Attributes) (bool, string, error) { | ||
func (a *openshiftAuthorizer) Authorize(attributes authorizer.Attributes) (authorizer.Decision, string, error) { | ||
if attributes.GetUser() == nil { | ||
return false, "", errors.New("no user available on context") | ||
return authorizer.DecisionDeny, "", errors.New("no user available on context") | ||
} | ||
allowed, delegateReason, err := a.delegate.Authorize(attributes) | ||
if allowed { | ||
return true, reason(attributes), nil | ||
authorized, delegateReason, err := a.delegate.Authorize(attributes) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should return authorized, not convert it to Deny There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
done |
||
if authorized == authorizer.DecisionAllow { | ||
return authorizer.DecisionAllow, reason(attributes), nil | ||
} | ||
// errors are allowed to occur | ||
if err != nil { | ||
return false, "", err | ||
return authorizer.DecisionDeny, "", err | ||
} | ||
|
||
denyReason, err := a.forbiddenMessageMaker.MakeMessage(attributes) | ||
|
@@ -48,7 +48,7 @@ func (a *openshiftAuthorizer) Authorize(attributes authorizer.Attributes) (bool, | |
denyReason += ": " + delegateReason | ||
} | ||
|
||
return false, denyReason, nil | ||
return authorizer.DecisionDeny, denyReason, nil | ||
} | ||
|
||
// GetAllowedSubjects returns the subjects it knows can perform the action. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,17 +26,17 @@ type subjectAccessTest struct { | |
} | ||
|
||
type testAuthorizer struct { | ||
allowed bool | ||
allowed kauthorizer.Decision | ||
reason string | ||
err string | ||
|
||
actualAttributes kauthorizer.Attributes | ||
} | ||
|
||
func (a *testAuthorizer) Authorize(passedAttributes kauthorizer.Attributes) (allowed bool, reason string, err error) { | ||
func (a *testAuthorizer) Authorize(passedAttributes kauthorizer.Attributes) (authorized kauthorizer.Decision, reason string, err error) { | ||
// allow the initial check for "can I run this SAR at all" | ||
if passedAttributes.GetResource() == "localsubjectaccessreviews" { | ||
return true, "", nil | ||
return kauthorizer.DecisionAllow, "", nil | ||
} | ||
|
||
a.actualAttributes = passedAttributes | ||
|
@@ -53,7 +53,7 @@ func (a *testAuthorizer) GetAllowedSubjects(passedAttributes kauthorizer.Attribu | |
func TestNoNamespace(t *testing.T) { | ||
test := &subjectAccessTest{ | ||
authorizer: &testAuthorizer{ | ||
allowed: false, | ||
allowed: kauthorizer.DecisionDeny, | ||
}, | ||
reviewRequest: &authorizationapi.LocalSubjectAccessReview{ | ||
Action: authorizationapi.Action{ | ||
|
@@ -72,7 +72,7 @@ func TestNoNamespace(t *testing.T) { | |
|
||
func TestConflictingNamespace(t *testing.T) { | ||
authorizer := &testAuthorizer{ | ||
allowed: false, | ||
allowed: kauthorizer.DecisionDeny, | ||
} | ||
reviewRequest := &authorizationapi.LocalSubjectAccessReview{ | ||
Action: authorizationapi.Action{ | ||
|
@@ -98,7 +98,7 @@ func TestConflictingNamespace(t *testing.T) { | |
func TestEmptyReturn(t *testing.T) { | ||
test := &subjectAccessTest{ | ||
authorizer: &testAuthorizer{ | ||
allowed: false, | ||
allowed: kauthorizer.DecisionDeny, | ||
reason: "because reasons", | ||
}, | ||
reviewRequest: &authorizationapi.LocalSubjectAccessReview{ | ||
|
@@ -123,7 +123,7 @@ func TestEmptyReturn(t *testing.T) { | |
func TestNoErrors(t *testing.T) { | ||
test := &subjectAccessTest{ | ||
authorizer: &testAuthorizer{ | ||
allowed: true, | ||
allowed: kauthorizer.DecisionAllow, | ||
reason: "because good things", | ||
}, | ||
reviewRequest: &authorizationapi.LocalSubjectAccessReview{ | ||
|
@@ -171,7 +171,7 @@ func TestErrors(t *testing.T) { | |
func TestRegularWithScopes(t *testing.T) { | ||
test := &subjectAccessTest{ | ||
authorizer: &testAuthorizer{ | ||
allowed: true, | ||
allowed: kauthorizer.DecisionAllow, | ||
reason: "because good things", | ||
}, | ||
reviewRequest: &authorizationapi.LocalSubjectAccessReview{ | ||
|
@@ -200,7 +200,7 @@ func TestRegularWithScopes(t *testing.T) { | |
func TestSelfWithDefaultScopes(t *testing.T) { | ||
test := &subjectAccessTest{ | ||
authorizer: &testAuthorizer{ | ||
allowed: true, | ||
allowed: kauthorizer.DecisionAllow, | ||
reason: "because good things", | ||
}, | ||
reviewRequest: &authorizationapi.LocalSubjectAccessReview{ | ||
|
@@ -228,7 +228,7 @@ func TestSelfWithDefaultScopes(t *testing.T) { | |
func TestSelfWithClearedScopes(t *testing.T) { | ||
test := &subjectAccessTest{ | ||
authorizer: &testAuthorizer{ | ||
allowed: true, | ||
allowed: kauthorizer.DecisionAllow, | ||
reason: "because good things", | ||
}, | ||
reviewRequest: &authorizationapi.LocalSubjectAccessReview{ | ||
|
@@ -259,7 +259,7 @@ func (r *subjectAccessTest) runTest(t *testing.T) { | |
|
||
expectedResponse := &authorizationapi.SubjectAccessReviewResponse{ | ||
Namespace: r.reviewRequest.Action.Namespace, | ||
Allowed: r.authorizer.allowed, | ||
Allowed: r.authorizer.allowed == kauthorizer.DecisionAllow, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Not for our object. |
||
Reason: r.authorizer.reason, | ||
EvaluationError: r.authorizer.err, | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mechanical change to return NoOpinion, not Deny (applies everywhere)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done