fix(auth api): fix review comments

Author: spaceluke

apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/auth/ConsumerPermissionValidator.java

@@ -25,7 +25,6 @@
 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("consumerPermissionValidator")
 public class ConsumerPermissionValidator implements PermissionValidator {
@@ -45,9 +44,9 @@ public boolean hasModifyNamespacePermission(String appId, String env, String clu
     if (hasCreateNamespacePermission(appId)) {
       return true;
     }
-    return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdByCtx(),
+    return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdFromCtx(),
         PermissionType.MODIFY_NAMESPACE, RoleUtils.buildNamespaceTargetId(appId, namespaceName))
-        || permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdByCtx(),
+        || permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdFromCtx(),
         PermissionType.MODIFY_NAMESPACE,
         RoleUtils.buildNamespaceTargetId(appId, namespaceName, env));
   }
@@ -58,34 +57,33 @@ public boolean hasReleaseNamespacePermission(String appId, String env, String cl
     if (hasCreateNamespacePermission(appId)) {
       return true;
     }
-    return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdByCtx(),
+    return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdFromCtx(),
         PermissionType.RELEASE_NAMESPACE, RoleUtils.buildNamespaceTargetId(appId, namespaceName))
-        || permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdByCtx(),
+        || permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdFromCtx(),
         PermissionType.RELEASE_NAMESPACE,
         RoleUtils.buildNamespaceTargetId(appId, namespaceName, env));
   }
 
   @Override
   public boolean hasAssignRolePermission(String appId) {
-    return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdByCtx(),
+    return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdFromCtx(),
         PermissionType.ASSIGN_ROLE, appId);
   }
 
   @Override
   public boolean hasCreateNamespacePermission(String appId) {
-    return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdByCtx(),
+    return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdFromCtx(),
         PermissionType.CREATE_NAMESPACE, appId);
   }
 
   @Override
   public boolean hasCreateAppNamespacePermission(String appId, AppNamespace appNamespace) {
-    // TODO: align OpenApiConfig with PortalConfig
-    return false;
+    throw new UnsupportedOperationException("Not supported operation");
   }
 
   @Override
   public boolean hasCreateClusterPermission(String appId) {
-    return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdByCtx(),
+    return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdFromCtx(),
         PermissionType.CREATE_CLUSTER, appId);
   }
 
@@ -98,19 +96,17 @@ public boolean isSuperAdmin() {
   @Override
   public boolean shouldHideConfigToCurrentUser(String appId, String env, String clusterName,
       String namespaceName) {
-    // TODO: align OpenApiConfig with PortalConfig
-    return false;
+    throw new UnsupportedOperationException("Not supported operation");
   }
 
   @Override
   public boolean hasCreateApplicationPermission() {
-    long consumerId = consumerAuthUtil.retrieveConsumerIdByCtx();
+    long consumerId = consumerAuthUtil.retrieveConsumerIdFromCtx();
     return permissionService.consumerHasPermission(consumerId, PermissionType.CREATE_APPLICATION, SYSTEM_PERMISSION_TARGET_ID);
   }
 
   @Override
   public boolean hasManageAppMasterPermission(String appId) {
-    // TODO: align OpenApiConfig with PortalConfig
-    return false;
+    throw new UnsupportedOperationException("Not supported operation");
   }
 }

apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/ConsumerAuthUtil.java

@@ -59,17 +59,12 @@ public long retrieveConsumerId(HttpServletRequest request) {
   }
 
   // retrieve from RequestContextHolder
-  public long retrieveConsumerIdByCtx() {
+  public long retrieveConsumerIdFromCtx() {
     ServletRequestAttributes attributes = (ServletRequestAttributes) RequestContextHolder.getRequestAttributes();
     if (attributes == null) {
-      throw new IllegalStateException("No consumer id!");
+      throw new IllegalStateException("No Request!");
     }
     HttpServletRequest request = attributes.getRequest();
-    Object value = request.getAttribute(CONSUMER_ID);
-    try {
-      return Long.parseLong(value.toString());
-    } catch (Throwable ex) {
-      throw new IllegalStateException("No consumer id!", ex);
-    }
+    return retrieveConsumerId(request);
   }
 }

apollo-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;
@@ -71,7 +70,7 @@ public void createApp(
     // create app
     this.appOpenApiService.createApp(req);
     if (req.isAssignAppRoleToSelf()) {
-      long consumerId = this.consumerAuthUtil.retrieveConsumerIdByCtx();
+      long consumerId = this.consumerAuthUtil.retrieveConsumerIdFromCtx();
       consumerService.assignAppRoleToConsumer(consumerId, app.getAppId());
     }
   }
@@ -95,7 +94,7 @@ public List<OpenAppDTO> findApps(@RequestParam(value = "appIds", required = fals
    */
   @GetMapping("/apps/authorized")
   public List<OpenAppDTO> findAppsAuthorized() {
-    long consumerId = this.consumerAuthUtil.retrieveConsumerIdByCtx();
+    long consumerId = this.consumerAuthUtil.retrieveConsumerIdFromCtx();
 
     Set<String> appIds = this.consumerService.findAppIdsAuthorizedByConsumerId(consumerId);
 

apollo-portal/src/test/java/com/ctrip/framework/apollo/openapi/v1/controller/AppControllerTest.java

@@ -107,7 +107,7 @@ public class AppControllerTest {
   @Test
   public void testFindAppsAuthorized() throws Exception {
     final long consumerId = 123456;
-    Mockito.when(this.consumerAuthUtil.retrieveConsumerIdByCtx()).thenReturn(consumerId);
+    Mockito.when(this.consumerAuthUtil.retrieveConsumerIdFromCtx()).thenReturn(consumerId);
 
     final List<ConsumerRole> consumerRoles = Arrays.asList(
         generateConsumerRoleByRoleId(6),