Skip to content

Commit 5eb320f

Browse files
spacelukespaceluke
authored
Refactor: align permission validator api (#5337)
* refactor(auth api): rename class name * refactor(auth api): rename bean name && make ConsumerPermissionValidator impl PermissionValidator * refactor(auth api): fix test error * refactor(auth api): add CHANGES.md * fix(auth api): fix review comments
1 parent cf2ed78 commit 5eb320f

35 files changed

Lines changed: 437 additions & 349 deletions

‎CHANGES.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ Release Notes.
55
Apollo 2.5.0
66

77
------------------
8+
* [Refactor: align permission validator api between openapi and portal](https://github.com/apolloconfig/apollo/pull/5337)
89
* [Feature: Provide a new configfiles API to return the raw content of configuration files directly](https://github.com/apolloconfig/apollo/pull/5336)
910

1011
------------------

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

Lines changed: 56 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,16 @@
1818

1919
import static com.ctrip.framework.apollo.portal.service.SystemRoleManagerService.SYSTEM_PERMISSION_TARGET_ID;
2020

21+
import com.ctrip.framework.apollo.common.entity.AppNamespace;
2122
import com.ctrip.framework.apollo.openapi.service.ConsumerRolePermissionService;
2223
import com.ctrip.framework.apollo.openapi.util.ConsumerAuthUtil;
24+
import com.ctrip.framework.apollo.portal.component.PermissionValidator;
2325
import com.ctrip.framework.apollo.portal.constant.PermissionType;
2426
import com.ctrip.framework.apollo.portal.util.RoleUtils;
2527
import org.springframework.stereotype.Component;
26-
import javax.servlet.http.HttpServletRequest;
2728

28-
@Component
29-
public class ConsumerPermissionValidator {
29+
@Component("consumerPermissionValidator")
30+
public class ConsumerPermissionValidator implements PermissionValidator {
3031

3132
private final ConsumerRolePermissionService permissionService;
3233
private final ConsumerAuthUtil consumerAuthUtil;
@@ -37,44 +38,75 @@ public ConsumerPermissionValidator(final ConsumerRolePermissionService permissio
3738
this.consumerAuthUtil = consumerAuthUtil;
3839
}
3940

40-
public boolean hasModifyNamespacePermission(HttpServletRequest request, String appId,
41-
String namespaceName, String env) {
42-
if (hasCreateNamespacePermission(request, appId)) {
41+
@Override
42+
public boolean hasModifyNamespacePermission(String appId, String env, String clusterName,
43+
String namespaceName) {
44+
if (hasCreateNamespacePermission(appId)) {
4345
return true;
4446
}
45-
return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
47+
return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdFromCtx(),
4648
PermissionType.MODIFY_NAMESPACE, RoleUtils.buildNamespaceTargetId(appId, namespaceName))
47-
|| permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
48-
PermissionType.MODIFY_NAMESPACE,
49-
RoleUtils.buildNamespaceTargetId(appId, namespaceName, env));
50-
49+
|| permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdFromCtx(),
50+
PermissionType.MODIFY_NAMESPACE,
51+
RoleUtils.buildNamespaceTargetId(appId, namespaceName, env));
5152
}
5253

53-
public boolean hasReleaseNamespacePermission(HttpServletRequest request, String appId,
54-
String namespaceName, String env) {
55-
if (hasCreateNamespacePermission(request, appId)) {
54+
@Override
55+
public boolean hasReleaseNamespacePermission(String appId, String env, String clusterName,
56+
String namespaceName) {
57+
if (hasCreateNamespacePermission(appId)) {
5658
return true;
5759
}
58-
return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
60+
return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdFromCtx(),
5961
PermissionType.RELEASE_NAMESPACE, RoleUtils.buildNamespaceTargetId(appId, namespaceName))
60-
|| permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
61-
PermissionType.RELEASE_NAMESPACE,
62-
RoleUtils.buildNamespaceTargetId(appId, namespaceName, env));
62+
|| permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdFromCtx(),
63+
PermissionType.RELEASE_NAMESPACE,
64+
RoleUtils.buildNamespaceTargetId(appId, namespaceName, env));
65+
}
6366

67+
@Override
68+
public boolean hasAssignRolePermission(String appId) {
69+
return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdFromCtx(),
70+
PermissionType.ASSIGN_ROLE, appId);
6471
}
6572

66-
public boolean hasCreateNamespacePermission(HttpServletRequest request, String appId) {
67-
return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
73+
@Override
74+
public boolean hasCreateNamespacePermission(String appId) {
75+
return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdFromCtx(),
6876
PermissionType.CREATE_NAMESPACE, appId);
6977
}
7078

71-
public boolean hasCreateClusterPermission(HttpServletRequest request, String appId) {
72-
return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
79+
@Override
80+
public boolean hasCreateAppNamespacePermission(String appId, AppNamespace appNamespace) {
81+
throw new UnsupportedOperationException("Not supported operation");
82+
}
83+
84+
@Override
85+
public boolean hasCreateClusterPermission(String appId) {
86+
return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerIdFromCtx(),
7387
PermissionType.CREATE_CLUSTER, appId);
7488
}
7589

76-
public boolean hasCreateApplicationPermission(HttpServletRequest request) {
77-
long consumerId = consumerAuthUtil.retrieveConsumerId(request);
90+
@Override
91+
public boolean isSuperAdmin() {
92+
// openapi shouldn't be
93+
return false;
94+
}
95+
96+
@Override
97+
public boolean shouldHideConfigToCurrentUser(String appId, String env, String clusterName,
98+
String namespaceName) {
99+
throw new UnsupportedOperationException("Not supported operation");
100+
}
101+
102+
@Override
103+
public boolean hasCreateApplicationPermission() {
104+
long consumerId = consumerAuthUtil.retrieveConsumerIdFromCtx();
78105
return permissionService.consumerHasPermission(consumerId, PermissionType.CREATE_APPLICATION, SYSTEM_PERMISSION_TARGET_ID);
79106
}
107+
108+
@Override
109+
public boolean hasManageAppMasterPermission(String appId) {
110+
throw new UnsupportedOperationException("Not supported operation");
111+
}
80112
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
import org.springframework.stereotype.Service;
2222

2323
import javax.servlet.http.HttpServletRequest;
24+
import org.springframework.web.context.request.RequestContextHolder;
25+
import org.springframework.web.context.request.ServletRequestAttributes;
2426

2527
/**
2628
* @author Jason Song(song_s@ctrip.com)
@@ -55,4 +57,14 @@ public long retrieveConsumerId(HttpServletRequest request) {
5557
throw new IllegalStateException("No consumer id!", ex);
5658
}
5759
}
60+
61+
// retrieve from RequestContextHolder
62+
public long retrieveConsumerIdFromCtx() {
63+
ServletRequestAttributes attributes = (ServletRequestAttributes) RequestContextHolder.getRequestAttributes();
64+
if (attributes == null) {
65+
throw new IllegalStateException("No Request!");
66+
}
67+
HttpServletRequest request = attributes.getRequest();
68+
return retrieveConsumerId(request);
69+
}
5870
}

‎apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/AppController.java‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import com.ctrip.framework.apollo.portal.entity.model.AppModel;
2727
import java.util.Arrays;
2828
import java.util.Set;
29-
import javax.servlet.http.HttpServletRequest;
3029
import javax.transaction.Transactional;
3130
import org.springframework.security.access.prepost.PreAuthorize;
3231
import org.springframework.util.StringUtils;
@@ -56,11 +55,10 @@ public AppController(
5655
* @see com.ctrip.framework.apollo.portal.controller.AppController#create(AppModel)
5756
*/
5857
@Transactional
59-
@PreAuthorize(value = "@consumerPermissionValidator.hasCreateApplicationPermission(#request)")
58+
@PreAuthorize(value = "@consumerPermissionValidator.hasCreateApplicationPermission()")
6059
@PostMapping(value = "/apps")
6160
public void createApp(
62-
@RequestBody OpenCreateAppDTO req,
63-
HttpServletRequest request
61+
@RequestBody OpenCreateAppDTO req
6462
) {
6563
if (null == req.getApp()) {
6664
throw new BadRequestException("App is null");
@@ -72,7 +70,7 @@ public void createApp(
7270
// create app
7371
this.appOpenApiService.createApp(req);
7472
if (req.isAssignAppRoleToSelf()) {
75-
long consumerId = this.consumerAuthUtil.retrieveConsumerId(request);
73+
long consumerId = this.consumerAuthUtil.retrieveConsumerIdFromCtx();
7674
consumerService.assignAppRoleToConsumer(consumerId, app.getAppId());
7775
}
7876
}
@@ -95,8 +93,8 @@ public List<OpenAppDTO> findApps(@RequestParam(value = "appIds", required = fals
9593
* @return which apps can be operated by open api
9694
*/
9795
@GetMapping("/apps/authorized")
98-
public List<OpenAppDTO> findAppsAuthorized(HttpServletRequest request) {
99-
long consumerId = this.consumerAuthUtil.retrieveConsumerId(request);
96+
public List<OpenAppDTO> findAppsAuthorized() {
97+
long consumerId = this.consumerAuthUtil.retrieveConsumerIdFromCtx();
10098

10199
Set<String> appIds = this.consumerService.findAppIdsAuthorizedByConsumerId(consumerId);
102100

‎apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/ClusterController.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,10 @@ public OpenClusterDTO getCluster(@PathVariable("appId") String appId, @PathVaria
5454
return this.clusterOpenApiService.getCluster(appId, env, clusterName);
5555
}
5656

57-
@PreAuthorize(value = "@consumerPermissionValidator.hasCreateClusterPermission(#request, #appId)")
57+
@PreAuthorize(value = "@consumerPermissionValidator.hasCreateClusterPermission(#appId)")
5858
@PostMapping(value = "apps/{appId}/clusters")
5959
public OpenClusterDTO createCluster(@PathVariable String appId, @PathVariable String env,
60-
@Valid @RequestBody OpenClusterDTO cluster, HttpServletRequest request) {
60+
@Valid @RequestBody OpenClusterDTO cluster) {
6161

6262
if (!Objects.equals(appId, cluster.getAppId())) {
6363
throw new BadRequestException(

‎apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/ItemController.java‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,11 @@ public OpenItemDTO getItemByEncodedKey(@PathVariable String appId, @PathVariable
7878
new String(Base64.getDecoder().decode(key.getBytes(StandardCharsets.UTF_8))));
7979
}
8080

81-
@PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#request, #appId, #namespaceName, #env)")
81+
@PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#appId, #env, #clusterName, #namespaceName)")
8282
@PostMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/items")
8383
public OpenItemDTO createItem(@PathVariable String appId, @PathVariable String env,
8484
@PathVariable String clusterName, @PathVariable String namespaceName,
85-
@RequestBody OpenItemDTO item, HttpServletRequest request) {
85+
@RequestBody OpenItemDTO item) {
8686

8787
RequestPrecondition.checkArguments(
8888
!StringUtils.isContainEmpty(item.getKey(), item.getDataChangeCreatedBy()),
@@ -99,12 +99,12 @@ public OpenItemDTO createItem(@PathVariable String appId, @PathVariable String e
9999
return this.itemOpenApiService.createItem(appId, env, clusterName, namespaceName, item);
100100
}
101101

102-
@PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#request, #appId, #namespaceName, #env)")
102+
@PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#appId, #env, #clusterName, #namespaceName)")
103103
@PutMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/items/{key:.+}")
104104
public void updateItem(@PathVariable String appId, @PathVariable String env,
105105
@PathVariable String clusterName, @PathVariable String namespaceName,
106106
@PathVariable String key, @RequestBody OpenItemDTO item,
107-
@RequestParam(defaultValue = "false") boolean createIfNotExists, HttpServletRequest request) {
107+
@RequestParam(defaultValue = "false") boolean createIfNotExists) {
108108

109109
RequestPrecondition.checkArguments(item != null, "item payload can not be empty");
110110

@@ -132,23 +132,22 @@ public void updateItem(@PathVariable String appId, @PathVariable String env,
132132
}
133133
}
134134

135-
@PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#request, #appId, #namespaceName, #env)")
135+
@PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#appId, #env, #clusterName, #namespaceName)")
136136
@PutMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/encodedItems/{key:.+}")
137137
public void updateItemByEncodedKey(@PathVariable String appId, @PathVariable String env,
138138
@PathVariable String clusterName, @PathVariable String namespaceName,
139139
@PathVariable String key, @RequestBody OpenItemDTO item,
140-
@RequestParam(defaultValue = "false") boolean createIfNotExists, HttpServletRequest request) {
140+
@RequestParam(defaultValue = "false") boolean createIfNotExists) {
141141
this.updateItem(appId, env, clusterName, namespaceName,
142142
new String(Base64.getDecoder().decode(key.getBytes(StandardCharsets.UTF_8))), item,
143-
createIfNotExists, request);
143+
createIfNotExists);
144144
}
145145

146-
@PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#request, #appId, #namespaceName, #env)")
146+
@PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#appId, #env, #clusterName, #namespaceName)")
147147
@DeleteMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/items/{key:.+}")
148148
public void deleteItem(@PathVariable String appId, @PathVariable String env,
149149
@PathVariable String clusterName, @PathVariable String namespaceName,
150-
@PathVariable String key, @RequestParam String operator,
151-
HttpServletRequest request) {
150+
@PathVariable String key, @RequestParam String operator) {
152151

153152
if (userService.findByUserId(operator) == null) {
154153
throw BadRequestException.userNotExists(operator);
@@ -162,15 +161,13 @@ public void deleteItem(@PathVariable String appId, @PathVariable String env,
162161
this.itemOpenApiService.removeItem(appId, env, clusterName, namespaceName, key, operator);
163162
}
164163

165-
@PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#request, #appId, #namespaceName, #env)")
164+
@PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#appId, #env, #clusterName, #namespaceName)")
166165
@DeleteMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/encodedItems/{key:.+}")
167166
public void deleteItemByEncodedKey(@PathVariable String appId, @PathVariable String env,
168167
@PathVariable String clusterName, @PathVariable String namespaceName,
169-
@PathVariable String key, @RequestParam String operator,
170-
HttpServletRequest request) {
168+
@PathVariable String key, @RequestParam String operator) {
171169
this.deleteItem(appId, env, clusterName, namespaceName,
172-
new String(Base64.getDecoder().decode(key.getBytes(StandardCharsets.UTF_8))), operator,
173-
request);
170+
new String(Base64.getDecoder().decode(key.getBytes(StandardCharsets.UTF_8))), operator);
174171
}
175172

176173
@GetMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/items")

‎apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/NamespaceBranchController.java‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,13 @@ public OpenNamespaceDTO findBranch(@PathVariable String appId,
7777
return OpenApiBeanUtils.transformFromNamespaceBO(namespaceBO);
7878
}
7979

80-
@PreAuthorize(value = "@consumerPermissionValidator.hasCreateNamespacePermission(#request, #appId)")
80+
@PreAuthorize(value = "@consumerPermissionValidator.hasCreateNamespacePermission(#appId)")
8181
@PostMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/branches")
8282
public OpenNamespaceDTO createBranch(@PathVariable String appId,
8383
@PathVariable String env,
8484
@PathVariable String clusterName,
8585
@PathVariable String namespaceName,
86-
@RequestParam("operator") String operator,
87-
HttpServletRequest request) {
86+
@RequestParam("operator") String operator) {
8887
RequestPrecondition.checkArguments(!StringUtils.isContainEmpty(operator),"operator can not be empty");
8988

9089
if (userService.findByUserId(operator) == null) {
@@ -98,23 +97,22 @@ public OpenNamespaceDTO createBranch(@PathVariable String appId,
9897
return BeanUtils.transform(OpenNamespaceDTO.class, namespaceDTO);
9998
}
10099

101-
@PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#request, #appId, #namespaceName, #env)")
100+
@PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#appId, #env, #clusterName, #namespaceName)")
102101
@DeleteMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/branches/{branchName}")
103102
public void deleteBranch(@PathVariable String appId,
104103
@PathVariable String env,
105104
@PathVariable String clusterName,
106105
@PathVariable String namespaceName,
107106
@PathVariable String branchName,
108-
@RequestParam("operator") String operator,
109-
HttpServletRequest request) {
107+
@RequestParam("operator") String operator) {
110108
RequestPrecondition.checkArguments(!StringUtils.isContainEmpty(operator),"operator can not be empty");
111109

112110
if (userService.findByUserId(operator) == null) {
113111
throw BadRequestException.userNotExists(operator);
114112
}
115113

116-
boolean canDelete = consumerPermissionValidator.hasReleaseNamespacePermission(request, appId, namespaceName, env) ||
117-
(consumerPermissionValidator.hasModifyNamespacePermission(request, appId, namespaceName, env) &&
114+
boolean canDelete = consumerPermissionValidator.hasReleaseNamespacePermission(appId, env, clusterName, namespaceName) ||
115+
(consumerPermissionValidator.hasModifyNamespacePermission(appId, env, clusterName, namespaceName) &&
118116
releaseService.loadLatestRelease(appId, Env.valueOf(env), branchName, namespaceName) == null);
119117

120118
if (!canDelete) {
@@ -139,13 +137,12 @@ public OpenGrayReleaseRuleDTO getBranchGrayRules(@PathVariable String appId, @Pa
139137
return OpenApiBeanUtils.transformFromGrayReleaseRuleDTO(grayReleaseRuleDTO);
140138
}
141139

142-
@PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#request, #appId, #namespaceName, #env)")
140+
@PreAuthorize(value = "@consumerPermissionValidator.hasModifyNamespacePermission(#appId, #env, #clusterName, #namespaceName)")
143141
@PutMapping(value = "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/branches/{branchName}/rules")
144142
public void updateBranchRules(@PathVariable String appId, @PathVariable String env,
145143
@PathVariable String clusterName, @PathVariable String namespaceName,
146144
@PathVariable String branchName, @RequestBody OpenGrayReleaseRuleDTO rules,
147-
@RequestParam("operator") String operator,
148-
HttpServletRequest request) {
145+
@RequestParam("operator") String operator) {
149146
RequestPrecondition.checkArguments(!StringUtils.isContainEmpty(operator),"operator can not be empty");
150147

151148
if (userService.findByUserId(operator) == null) {

‎apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/v1/controller/NamespaceController.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,10 @@ public NamespaceController(
5050
this.namespaceOpenApiService = namespaceOpenApiService;
5151
}
5252

53-
@PreAuthorize(value = "@consumerPermissionValidator.hasCreateNamespacePermission(#request, #appId)")
53+
@PreAuthorize(value = "@consumerPermissionValidator.hasCreateNamespacePermission(#appId)")
5454
@PostMapping(value = "/openapi/v1/apps/{appId}/appnamespaces")
5555
public OpenAppNamespaceDTO createNamespace(@PathVariable String appId,
56-
@RequestBody OpenAppNamespaceDTO appNamespaceDTO,
57-
HttpServletRequest request) {
56+
@RequestBody OpenAppNamespaceDTO appNamespaceDTO) {
5857

5958
if (!Objects.equals(appId, appNamespaceDTO.getAppId())) {
6059
throw new BadRequestException("AppId not equal. AppId in path = %s, AppId in payload = %s", appId,

0 commit comments

Comments
 (0)