diff --git a/staging/src/k8s.io/apiserver/pkg/authorization/union/union.go b/staging/src/k8s.io/apiserver/pkg/authorization/union/union.go index 367da59d1be..3246e4c0759 100644 --- a/staging/src/k8s.io/apiserver/pkg/authorization/union/union.go +++ b/staging/src/k8s.io/apiserver/pkg/authorization/union/union.go @@ -14,6 +14,14 @@ See the License for the specific language governing permissions and limitations under the License. */ +// Package union implements an authorizer that combines multiple subauthorizer. +// The union authorizer iterates over each subauthorizer and returns the first +// decision that is either an Allow decision or a Deny decision. If a +// subauthorizer returns a NoOpinion, then the union authorizer moves onto the +// next authorizer or, if the subauthorizer was the last authorizer, returns +// NoOpinion as the aggregate decision. I.e. union authorizer creates an +// aggregate decision and supports short-circut allows and denies from +// subauthorizers. package union import ( @@ -33,14 +41,14 @@ func New(authorizationHandlers ...authorizer.Authorizer) authorizer.Authorizer { } // Authorizes against a chain of authorizer.Authorizer objects and returns nil if successful and returns error if unsuccessful -func (authzHandler unionAuthzHandler) Authorize(a authorizer.Attributes) (bool, string, error) { +func (authzHandler unionAuthzHandler) Authorize(a authorizer.Attributes) (authorizer.Decision, string, error) { var ( errlist []error reasonlist []string ) for _, currAuthzHandler := range authzHandler { - authorized, reason, err := currAuthzHandler.Authorize(a) + decision, reason, err := currAuthzHandler.Authorize(a) if err != nil { errlist = append(errlist, err) @@ -48,13 +56,15 @@ func (authzHandler unionAuthzHandler) Authorize(a authorizer.Attributes) (bool, if len(reason) != 0 { reasonlist = append(reasonlist, reason) } - if !authorized { - continue + switch decision { + case authorizer.DecisionAllow, authorizer.DecisionDeny: + return decision, reason, err + case authorizer.DecisionNoOpinion: + // continue to the next authorizer } - return true, reason, nil } - return false, strings.Join(reasonlist, "\n"), utilerrors.NewAggregate(errlist) + return authorizer.DecisionNoOpinion, strings.Join(reasonlist, "\n"), utilerrors.NewAggregate(errlist) } // unionAuthzRulesHandler authorizer against a chain of authorizer.RuleResolver diff --git a/staging/src/k8s.io/apiserver/pkg/authorization/union/union_test.go b/staging/src/k8s.io/apiserver/pkg/authorization/union/union_test.go index 96d989fb67b..a6413897915 100644 --- a/staging/src/k8s.io/apiserver/pkg/authorization/union/union_test.go +++ b/staging/src/k8s.io/apiserver/pkg/authorization/union/union_test.go @@ -17,6 +17,7 @@ limitations under the License. package union import ( + "errors" "fmt" "reflect" "testing" @@ -26,49 +27,43 @@ import ( ) type mockAuthzHandler struct { - isAuthorized bool - err error + decision authorizer.Decision + err error } -func (mock *mockAuthzHandler) Authorize(a authorizer.Attributes) (bool, string, error) { - if mock.err != nil { - return false, "", mock.err - } - if !mock.isAuthorized { - return false, "", nil - } - return true, "", nil +func (mock *mockAuthzHandler) Authorize(a authorizer.Attributes) (authorizer.Decision, string, error) { + return mock.decision, "", mock.err } func TestAuthorizationSecondPasses(t *testing.T) { - handler1 := &mockAuthzHandler{isAuthorized: false} - handler2 := &mockAuthzHandler{isAuthorized: true} + handler1 := &mockAuthzHandler{decision: authorizer.DecisionNoOpinion} + handler2 := &mockAuthzHandler{decision: authorizer.DecisionAllow} authzHandler := New(handler1, handler2) authorized, _, _ := authzHandler.Authorize(nil) - if !authorized { + if authorized != authorizer.DecisionAllow { t.Errorf("Unexpected authorization failure") } } func TestAuthorizationFirstPasses(t *testing.T) { - handler1 := &mockAuthzHandler{isAuthorized: true} - handler2 := &mockAuthzHandler{isAuthorized: false} + handler1 := &mockAuthzHandler{decision: authorizer.DecisionAllow} + handler2 := &mockAuthzHandler{decision: authorizer.DecisionNoOpinion} authzHandler := New(handler1, handler2) authorized, _, _ := authzHandler.Authorize(nil) - if !authorized { + if authorized != authorizer.DecisionAllow { t.Errorf("Unexpected authorization failure") } } func TestAuthorizationNonePasses(t *testing.T) { - handler1 := &mockAuthzHandler{isAuthorized: false} - handler2 := &mockAuthzHandler{isAuthorized: false} + handler1 := &mockAuthzHandler{decision: authorizer.DecisionNoOpinion} + handler2 := &mockAuthzHandler{decision: authorizer.DecisionNoOpinion} authzHandler := New(handler1, handler2) authorized, _, _ := authzHandler.Authorize(nil) - if authorized { + if authorized == authorizer.DecisionAllow { t.Errorf("Expected failed authorization") } } @@ -223,3 +218,49 @@ func getNonResourceRules(infos []authorizer.NonResourceRuleInfo) []authorizer.De } return rules } + +func TestAuthorizationUnequivocalDeny(t *testing.T) { + cs := []struct { + authorizers []authorizer.Authorizer + decision authorizer.Decision + }{ + { + authorizers: []authorizer.Authorizer{}, + decision: authorizer.DecisionNoOpinion, + }, + { + authorizers: []authorizer.Authorizer{ + &mockAuthzHandler{decision: authorizer.DecisionNoOpinion}, + &mockAuthzHandler{decision: authorizer.DecisionAllow}, + &mockAuthzHandler{decision: authorizer.DecisionDeny}, + }, + decision: authorizer.DecisionAllow, + }, + { + authorizers: []authorizer.Authorizer{ + &mockAuthzHandler{decision: authorizer.DecisionNoOpinion}, + &mockAuthzHandler{decision: authorizer.DecisionDeny}, + &mockAuthzHandler{decision: authorizer.DecisionAllow}, + }, + decision: authorizer.DecisionDeny, + }, + { + authorizers: []authorizer.Authorizer{ + &mockAuthzHandler{decision: authorizer.DecisionNoOpinion}, + &mockAuthzHandler{decision: authorizer.DecisionDeny, err: errors.New("webhook failed closed")}, + &mockAuthzHandler{decision: authorizer.DecisionAllow}, + }, + decision: authorizer.DecisionDeny, + }, + } + for i, c := range cs { + t.Run(fmt.Sprintf("case %v", i), func(t *testing.T) { + authzHandler := New(c.authorizers...) + + decision, _, _ := authzHandler.Authorize(nil) + if decision != c.decision { + t.Errorf("Unexpected authorization failure: %v, expected: %v", decision, c.decision) + } + }) + } +}