refactor authz

ideahitme · May 29, 2017 · b799213 · b799213
1 parent d9ad9e3
commit b799213
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 30 deletions.
diff --git a/authz/handler.go b/authz/handler.go
@@ -15,8 +15,8 @@ type AuthorizationHandler struct {
 	reqParser             RequestParser
 }
 
-// NewAuthorizationHandler returns authentication http handler
-func NewAuthorizationHandler() *AuthorizationHandler {
+// CreateAuthorizationHandler returns authentication http handler
+func CreateAuthorizationHandler() *AuthorizationHandler {
 	h := &AuthorizationHandler{
 		resourceAuthorizer:    authorizer.ResourceUnauthorizer{},
 		nonResourceAuthorizer: authorizer.NonResourceUnauthorizer{},
@@ -56,37 +56,29 @@ func (h *AuthorizationHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 	}
 
 	userSpec := h.reqParser.ExtractUserSpecs()
+	var allowed bool
+	var err error
 
 	if h.reqParser.IsResourceRequest() {
 		resourceSpec := h.reqParser.ExtractResourceSpecs()
-		allowed, err := h.resourceAuthorizer.IsAuthorized(userSpec, resourceSpec)
-		if err != nil {
-			w.WriteHeader(http.StatusInternalServerError)
-			w.Write(h.resConstructor.NewFailResponse(err.Error()))
-			return
-		}
-		if !allowed {
-			w.WriteHeader(http.StatusUnauthorized)
-			w.Write(h.resConstructor.NewFailResponse("Unauthorized"))
-			return
-		}
+		allowed, err = h.resourceAuthorizer.IsAuthorized(userSpec, resourceSpec)
 	}
 
 	if h.reqParser.IsNonResourceRequest() {
 		nonResourceSpec := h.reqParser.ExtractNonResourceSpecs()
-		allowed, err := h.nonResourceAuthorizer.IsAuthorized(userSpec, nonResourceSpec)
-		if err != nil {
-			w.WriteHeader(http.StatusInternalServerError)
-			w.Write(h.resConstructor.NewFailResponse(err.Error()))
-			return
-		}
-		if !allowed {
-			w.WriteHeader(http.StatusUnauthorized)
-			w.Write(h.resConstructor.NewFailResponse("Unauthorized"))
-			return
-		}
+		allowed, err = h.nonResourceAuthorizer.IsAuthorized(userSpec, nonResourceSpec)
 	}
 
+	if err != nil {
+		w.WriteHeader(http.StatusInternalServerError)
+		w.Write(h.resConstructor.NewFailResponse(err.Error()))
+		return
+	}
+	if !allowed {
+		w.WriteHeader(http.StatusUnauthorized)
+		w.Write(h.resConstructor.NewFailResponse("Unauthorized"))
+		return
+	}
 	w.WriteHeader(http.StatusOK)
 	w.Write(h.resConstructor.NewSuccessResponse())
 }
diff --git a/authz/handler_test.go b/authz/handler_test.go
@@ -28,7 +28,7 @@ type AuthorizationHandlerSuite struct {
 }
 
 func (suite *AuthorizationHandlerSuite) SetupTest() {
-	unauthzServer := httptest.NewServer(NewAuthorizationHandler())
+	unauthzServer := httptest.NewServer(CreateAuthorizationHandler())
 	suite.unauthorizeEndpoint = unauthzServer.URL
 
 	suite.resourcePayload = `{
@@ -123,15 +123,15 @@ func (suite *AuthorizationHandlerSuite) SetupTest() {
 		Verb: "get",
 	}).Return(false, errors.New("hackers not allowed"))
 
-	mockHandler := NewAuthorizationHandler().
+	mockHandler := CreateAuthorizationHandler().
 		WithNonResourceAuthorizer(mockNonResourceHandler).
 		WithResourceAuthorizer(mockResourceHandler)
 	mockServer := httptest.NewServer(mockHandler)
 	suite.mockEndpoint = mockServer.URL
 }
 
-func (suite *AuthorizationHandlerSuite) TestNewAuthorizationHandler() {
-	h := NewAuthorizationHandler()
+func (suite *AuthorizationHandlerSuite) TestCreateAuthorizationHandler() {
+	h := CreateAuthorizationHandler()
 	suite.IsType(authorizer.ResourceUnauthorizer{}, h.resourceAuthorizer, "default should be unauthorizer")
 	suite.IsType(authorizer.NonResourceUnauthorizer{}, h.nonResourceAuthorizer, "default should be unauthorizer")
 	suite.IsType(&v1beta1.ResponseConstructor{}, h.resConstructor, "default should be v1beta1")
@@ -140,8 +140,7 @@ func (suite *AuthorizationHandlerSuite) TestNewAuthorizationHandler() {
 
 // TestExtensions makes sure with chaining works as expected
 func (suite *AuthorizationHandlerSuite) TestExtensions() {
-	h := NewAuthorizationHandler()
-	h.WithAPIVersion(V1Beta1).WithNonResourceAuthorizer(&authorizer.CasbinNonResource{}).
+	h := CreateAuthorizationHandler().WithAPIVersion(V1Beta1).WithNonResourceAuthorizer(&authorizer.CasbinNonResource{}).
 		WithResourceAuthorizer(&authorizer.CasbinResource{})
 	suite.IsType(&authorizer.CasbinResource{}, h.resourceAuthorizer, "default should be overriden with casbin")
 	suite.IsType(&authorizer.CasbinNonResource{}, h.nonResourceAuthorizer, "default should be overriden with casbin")