Remove unused DisableAuth from kubeops. Add decorator for user perms. (…

…#1453) * Remave disableAuth from kubeops tests since it's not used. * WIP: Add decorator to check user permissions in kubeops * Revert "WIP: Add decorator to check user permissions in kubeops" This reverts commit cabd95f.
vmware-tanzu · Jan 16, 2020 · e1d3c21 · e1d3c21
1 parent 6762ad3
commit e1d3c21
Show file tree

Hide file tree

Showing 4 changed files with 107 additions and 142 deletions.
diff --git a/cmd/kubeops/internal/handler/handler_test.go b/cmd/kubeops/internal/handler/handler_test.go
@@ -1,7 +1,6 @@
 package handler
 
 import (
-	"context"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -12,8 +11,6 @@ import (
 	"time"
 
 	"github.com/google/go-cmp/cmp"
-	"github.com/kubeapps/kubeapps/pkg/auth"
-	authFake "github.com/kubeapps/kubeapps/pkg/auth/fake"
 	chartFake "github.com/kubeapps/kubeapps/pkg/chart/fake"
 	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/chart"
@@ -83,7 +80,6 @@ func TestActions(t *testing.T) {
 		// Scenario params
 		Description      string
 		ExistingReleases []*release.Release
-		DisableAuth      bool
 		Skip             bool //TODO: Remove this when the memory bug is fixed
 		// Request params
 		RequestBody  string
@@ -99,9 +95,8 @@ func TestActions(t *testing.T) {
 	tests := []testScenario{
 		{
 			// Scenario params
-			Description:      "Create a simple release without auth",
+			Description:      "Create a simple release",
 			ExistingReleases: []*release.Release{},
-			DisableAuth:      true,
 			// Request params
 			RequestBody: `{"chartName": "foo", "releaseName": "foobar",	"version": "1.0.0"}`,
 			RequestQuery: "",
@@ -114,30 +109,12 @@ func TestActions(t *testing.T) {
 			},
 			ResponseBody: "",
 		},
-		{
-			// Scenario params
-			Description:      "Create a simple release with auth",
-			ExistingReleases: []*release.Release{},
-			DisableAuth:      true,
-			// Request params
-			RequestBody:  `{"chartName":"foo","releaseName":"foobar","version":"1.0.0"}`,
-			RequestQuery: "",
-			Action:       "create",
-			Params:       map[string]string{"namespace": "default"},
-			// Expected result
-			StatusCode: 200,
-			RemainingReleases: []*release.Release{
-				createRelease("foo", "foobar", "default", 1, release.StatusDeployed),
-			},
-			ResponseBody: "",
-		},
 		{
 			// Scenario params
 			Description: "Create a conflicting release",
 			ExistingReleases: []*release.Release{
 				createRelease("foo", "foobar", "default", 1, release.StatusDeployed),
 			},
-			DisableAuth: false,
 			// Request params
 			RequestBody: `{"chartName": "foo", "releaseName": "foobar",	"version": "1.0.0"}`,
 			RequestQuery: "",
@@ -154,7 +131,6 @@ func TestActions(t *testing.T) {
 			// Scenario params
 			Description:      "Get a non-existing release",
 			ExistingReleases: []*release.Release{},
-			DisableAuth:      true,
 			Skip:             true,
 			// Request params
 			RequestBody:  "",
@@ -171,7 +147,6 @@ func TestActions(t *testing.T) {
 			ExistingReleases: []*release.Release{
 				createRelease("foobarchart", "foobar", "default", 1, release.StatusDeployed),
 			},
-			DisableAuth: true,
 			// Request params
 			RequestBody:  "",
 			RequestQuery: "",
@@ -190,7 +165,6 @@ func TestActions(t *testing.T) {
 			ExistingReleases: []*release.Release{
 				createRelease("foobarchart", "foobar", "default", 1, release.StatusDeployed),
 			},
-			DisableAuth: true,
 			// Request params
 			RequestBody:  "",
 			RequestQuery: "?purge=true",
@@ -208,7 +182,6 @@ func TestActions(t *testing.T) {
 				createRelease("foo", "foobar", "default", 1, release.StatusDeployed),
 				createRelease("oof", "oofbar", "dev", 1, release.StatusDeployed),
 			},
-			DisableAuth: true,
 			// Request params
 			RequestBody:  "",
 			RequestQuery: "",
@@ -228,7 +201,6 @@ func TestActions(t *testing.T) {
 			ExistingReleases: []*release.Release{
 				createRelease("foo", "foobar", "default", 1, release.StatusUninstalled),
 			},
-			DisableAuth: true,
 			// Request params
 			RequestBody:  "",
 			RequestQuery: "",
@@ -247,7 +219,6 @@ func TestActions(t *testing.T) {
 			ExistingReleases: []*release.Release{
 				createRelease("foobarchart", "foobar", "default", 1, release.StatusDeployed),
 			},
-			DisableAuth: true,
 			// Request params
 			RequestBody:  "",
 			RequestQuery: "?purge=1",
@@ -262,7 +233,6 @@ func TestActions(t *testing.T) {
 			// Scenario params
 			Description:      "Delete a missing release",
 			ExistingReleases: []*release.Release{},
-			DisableAuth:      true,
 			Skip:             true,
 			// Request params
 			RequestBody:  "",
@@ -285,11 +255,6 @@ func TestActions(t *testing.T) {
 			}
 			// Initialize environment for test
 			req := httptest.NewRequest("GET", fmt.Sprintf("http://foo.bar%s", test.RequestQuery), strings.NewReader(test.RequestBody))
-			if !test.DisableAuth {
-				fauth := &authFake.FakeAuth{}
-				ctx := context.WithValue(req.Context(), auth.UserKey, fauth)
-				req = req.WithContext(ctx)
-			}
 			response := httptest.NewRecorder()
 			cfg := newConfigFixture(t)
 			createExistingReleases(t, cfg, test.ExistingReleases)

diff --git a/cmd/tiller-proxy/internal/handler/handler.go b/cmd/tiller-proxy/internal/handler/handler.go
@@ -43,10 +43,10 @@ func returnForbiddenActions(forbiddenActions []auth.Action, w http.ResponseWrite
 
 // TillerProxy client and configuration
 type TillerProxy struct {
-	DisableAuth bool
-	ListLimit   int
-	ChartClient chartUtils.Resolver
-	ProxyClient proxy.TillerClient
+	DisableUserAuthCheck bool
+	ListLimit            int
+	ChartClient          chartUtils.Resolver
+	ProxyClient          proxy.TillerClient
 }
 
 func (h *TillerProxy) logStatus(name string) {
@@ -67,7 +67,7 @@ func (h *TillerProxy) CreateRelease(w http.ResponseWriter, req *http.Request, pa
 		response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 		return
 	}
-	if !h.DisableAuth {
+	if !h.DisableUserAuthCheck {
 		manifest, err := h.ProxyClient.ResolveManifest(params["namespace"], chartDetails.Values, ch)
 		if err != nil {
 			response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
@@ -122,7 +122,7 @@ func (h *TillerProxy) RollbackRelease(w http.ResponseWriter, req *http.Request,
 		response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 		return
 	}
-	if !h.DisableAuth {
+	if !h.DisableUserAuthCheck {
 		manifest, err := h.ProxyClient.ResolveManifestFromRelease(params["releaseName"], int32(revisionInt))
 		if err != nil {
 			response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
@@ -159,7 +159,7 @@ func (h *TillerProxy) UpgradeRelease(w http.ResponseWriter, req *http.Request, p
 		response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 		return
 	}
-	if !h.DisableAuth {
+	if !h.DisableUserAuthCheck {
 		manifest, err := h.ProxyClient.ResolveManifest(params["namespace"], chartDetails.Values, ch)
 		if err != nil {
 			response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
@@ -209,7 +209,7 @@ func (h *TillerProxy) ListReleases(w http.ResponseWriter, req *http.Request, par
 // TestRelease in the namespace given as Param
 func (h *TillerProxy) TestRelease(w http.ResponseWriter, req *http.Request, params handlerutil.Params) {
 
-	if !h.DisableAuth {
+	if !h.DisableUserAuthCheck {
 		userAuth := req.Context().Value(auth.UserKey).(auth.Checker)
 		// helm tests only create pods so we only need to check that
 		manifest := "apiVersion: v1\nkind: Pod"
@@ -239,7 +239,7 @@ func (h *TillerProxy) GetRelease(w http.ResponseWriter, req *http.Request, param
 		response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
 		return
 	}
-	if !h.DisableAuth {
+	if !h.DisableUserAuthCheck {
 		manifest, err := h.ProxyClient.ResolveManifest(params["namespace"], rel.Config.Raw, rel.Chart)
 		if err != nil {
 			response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)
@@ -261,7 +261,7 @@ func (h *TillerProxy) GetRelease(w http.ResponseWriter, req *http.Request, param
 
 // DeleteRelease removes a release from a namespace
 func (h *TillerProxy) DeleteRelease(w http.ResponseWriter, req *http.Request, params handlerutil.Params) {
-	if !h.DisableAuth {
+	if !h.DisableUserAuthCheck {
 		rel, err := h.ProxyClient.GetRelease(params["releaseName"], params["namespace"])
 		if err != nil {
 			response.NewErrorResponse(handlerutil.ErrorCode(err), err.Error()).Write(w)