implement authorizer logic

authn/handler.go

@@ -14,32 +14,24 @@ type AuthenticationHandler struct {
 	reqParser      RequestParser
 }
 
-// Option extends default AuthenticationHandler
-type Option func(*AuthenticationHandler)
-
 // NewAuthenticationHandler returns authentication http handler
-func NewAuthenticationHandler(p authenticator.Authenticator, opts ...Option) *AuthenticationHandler {
+func NewAuthenticationHandler(p authenticator.Authenticator) *AuthenticationHandler {
 	h := &AuthenticationHandler{
 		authProvider:   p,
 		resConstructor: v1beta1.ResponseConstructor{},
 		reqParser:      v1beta1.RequestParser{},
 	}
 
-	for _, opt := range opts {
-		opt(h)
-	}
-
 	return h
 }
 
 // WithAPIVersion specify API version to use for handling authentication requests
-func WithAPIVersion(apiVersion APIVersion) func(*AuthenticationHandler) {
-	return func(h *AuthenticationHandler) {
-		if apiVersion == V1Beta1 {
-			h.resConstructor = v1beta1.ResponseConstructor{}
-			h.reqParser = v1beta1.RequestParser{}
-		}
+func (h *AuthenticationHandler) WithAPIVersion(apiVersion APIVersion) *AuthenticationHandler {
+	if apiVersion == V1Beta1 {
+		h.resConstructor = v1beta1.ResponseConstructor{}
+		h.reqParser = v1beta1.RequestParser{}
 	}
+	return h
 }
 
 func (h *AuthenticationHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
@@ -49,7 +41,6 @@ func (h *AuthenticationHandler) ServeHTTP(w http.ResponseWriter, r *http.Request
 		w.Write(h.resConstructor.NewFailResponse())
 		return
 	}
-	defer r.Body.Close()
 
 	user, err := h.authProvider.Authenticate(token)
 	if err != nil {

authn/handler_test.go

@@ -18,17 +18,16 @@ import (
 )
 
 func TestNewAuthenticationHandler(t *testing.T) {
-	h := NewAuthenticationHandler(authenticator.Static{}, WithAPIVersion(V1Beta1))
+	h := NewAuthenticationHandler(authenticator.Static{}).WithAPIVersion(V1Beta1)
 	assert.Equal(t, h.authProvider, authenticator.Static{})
 }
 
 func TestWithAPIVersion(t *testing.T) {
-	h := &AuthenticationHandler{}
-	WithAPIVersion(V1Beta1)(h)
+	h := (&AuthenticationHandler{}).WithAPIVersion(V1Beta1)
 	assert.Equal(t, h.reqParser, v1beta1.RequestParser{})
 	assert.Equal(t, h.resConstructor, v1beta1.ResponseConstructor{})
 
-	WithAPIVersion(-1)(h)
+	h = h.WithAPIVersion(-1)
 	assert.Equal(t, h.reqParser, v1beta1.RequestParser{})
 	assert.Equal(t, h.resConstructor, v1beta1.ResponseConstructor{})
 }

authz/authorizer/authorizer.go

@@ -4,8 +4,12 @@ import (
 	"github.com/ideahitme/k8s-api-webhook/authz/unversioned"
 )
 
-// Authorizer defines the interface required to handle access to resource/non-resource endpoints of k8s API server
-type Authorizer interface {
-	ResourceEnforce(*unversioned.UserSpec, *unversioned.ResourceSpec) (allowed bool, err error)
-	NonResourceEnforce(*unversioned.UserSpec, *unversioned.NonResourceSpec) (allowed bool, err error)
+// ResourceAuthorizer interface to be implemented by authorizer based on resource
+type ResourceAuthorizer interface {
+	IsAuthorized(*unversioned.UserSpec, *unversioned.ResourceSpec) (bool, error)
+}
+
+// NonResourceAuthorizer interface to be implemented by authorizer based on non-resources
+type NonResourceAuthorizer interface {
+	IsAuthorized(*unversioned.UserSpec, *unversioned.NonResourceSpec) (bool, error)
 }

authz/authorizer/casbin.go

@@ -9,8 +9,13 @@ import (
 Authorizer enabled via casbin style file and using casbin package (github.com/casbin/casbin)
 */
 
-// Casbin type authorizer
-type Casbin struct {
+// ResourceCasbin type authorizer for resources
+type ResourceCasbin struct {
+	casbin *casbin.Enforcer
+}
+
+// NonResourceCasbin type authorizer for non-resources
+type NonResourceCasbin struct {
 	casbin *casbin.Enforcer
 }
 
@@ -19,20 +24,24 @@ const (
 	casbinModelPath  = ""
 )
 
-// NewCasbin reads csv policy file and constructs required policy enforcer
-func NewCasbin(policyFile string) (*Casbin, error) {
+// NewCasbinResource reads csv policy file and constructs required policy enforcer
+func NewCasbinResource(policyFile string) (*ResourceCasbin, error) {
+	return nil, nil
+}
 
+// NewCasbinNonResource reads csv policy file and constructs required policy enforcer
+func NewCasbinNonResource(policyFile string) (*NonResourceCasbin, error) {
 	return nil, nil
 }
 
-// ResourceEnforce returns true, nil if the user is allowed to access specified
+// IsAuthorized returns true, nil if the user is allowed to access specified
 // resource object
-func (c *Casbin) ResourceEnforce(*unversioned.UserSpec, *unversioned.ResourceSpec) (bool, error) {
+func (c *ResourceCasbin) IsAuthorized(*unversioned.UserSpec, *unversioned.ResourceSpec) (bool, error) {
 	return false, nil
 }
 
-// NonResourceEnforce returns true, nil if the user is allowed to access specified
+// IsAuthorized returns true, nil if the user is allowed to access specified
 // non resource object
-func (c *Casbin) NonResourceEnforce(*unversioned.UserSpec, *unversioned.NonResourceSpec) (bool, error) {
+func (c *NonResourceCasbin) IsAuthorized(*unversioned.UserSpec, *unversioned.NonResourceSpec) (bool, error) {
 	return false, nil
 }

authz/authorizer/casbin_test.go

@@ -1,3 +1,4 @@
 package authorizer
 
-var _ Authorizer = &Casbin{}
+var _ ResourceAuthorizer = &ResourceCasbin{}
+var _ NonResourceAuthorizer = &NonResourceCasbin{}

authz/authorizer/unauthorizer.go

@@ -0,0 +1,19 @@
+package authorizer
+
+import "github.com/ideahitme/k8s-api-webhook/authz/unversioned"
+
+// ResourceUnauthorizer is authorizer which does not allow access to any of the resources
+type ResourceUnauthorizer struct{}
+
+// NonResourceUnauthorizer is authorizer which does not allow access to any of the non-resources
+type NonResourceUnauthorizer struct{}
+
+// IsAuthorized always returns false
+func (ResourceUnauthorizer) IsAuthorized(*unversioned.UserSpec, *unversioned.ResourceSpec) (bool, error) {
+	return false, nil
+}
+
+// IsAuthorized always returns false
+func (NonResourceUnauthorizer) IsAuthorized(*unversioned.UserSpec, *unversioned.NonResourceSpec) (bool, error) {
+	return false, nil
+}

authz/authorizer/unauthorizer_test.go

@@ -0,0 +1,14 @@
+package authorizer
+
+import "testing"
+import "github.com/stretchr/testify/assert"
+
+func TestUnauthorizer(t *testing.T) {
+	allowed, err := NonResourceUnauthorizer{}.IsAuthorized(nil, nil)
+	assert.False(t, allowed, "always false")
+	assert.Nil(t, err, "error should be nil")
+
+	allowed, err = ResourceUnauthorizer{}.IsAuthorized(nil, nil)
+	assert.False(t, allowed, "always false")
+	assert.Nil(t, err, "error should be nil")
+}

authz/handler.go

@@ -9,37 +9,43 @@ import (
 
 // AuthorizationHandler implements the webhook handler
 type AuthorizationHandler struct {
-	authorizer     authorizer.Authorizer
-	resConstructor ResponseConstructor
-	reqParser      RequestParser
+	resourceAuthorizer    authorizer.ResourceAuthorizer
+	nonResourceAuthorizer authorizer.NonResourceAuthorizer
+	resConstructor        ResponseConstructor
+	reqParser             RequestParser
 }
 
-// Option extends default AuthorizationHandler
-type Option func(*AuthorizationHandler)
-
 // NewAuthorizationHandler returns authentication http handler
-func NewAuthorizationHandler(authz authorizer.Authorizer, opts ...Option) *AuthorizationHandler {
+func NewAuthorizationHandler() *AuthorizationHandler {
 	h := &AuthorizationHandler{
-		authorizer:     authz,
-		resConstructor: v1beta1.ResponseConstructor{},
-		reqParser:      v1beta1.RequestParser{},
-	}
-
-	for _, opt := range opts {
-		opt(h)
+		resourceAuthorizer:    authorizer.ResourceUnauthorizer{},
+		nonResourceAuthorizer: authorizer.NonResourceUnauthorizer{},
+		resConstructor:        &v1beta1.ResponseConstructor{},
+		reqParser:             &v1beta1.RequestParser{},
 	}
 
 	return h
 }
 
 // WithAPIVersion specify API version to use for handling authentication requests
-func WithAPIVersion(apiVersion APIVersion) func(*AuthorizationHandler) {
-	return func(h *AuthorizationHandler) {
-		if apiVersion == V1Beta1 {
-			h.resConstructor = v1beta1.ResponseConstructor{}
-			h.reqParser = v1beta1.RequestParser{}
-		}
+func (h *AuthorizationHandler) WithAPIVersion(apiVersion APIVersion) *AuthorizationHandler {
+	if apiVersion == V1Beta1 {
+		h.resConstructor = &v1beta1.ResponseConstructor{}
+		h.reqParser = &v1beta1.RequestParser{}
 	}
+	return h
+}
+
+// WithResourceAuthorizer specify API version to use for handling authentication requests
+func (h *AuthorizationHandler) WithResourceAuthorizer(authz authorizer.ResourceAuthorizer) *AuthorizationHandler {
+	h.resourceAuthorizer = authz
+	return h
+}
+
+// WithNonResourceAuthorizer specify API version to use for handling authentication requests
+func (h *AuthorizationHandler) WithNonResourceAuthorizer(authz authorizer.NonResourceAuthorizer) *AuthorizationHandler {
+	h.nonResourceAuthorizer = authz
+	return h
 }
 
 func (h *AuthorizationHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
@@ -48,13 +54,12 @@ func (h *AuthorizationHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 		w.Write(h.resConstructor.NewFailResponse(err.Error()))
 		return
 	}
-	defer r.Body.Close()
 
 	userSpec := h.reqParser.ExtractUserSpecs()
 
 	if h.reqParser.IsResourceRequest() {
 		resourceSpec := h.reqParser.ExtractResourceSpecs()
-		allowed, err := h.authorizer.ResourceEnforce(userSpec, resourceSpec)
+		allowed, err := h.resourceAuthorizer.IsAuthorized(userSpec, resourceSpec)
 		if err != nil {
 			w.WriteHeader(http.StatusInternalServerError)
 			w.Write(h.resConstructor.NewFailResponse(err.Error()))
@@ -69,7 +74,7 @@ func (h *AuthorizationHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 
 	if h.reqParser.IsNonResourceRequest() {
 		nonResourceSpec := h.reqParser.ExtractNonResourceSpecs()
-		allowed, err := h.authorizer.NonResourceEnforce(userSpec, nonResourceSpec)
+		allowed, err := h.nonResourceAuthorizer.IsAuthorized(userSpec, nonResourceSpec)
 		if err != nil {
 			w.WriteHeader(http.StatusInternalServerError)
 			w.Write(h.resConstructor.NewFailResponse(err.Error()))

authz/v1beta1/request.go

@@ -95,20 +95,21 @@ type SubjectAccessReviewSpec struct {
 	User string `json:"user,omitempty"`
 	// Groups is the groups you're testing for.
 	Groups []string `json:"group,omitempty"`
-	// Extra corresponds to the user.Info.GetExtra() method from the authenticator.  Since that is input to the authorizer
-	// it needs a reflection here.
-	Extra map[string][]string `json:"extra,omitempty"`
+	// Extra map[string][]string `json:"extra,omitempty"` ignore extra fields
 }
 
 // RequestParser implements extraction of the spec according to the official requirement for v1beta1 version
 type RequestParser struct {
-	body SubjectAccessReview
+	body *SubjectAccessReview
 }
 
-func (req RequestParser) ReadBody(body io.ReadCloser) error {
+// ReadBody parses the request body into k8s API Server specified format
+// it should be called before extracting specs
+func (req *RequestParser) ReadBody(body io.ReadCloser) error {
 	decoder := json.NewDecoder(body)
-	requestBody := SubjectAccessReview{}
-	err := decoder.Decode(&requestBody)
+	requestBody := &SubjectAccessReview{}
+
+	err := decoder.Decode(requestBody)
 	if err != nil {
 		return err
 	}
@@ -118,15 +119,21 @@ func (req RequestParser) ReadBody(body io.ReadCloser) error {
 	return nil
 }
 
-func (req RequestParser) IsResourceRequest() bool {
+// IsResourceRequest returns true if the request is targeted for a resource
+// for example "create pod"
+func (req *RequestParser) IsResourceRequest() bool {
 	return req.body.Spec.ResourceAttributes != nil
 }
 
-func (req RequestParser) IsNonResourceRequest() bool {
+// IsNonResourceRequest returns true if the request is targeted for a non-resource,
+// for example "metrics" exposed by API Server
+func (req *RequestParser) IsNonResourceRequest() bool {
 	return req.body.Spec.NonResourceAttributes != nil
 }
 
-func (req RequestParser) ExtractResourceSpecs() *unversioned.ResourceSpec {
+// ExtractResourceSpecs extracts resource related fields from previously read request body
+// see ReadBody method
+func (req *RequestParser) ExtractResourceSpecs() *unversioned.ResourceSpec {
 	if !req.IsResourceRequest() {
 		return nil
 	}
@@ -137,7 +144,9 @@ func (req RequestParser) ExtractResourceSpecs() *unversioned.ResourceSpec {
 	}
 }
 
-func (req RequestParser) ExtractNonResourceSpecs() *unversioned.NonResourceSpec {
+// ExtractNonResourceSpecs extracts non-resource related fields from previously read request body
+// see ReadBody method
+func (req *RequestParser) ExtractNonResourceSpecs() *unversioned.NonResourceSpec {
 	if !req.IsNonResourceRequest() {
 		return nil
 	}
@@ -148,7 +157,7 @@ func (req RequestParser) ExtractNonResourceSpecs() *unversioned.NonResourceSpec
 }
 
 // ExtractUserSpecs reads the request body received from API server and extracts all required scopes by the user
-func (req RequestParser) ExtractUserSpecs() *unversioned.UserSpec {
+func (req *RequestParser) ExtractUserSpecs() *unversioned.UserSpec {
 	return &unversioned.UserSpec{
 		User:   req.body.Spec.User,
 		Groups: req.body.Spec.Groups,