Merge check implementations

netobserv · Feb 6, 2023 · cd5b64d · cd5b64d
1 parent 3565287
commit cd5b64d
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 36 deletions.
diff --git a/cmd/plugin-backend.go b/cmd/plugin-backend.go
@@ -11,6 +11,7 @@ import (
 	"github.com/sirupsen/logrus"
 
 	"github.com/netobserv/network-observability-console-plugin/pkg/kubernetes/auth"
+	"github.com/netobserv/network-observability-console-plugin/pkg/kubernetes/client"
 	"github.com/netobserv/network-observability-console-plugin/pkg/loki"
 	"github.com/netobserv/network-observability-console-plugin/pkg/server"
 )
@@ -86,7 +87,7 @@ func main() {
 	if checkType == auth.CheckNone {
 		log.Warn("INSECURE: auth checker is disabled")
 	}
-	checker, err := auth.NewChecker(checkType)
+	checker, err := auth.NewChecker(checkType, client.NewInCluster)
 	if err != nil {
 		log.WithError(err).Fatal("auth checker error")
 	}

diff --git a/pkg/kubernetes/auth/check_auth.go b/pkg/kubernetes/auth/check_auth.go
@@ -28,14 +28,14 @@ type Checker interface {
 	CheckAuth(ctx context.Context, header http.Header) error
 }
 
-func NewChecker(typez CheckType) (Checker, error) {
+func NewChecker(typez CheckType, apiProvider client.APIProvider) (Checker, error) {
 	switch typez {
 	case CheckNone:
 		return &NoopChecker{}, nil
 	case CheckAuthenticated:
-		return &ValidBearerTokenChecker{apiProvider: client.NewInCluster}, nil
+		return &BearerTokenChecker{apiProvider: apiProvider, predicates: []tokenReviewPredicate{mustBeAuthenticated}}, nil
 	case CheckAdmin:
-		return &AdminBearerTokenChecker{apiProvider: client.NewInCluster}, nil
+		return &BearerTokenChecker{apiProvider: apiProvider, predicates: []tokenReviewPredicate{mustBeAuthenticated, mustBeClusterAdmin}}, nil
 	}
 	return nil, fmt.Errorf("auth checker type unknown: %s. Must be one of %s, %s, %s", typez, CheckAdmin, CheckAuthenticated, CheckNone)
 }
@@ -101,39 +101,20 @@ func mustBeClusterAdmin(rvw *authv1.TokenReview) error {
 	return errors.New("user not in cluster-admins group")
 }
 
-type ValidBearerTokenChecker struct {
+type BearerTokenChecker struct {
 	Checker
 	apiProvider client.APIProvider
+	predicates  []tokenReviewPredicate
 }
 
-func (c *ValidBearerTokenChecker) CheckAuth(ctx context.Context, header http.Header) error {
+func (c *BearerTokenChecker) CheckAuth(ctx context.Context, header http.Header) error {
 	hlog.Debug("Checking authenticated user")
 	token, err := getUserToken(header)
 	if err != nil {
 		return err
 	}
 	hlog.Debug("Checking auth: token found")
-	if err = runTokenReview(ctx, c.apiProvider, token, []tokenReviewPredicate{mustBeAuthenticated}); err != nil {
-		return err
-	}
-
-	hlog.Debug("Checking auth: passed")
-	return nil
-}
-
-type AdminBearerTokenChecker struct {
-	Checker
-	apiProvider client.APIProvider
-}
-
-func (c *AdminBearerTokenChecker) CheckAuth(ctx context.Context, header http.Header) error {
-	hlog.Debug("Checking authenticated user")
-	token, err := getUserToken(header)
-	if err != nil {
-		return err
-	}
-	hlog.Debug("Checking auth: token found")
-	if err = runTokenReview(ctx, c.apiProvider, token, []tokenReviewPredicate{mustBeAuthenticated, mustBeClusterAdmin}); err != nil {
+	if err = runTokenReview(ctx, c.apiProvider, token, c.predicates); err != nil {
 		return err
 	}
 

diff --git a/pkg/kubernetes/auth/check_auth_test.go b/pkg/kubernetes/auth/check_auth_test.go
@@ -13,12 +13,17 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+func setupChecker(typez CheckType, m *TokenReviewMock) Checker {
+	checker, _ := NewChecker(typez, func() (client.KubeAPI, error) { return m, nil })
+	return checker
+}
+
 func TestCheckAuth_NoAuth(t *testing.T) {
 	m := TokenReviewMock{}
 	m.mockNoAuth()
 
 	// Any user authenticated mode
-	checkAny := ValidBearerTokenChecker{apiProvider: func() (client.KubeAPI, error) { return &m, nil }}
+	checkAny := setupChecker(CheckAuthenticated, &m)
 
 	// No header => fail
 	err := checkAny.CheckAuth(context.TODO(), http.Header{})
@@ -30,7 +35,7 @@ func TestCheckAuth_NoAuth(t *testing.T) {
 	require.Equal(t, "user not authenticated", err.Error())
 
 	// Admin mode
-	checkerAdmin := AdminBearerTokenChecker{apiProvider: func() (client.KubeAPI, error) { return &m, nil }}
+	checkerAdmin := setupChecker(CheckAdmin, &m)
 
 	// No header => fail
 	err = checkerAdmin.CheckAuth(context.TODO(), http.Header{})
@@ -54,7 +59,7 @@ func TestCheckAuth_NormalUser(t *testing.T) {
 	m.mockNormalUser()
 
 	// Any user authenticated mode
-	checkAny := ValidBearerTokenChecker{apiProvider: func() (client.KubeAPI, error) { return &m, nil }}
+	checkAny := setupChecker(CheckAuthenticated, &m)
 
 	// No header => fail
 	err := checkAny.CheckAuth(context.TODO(), http.Header{})
@@ -65,7 +70,7 @@ func TestCheckAuth_NormalUser(t *testing.T) {
 	require.NoError(t, err)
 
 	// Admin mode
-	checkerAdmin := AdminBearerTokenChecker{apiProvider: func() (client.KubeAPI, error) { return &m, nil }}
+	checkerAdmin := setupChecker(CheckAdmin, &m)
 
 	// No header => fail
 	err = checkerAdmin.CheckAuth(context.TODO(), http.Header{})
@@ -89,7 +94,7 @@ func TestCheckAuth_Admin(t *testing.T) {
 	m.mockAdmin()
 
 	// Any user authenticated mode
-	checkAny := ValidBearerTokenChecker{apiProvider: func() (client.KubeAPI, error) { return &m, nil }}
+	checkAny := setupChecker(CheckAuthenticated, &m)
 
 	// No header => fail
 	err := checkAny.CheckAuth(context.TODO(), http.Header{})
@@ -100,7 +105,7 @@ func TestCheckAuth_Admin(t *testing.T) {
 	require.NoError(t, err)
 
 	// Admin mode
-	checkerAdmin := AdminBearerTokenChecker{apiProvider: func() (client.KubeAPI, error) { return &m, nil }}
+	checkerAdmin := setupChecker(CheckAdmin, &m)
 
 	// No header => fail
 	err = checkerAdmin.CheckAuth(context.TODO(), http.Header{})
@@ -125,7 +130,7 @@ func TestCheckAuth_APIError(t *testing.T) {
 	m.mockError()
 
 	// Any user authenticated mode
-	checkAny := ValidBearerTokenChecker{apiProvider: func() (client.KubeAPI, error) { return &m, nil }}
+	checkAny := setupChecker(CheckAuthenticated, &m)
 
 	// No header => fail
 	err := checkAny.CheckAuth(context.TODO(), http.Header{})
@@ -137,7 +142,7 @@ func TestCheckAuth_APIError(t *testing.T) {
 	require.Equal(t, fakeError, err.Error())
 
 	// Admin mode
-	checkerAdmin := AdminBearerTokenChecker{apiProvider: func() (client.KubeAPI, error) { return &m, nil }}
+	checkerAdmin := setupChecker(CheckAdmin, &m)
 
 	// No header => fail
 	err = checkerAdmin.CheckAuth(context.TODO(), http.Header{})

diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go
@@ -115,7 +115,7 @@ func TestServerUnauthorized(t *testing.T) {
 				URL: &url.URL{Scheme: "http", Host: "localhost:3100"},
 			},
 			Port: testPort,
-		}, &auth.AdminBearerTokenChecker{})
+		}, &auth.BearerTokenChecker{})
 	}()
 
 	t.Logf("Started test http server: %v", serverURL)