INFRA-3970 | Dhruv | fix manifest read condition

This commit is contained in:
dhruvjoshi
2024-12-20 14:23:55 +05:30
parent d1bf0a804f
commit 019f3392fc
3 changed files with 15 additions and 7 deletions

View File

@@ -2,6 +2,7 @@ package com.navi.infra.portal.service.user;
import static com.navi.infra.portal.v2.privilege.Action.APPROVAL_WRITE;
import static com.navi.infra.portal.v2.privilege.Action.KUBE_DELETE;
import static com.navi.infra.portal.v2.privilege.Action.MANIFEST_READ;
import static com.navi.infra.portal.v2.privilege.Action.MANIFEST_SECRET_READ;
import static com.navi.infra.portal.v2.privilege.Action.MANIFEST_SECRET_WRITE;
import static com.navi.infra.portal.v2.privilege.Action.MANIFEST_SUBSTITUTE_SECRETS;
@@ -38,7 +39,7 @@ public class PrivilegeUtilService {
public boolean canReadManifest(String team, String environment) {
return authorizationFilter.hasPermissions(MANIFEST, team, environment,
MANIFEST_SECRET_WRITE);
MANIFEST_READ);
}
public boolean canSubstituteEnvironmentVariable(Manifest manifest) {

View File

@@ -22,6 +22,7 @@ import com.navi.infra.portal.domain.manifest.Manifest;
import com.navi.infra.portal.domain.user.User;
import com.navi.infra.portal.security.authorization.AuthorizationContext;
import com.navi.infra.portal.service.manifest.ManifestService;
import com.navi.infra.portal.service.user.PrivilegeUtilService;
import com.navi.infra.portal.service.user.UserService;
import com.navi.infra.portal.util.MapUtil;
import com.navi.infra.portal.util.manifest.ChangeRequestUtils;
@@ -69,6 +70,7 @@ public class ChangeRequestServiceImpl implements ChangeRequestService {
private final MapUtil mapUtil;
private final AuthUtil authUtil;
private final Set<String> ignoredFieldsForMerge;
private final PrivilegeUtilService privilegeUtilService;
public ChangeRequestServiceImpl(
ChangeRequestRepository repository,
@@ -79,6 +81,7 @@ public class ChangeRequestServiceImpl implements ChangeRequestService {
ObjectMapper objectMapper,
MapUtil mapUtil,
AuthUtil authUtil,
PrivilegeUtilService privilegeUtilService,
@Value("version") Set<String> ignoredFieldsForMerge
) {
this.repository = repository;
@@ -89,6 +92,7 @@ public class ChangeRequestServiceImpl implements ChangeRequestService {
this.objectMapper = objectMapper;
this.mapUtil = mapUtil;
this.authUtil = authUtil;
this.privilegeUtilService = privilegeUtilService;
this.ignoredFieldsForMerge = unmodifiableSet(
requireNonNullElse(ignoredFieldsForMerge, emptySet()));
}
@@ -199,17 +203,17 @@ public class ChangeRequestServiceImpl implements ChangeRequestService {
.orElseThrow(() -> new NotFoundException("Change request not found"));
final var approvals = approvalService.findAllByRequestTypeAndRequestIds(
RequestType.CHANGE_REQUEST, List.of(requestId));
final var canView = approvalService.canApproveSomeApprovalRequest(
AuthorizationContext.getUserId(), approvals);
Manifest manifest = manifestService.fetchByIdWithoutAuthorization(
changeRequest.getManifestId());
var canView = approvalService.
canApproveSomeApprovalRequest(AuthorizationContext.getUserId(), approvals) ||
privilegeUtilService.canReadManifest(manifest.getTeam(), manifest.getEnvironment());
if (!canView) {
log.warn("User {} does not have access to view CR: {}",
AuthorizationContext.getUserEmail(),
requestId);
return null;
}
Manifest manifest = manifestService.fetchByIdWithoutAuthorization(
changeRequest.getManifestId());
return createCrDto(List.of(manifest), List.of(changeRequest), approvals).get(0);
}

View File

@@ -16,6 +16,7 @@ import com.navi.infra.portal.domain.user.Team;
import com.navi.infra.portal.domain.user.User;
import com.navi.infra.portal.exceptions.ManifestNotFoundException;
import com.navi.infra.portal.service.manifest.ManifestService;
import com.navi.infra.portal.service.user.PrivilegeUtilService;
import com.navi.infra.portal.service.user.UserService;
import com.navi.infra.portal.util.MapUtil;
import com.navi.infra.portal.v2.approvalflow.dto.ApprovalRequestDto;
@@ -63,6 +64,8 @@ class ChangeRequestServiceImplTest {
@Mock
private ManifestService manifestService;
@Mock
private PrivilegeUtilService privilegeUtilService;
@Mock
private ChangeRequestSlackService changeRequestSlackService;
@Mock
private UserService userService;
@@ -92,7 +95,7 @@ class ChangeRequestServiceImplTest {
service = new ChangeRequestServiceImpl(repo, approvalRequestService,
changeRequestSlackService,
manifestService, userService, new ObjectMapper(),
new MapUtil(), null, null);
new MapUtil(), null, privilegeUtilService, null);
}