Merge pull request #781 from helixml/revert-743-admin-src

Revert "Introduce ADMIN_USER_SOURCE for checking admin users"

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 {

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,87 +49,32 @@ 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
 		}
 	}
 	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
 }

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 {

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
 }

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,
 			},
 		),

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"

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

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

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