From 5c8b373dd789a152f855041b859849d6361154d9 Mon Sep 17 00:00:00 2001 From: lukemarsden Date: Wed, 29 Jan 2025 10:34:04 +0000 Subject: [PATCH] Revert "Introduce ADMIN_USER_SOURCE for checking admin users" --- api/pkg/config/config.go | 27 +----- api/pkg/server/auth_middleware.go | 85 ++++--------------- api/pkg/server/auth_utils.go | 3 +- api/pkg/server/handlers.go | 34 +++----- api/pkg/server/server.go | 1 - charts/helix-controlplane/values-example.yaml | 1 - docker-compose.dev.yaml | 1 - docker-compose.yaml | 1 - scripts/kind_helm_install.sh | 3 +- 9 files changed, 33 insertions(+), 123 deletions(-) diff --git a/api/pkg/config/config.go b/api/pkg/config/config.go index 087b0cb95..3925e01fa 100644 --- a/api/pkg/config/config.go +++ b/api/pkg/config/config.go @@ -1,7 +1,6 @@ package config import ( - "fmt" "time" "github.com/helixml/helix/api/pkg/types" @@ -265,11 +264,8 @@ type WebServer struct { RunnerToken string `envconfig:"RUNNER_TOKEN" description:"The token for runner auth."` // a list of keycloak ids that are considered admins - // if the string 'all' is included it means ALL users + // if the string '*' is included it means ALL users AdminIDs []string `envconfig:"ADMIN_USER_IDS" description:"Keycloak admin IDs."` - // Specifies the source of the Admin user IDs. - // By default AdminSrc is set to env. - AdminSrc AdminSrcType `envconfig:"ADMIN_USER_SOURCE" default:"env" description:"Source of admin IDs"` // if this is specified then we provide the option to clone entire // sessions into this user without having to logout and login EvalUserID string `envconfig:"EVAL_USER_ID" description:""` @@ -282,27 +278,6 @@ type WebServer struct { LocalFilestorePath string } -// AdminSrcType is an enum specifyin the type of Admin ID source. -// It currently supports only two sources: -// * env: ADMIN_USER_IDS env var -// * jwt: admin JWT token claim -type AdminSrcType string - -const ( - AdminSrcTypeEnv AdminSrcType = "env" - AdminSrcTypeJWT AdminSrcType = "jwt" -) - -// Decode implements envconfig.Decoder for value validation. -func (a *AdminSrcType) Decode(value string) error { - switch value { - case string(AdminSrcTypeEnv), string(AdminSrcTypeJWT): - return nil - default: - return fmt.Errorf("invalid source of admin IDs: %q", value) - } -} - type SubscriptionQuotas struct { Enabled bool `envconfig:"SUBSCRIPTION_QUOTAS_ENABLED" default:"true"` Finetuning struct { diff --git a/api/pkg/server/auth_middleware.go b/api/pkg/server/auth_middleware.go index b2f9968e8..d928dab98 100644 --- a/api/pkg/server/auth_middleware.go +++ b/api/pkg/server/auth_middleware.go @@ -9,7 +9,6 @@ import ( jwt "github.com/golang-jwt/jwt/v5" "github.com/helixml/helix/api/pkg/auth" - "github.com/helixml/helix/api/pkg/config" "github.com/helixml/helix/api/pkg/store" "github.com/helixml/helix/api/pkg/types" ) @@ -31,14 +30,17 @@ var ( type authMiddlewareConfig struct { adminUserIDs []string - adminUserSrc config.AdminSrcType runnerToken string } type authMiddleware struct { authenticator auth.Authenticator store store.Store - cfg authMiddlewareConfig + adminUserIDs []string + runnerToken string + // this means ALL users + // if '*' is included in the list + developmentMode bool } func newAuthMiddleware( @@ -47,65 +49,19 @@ func newAuthMiddleware( cfg authMiddlewareConfig, ) *authMiddleware { return &authMiddleware{ - authenticator: authenticator, - store: store, - cfg: cfg, + authenticator: authenticator, + store: store, + adminUserIDs: cfg.adminUserIDs, + runnerToken: cfg.runnerToken, + developmentMode: isDevelopmentMode(cfg.adminUserIDs), } } -type account struct { - userID string - token *jwt.Token -} - -type accountType string - -const ( - accountTypeUser accountType = "user" - accountTypeToken accountType = "token" - accountTypeInvalid accountType = "invalid" -) - -func (a *account) Type() accountType { - switch { - case a.userID != "": - return accountTypeUser - case a.token != nil: - return accountTypeToken - } - return accountTypeInvalid -} - -func (auth *authMiddleware) isAdmin(acct account) bool { - if acct.Type() == accountTypeInvalid { - return false - } - - switch auth.cfg.adminUserSrc { - case config.AdminSrcTypeEnv: - if acct.Type() != accountTypeUser { - return false - } - return auth.isUserAdmin(acct.userID) - case config.AdminSrcTypeJWT: - if acct.Type() != accountTypeToken { - return false - } - return auth.isTokenAdmin(acct.token) - } - return false -} - func (auth *authMiddleware) isUserAdmin(userID string) bool { - if userID == "" { - return false + if auth.developmentMode { + return true } - - for _, adminID := range auth.cfg.adminUserIDs { - // development mode everyone is an admin - if adminID == types.AdminAllUsers { - return true - } + for _, adminID := range auth.adminUserIDs { if adminID == userID { return true } @@ -113,21 +69,12 @@ func (auth *authMiddleware) isUserAdmin(userID string) bool { return false } -func (auth *authMiddleware) isTokenAdmin(token *jwt.Token) bool { - if token == nil { - return false - } - mc := token.Claims.(jwt.MapClaims) - isAdmin := mc["admin"].(bool) - return isAdmin -} - func (auth *authMiddleware) getUserFromToken(ctx context.Context, token string) (*types.User, error) { if token == "" { return nil, nil } - if token == auth.cfg.runnerToken { + if token == auth.runnerToken { // if the api key is our runner token then we are in runner mode return &types.User{ Token: token, @@ -154,7 +101,7 @@ func (auth *authMiddleware) getUserFromToken(ctx context.Context, token string) user.TokenType = types.TokenTypeAPIKey user.ID = apiKey.Owner user.Type = apiKey.OwnerType - user.Admin = auth.isAdmin(account{userID: user.ID}) + user.Admin = auth.isUserAdmin(user.ID) if apiKey.AppID != nil && apiKey.AppID.Valid { user.AppID = apiKey.AppID.String } @@ -183,7 +130,7 @@ func (auth *authMiddleware) getUserFromToken(ctx context.Context, token string) user.TokenType = types.TokenTypeKeycloak user.ID = keycloakUserID user.Type = types.OwnerTypeUser - user.Admin = auth.isAdmin(account{token: keycloakJWT}) + user.Admin = auth.isUserAdmin(user.ID) return user, nil } diff --git a/api/pkg/server/auth_utils.go b/api/pkg/server/auth_utils.go index 84ba32826..f00622e31 100644 --- a/api/pkg/server/auth_utils.go +++ b/api/pkg/server/auth_utils.go @@ -117,8 +117,7 @@ func addCorsHeaders(w http.ResponseWriter) { Access Control - */ -// if any of the admin users IDs is "all" then we are in dev mode and every user is an admin -// nolint:unused +// if any of the admin users IDs is "*" then we are in dev mode and every user is an admin func isDevelopmentMode(adminUserIDs []string) bool { for _, id := range adminUserIDs { if id == types.AdminAllUsers { diff --git a/api/pkg/server/handlers.go b/api/pkg/server/handlers.go index c53306288..f2b072177 100644 --- a/api/pkg/server/handlers.go +++ b/api/pkg/server/handlers.go @@ -3,19 +3,18 @@ package server import ( "archive/tar" "bytes" - "context" "database/sql" "encoding/json" "fmt" "io" "net/http" + "os" "path/filepath" "strconv" "strings" "github.com/gorilla/mux" - "github.com/helixml/helix/api/pkg/config" "github.com/helixml/helix/api/pkg/controller" "github.com/helixml/helix/api/pkg/data" "github.com/helixml/helix/api/pkg/filestore" @@ -502,6 +501,11 @@ func (apiServer *HelixAPIServer) retryTextFinetune(_ http.ResponseWriter, req *h } func (apiServer *HelixAPIServer) cloneFinetuneInteraction(_ http.ResponseWriter, req *http.Request) (*types.Session, *system.HTTPError) { + vars := mux.Vars(req) + + user := getRequestUser(req) + ctx := req.Context() + // clone the session into the eval user account // only admins can do this cloneIntoEvalUser := req.URL.Query().Get("clone_into_eval_user") @@ -517,10 +521,6 @@ func (apiServer *HelixAPIServer) cloneFinetuneInteraction(_ http.ResponseWriter, return nil, httpError } - vars := mux.Vars(req) - user := getRequestUser(req) - ctx := req.Context() - // if we own the session then we don't need to copy all files copyAllFiles := true if doesOwnSession(user, session) { @@ -654,22 +654,16 @@ func (apiServer *HelixAPIServer) updateSessionMeta(_ http.ResponseWriter, req *h } func (apiServer *HelixAPIServer) isAdmin(req *http.Request) bool { - auth := apiServer.authMiddleware - - switch auth.cfg.adminUserSrc { - case config.AdminSrcTypeEnv: - user := getRequestUser(req) - return auth.isUserAdmin(user.ID) - case config.AdminSrcTypeJWT: - token := getRequestToken(req) - if token == "" { - return false + user := getRequestUser(req) + adminUserIDs := strings.Split(os.Getenv("ADMIN_USER_IDS"), ",") + for _, a := range adminUserIDs { + // development mode everyone is an admin + if a == "all" { + return true } - jwtToken, err := auth.authenticator.ValidateUserToken(context.Background(), token) - if err != nil { - return false + if a == user.ID { + return true } - return auth.isTokenAdmin(jwtToken) } return false } diff --git a/api/pkg/server/server.go b/api/pkg/server/server.go index 53c565841..917cd5cde 100644 --- a/api/pkg/server/server.go +++ b/api/pkg/server/server.go @@ -118,7 +118,6 @@ func NewServer( store, authMiddlewareConfig{ adminUserIDs: cfg.WebServer.AdminIDs, - adminUserSrc: cfg.WebServer.AdminSrc, runnerToken: cfg.WebServer.RunnerToken, }, ), diff --git a/charts/helix-controlplane/values-example.yaml b/charts/helix-controlplane/values-example.yaml index 5a7d84507..cd5ad75db 100644 --- a/charts/helix-controlplane/values-example.yaml +++ b/charts/helix-controlplane/values-example.yaml @@ -36,7 +36,6 @@ envVariables: KEYCLOAK_PASSWORD: "oh-hallo-insecure-password" # Dashboard ADMIN_USER_IDS: "all" - ADMIN_USER_SOURCE: "env" # Evals EVAL_USER_ID: "" HELIX_API_KEY: "helix_api_key" diff --git a/docker-compose.dev.yaml b/docker-compose.dev.yaml index 34a8672b7..c9f839c05 100644 --- a/docker-compose.dev.yaml +++ b/docker-compose.dev.yaml @@ -35,7 +35,6 @@ services: - KEYCLOAK_FRONTEND_URL=${KEYCLOAK_FRONTEND_URL:-http://localhost:8080/auth/} # lock down dashboard in production - ADMIN_USER_IDS=${ADMIN_USER_IDS-all} - - ADMIN_USER_SOURCE=${ADMIN_USER_SOURCE-env} - TEXT_EXTRACTION_URL=http://llamaindex:5000/api/v1/extract - RAG_INDEX_URL=http://llamaindex:5000/api/v1/rag/chunk - RAG_QUERY_URL=http://llamaindex:5000/api/v1/rag/query diff --git a/docker-compose.yaml b/docker-compose.yaml index 3d2b2ce90..d4e55e7ba 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -32,7 +32,6 @@ services: - KEYCLOAK_FRONTEND_URL=${KEYCLOAK_FRONTEND_URL:-http://localhost:8080/auth/} # lock down dashboard in production - ADMIN_USER_IDS=${ADMIN_USER_IDS-all} - - ADMIN_USER_SOURCE=${ADMIN_USER_SOURCE-env} - TEXT_EXTRACTION_URL=http://llamaindex:5000/api/v1/extract - RAG_INDEX_URL=http://llamaindex:5000/api/v1/rag/chunk - RAG_QUERY_URL=http://llamaindex:5000/api/v1/rag/query diff --git a/scripts/kind_helm_install.sh b/scripts/kind_helm_install.sh index 38fd0b059..5e51e0747 100755 --- a/scripts/kind_helm_install.sh +++ b/scripts/kind_helm_install.sh @@ -124,7 +124,6 @@ for env_var in \ KEYCLOAK_USER \ KEYCLOAK_PASSWORD \ ADMIN_USER_IDS \ - ADMIN_USER_SOURCE \ EVAL_USER_ID \ HELIX_API_KEY; do eval value=\${$env_var:-} @@ -136,4 +135,4 @@ done # Execute helm command with all accumulated values helm upgrade --install my-helix-controlplane $CHART "${HELM_VALUES[@]}" -kubectl wait --for=condition=ready pod -l app.kubernetes.io/name=helix-controlplane --timeout=300s +kubectl wait --for=condition=ready pod -l app.kubernetes.io/name=helix-controlplane --timeout=300s \ No newline at end of file