From 3031d82c54eed366b69c83d0da25b79f62b29ccd Mon Sep 17 00:00:00 2001 From: Miguel Caballer Date: Thu, 21 Nov 2024 12:25:28 +0100 Subject: [PATCH 01/10] Add multitenancy test --- pkg/utils/auth/multitenancy_test.go | 149 ++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 pkg/utils/auth/multitenancy_test.go diff --git a/pkg/utils/auth/multitenancy_test.go b/pkg/utils/auth/multitenancy_test.go new file mode 100644 index 00000000..2f93a055 --- /dev/null +++ b/pkg/utils/auth/multitenancy_test.go @@ -0,0 +1,149 @@ +/* +Copyright (C) GRyCAP - I3M - UPV + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package auth + +import ( + "context" + "encoding/base64" + "testing" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" +) + +func TestNewMultitenancyConfig(t *testing.T) { + clientset := fake.NewSimpleClientset() + uid := "test-uid" + mc := NewMultitenancyConfig(clientset, uid) + + if mc.owner_uid != uid { + t.Errorf("expected owner_uid to be %s, got %s", uid, mc.owner_uid) + } +} + +func TestUpdateCache(t *testing.T) { + clientset := fake.NewSimpleClientset() + mc := NewMultitenancyConfig(clientset, "test-uid") + + mc.UpdateCache("user1") + if len(mc.usersCache) != 1 { + t.Errorf("expected usersCache length to be 1, got %d", len(mc.usersCache)) + } +} + +func TestClearCache(t *testing.T) { + clientset := fake.NewSimpleClientset() + mc := NewMultitenancyConfig(clientset, "test-uid") + + mc.UpdateCache("user1") + mc.ClearCache() + if len(mc.usersCache) != 0 { + t.Errorf("expected usersCache length to be 0, got %d", len(mc.usersCache)) + } +} + +func TestUserExists(t *testing.T) { + clientset := fake.NewSimpleClientset() + mc := NewMultitenancyConfig(clientset, "test-uid") + + secret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "user1", + Namespace: ServicesNamespace, + }, + } + clientset.CoreV1().Secrets(ServicesNamespace).Create(context.TODO(), secret, metav1.CreateOptions{}) + + exists := mc.UserExists("user1@egi.eu") + if !exists { + t.Errorf("expected user1 to exist") + } +} + +func TestCreateSecretForOIDC(t *testing.T) { + clientset := fake.NewSimpleClientset() + mc := NewMultitenancyConfig(clientset, "test-uid") + + err := mc.CreateSecretForOIDC("user1@egi.eu", "secret-key") + if err != nil { + t.Errorf("expected no error, got %v", err) + } + + secret, err := clientset.CoreV1().Secrets(ServicesNamespace).Get(context.TODO(), "user1", metav1.GetOptions{}) + if err != nil { + t.Errorf("expected no error, got %v", err) + } + + if secret.StringData["secretKey"] != "secret-key" { + t.Errorf("expected secretKey to be 'secret-key', got %s", secret.StringData["secretKey"]) + } +} + +func TestGetUserCredentials(t *testing.T) { + clientset := fake.NewSimpleClientset() + mc := NewMultitenancyConfig(clientset, "test-uid") + + secret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "user1", + Namespace: ServicesNamespace, + }, + Data: map[string][]byte{ + "accessKey": []byte("access-key"), + "secretKey": []byte("secret-key"), + }, + } + clientset.CoreV1().Secrets(ServicesNamespace).Create(context.TODO(), secret, metav1.CreateOptions{}) + + accessKey, secretKey, err := mc.GetUserCredentials("user1@egi.eu") + if err != nil { + t.Errorf("expected no error, got %v", err) + } + + if accessKey != "access-key" { + t.Errorf("expected accessKey to be 'access-key', got %s", accessKey) + } + + if secretKey != "secret-key" { + t.Errorf("expected secretKey to be 'secret-key', got %s", secretKey) + } +} + +func TestGenerateRandomKey(t *testing.T) { + key, err := GenerateRandomKey(32) + if err != nil { + t.Errorf("expected no error, got %v", err) + } + dkey, _ := base64.RawURLEncoding.DecodeString(key) + if len(dkey) != 32 { + t.Errorf("expected key length to be 32, got %d", len(key)) + } +} + +func TestCheckUsersInCache(t *testing.T) { + clientset := fake.NewSimpleClientset() + mc := NewMultitenancyConfig(clientset, "test-uid") + + mc.UpdateCache("user1") + mc.UpdateCache("user2") + + notFoundUsers := mc.CheckUsersInCache([]string{"user1", "user3"}) + if len(notFoundUsers) != 1 { + t.Errorf("expected notFoundUsers length to be 1, got %d", len(notFoundUsers)) + } +} From 40ea41aff66b76e51e2fccec9ba143bdb95f485c Mon Sep 17 00:00:00 2001 From: Miguel Caballer Date: Thu, 21 Nov 2024 12:51:27 +0100 Subject: [PATCH 02/10] Add multitenancy auth test --- pkg/handlers/update_test.go | 2 +- pkg/utils/auth/auth.go | 4 +- pkg/utils/auth/auth_test.go | 102 ++++++++++++++++++++++++++++++++++++ pkg/utils/auth/oidc.go | 2 +- 4 files changed, 106 insertions(+), 4 deletions(-) create mode 100644 pkg/utils/auth/auth_test.go diff --git a/pkg/handlers/update_test.go b/pkg/handlers/update_test.go index 636caa9a..034b3fed 100644 --- a/pkg/handlers/update_test.go +++ b/pkg/handlers/update_test.go @@ -96,7 +96,7 @@ func TestMakeUpdateHandler(t *testing.T) { } } }, - "allowed_users": ["user1", "user2"] + "allowed_users": ["somelonguid1@egi.eu", "somelonguid2@egi.eu"] } `) req, _ := http.NewRequest("PUT", "/system/services", body) diff --git a/pkg/utils/auth/auth.go b/pkg/utils/auth/auth.go index 559c6aad..766e4963 100644 --- a/pkg/utils/auth/auth.go +++ b/pkg/utils/auth/auth.go @@ -29,7 +29,7 @@ import ( ) // GetAuthMiddleware returns the appropriate gin auth middleware -func GetAuthMiddleware(cfg *types.Config, kubeClientset *kubernetes.Clientset) gin.HandlerFunc { +func GetAuthMiddleware(cfg *types.Config, kubeClientset kubernetes.Interface) gin.HandlerFunc { if !cfg.OIDCEnable { return gin.BasicAuth(gin.Accounts{ // Use the config's username and password for basic auth @@ -40,7 +40,7 @@ func GetAuthMiddleware(cfg *types.Config, kubeClientset *kubernetes.Clientset) g } // CustomAuth returns a custom auth handler (gin middleware) -func CustomAuth(cfg *types.Config, kubeClientset *kubernetes.Clientset) gin.HandlerFunc { +func CustomAuth(cfg *types.Config, kubeClientset kubernetes.Interface) gin.HandlerFunc { basicAuthHandler := gin.BasicAuth(gin.Accounts{ // Use the config's username and password for basic auth cfg.Username: cfg.Password, diff --git a/pkg/utils/auth/auth_test.go b/pkg/utils/auth/auth_test.go new file mode 100644 index 00000000..f92c553b --- /dev/null +++ b/pkg/utils/auth/auth_test.go @@ -0,0 +1,102 @@ +/* +Copyright (C) GRyCAP - I3M - UPV + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package auth + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + "github.com/grycap/oscar/v3/pkg/types" + "k8s.io/client-go/kubernetes/fake" +) + +func TestGetAuthMiddleware(t *testing.T) { + cfg := &types.Config{ + OIDCEnable: false, + Username: "testuser", + Password: "testpass", + } + kubeClientset := fake.NewSimpleClientset() + + router := gin.New() + router.Use(GetAuthMiddleware(cfg, kubeClientset)) + router.GET("/", func(c *gin.Context) { + c.JSON(http.StatusOK, "") + }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/", nil) + req.SetBasicAuth("testuser", "testpass") + router.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected status %v, got %v", http.StatusOK, w.Code) + } + + we := httptest.NewRecorder() + reqe, _ := http.NewRequest("GET", "/", nil) + reqe.SetBasicAuth("testuser", "otherpass") + router.ServeHTTP(we, reqe) + + if we.Code != http.StatusUnauthorized { + t.Errorf("expected status %v, got %v", http.StatusUnauthorized, we.Code) + } +} + +func TestGetLoggerMiddleware(t *testing.T) { + router := gin.New() + router.Use(GetLoggerMiddleware()) + router.GET("/", func(c *gin.Context) { + c.JSON(http.StatusOK, "") + }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/", nil) + router.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected status %v, got %v", http.StatusOK, w.Code) + } +} + +func TestGetUIDFromContext(t *testing.T) { + c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c.Set("uidOrigin", "testuid") + + uid, err := GetUIDFromContext(c) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if uid != "testuid" { + t.Errorf("expected uid %v, got %v", "testuid", uid) + } +} + +func TestGetMultitenancyConfigFromContext(t *testing.T) { + c, _ := gin.CreateTestContext(httptest.NewRecorder()) + mc := &MultitenancyConfig{} + c.Set("multitenancyConfig", mc) + + mcFromContext, err := GetMultitenancyConfigFromContext(c) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if mcFromContext != mc { + t.Errorf("expected multitenancyConfig %v, got %v", mc, mcFromContext) + } +} diff --git a/pkg/utils/auth/oidc.go b/pkg/utils/auth/oidc.go index 7582695f..19da4d4f 100644 --- a/pkg/utils/auth/oidc.go +++ b/pkg/utils/auth/oidc.go @@ -76,7 +76,7 @@ func NewOIDCManager(issuer string, subject string, groups []string) (*oidcManage } // getIODCMiddleware returns the Gin's handler middleware to validate OIDC-based auth -func getOIDCMiddleware(kubeClientset *kubernetes.Clientset, minIOAdminClient *utils.MinIOAdminClient, issuer string, subject string, groups []string) gin.HandlerFunc { +func getOIDCMiddleware(kubeClientset kubernetes.Interface, minIOAdminClient *utils.MinIOAdminClient, issuer string, subject string, groups []string) gin.HandlerFunc { oidcManager, err := NewOIDCManager(issuer, subject, groups) if err != nil { return func(c *gin.Context) { From 4f37240bfa6a060a85df2d3167a5a0c763faf3f7 Mon Sep 17 00:00:00 2001 From: Miguel Caballer Date: Thu, 21 Nov 2024 13:15:41 +0100 Subject: [PATCH 03/10] Add codacy conf file --- .codacy.yaml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .codacy.yaml diff --git a/.codacy.yaml b/.codacy.yaml new file mode 100644 index 00000000..7d2c2a01 --- /dev/null +++ b/.codacy.yaml @@ -0,0 +1,2 @@ +exclude_paths: + - "**/*_test.go" From bcb946e786297dac858813d44e6a9d6dd7d6e52e Mon Sep 17 00:00:00 2001 From: Miguel Caballer Date: Thu, 21 Nov 2024 13:23:16 +0100 Subject: [PATCH 04/10] Add codacy conf file --- .codacy.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.codacy.yaml b/.codacy.yaml index 7d2c2a01..4cb1c2ba 100644 --- a/.codacy.yaml +++ b/.codacy.yaml @@ -1,2 +1,3 @@ +--- exclude_paths: - "**/*_test.go" From 693f7dd656a7eb28655ae970ec5b0881760b1ada Mon Sep 17 00:00:00 2001 From: Miguel Caballer Date: Thu, 21 Nov 2024 13:29:28 +0100 Subject: [PATCH 05/10] Del codacy conf file --- .codacy.yaml | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 .codacy.yaml diff --git a/.codacy.yaml b/.codacy.yaml deleted file mode 100644 index 4cb1c2ba..00000000 --- a/.codacy.yaml +++ /dev/null @@ -1,3 +0,0 @@ ---- -exclude_paths: - - "**/*_test.go" From f839b527ef36d2a85b4bc4de91ecad3e1909594d Mon Sep 17 00:00:00 2001 From: Miguel Caballer Date: Thu, 21 Nov 2024 16:53:22 +0100 Subject: [PATCH 06/10] Add tests --- go.mod | 1 + go.sum | 13 ++++ pkg/utils/auth/auth_test.go | 43 +++++++++++ pkg/utils/auth/oidc_test.go | 147 ++++++++++++++++++++++++++++++++++++ 4 files changed, 204 insertions(+) create mode 100644 pkg/utils/auth/oidc_test.go diff --git a/go.mod b/go.mod index 508637a0..c29f3ffe 100644 --- a/go.mod +++ b/go.mod @@ -34,6 +34,7 @@ require ( require ( github.com/fatih/color v1.14.1 // indirect + github.com/golang-jwt/jwt/v4 v4.5.1 // indirect github.com/google/go-cmp v0.6.0 // indirect github.com/mattn/go-colorable v0.1.13 // indirect github.com/rs/xid v1.4.0 // indirect diff --git a/go.sum b/go.sum index 67ddba40..3ef351e8 100644 --- a/go.sum +++ b/go.sum @@ -108,6 +108,8 @@ github.com/goccy/go-yaml v1.9.8/go.mod h1:JubOolP3gh0HpiBc4BLRD4YmjEjHAmIIB2aaXK github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= +github.com/golang-jwt/jwt/v4 v4.5.1 h1:JdqV9zKUdtaa9gdPlywC3aeoEsR681PlKC+4F5gQgeo= +github.com/golang-jwt/jwt/v4 v4.5.1/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da h1:oI5xCqsCo564l8iNU+DwB5epxmsaqB+rhGL0m5jtYqE= github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= @@ -151,6 +153,7 @@ github.com/grycap/cdmi-client-go v0.1.1/go.mod h1:ZqWeQS3YBJVXxg3HOIkAu1MLNJ4+7s github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI= github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg= github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo= +github.com/jmespath/go-jmespath/internal/testify v1.5.1/go.mod h1:L3OGu8Wl2/fWfCI6z80xFu9LTZmf1ZRjMHUOPmWr69U= github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= github.com/jpillora/backoff v1.0.0 h1:uvFg412JmmHBHw7iwprIxkPMI+sGQ4kzOWsMeHnm2EA= @@ -274,6 +277,16 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+ github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= +github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= +github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= +github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +github.com/stretchr/testify v1.8.3/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/tinylib/msgp v1.1.8 h1:FCXC1xanKO4I8plpHGH2P7koL/RzZs12l/+r7vakfm0= github.com/tinylib/msgp v1.1.8/go.mod h1:qkpG+2ldGg4xRFmx+jfTvZPxfGFhi64BcnL9vkCm/Tw= github.com/tklauser/go-sysconf v0.3.11 h1:89WgdJhk5SNwJfu+GKyYveZ4IaJ7xAkecBo+KdJV0CM= diff --git a/pkg/utils/auth/auth_test.go b/pkg/utils/auth/auth_test.go index f92c553b..1a9c50b5 100644 --- a/pkg/utils/auth/auth_test.go +++ b/pkg/utils/auth/auth_test.go @@ -18,6 +18,7 @@ package auth import ( "net/http" "net/http/httptest" + "strings" "testing" "github.com/gin-gonic/gin" @@ -100,3 +101,45 @@ func TestGetMultitenancyConfigFromContext(t *testing.T) { t.Errorf("expected multitenancyConfig %v, got %v", mc, mcFromContext) } } + +func TestCustomAuth(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, hreq *http.Request) { + if !strings.HasPrefix(hreq.URL.Path, "/minio/admin/v3/") { + t.Errorf("Unexpected path in request, got: %s", hreq.URL.Path) + } + if hreq.URL.Path == "/minio/admin/v3/info" { + rw.WriteHeader(http.StatusOK) + rw.Write([]byte(`{"Mode": "local", "Region": "us-east-1"}`)) + } else { + rw.WriteHeader(http.StatusOK) + rw.Write([]byte(`{"status": "success"}`)) + } + })) + + cfg := &types.Config{ + OIDCEnable: false, + Username: "testuser", + Password: "testpass", + MinIOProvider: &types.MinIOProvider{ + Endpoint: server.URL, + AccessKey: "minio", + SecretKey: "minio123", + }, + } + kubeClientset := fake.NewSimpleClientset() + + router := gin.New() + router.Use(CustomAuth(cfg, kubeClientset)) + router.GET("/", func(c *gin.Context) { + c.JSON(http.StatusOK, "") + }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/", nil) + req.SetBasicAuth("testuser", "testpass") + router.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected status %v, got %v", http.StatusOK, w.Code) + } +} diff --git a/pkg/utils/auth/oidc_test.go b/pkg/utils/auth/oidc_test.go new file mode 100644 index 00000000..f8cc8526 --- /dev/null +++ b/pkg/utils/auth/oidc_test.go @@ -0,0 +1,147 @@ +/* +Copyright (C) GRyCAP - I3M - UPV + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package auth + +import ( + "crypto/rand" + "crypto/rsa" + "fmt" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/gin-gonic/gin" + "github.com/golang-jwt/jwt/v4" + "github.com/grycap/oscar/v3/pkg/utils" + "k8s.io/client-go/kubernetes/fake" +) + +func TestNewOIDCManager(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, hreq *http.Request) { + if hreq.URL.Path == "/.well-known/openid-configuration" { + rw.Write([]byte(`{"issuer": "http://` + hreq.Host + `"}`)) + } + })) + + issuer := server.URL + subject := "test-subject" + groups := []string{"group1", "group2"} + + oidcManager, err := NewOIDCManager(issuer, subject, groups) + if err != nil { + t.Errorf("expected no error, got %v", err) + } + if oidcManager == nil { + t.Errorf("expected oidcManager to be non-nil") + } +} + +func TestGetUserInfo(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, hreq *http.Request) { + fmt.Println(hreq.URL.Path) + rw.Header().Set("Content-Type", "application/json") + if hreq.URL.Path == "/.well-known/openid-configuration" { + rw.Write([]byte(`{"issuer": "http://` + hreq.Host + `", "userinfo_endpoint": "http://` + hreq.Host + `/userinfo"}`)) + } else if hreq.URL.Path == "/userinfo" { + rw.Write([]byte(`{"sub": "test-subject", "eduperson_entitlement": ["urn:mace:egi.eu:group:group1"]}`)) + } + })) + + issuer := server.URL + subject := "test-subject" + groups := []string{"group1", "group2"} + + oidcManager, err := NewOIDCManager(issuer, subject, groups) + if err != nil { + t.Errorf("expected no error, got %v", err) + } + + rawToken := "test-token" + ui, err := oidcManager.GetUserInfo(rawToken) + if err != nil { + t.Errorf("expected no error, got %v", err) + } + if ui.Subject != "test-subject" { + t.Errorf("expected subject to be %v, got %v", "test-subject", ui.Subject) + } + if len(ui.Groups) != 1 || ui.Groups[0] != "group1" { + t.Errorf("expected groups to be %v, got %v", []string{"group1"}, ui.Groups) + } +} + +func TestIsAuthorised(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, hreq *http.Request) { + rw.Header().Set("Content-Type", "application/json") + if hreq.URL.Path == "/.well-known/openid-configuration" { + rw.Write([]byte(`{"issuer": "http://` + hreq.Host + `", "userinfo_endpoint": "http://` + hreq.Host + `/userinfo"}`)) + } else if hreq.URL.Path == "/userinfo" { + rw.Write([]byte(`{"sub": "test-subject", "eduperson_entitlement": ["urn:mace:egi.eu:group:group1"]}`)) + } + })) + + issuer := server.URL + subject := "test-subject" + groups := []string{"group1", "group2"} + + oidcManager, err := NewOIDCManager(issuer, subject, groups) + if err != nil { + t.Errorf("expected no error, got %v", err) + } + + claims := jwt.MapClaims{ + "iss": issuer, + "sub": subject, + "exp": time.Now().Add(1 * time.Hour).Unix(), + "iat": time.Now().Unix(), + } + token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims) + privateKey, _ := rsa.GenerateKey(rand.Reader, 1024) + rawToken, _ := token.SignedString(privateKey) + oidcManager.config.InsecureSkipSignatureCheck = true + + if !oidcManager.IsAuthorised(rawToken) { + t.Errorf("expected token to be authorised") + } +} + +func TestGetOIDCMiddleware(t *testing.T) { + kubeClientset := fake.NewSimpleClientset() + minIOAdminClient := &utils.MinIOAdminClient{} + issuer := "https://example.com" + subject := "test-subject" + groups := []string{"group1", "group2"} + + middleware := getOIDCMiddleware(kubeClientset, minIOAdminClient, issuer, subject, groups) + if middleware == nil { + t.Errorf("expected middleware to be non-nil") + } + + // Create a new Gin context + gin.SetMode(gin.TestMode) + c, _ := gin.CreateTestContext(nil) + + // Test the middleware with an invalid token + c.Request = &http.Request{ + Header: http.Header{ + "Authorization": []string{"Bearer invalid-token"}, + }, + } + middleware(c) + if c.Writer.Status() != http.StatusUnauthorized { + t.Errorf("expected status to be %v, got %v", http.StatusUnauthorized, c.Writer.Status()) + } +} From 93cf1c112dd3f3273451be7ebf857f1737862496 Mon Sep 17 00:00:00 2001 From: Miguel Caballer Date: Fri, 22 Nov 2024 08:19:29 +0100 Subject: [PATCH 07/10] Fix test --- pkg/utils/auth/oidc_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/utils/auth/oidc_test.go b/pkg/utils/auth/oidc_test.go index f8cc8526..680db785 100644 --- a/pkg/utils/auth/oidc_test.go +++ b/pkg/utils/auth/oidc_test.go @@ -132,7 +132,8 @@ func TestGetOIDCMiddleware(t *testing.T) { // Create a new Gin context gin.SetMode(gin.TestMode) - c, _ := gin.CreateTestContext(nil) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) // Test the middleware with an invalid token c.Request = &http.Request{ From ef3e9da50a8ad47b8a8c8125affb5953d7405e0a Mon Sep 17 00:00:00 2001 From: Miguel Caballer Date: Fri, 22 Nov 2024 09:49:32 +0100 Subject: [PATCH 08/10] Improve tests --- pkg/utils/auth/auth.go | 2 +- pkg/utils/auth/oidc.go | 7 +- pkg/utils/auth/oidc_test.go | 125 ++++++++++++++++++++++++++++-------- 3 files changed, 104 insertions(+), 30 deletions(-) diff --git a/pkg/utils/auth/auth.go b/pkg/utils/auth/auth.go index 766e4963..57795a48 100644 --- a/pkg/utils/auth/auth.go +++ b/pkg/utils/auth/auth.go @@ -53,7 +53,7 @@ func CustomAuth(cfg *types.Config, kubeClientset kubernetes.Interface) gin.Handl minIOAdminClient.CreateAllUsersGroup() minIOAdminClient.UpdateUsersInGroup(oscarUser, "all_users_group", false) - oidcHandler := getOIDCMiddleware(kubeClientset, minIOAdminClient, cfg.OIDCIssuer, cfg.OIDCSubject, cfg.OIDCGroups) + oidcHandler := getOIDCMiddleware(kubeClientset, minIOAdminClient, cfg.OIDCIssuer, cfg.OIDCSubject, cfg.OIDCGroups, nil) return func(c *gin.Context) { authHeader := c.GetHeader("Authorization") if strings.HasPrefix(authHeader, "Bearer ") { diff --git a/pkg/utils/auth/oidc.go b/pkg/utils/auth/oidc.go index 19da4d4f..eab75485 100644 --- a/pkg/utils/auth/oidc.go +++ b/pkg/utils/auth/oidc.go @@ -76,8 +76,11 @@ func NewOIDCManager(issuer string, subject string, groups []string) (*oidcManage } // getIODCMiddleware returns the Gin's handler middleware to validate OIDC-based auth -func getOIDCMiddleware(kubeClientset kubernetes.Interface, minIOAdminClient *utils.MinIOAdminClient, issuer string, subject string, groups []string) gin.HandlerFunc { +func getOIDCMiddleware(kubeClientset kubernetes.Interface, minIOAdminClient *utils.MinIOAdminClient, issuer string, subject string, groups []string, oidcConfig *oidc.Config) gin.HandlerFunc { oidcManager, err := NewOIDCManager(issuer, subject, groups) + if oidcConfig != nil { + oidcManager.config = oidcConfig + } if err != nil { return func(c *gin.Context) { c.AbortWithStatus(http.StatusUnauthorized) @@ -199,7 +202,7 @@ func (om *oidcManager) UserHasVO(rawToken string, vo string) (bool, error) { func (om *oidcManager) GetUID(rawToken string) (string, error) { ui, err := om.GetUserInfo(rawToken) oidcLogger.Println("received uid: ", ui.Subject) - if err != nil { + if err == nil { return ui.Subject, nil } return "", err diff --git a/pkg/utils/auth/oidc_test.go b/pkg/utils/auth/oidc_test.go index 680db785..142e520f 100644 --- a/pkg/utils/auth/oidc_test.go +++ b/pkg/utils/auth/oidc_test.go @@ -24,8 +24,10 @@ import ( "testing" "time" + "github.com/coreos/go-oidc/v3/oidc" "github.com/gin-gonic/gin" "github.com/golang-jwt/jwt/v4" + "github.com/grycap/oscar/v3/pkg/types" "github.com/grycap/oscar/v3/pkg/utils" "k8s.io/client-go/kubernetes/fake" ) @@ -89,12 +91,12 @@ func TestIsAuthorised(t *testing.T) { if hreq.URL.Path == "/.well-known/openid-configuration" { rw.Write([]byte(`{"issuer": "http://` + hreq.Host + `", "userinfo_endpoint": "http://` + hreq.Host + `/userinfo"}`)) } else if hreq.URL.Path == "/userinfo" { - rw.Write([]byte(`{"sub": "test-subject", "eduperson_entitlement": ["urn:mace:egi.eu:group:group1"]}`)) + rw.Write([]byte(`{"sub": "user1@egi.eu", "eduperson_entitlement": ["urn:mace:egi.eu:group:group1"]}`)) } })) issuer := server.URL - subject := "test-subject" + subject := "user1@egi.eu" groups := []string{"group1", "group2"} oidcManager, err := NewOIDCManager(issuer, subject, groups) @@ -102,47 +104,116 @@ func TestIsAuthorised(t *testing.T) { t.Errorf("expected no error, got %v", err) } - claims := jwt.MapClaims{ - "iss": issuer, - "sub": subject, - "exp": time.Now().Add(1 * time.Hour).Unix(), - "iat": time.Now().Unix(), - } - token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims) - privateKey, _ := rsa.GenerateKey(rand.Reader, 1024) - rawToken, _ := token.SignedString(privateKey) + rawToken := GetToken(issuer, subject) oidcManager.config.InsecureSkipSignatureCheck = true if !oidcManager.IsAuthorised(rawToken) { t.Errorf("expected token to be authorised") } + + resg1, err2 := oidcManager.UserHasVO(rawToken, "group1") + if err2 != nil { + t.Errorf("expected no error, got %v", err) + } + if !resg1 { + t.Errorf("expected user to have VO") + } + resg2, err3 := oidcManager.UserHasVO(rawToken, "group2") + if err3 != nil { + t.Errorf("expected no error, got %v", err) + } + if resg2 { + t.Errorf("expected user not to have VO") + } + + uid, _ := oidcManager.GetUID(rawToken) + if uid != subject { + t.Errorf("expected uid to be %v, got %v", subject, uid) + } } func TestGetOIDCMiddleware(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, hreq *http.Request) { + if hreq.URL.Path == "/.well-known/openid-configuration" { + rw.Write([]byte(`{"issuer": "http://` + hreq.Host + `", "userinfo_endpoint": "http://` + hreq.Host + `/userinfo"}`)) + } else if hreq.URL.Path == "/userinfo" { + rw.Write([]byte(`{"sub": "user@egi.eu", "eduperson_entitlement": ["urn:mace:egi.eu:group:group1"]}`)) + } else if hreq.URL.Path == "/minio/admin/v3/info" { + rw.WriteHeader(http.StatusOK) + rw.Write([]byte(`{"Mode": "local", "Region": "us-east-1"}`)) + } else { + rw.WriteHeader(http.StatusOK) + rw.Write([]byte(`{"status": "success"}`)) + } + })) + kubeClientset := fake.NewSimpleClientset() - minIOAdminClient := &utils.MinIOAdminClient{} - issuer := "https://example.com" - subject := "test-subject" + cfg := types.Config{ + MinIOProvider: &types.MinIOProvider{ + Endpoint: server.URL, + Verify: false, + }, + } + minIOAdminClient, _ := utils.MakeMinIOAdminClient(&cfg) + issuer := server.URL + subject := "user@egi.eu" groups := []string{"group1", "group2"} - middleware := getOIDCMiddleware(kubeClientset, minIOAdminClient, issuer, subject, groups) + oidcConfig := &oidc.Config{ + InsecureSkipSignatureCheck: true, + SkipClientIDCheck: true, + } + middleware := getOIDCMiddleware(kubeClientset, minIOAdminClient, issuer, subject, groups, oidcConfig) if middleware == nil { t.Errorf("expected middleware to be non-nil") } - // Create a new Gin context - gin.SetMode(gin.TestMode) - w := httptest.NewRecorder() - c, _ := gin.CreateTestContext(w) - - // Test the middleware with an invalid token - c.Request = &http.Request{ - Header: http.Header{ - "Authorization": []string{"Bearer invalid-token"}, + scenarios := []struct { + token string + code int + name string + }{ + { + name: "invalid-token", + token: "invalid-token", + code: http.StatusUnauthorized, + }, + { + name: "valid-token", + token: GetToken(issuer, subject), + code: http.StatusOK, }, } - middleware(c) - if c.Writer.Status() != http.StatusUnauthorized { - t.Errorf("expected status to be %v, got %v", http.StatusUnauthorized, c.Writer.Status()) + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + // Create a new Gin context + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + + // Test the middleware with an invalid token + c.Request = &http.Request{ + Header: http.Header{ + "Authorization": []string{"Bearer " + s.token}, + }, + } + middleware(c) + if c.Writer.Status() != s.code { + t.Errorf("expected status to be %v, got %v", s.code, c.Writer.Status()) + } + }) } } + +func GetToken(issuer string, subject string) string { + claims := jwt.MapClaims{ + "iss": issuer, + "sub": subject, + "exp": time.Now().Add(1 * time.Hour).Unix(), + "iat": time.Now().Unix(), + } + token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims) + privateKey, _ := rsa.GenerateKey(rand.Reader, 1024) + rawToken, _ := token.SignedString(privateKey) + return rawToken +} From d6c62e42551b9318a3f89c9c83795ac28bb13a06 Mon Sep 17 00:00:00 2001 From: Miguel Caballer Date: Fri, 22 Nov 2024 11:17:59 +0100 Subject: [PATCH 09/10] Improve tests --- pkg/types/expose_test.go | 140 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 140 insertions(+) diff --git a/pkg/types/expose_test.go b/pkg/types/expose_test.go index e0185f09..69bab5b3 100644 --- a/pkg/types/expose_test.go +++ b/pkg/types/expose_test.go @@ -22,6 +22,7 @@ import ( appsv1 "k8s.io/api/apps/v1" autoscalingv1 "k8s.io/api/autoscaling/v1" corev1 "k8s.io/api/core/v1" + netv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" testclient "k8s.io/client-go/kubernetes/fake" @@ -229,3 +230,142 @@ func TestHortizontalAutoScaleSpec(t *testing.T) { t.Errorf("Expected target cpu 40 but got %d", res.Spec.TargetCPUUtilizationPercentage) } } + +func TestListIngress(t *testing.T) { + + K8sObjects := []runtime.Object{ + &netv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service-ing", + Namespace: "namespace", + }, + }, + } + + kubeClientset := testclient.NewSimpleClientset(K8sObjects...) + cfg := &Config{ServicesNamespace: "namespace"} + + _, err := listIngress(kubeClientset, cfg) + + if err != nil { + t.Errorf("Error listing ingresses: %v", err) + } +} + +func TestUpdateIngress(t *testing.T) { + + K8sObjects := []runtime.Object{ + &netv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service-ing", + Namespace: "namespace", + }, + }, + } + + service := Service{ + Name: "service", + } + + kubeClientset := testclient.NewSimpleClientset(K8sObjects...) + cfg := &Config{ServicesNamespace: "namespace"} + + err := updateIngress(service, kubeClientset, cfg) + + if err != nil { + t.Errorf("Error updating ingress: %v", err) + } +} + +func TestDeleteIngress(t *testing.T) { + + K8sObjects := []runtime.Object{ + &netv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service-ing", + Namespace: "namespace", + }, + }, + } + + kubeClientset := testclient.NewSimpleClientset(K8sObjects...) + cfg := &Config{ServicesNamespace: "namespace"} + + err := deleteIngress("service-ing", kubeClientset, cfg) + + if err != nil { + t.Errorf("Error deleting ingress: %v", err) + } +} + +func TestUpdateSecret(t *testing.T) { + + K8sObjects := []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service-auth-expose", + Namespace: "namespace", + }, + }, + } + service := Service{ + Name: "service", + } + + kubeClientset := testclient.NewSimpleClientset(K8sObjects...) + cfg := &Config{ServicesNamespace: "namespace"} + + err := updateSecret(service, kubeClientset, cfg) + + if err != nil { + t.Errorf("Error updating secret: %v", err) + } +} + +func TestDeleteSecret(t *testing.T) { + + K8sObjects := []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service-auth-expose", + Namespace: "namespace", + }, + }, + } + + kubeClientset := testclient.NewSimpleClientset(K8sObjects...) + cfg := &Config{ServicesNamespace: "namespace"} + + err := deleteSecret("service", kubeClientset, cfg) + + if err != nil { + t.Errorf("Error deleting secret: %v", err) + } +} + +func TestExistsSecret(t *testing.T) { + + K8sObjects := []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service-auth-expose", + Namespace: "namespace", + }, + }, + } + + kubeClientset := testclient.NewSimpleClientset(K8sObjects...) + cfg := &Config{ServicesNamespace: "namespace"} + + exists := existsSecret("service", kubeClientset, cfg) + + if exists != true { + t.Errorf("Expected secret to exist but got %v", exists) + } + + notexists := existsSecret("service1", kubeClientset, cfg) + + if notexists != false { + t.Errorf("Expected secret not to exist but got %v", notexists) + } +} From eca143630084eb6006f8bca12b076aff98b621fb Mon Sep 17 00:00:00 2001 From: Miguel Caballer Date: Fri, 22 Nov 2024 12:14:47 +0100 Subject: [PATCH 10/10] Remove testify --- go.sum | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/go.sum b/go.sum index 3ef351e8..0fadf17e 100644 --- a/go.sum +++ b/go.sum @@ -153,7 +153,6 @@ github.com/grycap/cdmi-client-go v0.1.1/go.mod h1:ZqWeQS3YBJVXxg3HOIkAu1MLNJ4+7s github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI= github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg= github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo= -github.com/jmespath/go-jmespath/internal/testify v1.5.1/go.mod h1:L3OGu8Wl2/fWfCI6z80xFu9LTZmf1ZRjMHUOPmWr69U= github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= github.com/jpillora/backoff v1.0.0 h1:uvFg412JmmHBHw7iwprIxkPMI+sGQ4kzOWsMeHnm2EA= @@ -277,16 +276,6 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+ github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= -github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= -github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= -github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= -github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= -github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= -github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= -github.com/stretchr/testify v1.8.3/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/tinylib/msgp v1.1.8 h1:FCXC1xanKO4I8plpHGH2P7koL/RzZs12l/+r7vakfm0= github.com/tinylib/msgp v1.1.8/go.mod h1:qkpG+2ldGg4xRFmx+jfTvZPxfGFhi64BcnL9vkCm/Tw= github.com/tklauser/go-sysconf v0.3.11 h1:89WgdJhk5SNwJfu+GKyYveZ4IaJ7xAkecBo+KdJV0CM=