From 7d10d1baea5fb2e67d1557d5d4259dd4165feb43 Mon Sep 17 00:00:00 2001 From: catttam Date: Fri, 21 Feb 2025 10:54:20 +0100 Subject: [PATCH] Changes on OIDC to support multiple issuers --- go.mod | 1 + go.sum | 13 ++++++++++ pkg/handlers/create.go | 14 +++++++--- pkg/handlers/job.go | 11 ++++++-- pkg/handlers/run.go | 12 +++++++-- pkg/handlers/update.go | 2 +- pkg/types/config.go | 6 ++--- pkg/utils/auth/auth.go | 2 +- pkg/utils/auth/oidc.go | 52 ++++++++++++++++++++++++++++++------- pkg/utils/auth/oidc_test.go | 12 +++++---- ui | 2 +- 11 files changed, 98 insertions(+), 29 deletions(-) diff --git a/go.mod b/go.mod index c29f3ffe..815d51d8 100644 --- a/go.mod +++ b/go.mod @@ -74,6 +74,7 @@ require ( github.com/go-playground/universal-translator v0.18.1 // indirect github.com/goccy/go-json v0.10.2 // indirect github.com/gogo/protobuf v1.3.2 // indirect + github.com/golang-jwt/jwt/v5 v5.2.1 github.com/golang/protobuf v1.5.3 // indirect github.com/google/gnostic-models v0.6.8 // indirect github.com/google/go-containerregistry v0.13.0 // indirect diff --git a/go.sum b/go.sum index 0fadf17e..e7244394 100644 --- a/go.sum +++ b/go.sum @@ -110,6 +110,8 @@ 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-jwt/jwt/v5 v5.2.1 h1:OuVbFODueb089Lh128TAcimifWaLhJwVflnrgM17wHk= +github.com/golang-jwt/jwt/v5 v5.2.1/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= 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= @@ -153,6 +155,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= @@ -276,6 +279,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/handlers/create.go b/pkg/handlers/create.go index 7458a7d2..ea38f3bd 100644 --- a/pkg/handlers/create.go +++ b/pkg/handlers/create.go @@ -95,7 +95,7 @@ func MakeCreateHandler(cfg *types.Config, back types.ServerlessBackend) gin.Hand if service.VO != "" { for _, vo := range cfg.OIDCGroups { if vo == service.VO { - err := checkIdentity(&service, cfg, authHeader) + err := checkIdentity(&service, authHeader) if err != nil { c.String(http.StatusBadRequest, fmt.Sprintln(err)) return @@ -609,10 +609,16 @@ func getProviderInfo(rawInfo string) (string, string) { return provID, provName } -func checkIdentity(service *types.Service, cfg *types.Config, authHeader string) error { - oidcManager, _ := auth.NewOIDCManager(cfg.OIDCIssuer, cfg.OIDCSubject, cfg.OIDCGroups) +func checkIdentity(service *types.Service, authHeader string) error { rawToken := strings.TrimPrefix(authHeader, "Bearer ") - + issuer, err := auth.GetIssuerFromToken(rawToken) + if err != nil { + return err + } + oidcManager := auth.ClusterOidcManagers[issuer] + if oidcManager == nil { + return err + } hasVO, err := oidcManager.UserHasVO(rawToken, service.VO) if err != nil { diff --git a/pkg/handlers/job.go b/pkg/handlers/job.go index 977a1da7..d5da9346 100644 --- a/pkg/handlers/job.go +++ b/pkg/handlers/job.go @@ -104,8 +104,15 @@ func MakeJobHandler(cfg *types.Config, kubeClientset kubernetes.Interface, back // If isn't service token check if it is an oidc token var uidFromToken string if len(rawToken) != tokenLength { - oidcManager, _ := auth.NewOIDCManager(cfg.OIDCIssuer, cfg.OIDCSubject, cfg.OIDCGroups) - + issuer, err := auth.GetIssuerFromToken(rawToken) + if err != nil { + c.String(http.StatusBadGateway, fmt.Sprintf("%v", err)) + } + oidcManager := auth.ClusterOidcManagers[issuer] + if oidcManager == nil { + c.String(http.StatusBadRequest, fmt.Sprintf("Error getting oidc manager for issuer '%s'", issuer)) + return + } if !oidcManager.IsAuthorised(rawToken) { c.Status(http.StatusUnauthorized) return diff --git a/pkg/handlers/run.go b/pkg/handlers/run.go index ee7dec56..ba61650b 100644 --- a/pkg/handlers/run.go +++ b/pkg/handlers/run.go @@ -17,6 +17,7 @@ limitations under the License. package handlers import ( + "fmt" "net/http" "net/http/httputil" "strings" @@ -62,8 +63,15 @@ func MakeRunHandler(cfg *types.Config, back types.SyncBackend) gin.HandlerFunc { return } } else { - oidcManager, _ := auth.NewOIDCManager(cfg.OIDCIssuer, cfg.OIDCSubject, cfg.OIDCGroups) - + issuer, err := auth.GetIssuerFromToken(rawToken) + if err != nil { + c.String(http.StatusBadGateway, err.Error()) + } + oidcManager := auth.ClusterOidcManagers[issuer] + if oidcManager == nil { + c.String(http.StatusBadRequest, fmt.Sprintf("Error getting oidc manager for issuer '%s'", issuer)) + return + } if !oidcManager.IsAuthorised(rawToken) { c.Status(http.StatusUnauthorized) return diff --git a/pkg/handlers/update.go b/pkg/handlers/update.go index 62a83171..f9c6f894 100644 --- a/pkg/handlers/update.go +++ b/pkg/handlers/update.go @@ -83,7 +83,7 @@ func MakeUpdateHandler(cfg *types.Config, back types.ServerlessBackend) gin.Hand for _, vo := range cfg.OIDCGroups { if vo == newService.VO { authHeader := c.GetHeader("Authorization") - err := checkIdentity(&newService, cfg, authHeader) + err := checkIdentity(&newService, authHeader) if err != nil { c.String(http.StatusBadRequest, fmt.Sprintln(err)) } diff --git a/pkg/types/config.go b/pkg/types/config.go index 6ad0a3a9..6126a09e 100644 --- a/pkg/types/config.go +++ b/pkg/types/config.go @@ -174,8 +174,8 @@ type Config struct { // OIDCEnable parameter to enable OIDC support OIDCEnable bool `json:"-"` - // OIDCIssuer OpenID Connect issuer as returned in the "iss" field of the JWT payload - OIDCIssuer string `json:"-"` + // OIDCValidIssuers List of allowed providers to authenticate + OIDCValidIssuers []string `json:"-"` // OIDCSubject OpenID Connect Subject (user identifier) OIDCSubject string `json:"-"` @@ -234,7 +234,7 @@ var configVars = []configVar{ {"ReSchedulerInterval", "RESCHEDULER_INTERVAL", false, intType, "15"}, {"ReSchedulerThreshold", "RESCHEDULER_THRESHOLD", false, intType, "30"}, {"OIDCEnable", "OIDC_ENABLE", false, boolType, "false"}, - {"OIDCIssuer", "OIDC_ISSUER", false, stringType, "https://aai.egi.eu/oidc/"}, + {"OIDCValidIssuers", "OIDC_ISSUERS", false, stringSliceType, ""}, {"OIDCSubject", "OIDC_SUBJECT", false, stringType, ""}, {"OIDCGroups", "OIDC_GROUPS", false, stringSliceType, ""}, {"IngressHost", "INGRESS_HOST", false, stringType, ""}, diff --git a/pkg/utils/auth/auth.go b/pkg/utils/auth/auth.go index cea4ad6b..0a4e3635 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() // #nosec G104 minIOAdminClient.UpdateUsersInGroup(oscarUser, "all_users_group", false) // #nosec G104 - oidcHandler := getOIDCMiddleware(kubeClientset, minIOAdminClient, cfg.OIDCIssuer, cfg.OIDCSubject, cfg.OIDCGroups, nil) + oidcHandler := getOIDCMiddleware(kubeClientset, minIOAdminClient, cfg, 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 b7ad6c30..8c068096 100644 --- a/pkg/utils/auth/oidc.go +++ b/pkg/utils/auth/oidc.go @@ -27,6 +27,8 @@ import ( "github.com/coreos/go-oidc/v3/oidc" "github.com/gin-gonic/gin" + "github.com/golang-jwt/jwt/v5" + "github.com/grycap/oscar/v3/pkg/types" "github.com/grycap/oscar/v3/pkg/utils" "golang.org/x/oauth2" "k8s.io/client-go/kubernetes" @@ -39,6 +41,7 @@ const ( ) var oidcLogger = log.New(os.Stdout, "[OIDC-AUTH] ", log.Flags()) +var ClusterOidcManagers = make(map[string]*oidcManager) // oidcManager struct to represent a OIDC manager, including a cache of tokens type oidcManager struct { @@ -76,18 +79,25 @@ 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, 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) +func getOIDCMiddleware(kubeClientset kubernetes.Interface, minIOAdminClient *utils.MinIOAdminClient, cfg *types.Config, oidcConfig *oidc.Config) gin.HandlerFunc { + + for _, iss := range cfg.OIDCValidIssuers { + issuerManager, err := NewOIDCManager(iss, cfg.OIDCSubject, cfg.OIDCGroups) + if oidcConfig != nil { + issuerManager.config = oidcConfig + } + if err != nil { + return func(c *gin.Context) { + c.AbortWithStatus(http.StatusUnauthorized) + return + } } + + ClusterOidcManagers[iss] = issuerManager + } - mc := NewMultitenancyConfig(kubeClientset, subject) + mc := NewMultitenancyConfig(kubeClientset, cfg.OIDCSubject) return func(c *gin.Context) { // Get token from headers @@ -97,7 +107,16 @@ func getOIDCMiddleware(kubeClientset kubernetes.Interface, minIOAdminClient *uti return } rawToken := strings.TrimPrefix(authHeader, "Bearer ") - + iss, err := GetIssuerFromToken(rawToken) + if err != nil { + c.String(http.StatusBadRequest, fmt.Sprintf("%v", err)) + return + } + oidcManager := ClusterOidcManagers[iss] + if oidcManager == nil { + c.String(http.StatusBadRequest, fmt.Sprintf("Error getting oidc manager for issuer '%s'", iss)) + return + } // Check the token if !oidcManager.IsAuthorised(rawToken) { c.AbortWithStatus(http.StatusUnauthorized) @@ -188,6 +207,19 @@ func getGroups(urns []string) []string { return groups } +func GetIssuerFromToken(rawToken string) (string, error) { + token, _, err := new(jwt.Parser).ParseUnverified(rawToken, jwt.MapClaims{}) + if err != nil { + return "", err + } + claims, _ := token.Claims.(jwt.MapClaims) + iss, err := claims.GetIssuer() + if err != nil { + return "", err + } + return iss, nil +} + // UserHasVO checks if the user contained on the request token is enrolled on a specific VO func (om *oidcManager) UserHasVO(rawToken string, vo string) (bool, error) { ui, err := om.GetUserInfo(rawToken) diff --git a/pkg/utils/auth/oidc_test.go b/pkg/utils/auth/oidc_test.go index 142e520f..30345bf9 100644 --- a/pkg/utils/auth/oidc_test.go +++ b/pkg/utils/auth/oidc_test.go @@ -153,17 +153,19 @@ func TestGetOIDCMiddleware(t *testing.T) { Endpoint: server.URL, Verify: false, }, + OIDCEnable: true, + OIDCSubject: "user@egi.eu", + OIDCValidIssuers: []string{server.URL}, + OIDCGroups: []string{"group1", "group2"}, } minIOAdminClient, _ := utils.MakeMinIOAdminClient(&cfg) issuer := server.URL - subject := "user@egi.eu" - groups := []string{"group1", "group2"} oidcConfig := &oidc.Config{ InsecureSkipSignatureCheck: true, SkipClientIDCheck: true, } - middleware := getOIDCMiddleware(kubeClientset, minIOAdminClient, issuer, subject, groups, oidcConfig) + middleware := getOIDCMiddleware(kubeClientset, minIOAdminClient, &cfg, oidcConfig) if middleware == nil { t.Errorf("expected middleware to be non-nil") } @@ -176,11 +178,11 @@ func TestGetOIDCMiddleware(t *testing.T) { { name: "invalid-token", token: "invalid-token", - code: http.StatusUnauthorized, + code: http.StatusBadRequest, }, { name: "valid-token", - token: GetToken(issuer, subject), + token: GetToken(issuer, cfg.OIDCSubject), code: http.StatusOK, }, } diff --git a/ui b/ui index 329e070b..3ce8470b 160000 --- a/ui +++ b/ui @@ -1 +1 @@ -Subproject commit 329e070b00fbd601acd3e364acce97a34fa8129c +Subproject commit 3ce8470b994e73789ec086e612af66b9721f2aff