From fbd88da02e3d6ffc627b43a4c94cd0741e6e98e3 Mon Sep 17 00:00:00 2001 From: Gaius Date: Fri, 14 Mar 2025 13:31:47 +0800 Subject: [PATCH] feat(manager): an empty permissions list grants all permissions (#3889) Signed-off-by: Gaius --- client-rs | 2 +- manager/job/preheat.go | 4 -- manager/job/types.go | 6 +- manager/middlewares/personal_access_token.go | 73 ++++++++++++++------ manager/service/personal_access_token.go | 8 +++ manager/types/persional_access_token.go | 8 ++- 6 files changed, 69 insertions(+), 32 deletions(-) diff --git a/client-rs b/client-rs index 316cb541d..beff3f87b 160000 --- a/client-rs +++ b/client-rs @@ -1 +1 @@ -Subproject commit 316cb541dde8b32f66c291b66d6347ec17cb1cca +Subproject commit beff3f87b3b2b9f1161984b21aca984b448b3d93 diff --git a/manager/job/preheat.go b/manager/job/preheat.go index e5d08ef97..8489d58a7 100644 --- a/manager/job/preheat.go +++ b/manager/job/preheat.go @@ -27,7 +27,6 @@ import ( "io" "net/http" "net/url" - "regexp" "time" machineryv1tasks "github.com/RichardKnop/machinery/v1/tasks" @@ -73,9 +72,6 @@ var defaultHTTPTransport = &http.Transport{ IdleConnTimeout: 120 * time.Second, } -// accessURLPattern is the pattern of access url. -var accessURLPattern, _ = regexp.Compile("^(.*)://(.*)/v2/(.*)/manifests/(.*)") - // Preheat is an interface for preheat job. type Preheat interface { // CreatePreheat creates a preheat job. diff --git a/manager/job/types.go b/manager/job/types.go index 3115d7769..0d829b9ad 100644 --- a/manager/job/types.go +++ b/manager/job/types.go @@ -3,8 +3,12 @@ package job import ( "errors" "fmt" + "regexp" ) +// accessURLRegexp is the regular expression for parsing access url. +var accessURLRegexp, _ = regexp.Compile("^(.*)://(.*)/v2/(.*)/manifests/(.*)") + // preheatImage is image information for preheat. type preheatImage struct { protocol string @@ -23,7 +27,7 @@ func (p *preheatImage) blobsURL(digest string) string { // parseManifestURL parses manifest url. func parseManifestURL(url string) (*preheatImage, error) { - r := accessURLPattern.FindStringSubmatch(url) + r := accessURLRegexp.FindStringSubmatch(url) if len(r) != 5 { return nil, errors.New("parse access url failed") } diff --git a/manager/middlewares/personal_access_token.go b/manager/middlewares/personal_access_token.go index b71fb7e65..a7cec4b61 100644 --- a/manager/middlewares/personal_access_token.go +++ b/manager/middlewares/personal_access_token.go @@ -33,6 +33,8 @@ import ( ) var ( + // oapiResourceRegexp is a regular expression to extract the resource type from the path. + // Example: /oapi/v1/jobs/1 -> jobs. oapiResourceRegexp = regexp.MustCompile(`^/oapi/v[0-9]+/([-_a-zA-Z]*)[/.*]*`) ) @@ -45,6 +47,7 @@ func PersonalAccessToken(gdb *gorm.DB) gin.HandlerFunc { c.JSON(http.StatusUnauthorized, ErrorResponse{ Message: http.StatusText(http.StatusUnauthorized), }) + c.Abort() return } @@ -53,51 +56,56 @@ func PersonalAccessToken(gdb *gorm.DB) gin.HandlerFunc { personalAccessToken := tokenFields[1] var token models.PersonalAccessToken if err := gdb.WithContext(c).Where("token = ?", personalAccessToken).First(&token).Error; err != nil { - logger.Errorf("Invalid personal access token attempt: %s, error: %v", c.Request.URL.Path, err) + logger.Errorf("invalid personal access token attempt: %s, error: %v", c.Request.URL.Path, err) c.JSON(http.StatusUnauthorized, ErrorResponse{ Message: http.StatusText(http.StatusUnauthorized), }) + c.Abort() return } // Check if the token is active. if token.State != models.PersonalAccessTokenStateActive { - logger.Errorf("Inactive token used: %s, token name: %s, user_id: %d", c.Request.URL.Path, token.Name, token.UserID) + logger.Errorf("inactive token used: %s, token name: %s, user_id: %d", c.Request.URL.Path, token.Name, token.UserID) c.JSON(http.StatusForbidden, ErrorResponse{ Message: "Token is inactive", }) + c.Abort() return } // Check if the token has expired. if time.Now().After(token.ExpiredAt) { - logger.Errorf("Expired token used: %s, token name: %s, user_id: %d, expired: %v", + logger.Errorf("expired token used: %s, token name: %s, user_id: %d, expired: %v", c.Request.URL.Path, token.Name, token.UserID, token.ExpiredAt) c.JSON(http.StatusForbidden, ErrorResponse{ Message: "Token has expired", }) + c.Abort() return } // Check if the token's scopes include the required resource type. - hasScope := false - resourceType := getAPIResourceType(c.Request.URL.Path) - for _, scope := range token.Scopes { - if scope == resourceType { - hasScope = true - break - } + requiredPermission, err := requiredPermission(c.Request.URL.Path) + if err != nil { + logger.Errorf("failed to extract resource type from path: %s, error: %v", c.Request.URL.Path, err) + c.JSON(http.StatusForbidden, ErrorResponse{ + Message: fmt.Sprintf("Failed to extract resource type from path: %s", c.Request.URL.Path), + }) + + c.Abort() + return } - if !hasScope { - logger.Errorf("Insufficient scope token used: %s, token name: %s, user_id: %d, required: %s, available: %v", - c.Request.URL.Path, token.Name, token.UserID, resourceType, token.Scopes) + if !hasPermission(token.Scopes, requiredPermission) { + logger.Errorf("insufficient scope token used %s. Required permission: %s", token.Name, requiredPermission) c.JSON(http.StatusForbidden, ErrorResponse{ - Message: fmt.Sprintf("Token doesn't have permission to access this resource. Required scope: %s", resourceType), + Message: fmt.Sprintf("Token doesn't have permission to access this resource. Required permission: %s", requiredPermission), }) + c.Abort() return } @@ -106,23 +114,42 @@ func PersonalAccessToken(gdb *gorm.DB) gin.HandlerFunc { } } -// getAPIResourceType extracts the resource type from the path. -// For example: /oapi/v1/jobs -> job, /oapi/v1/clusters -> cluster. -func getAPIResourceType(path string) string { +// hasPermission checks if the required permission exists in the provided permissions list. +// For backward compatibility, an empty permissions list grants all permissions. +// This allows existing systems that don't have explicit permissions set to continue +// working without interruption. +// +// Returns true if: +// 1. The permissions list is empty (backward compatibility mode) +// 2. The requiredPermission is found in the permissions list +func hasPermission(permissions []string, requiredPermission string) bool { + if len(permissions) == 0 { + return true + } + + for _, permission := range permissions { + if permission == requiredPermission { + return true + } + } + + return false +} + +// requiredPermission extracts the resource type from the path and returns the required permission. +func requiredPermission(path string) (string, error) { matches := oapiResourceRegexp.FindStringSubmatch(path) if len(matches) != 2 { - return "" + return "", fmt.Errorf("failed to extract resource type from path: %s", path) } resource := strings.ToLower(matches[1]) switch resource { case "jobs": - return types.PersonalAccessTokenScopeJob + return types.PersonalAccessTokenScopeJob, nil case "clusters": - return types.PersonalAccessTokenScopeCluster - case "preheats": - return types.PersonalAccessTokenScopePreheat + return types.PersonalAccessTokenScopeCluster, nil default: - return resource + return "", fmt.Errorf("unsupported resource type: %s", resource) } } diff --git a/manager/service/personal_access_token.go b/manager/service/personal_access_token.go index 49c5771c9..f98398711 100644 --- a/manager/service/personal_access_token.go +++ b/manager/service/personal_access_token.go @@ -27,6 +27,10 @@ import ( ) func (s *service) CreatePersonalAccessToken(ctx context.Context, json types.CreatePersonalAccessTokenRequest) (*models.PersonalAccessToken, error) { + if len(json.Scopes) == 0 { + json.Scopes = types.DefaultPersonalAccessTokenScopes + } + personalAccessToken := models.PersonalAccessToken{ Name: json.Name, BIO: json.BIO, @@ -58,6 +62,10 @@ func (s *service) DestroyPersonalAccessToken(ctx context.Context, id uint) error } func (s *service) UpdatePersonalAccessToken(ctx context.Context, id uint, json types.UpdatePersonalAccessTokenRequest) (*models.PersonalAccessToken, error) { + if len(json.Scopes) == 0 { + json.Scopes = types.DefaultPersonalAccessTokenScopes + } + personalAccessToken := models.PersonalAccessToken{} if err := s.db.WithContext(ctx).Preload("User").First(&personalAccessToken, id).Updates(models.PersonalAccessToken{ BIO: json.BIO, diff --git a/manager/types/persional_access_token.go b/manager/types/persional_access_token.go index a906fc834..9d9fc4f4f 100644 --- a/manager/types/persional_access_token.go +++ b/manager/types/persional_access_token.go @@ -19,9 +19,6 @@ package types import "time" const ( - // PersonalAccessTokenScopePreheat represents the personal access token whose scope is preheat. - PersonalAccessTokenScopePreheat = "preheat" - // PersonalAccessTokenScopeJob represents the personal access token whose scope is job. PersonalAccessTokenScopeJob = "job" @@ -29,6 +26,11 @@ const ( PersonalAccessTokenScopeCluster = "cluster" ) +var ( + // DefaultPersonalAccessTokenScopes represents the default scopes of personal access token. + DefaultPersonalAccessTokenScopes = []string{PersonalAccessTokenScopeJob, PersonalAccessTokenScopeCluster} +) + type CreatePersonalAccessTokenRequest struct { Name string `json:"name" binding:"required"` BIO string `json:"bio" binding:"omitempty"`