apolloconfig · nobodyiam · Feb 18, 2025 · Feb 9, 2025 · Feb 9, 2025 · Feb 9, 2025
diff --git a/CHANGES.md b/CHANGES.md
@@ -5,6 +5,7 @@ Release Notes.
 Apollo 2.5.0
 
 ------------------
+* [Refactor: align permission validator api between openapi and portal](https://github.com/apolloconfig/apollo/pull/5337)
 * [Feature: Provide a new configfiles API to return the raw content of configuration files directly](https://github.com/apolloconfig/apollo/pull/5336)
 
 ------------------

diff --git a/...al/src/main/java/com/ctrip/framework/apollo/openapi/auth/ConsumerPermissionValidator.java b/...al/src/main/java/com/ctrip/framework/apollo/openapi/auth/ConsumerPermissionValidator.java
@@ -18,15 +18,16 @@
 
 import static com.ctrip.framework.apollo.portal.service.SystemRoleManagerService.SYSTEM_PERMISSION_TARGET_ID;
 
+import com.ctrip.framework.apollo.common.entity.AppNamespace;
 import com.ctrip.framework.apollo.openapi.service.ConsumerRolePermissionService;
 import com.ctrip.framework.apollo.openapi.util.ConsumerAuthUtil;
+import com.ctrip.framework.apollo.portal.component.PermissionValidator;
 import com.ctrip.framework.apollo.portal.constant.PermissionType;
 import com.ctrip.framework.apollo.portal.util.RoleUtils;
 import org.springframework.stereotype.Component;
-import javax.servlet.http.HttpServletRequest;
 
-@Component
-public class ConsumerPermissionValidator {
+@Component("consumerPermissionValidator")
+public class ConsumerPermissionValidator implements PermissionValidator {
 
   private final ConsumerRolePermissionService permissionService;
   private final ConsumerAuthUtil consumerAuthUtil;
@@ -37,44 +38,75 @@ public ConsumerPermissionValidator(final ConsumerRolePermissionService permissio
     this.consumerAuthUtil = consumerAuthUtil;
   }
 
-  public boolean hasModifyNamespacePermission(HttpServletRequest request, String appId,
-      String namespaceName, String env) {
-    if (hasCreateNamespacePermission(request, appId)) {
+  @Override
+  public boolean hasModifyNamespacePermission(String appId, String env, String clusterName,
+      String namespaceName) {
+    if (hasCreateNamespacePermission(appId)) {
       return true;
     }
-    return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
+    return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdFromCtx(),
         PermissionType.MODIFY_NAMESPACE, RoleUtils.buildNamespaceTargetId(appId, namespaceName))
-        || permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
-            PermissionType.MODIFY_NAMESPACE,
-            RoleUtils.buildNamespaceTargetId(appId, namespaceName, env));
-
+        || permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdFromCtx(),
+        PermissionType.MODIFY_NAMESPACE,
+        RoleUtils.buildNamespaceTargetId(appId, namespaceName, env));
   }
 
-  public boolean hasReleaseNamespacePermission(HttpServletRequest request, String appId,
-      String namespaceName, String env) {
-    if (hasCreateNamespacePermission(request, appId)) {
+  @Override
+  public boolean hasReleaseNamespacePermission(String appId, String env, String clusterName,
+      String namespaceName) {
+    if (hasCreateNamespacePermission(appId)) {
       return true;
     }
-    return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
+    return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdFromCtx(),
         PermissionType.RELEASE_NAMESPACE, RoleUtils.buildNamespaceTargetId(appId, namespaceName))
-        || permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
-            PermissionType.RELEASE_NAMESPACE,
-            RoleUtils.buildNamespaceTargetId(appId, namespaceName, env));
+        || permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdFromCtx(),
+        PermissionType.RELEASE_NAMESPACE,
+        RoleUtils.buildNamespaceTargetId(appId, namespaceName, env));
+  }
 
+  @Override
+  public boolean hasAssignRolePermission(String appId) {
+    return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdFromCtx(),
+        PermissionType.ASSIGN_ROLE, appId);
   }
 
-  public boolean hasCreateNamespacePermission(HttpServletRequest request, String appId) {
-    return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
+  @Override
+  public boolean hasCreateNamespacePermission(String appId) {
+    return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdFromCtx(),
         PermissionType.CREATE_NAMESPACE, appId);
   }
 
-  public boolean hasCreateClusterPermission(HttpServletRequest request, String appId) {
-    return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
+  @Override
+  public boolean hasCreateAppNamespacePermission(String appId, AppNamespace appNamespace) {
+    throw new UnsupportedOperationException("Not supported operation");
+  }
+
+  @Override
+  public boolean hasCreateClusterPermission(String appId) {
+    return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdFromCtx(),
         PermissionType.CREATE_CLUSTER, appId);
   }
 
-  public boolean hasCreateApplicationPermission(HttpServletRequest request) {
-    long consumerId = consumerAuthUtil.retrieveConsumerId(request);
+  @Override
+  public boolean isSuperAdmin() {
+    // openapi shouldn't be
+    return false;
+  }
+
+  @Override
+  public boolean shouldHideConfigToCurrentUser(String appId, String env, String clusterName,
+      String namespaceName) {
+    throw new UnsupportedOperationException("Not supported operation");
+  }
+
+  @Override
+  public boolean hasCreateApplicationPermission() {
+    long consumerId = consumerAuthUtil.retrieveConsumerIdFromCtx();
     return permissionService.consumerHasPermission(consumerId, PermissionType.CREATE_APPLICATION, SYSTEM_PERMISSION_TARGET_ID);
   }
+
+  @Override
+  public boolean hasManageAppMasterPermission(String appId) {
+    throw new UnsupportedOperationException("Not supported operation");
+  }
 }
diff --git a/apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/ConsumerAuthUtil.java b/apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/ConsumerAuthUtil.java
@@ -21,6 +21,8 @@
 import org.springframework.stereotype.Service;
 
 import javax.servlet.http.HttpServletRequest;
+import org.springframework.web.context.request.RequestContextHolder;
+import org.springframework.web.context.request.ServletRequestAttributes;
 
 /**
  * @author Jason Song([email protected])
@@ -55,4 +57,14 @@ public long retrieveConsumerId(HttpServletRequest request) {
       throw new IllegalStateException("No consumer id!", ex);
     }
   }
+
+  // retrieve from RequestContextHolder
+  public long retrieveConsumerIdFromCtx() {
+    ServletRequestAttributes attributes = (ServletRequestAttributes) RequestContextHolder.getRequestAttributes();
+    if (attributes == null) {
+      throw new IllegalStateException("No Request!");
+    }
+    HttpServletRequest request = attributes.getRequest();
+    return retrieveConsumerId(request);
+  }
 }
diff --git a/...-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/AppController.java b/...-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/AppController.java
@@ -26,7 +26,6 @@
 import com.ctrip.framework.apollo.portal.entity.model.AppModel;
 import java.util.Arrays;
 import java.util.Set;
-import javax.servlet.http.HttpServletRequest;
 import javax.transaction.Transactional;
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.util.StringUtils;
@@ -56,11 +55,10 @@ public AppController(
    * @see com.ctrip.framework.apollo.portal.controller.AppController#create(AppModel)
    */
   @Transactional
-  @PreAuthorize(value = "@consumerPermissionValidator.hasCreateApplicationPermission(#request)")
+  @PreAuthorize(value = "@consumerPermissionValidator.hasCreateApplicationPermission()")
   @PostMapping(value = "/apps")
   public void createApp(
-      @RequestBody OpenCreateAppDTO req,
-      HttpServletRequest request
+      @RequestBody OpenCreateAppDTO req
   ) {
     if (null == req.getApp()) {
       throw new BadRequestException("App is null");
@@ -72,7 +70,7 @@ public void createApp(
     // create app
     this.appOpenApiService.createApp(req);
     if (req.isAssignAppRoleToSelf()) {
-      long consumerId = this.consumerAuthUtil.retrieveConsumerId(request);
+      long consumerId = this.consumerAuthUtil.retrieveConsumerIdFromCtx();
       consumerService.assignAppRoleToConsumer(consumerId, app.getAppId());
     }
   }
@@ -95,8 +93,8 @@ public List<OpenAppDTO> findApps(@RequestParam(value = "appIds", required = fals
    * @return which apps can be operated by open api
    */
   @GetMapping("/apps/authorized")
-  public List<OpenAppDTO> findAppsAuthorized(HttpServletRequest request) {
-    long consumerId = this.consumerAuthUtil.retrieveConsumerId(request);
+  public List<OpenAppDTO> findAppsAuthorized() {
+    long consumerId = this.consumerAuthUtil.retrieveConsumerIdFromCtx();
 
     Set<String> appIds = this.consumerService.findAppIdsAuthorizedByConsumerId(consumerId);
 

diff --git a/...tal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/ClusterController.java b/...tal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/ClusterController.java
@@ -54,10 +54,10 @@ public OpenClusterDTO getCluster(@PathVariable("appId") String appId, @PathVaria
     return this.clusterOpenApiService.getCluster(appId, env, clusterName);
   }
 
-  @PreAuthorize(value = "@consumerPermissionValidator.hasCreateClusterPermission(#request, #appId)")
+  @PreAuthorize(value = "@consumerPermissionValidator.hasCreateClusterPermission(#appId)")
   @PostMapping(value = "apps/{appId}/clusters")
   public OpenClusterDTO createCluster(@PathVariable String appId, @PathVariable String env,
-      @Valid @RequestBody OpenClusterDTO cluster, HttpServletRequest request) {
+      @Valid @RequestBody OpenClusterDTO cluster) {
 
     if (!Objects.equals(appId, cluster.getAppId())) {
       throw new BadRequestException(

diff --git a/...portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/ItemController.java b/...portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/ItemController.java
@@ -78,11 +78,11 @@ public OpenItemDTO getItemByEncodedKey(@PathVariable String appId, @PathVariable
         new String(Base64.getDecoder().decode(key.getBytes(StandardCharsets.UTF_8))));
   }
 
-  @PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#request, #appId, #namespaceName, #env)")
+  @PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#appId, #env, #clusterName, #namespaceName)")
   @PostMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/items")
   public OpenItemDTO createItem(@PathVariable String appId, @PathVariable String env,
                                 @PathVariable String clusterName, @PathVariable String namespaceName,
-                                @RequestBody OpenItemDTO item, HttpServletRequest request) {
+                                @RequestBody OpenItemDTO item) {
 
     RequestPrecondition.checkArguments(
         !StringUtils.isContainEmpty(item.getKey(), item.getDataChangeCreatedBy()),
@@ -99,12 +99,12 @@ public OpenItemDTO createItem(@PathVariable String appId, @PathVariable String e
     return this.itemOpenApiService.createItem(appId, env, clusterName, namespaceName, item);
   }
 
-  @PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#request, #appId, #namespaceName, #env)")
+  @PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#appId, #env, #clusterName, #namespaceName)")
   @PutMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/items/{key:.+}")
   public void updateItem(@PathVariable String appId, @PathVariable String env,
                          @PathVariable String clusterName, @PathVariable String namespaceName,
                          @PathVariable String key, @RequestBody OpenItemDTO item,
-                         @RequestParam(defaultValue = "false") boolean createIfNotExists, HttpServletRequest request) {
+                         @RequestParam(defaultValue = "false") boolean createIfNotExists) {
 
     RequestPrecondition.checkArguments(item != null, "item payload can not be empty");
 
@@ -132,23 +132,22 @@ public void updateItem(@PathVariable String appId, @PathVariable String env,
     }
   }
 
-  @PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#request, #appId, #namespaceName, #env)")
+  @PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#appId, #env, #clusterName, #namespaceName)")
   @PutMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/encodedItems/{key:.+}")
   public void updateItemByEncodedKey(@PathVariable String appId, @PathVariable String env,
       @PathVariable String clusterName, @PathVariable String namespaceName,
       @PathVariable String key, @RequestBody OpenItemDTO item,
-      @RequestParam(defaultValue = "false") boolean createIfNotExists, HttpServletRequest request) {
+      @RequestParam(defaultValue = "false") boolean createIfNotExists) {
     this.updateItem(appId, env, clusterName, namespaceName,
         new String(Base64.getDecoder().decode(key.getBytes(StandardCharsets.UTF_8))), item,
-        createIfNotExists, request);
+        createIfNotExists);
   }
 
-  @PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#request, #appId, #namespaceName, #env)")
+  @PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#appId, #env, #clusterName, #namespaceName)")
   @DeleteMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/items/{key:.+}")
   public void deleteItem(@PathVariable String appId, @PathVariable String env,
                          @PathVariable String clusterName, @PathVariable String namespaceName,
-                         @PathVariable String key, @RequestParam String operator,
-                         HttpServletRequest request) {
+                         @PathVariable String key, @RequestParam String operator) {
 
     if (userService.findByUserId(operator) == null) {
       throw BadRequestException.userNotExists(operator);
@@ -162,15 +161,13 @@ public void deleteItem(@PathVariable String appId, @PathVariable String env,
     this.itemOpenApiService.removeItem(appId, env, clusterName, namespaceName, key, operator);
   }
 
-  @PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#request, #appId, #namespaceName, #env)")
+  @PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#appId, #env, #clusterName, #namespaceName)")
   @DeleteMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/encodedItems/{key:.+}")
   public void deleteItemByEncodedKey(@PathVariable String appId, @PathVariable String env,
       @PathVariable String clusterName, @PathVariable String namespaceName,
-      @PathVariable String key, @RequestParam String operator,
-      HttpServletRequest request) {
+      @PathVariable String key, @RequestParam String operator) {
     this.deleteItem(appId, env, clusterName, namespaceName,
-        new String(Base64.getDecoder().decode(key.getBytes(StandardCharsets.UTF_8))), operator,
-        request);
+        new String(Base64.getDecoder().decode(key.getBytes(StandardCharsets.UTF_8))), operator);
   }
 
   @GetMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/items")

diff --git a/...main/java/com/ctrip/framework/apollo/openapi/v1/controller/NamespaceBranchController.java b/...main/java/com/ctrip/framework/apollo/openapi/v1/controller/NamespaceBranchController.java
@@ -77,14 +77,13 @@ public OpenNamespaceDTO findBranch(@PathVariable String appId,
         return OpenApiBeanUtils.transformFromNamespaceBO(namespaceBO);
     }
 
-    @PreAuthorize(value = "@consumerPermissionValidator.hasCreateNamespacePermission(#request, #appId)")
+    @PreAuthorize(value = "@consumerPermissionValidator.hasCreateNamespacePermission(#appId)")
     @PostMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/branches")
     public OpenNamespaceDTO createBranch(@PathVariable String appId,
                                          @PathVariable String env,
                                          @PathVariable String clusterName,
                                          @PathVariable String namespaceName,
-                                         @RequestParam("operator") String operator,
-                                         HttpServletRequest request) {
+                                         @RequestParam("operator") String operator) {
         RequestPrecondition.checkArguments(!StringUtils.isContainEmpty(operator),"operator can not be empty");
 
         if (userService.findByUserId(operator) == null) {
@@ -98,23 +97,22 @@ public OpenNamespaceDTO createBranch(@PathVariable String appId,
         return BeanUtils.transform(OpenNamespaceDTO.class, namespaceDTO);
     }
 
-    @PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#request, #appId, #namespaceName, #env)")
+    @PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#appId, #env, #clusterName, #namespaceName)")
     @DeleteMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/branches/{branchName}")
     public void deleteBranch(@PathVariable String appId,
                              @PathVariable String env,
                              @PathVariable String clusterName,
                              @PathVariable String namespaceName,
                              @PathVariable String branchName,
-                             @RequestParam("operator") String operator,
-                             HttpServletRequest request) {
+                             @RequestParam("operator") String operator) {
         RequestPrecondition.checkArguments(!StringUtils.isContainEmpty(operator),"operator can not be empty");
 
         if (userService.findByUserId(operator) == null) {
             throw BadRequestException.userNotExists(operator);
         }
 
-        boolean canDelete = consumerPermissionValidator.hasReleaseNamespacePermission(request, appId, namespaceName, env) ||
-            (consumerPermissionValidator.hasModifyNamespacePermission(request, appId, namespaceName, env) &&
+        boolean canDelete = consumerPermissionValidator.hasReleaseNamespacePermission(appId, env, clusterName, namespaceName) ||
+            (consumerPermissionValidator.hasModifyNamespacePermission(appId, env, clusterName, namespaceName) &&
                 releaseService.loadLatestRelease(appId, Env.valueOf(env), branchName, namespaceName) == null);
 
         if (!canDelete) {
@@ -139,13 +137,12 @@ public OpenGrayReleaseRuleDTO getBranchGrayRules(@PathVariable String appId, @Pa
         return OpenApiBeanUtils.transformFromGrayReleaseRuleDTO(grayReleaseRuleDTO);
     }
 
-    @PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#request, #appId, #namespaceName, #env)")
+    @PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#appId, #env, #clusterName, #namespaceName)")
     @PutMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/branches/{branchName}/rules")
     public void updateBranchRules(@PathVariable String appId, @PathVariable String env,
                                   @PathVariable String clusterName, @PathVariable String namespaceName,
                                   @PathVariable String branchName, @RequestBody OpenGrayReleaseRuleDTO rules,
-                                  @RequestParam("operator") String operator,
-                                  HttpServletRequest request) {
+                                  @RequestParam("operator") String operator) {
         RequestPrecondition.checkArguments(!StringUtils.isContainEmpty(operator),"operator can not be empty");
 
         if (userService.findByUserId(operator) == null) {

diff --git a/...l/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/NamespaceController.java b/...l/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/NamespaceController.java
@@ -50,11 +50,10 @@ public NamespaceController(
     this.namespaceOpenApiService = namespaceOpenApiService;
   }
 
-  @PreAuthorize(value = "@consumerPermissionValidator.hasCreateNamespacePermission(#request, #appId)")
+  @PreAuthorize(value = "@consumerPermissionValidator.hasCreateNamespacePermission(#appId)")
   @PostMapping(value = "/openapi/v1/apps/{appId}/appnamespaces")
   public OpenAppNamespaceDTO createNamespace(@PathVariable String appId,
-                                             @RequestBody OpenAppNamespaceDTO appNamespaceDTO,
-                                             HttpServletRequest request) {
+                                             @RequestBody OpenAppNamespaceDTO appNamespaceDTO) {
 
     if (!Objects.equals(appId, appNamespaceDTO.getAppId())) {
       throw new BadRequestException("AppId not equal. AppId in path = %s, AppId in payload = %s", appId,
-Original file line number
+Diff line change
@@ Expand Up / @@ -5,6 +5,7 @@ Release Notes. @@
     Apollo 2.5.0
     ------------------
+    * [Refactor: align permission validator api between openapi and portal](https://github.com/apolloconfig/apollo/pull/5337)
     * [Feature: Provide a new configfiles API to return the raw content of configuration files directly](https://github.com/apolloconfig/apollo/pull/5336)
     ------------------
@@ Expand Down @@