Add "login required" endpoints behind "admin" check and fix bug in admin auth handler

web_ui/authentication.go

@@ -212,14 +212,15 @@ func AdminAuthHandler(ctx *gin.Context) {
 	user := ctx.GetString("User")
 	// This should be done by a regular auth handler from the upstream, but we check here just in case
 	if user == "" {
-		ctx.JSON(http.StatusUnauthorized, gin.H{"error": "Login required to view this page"})
+		ctx.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "Login required to view this page"})
+		return
 	}
 	isAdmin, msg := CheckAdmin(user)
 	if isAdmin {
 		ctx.Next()
 		return
 	} else {
-		ctx.JSON(http.StatusForbidden, gin.H{"error": msg})
+		ctx.AbortWithStatusJSON(http.StatusForbidden, gin.H{"error": msg})
 	}
 }
 
@@ -370,7 +371,7 @@ func configureAuthEndpoints(ctx context.Context, router *gin.Engine, egrp *errgr
 	group.POST("/login", mw, loginHandler)
 	group.POST("/logout", AuthHandler, logoutHandler)
 	group.POST("/initLogin", initLoginHandler)
-	group.POST("/resetLogin", AuthHandler, resetLoginHandler)
+	group.POST("/resetLogin", AuthHandler, AdminAuthHandler, resetLoginHandler)
 	// Pass csrfhanlder only to the whoami route to generate CSRF token
 	// while leaving other routes free of CSRF check (we might want to do it some time in the future)
 	group.GET("/whoami", csrfHandler, whoamiHandler)

web_ui/authentication_test.go

@@ -37,6 +37,8 @@ import (
 	"github.com/pelicanplatform/pelican/config"
 	"github.com/pelicanplatform/pelican/param"
 	"github.com/pelicanplatform/pelican/test_utils"
+	"github.com/pelicanplatform/pelican/token"
+	"github.com/pelicanplatform/pelican/token_scopes"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -174,10 +176,10 @@ func TestPasswordResetAPI(t *testing.T) {
 	assert.NoError(t, err)
 
 	//Create a user for testing
-	err = WritePasswordEntry("user", "password")
+	user := "admin" // With admin privilege
+	err = WritePasswordEntry(user, "password")
 	assert.NoError(t, err, "error writing a user")
 	password := "password"
-	user := "user"
 	payload := fmt.Sprintf(`{"user": "%s", "password": "%s"}`, user, password)
 
 	//Create a request
@@ -220,7 +222,7 @@ func TestPasswordResetAPI(t *testing.T) {
 		assert.JSONEq(t, `{"msg":"Success"}`, recorderReset.Body.String())
 
 		//After password reset, test authorization with newly generated password
-		loginWithNewPasswordPayload := `{"user": "user", "password": "newpassword"}`
+		loginWithNewPasswordPayload := `{"user": "admin", "password": "newpassword"}`
 
 		reqLoginWithNewPassword, err := http.NewRequest("POST", "/api/v1.0/auth/login", strings.NewReader(loginWithNewPasswordPayload))
 		assert.NoError(t, err)
@@ -237,6 +239,39 @@ func TestPasswordResetAPI(t *testing.T) {
 		assert.JSONEq(t, `{"msg":"Success"}`, recorderLoginWithNewPassword.Body.String())
 	})
 
+	//Invoking password reset without a cookie should result in failure
+	t.Run("Without admin privilege", func(t *testing.T) {
+		resetPayload := `{"password": "newpassword"}`
+		reqReset, err := http.NewRequest("POST", "/api/v1.0/auth/resetLogin", strings.NewReader(resetPayload))
+		assert.NoError(t, err)
+
+		reqReset.Header.Set("Content-Type", "application/json")
+
+		loginCookieTokenCfg := token.NewWLCGToken()
+		loginCookieTokenCfg.Lifetime = 30 * time.Minute
+		loginCookieTokenCfg.Issuer = param.Server_ExternalWebUrl.GetString()
+		loginCookieTokenCfg.AddAudiences(param.Server_ExternalWebUrl.GetString())
+		loginCookieTokenCfg.Subject = "user" // general user
+		loginCookieTokenCfg.AddScopes(token_scopes.WebUi_Access, token_scopes.Monitoring_Query, token_scopes.Monitoring_Scrape)
+
+		// CreateToken also handles validation for us
+		tok, err := loginCookieTokenCfg.CreateToken()
+		require.NoError(t, err)
+
+		reqReset.AddCookie(&http.Cookie{
+			Name:  "login",
+			Value: tok,
+		})
+
+		recorderReset := httptest.NewRecorder()
+		router.ServeHTTP(recorderReset, reqReset)
+
+		//Check ok http reponse
+		assert.Equal(t, 403, recorderReset.Code)
+		//Check that success message returned
+		assert.JSONEq(t, `{"error":"Registry.AdminUsers is not set, and user is not root user. Admin check returns false"}`, recorderReset.Body.String())
+	})
+
 	//Invoking password reset without a cookie should result in failure
 	t.Run("Without valid cookie", func(t *testing.T) {
 		resetPayload := `{"password": "newpassword"}`
@@ -464,9 +499,6 @@ func TestWhoamiAPI(t *testing.T) {
 }
 
 func TestAdminAuthHandler(t *testing.T) {
-	// Initialize Gin and set it to test mode
-	gin.SetMode(gin.TestMode)
-
 	// Define test cases
 	testCases := []struct {
 		name          string
@@ -527,13 +559,22 @@ func TestAdminAuthHandler(t *testing.T) {
 		},
 	}
 
+	// Initialize Gin and set it to test mode
+	gin.SetMode(gin.TestMode)
+
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			w := httptest.NewRecorder()
-			ctx, _ := gin.CreateTestContext(w)
-			tc.setupUserFunc(ctx)
-
-			AdminAuthHandler(ctx)
+			router := gin.Default()
+			// If admin middleware didn't abort, the response will have status code == 200
+			router.GET("/test",
+				func(ctx *gin.Context) { tc.setupUserFunc(ctx) },
+				AdminAuthHandler,
+				func(ctx *gin.Context) { ctx.AbortWithStatus(http.StatusOK) },
+			)
+			req, err := http.NewRequest("GET", "/test", nil)
+			require.NoError(t, err)
+			router.ServeHTTP(w, req)
 
 			assert.Equal(t, tc.expectedCode, w.Code)
 			if tc.expectedError != "" {

web_ui/frontend/app/api/docs/pelican-swagger.yaml

@@ -14,6 +14,8 @@ info:
     The cookie is issued after a successful call to `/api/v1.0/auth/login`. However, OpenAPI 2.0 does not support specifying cookie-based security check.
     Therefore, as an alternative, we will add `Authentication Required` to the API description where needed.
 
+    For endpoint that is only accesible by users with admin privilege, we mark the endpoint as `Admin privilege Required`.
+
 
     For how to set up Pelican servers, please refer to the documentation at [docs.pelicanplatform.org](https://docs.pelicanplatform.org/)"
   license:
@@ -398,7 +400,8 @@ paths:
       tags:
         - common
       summary: Return the configuration values of the server and their type
-      description: "`Authentication Required`"
+      description: >-
+        `Authentication Required` `Admin privilege Required`
       produces:
         - application/json
       responses:
@@ -446,7 +449,7 @@ paths:
         - "common"
       summary: Update the server configuration with user-provided changes
       description:
-        "`Authentication Required` `Admin Previlege Required`
+        "`Authentication Required` `Admin privilege Required`
 
 
         The server will restart promptly after a  successful request. You may query against
@@ -531,7 +534,7 @@ paths:
       tags:
         - metrics
       summary: Returns the health status of server components
-      description: "`Authentication Required`"
+      description: "`Authentication Required` `Admin Privilege Required"
       produces:
         - application/json
       responses:
@@ -638,7 +641,7 @@ paths:
       tags:
         - auth
       summary: Reset the password for the user
-      description: "`Authentication Required`"
+      description: "`Authentication Required` `Admin Privilege Required`"
       consumes:
         - application/json
       produces:
@@ -912,7 +915,7 @@ paths:
       description: "`Authentication Required`
 
 
-        For user with admin previlege, it returns for all valid namespace request.
+        For user with admin privilege, it returns for all valid namespace request.
 
 
         For general users, it only returns namespace belonging to the user, or it returns 404
@@ -943,7 +946,7 @@ paths:
             type: object
             $ref: "#/definitions/ErrorModel"
         "404":
-          description: Namespace not found, either does not exists or the user doesn't have previlege to get it
+          description: Namespace not found, either does not exists or the user doesn't have privilege to get it
           schema:
             type: object
             $ref: "#/definitions/ErrorModel"
@@ -959,7 +962,7 @@ paths:
       description: "`Authentication Required`
 
 
-        For user with admin previlege, they can update any valid namespace.
+        For user with admin privilege, they can update any valid namespace.
 
 
         Non-admin users can update only namespaces they own and only if the approval status is `admin_metadata.status == approved`, otherwise the endpoint returns 404.
@@ -1003,7 +1006,7 @@ paths:
             type: object
             $ref: "#/definitions/ErrorModel"
         "403":
-          description: The user does not have previlege to update the namespace
+          description: The user does not have privilege to update the namespace
           schema:
             type: object
             $ref: "#/definitions/ErrorModel"
@@ -1019,7 +1022,7 @@ paths:
             $ref: "#/definitions/ErrorModel"
     delete:
       summary: Delete a namespace registration by ID
-      description: "`Authentication Required` `Admin`
+      description: "`Authentication Required` `Admin privilege Required`
 
         Only admin users have privilege to this action.
         "
@@ -1045,7 +1048,7 @@ paths:
             type: object
             $ref: "#/definitions/ErrorModel"
         "403":
-          description: The user does not have previlege to update the namespace
+          description: The user does not have privilege to update the namespace
           schema:
             type: object
             $ref: "#/definitions/ErrorModel"
@@ -1081,7 +1084,7 @@ paths:
             type: object
             $ref: "#/definitions/ErrorModel"
         "404":
-          description: Namespace not found, either does not exist or the user doesn't have previlege to get it
+          description: Namespace not found, either does not exist or the user doesn't have privilege to get it
           schema:
             type: object
             $ref: "#/definitions/ErrorModel"
@@ -1101,7 +1104,7 @@ paths:
         Update namespace status to `approved` by namespace `id`.
 
 
-        This action requires admin previlege to perform.
+        This action requires admin privilege to perform.
         "
       parameters:
         - name: id
@@ -1133,7 +1136,7 @@ paths:
             type: object
             $ref: "#/definitions/ErrorModel"
         "403":
-          description: The user does not have previlege to update the namespace status
+          description: The user does not have privilege to update the namespace status
           schema:
             type: object
             $ref: "#/definitions/ErrorModel"
@@ -1158,7 +1161,7 @@ paths:
         Update namespace status to `denied` by namespace `id`.
 
 
-        This action requires admin previlege to perform.
+        This action requires admin privilege to perform.
         "
       parameters:
         - name: id
@@ -1190,7 +1193,7 @@ paths:
             type: object
             $ref: "#/definitions/ErrorModel"
         "403":
-          description: The user does not have previlege to update the namespace status
+          description: The user does not have privilege to update the namespace status
           schema:
             type: object
             $ref: "#/definitions/ErrorModel"
@@ -1261,7 +1264,7 @@ paths:
     patch:
       summary: Filter a server from director redirecting
       description: |
-        `Authentication Required` `Admin`
+        `Authentication Required` `Admin privilege Required`
 
 
         **The filter rules registered by this endpoint are in-memory and will be reset at the server restart.**

web_ui/ui.go

@@ -240,7 +240,7 @@ func configureWebResource(engine *gin.Engine) error {
 
 // Configure common endpoint available to all server web UI which are located at /api/v1.0/*
 func configureCommonEndpoints(engine *gin.Engine) error {
-	engine.GET("/api/v1.0/config", AuthHandler, getConfigValues)
+	engine.GET("/api/v1.0/config", AuthHandler, AdminAuthHandler, getConfigValues)
 	engine.PATCH("/api/v1.0/config", AuthHandler, AdminAuthHandler, updateConfigValues)
 	engine.GET("/api/v1.0/servers", getEnabledServers)
 	// Health check endpoint for web engine
@@ -258,7 +258,7 @@ func configureMetrics(engine *gin.Engine) error {
 	prometheusMonitor := ginprometheus.NewPrometheus("gin")
 	prometheusMonitor.Use(engine)
 
-	engine.GET("/api/v1.0/metrics/health", AuthHandler, func(ctx *gin.Context) {
+	engine.GET("/api/v1.0/metrics/health", AuthHandler, AdminAuthHandler, func(ctx *gin.Context) {
 		healthStatus := metrics.GetHealthStatus()
 		ctx.JSON(http.StatusOK, healthStatus)
 	})