Skip to content

Commit 14e13af

Browse files
authored
Dmp 5090 fix deactivate own account error (#2987)
1 parent b0c4ae7 commit 14e13af

File tree

12 files changed

+243
-56
lines changed

12 files changed

+243
-56
lines changed

src/integrationTest/java/uk/gov/hmcts/darts/audio/controller/AudioControllerGetAdminMediaVersionsByIdIntTest.java

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import java.time.Duration;
1919
import java.time.OffsetDateTime;
20+
import java.time.ZoneOffset;
2021
import java.time.format.DateTimeFormatter;
2122

2223
import static org.junit.jupiter.params.provider.EnumSource.Mode.EXCLUDE;
@@ -62,9 +63,12 @@ void shouldReturn200_whenChronicleIdHasBothCurrentAndNonCurrentVersions() throws
6263

6364
String expectedResponse = getContentsFromFile(
6465
"tests/audio/AudioControllerGetAdminMediaVersionsByIdIntTest/expectedResponseTypical.json")
65-
.replace("<created_at_current>", currentMediaEntity.getCreatedDateTime().format(DateTimeFormatter.ISO_DATE_TIME))
66-
.replace("<versioned_at_current_1>", versionedMediaEntity2.getCreatedDateTime().format(DateTimeFormatter.ISO_DATE_TIME))
67-
.replace("<versioned_at_current_2>", versionedMediaEntity1.getCreatedDateTime().format(DateTimeFormatter.ISO_DATE_TIME));
66+
.replace("<created_at_current>",
67+
currentMediaEntity.getCreatedDateTime().atZoneSameInstant(ZoneOffset.UTC).format(DateTimeFormatter.ISO_DATE_TIME))
68+
.replace("<versioned_at_current_1>",
69+
versionedMediaEntity2.getCreatedDateTime().atZoneSameInstant(ZoneOffset.UTC).format(DateTimeFormatter.ISO_DATE_TIME))
70+
.replace("<versioned_at_current_2>",
71+
versionedMediaEntity1.getCreatedDateTime().atZoneSameInstant(ZoneOffset.UTC).format(DateTimeFormatter.ISO_DATE_TIME));
6872
JSONAssert.assertEquals(expectedResponse, mvcResult.getResponse().getContentAsString(), JSONCompareMode.STRICT);
6973
}
7074

@@ -86,9 +90,12 @@ void shouldReturn200_whenMediaIdPassedIsNotCurrent_shouldReturnTheCorrectCorrent
8690

8791
String expectedResponse = getContentsFromFile(
8892
"tests/audio/AudioControllerGetAdminMediaVersionsByIdIntTest/expectedResponseTypical.json")
89-
.replace("<created_at_current>", currentMediaEntity.getCreatedDateTime().format(DateTimeFormatter.ISO_DATE_TIME))
90-
.replace("<versioned_at_current_1>", versionedMediaEntity2.getCreatedDateTime().format(DateTimeFormatter.ISO_DATE_TIME))
91-
.replace("<versioned_at_current_2>", versionedMediaEntity1.getCreatedDateTime().format(DateTimeFormatter.ISO_DATE_TIME));
93+
.replace("<created_at_current>", currentMediaEntity.getCreatedDateTime().atZoneSameInstant(ZoneOffset.UTC)
94+
.format(DateTimeFormatter.ISO_DATE_TIME))
95+
.replace("<versioned_at_current_1>", versionedMediaEntity2.getCreatedDateTime().atZoneSameInstant(ZoneOffset.UTC)
96+
.format(DateTimeFormatter.ISO_DATE_TIME))
97+
.replace("<versioned_at_current_2>", versionedMediaEntity1.getCreatedDateTime().atZoneSameInstant(ZoneOffset.UTC)
98+
.format(DateTimeFormatter.ISO_DATE_TIME));
9299
JSONAssert.assertEquals(expectedResponse, mvcResult.getResponse().getContentAsString(), JSONCompareMode.STRICT);
93100
}
94101

@@ -108,7 +115,8 @@ void shouldReturn200_whenChronicleIdHasOnlyCurrentAndNoVersions() throws Excepti
108115

109116
String expectedResponse = getContentsFromFile(
110117
"tests/audio/AudioControllerGetAdminMediaVersionsByIdIntTest/expectedResponseNoVersions.json")
111-
.replace("<created_at_current>", currentMediaEntity.getCreatedDateTime().format(DateTimeFormatter.ISO_DATE_TIME));
118+
.replace("<created_at_current>", currentMediaEntity.getCreatedDateTime().atZoneSameInstant(ZoneOffset.UTC)
119+
.format(DateTimeFormatter.ISO_DATE_TIME));
112120
JSONAssert.assertEquals(expectedResponse, mvcResult.getResponse().getContentAsString(), JSONCompareMode.STRICT);
113121
}
114122

src/integrationTest/java/uk/gov/hmcts/darts/usermanagement/controller/PatchUserIntTest.java

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import org.hamcrest.Matchers;
44
import org.junit.jupiter.api.BeforeEach;
55
import org.junit.jupiter.api.Test;
6+
import org.junit.jupiter.api.parallel.Isolated;
67
import org.springframework.beans.factory.annotation.Autowired;
78
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
89
import org.springframework.test.context.bean.override.mockito.MockitoBean;
@@ -37,6 +38,7 @@
3738
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
3839

3940
@AutoConfigureMockMvc
41+
@Isolated
4042
class PatchUserIntTest extends IntegrationBase {
4143

4244
private static final String ORIGINAL_USERNAME = "James Smith";
@@ -73,7 +75,7 @@ void setUp() {
7375
}
7476

7577
@Test
76-
void patchUserShouldSucceedWhenProvidedWithValidValueForSubsetOfAllowableFields() throws Exception {
78+
void patchUser_ShouldSucceed_WhenProvidedWithValidValueForSubsetOfAllowableFields() throws Exception {
7779
UserAccountEntity user = superAdminUserStub.givenSystemAdminIsAuthorised(userIdentity);
7880

7981
UserAccountEntity existingAccount = createEnabledUserAccountEntity(user);
@@ -121,7 +123,7 @@ void patchUserShouldSucceedWhenProvidedWithValidValueForSubsetOfAllowableFields(
121123
}
122124

123125
@Test
124-
void patchUserShouldSucceedWhenProvidedWithValidValuesForAllAllowableFields() throws Exception {
126+
void patchUser_ShouldSucceed_WhenProvidedWithValidValuesForAllAllowableFields() throws Exception {
125127
UserAccountEntity user = superAdminUserStub.givenSystemAdminIsAuthorised(userIdentity);
126128

127129
UserAccountEntity existingAccount = createEnabledUserAccountEntity(user);
@@ -168,7 +170,7 @@ void patchUserShouldSucceedWhenProvidedWithValidValuesForAllAllowableFields() th
168170

169171
// Regression test to cover bug DMP-2459
170172
@Test
171-
void patchUserShouldSucceedWhenAnExistingDescriptionIsRemoved() throws Exception {
173+
void patchUser_ShouldSucceed_WhenAnExistingDescriptionIsRemoved() throws Exception {
172174
UserAccountEntity user = superAdminUserStub.givenSystemAdminIsAuthorised(userIdentity);
173175

174176
UserAccountEntity existingAccount = createEnabledUserAccountEntity(user);
@@ -215,7 +217,7 @@ void patchUserShouldSucceedWhenAnExistingDescriptionIsRemoved() throws Exception
215217
}
216218

217219
@Test
218-
void patchUserShouldFailIfChangeWithInvalidDataIsAttempted() throws Exception {
220+
void patchUser_ShouldFail_IfChangeWithInvalidDataIsAttempted() throws Exception {
219221
UserAccountEntity user = superAdminUserStub.givenSystemAdminIsAuthorised(userIdentity);
220222

221223
UserAccountEntity existingAccount = createEnabledUserAccountEntity(user);
@@ -234,7 +236,7 @@ void patchUserShouldFailIfChangeWithInvalidDataIsAttempted() throws Exception {
234236
}
235237

236238
@Test
237-
void patchUserShouldFailIfProvidedUserIdDoesNotExistInDB() throws Exception {
239+
void patchUser_ShouldFail_IfProvidedUserIdDoesNotExistInDB() throws Exception {
238240
superAdminUserStub.givenUserIsAuthorised(userIdentity);
239241

240242
MockHttpServletRequestBuilder request = buildRequest(818_231)
@@ -249,7 +251,7 @@ void patchUserShouldFailIfProvidedUserIdDoesNotExistInDB() throws Exception {
249251
}
250252

251253
@Test
252-
void patchUserShouldSucceedAndClearSecurityGroupsWhenAccountGetsDisabledAndNoSecurityGroupsAreExplicitlyProvided() throws Exception {
254+
void patchUser_ShouldSucceedAndClearSecurityGroups_WhenAccountGetsDisabledAndNoSecurityGroupsAreExplicitlyProvided() throws Exception {
253255
UserAccountEntity user = superAdminUserStub.givenSystemAdminIsAuthorised(userIdentity);
254256

255257
UserAccountEntity existingAccount = createEnabledUserAccountEntity(user);
@@ -283,7 +285,37 @@ void patchUserShouldSucceedAndClearSecurityGroupsWhenAccountGetsDisabledAndNoSec
283285
}
284286

285287
@Test
286-
void patchUserShouldSucceedWhenAccountGetsEnabled() throws Exception {
288+
void patchUser_ShouldFail_WhenAccountGetsDeactivatedBySameUser() throws Exception {
289+
UserAccountEntity user = superAdminUserStub.givenSystemAdminIsAuthorised(userIdentity);
290+
291+
Integer userId = user.getId();
292+
293+
MockHttpServletRequestBuilder request = buildRequest(userId)
294+
.content("""
295+
{
296+
"active": false
297+
}
298+
""");
299+
mockMvc.perform(request)
300+
.andExpect(status().is4xxClientError())
301+
.andExpect(jsonPath("$.type").value("AUTHORISATION_115"))
302+
.andExpect(jsonPath("$.title").value("User is not authorised to modify own account"))
303+
.andExpect(jsonPath("$.status").value(403));
304+
305+
transactionTemplate.execute(status -> {
306+
UserAccountEntity latestUserAccountEntity = dartsDatabase.getUserAccountRepository()
307+
.findById(userId)
308+
.orElseThrow();
309+
310+
assertEquals(true, latestUserAccountEntity.isActive());
311+
assertThat(getSecurityGroupIds(latestUserAccountEntity).size(), greaterThan(0));
312+
313+
return null;
314+
});
315+
}
316+
317+
@Test
318+
void patchUser_ShouldSucceed_WhenAccountGetsEnabled() throws Exception {
287319
UserAccountEntity user = superAdminUserStub.givenUserIsAuthorised(userIdentity);
288320

289321
UserAccountEntity existingAccount = createDisabledUserAccountEntity(user);
@@ -317,7 +349,7 @@ void patchUserShouldSucceedWhenAccountGetsEnabled() throws Exception {
317349
}
318350

319351
@Test
320-
void patchUserShouldSucceedWhenSecurityGroupsAreUpdated() throws Exception {
352+
void patchUser_ShouldSucceed_WhenSecurityGroupsAreUpdated() throws Exception {
321353
UserAccountEntity user = superAdminUserStub.givenSystemAdminIsAuthorised(userIdentity);
322354

323355
UserAccountEntity existingAccount = createEnabledUserAccountEntity(user);
@@ -361,7 +393,7 @@ void patchUserShouldSucceedWhenSecurityGroupsAreUpdated() throws Exception {
361393
}
362394

363395
@Test
364-
void patchUserShouldSucceedIfEmailAddressChangeDifferent() throws Exception {
396+
void patchUser_ShouldSucceed_IfEmailAddressChangeDifferent() throws Exception {
365397
UserAccountEntity user = superAdminUserStub.givenSystemAdminIsAuthorised(userIdentity);
366398

367399
UserAccountEntity existingAccount = createEnabledUserAccountEntity(user);
@@ -382,7 +414,7 @@ void patchUserShouldSucceedIfEmailAddressChangeDifferent() throws Exception {
382414
}
383415

384416
@Test
385-
void patchUserSameEmailShouldBeOkAndDataShouldRemainUnchanged() throws Exception {
417+
void patchUser_ShouldRemainUnchanged_SameEmailShouldBeOkAndData() throws Exception {
386418
UserAccountEntity user = superAdminUserStub.givenSystemAdminIsAuthorised(userIdentity);
387419

388420
UserAccountEntity existingAccount = createEnabledUserAccountEntity(user);
@@ -401,7 +433,7 @@ void patchUserSameEmailShouldBeOkAndDataShouldRemainUnchanged() throws Exception
401433
}
402434

403435
@Test
404-
void patchUserDuplicateEmailShouldFail() throws Exception {
436+
void patchUser_DuplicateEmailShouldFail() throws Exception {
405437
UserAccountEntity user = superAdminUserStub.givenSystemAdminIsAuthorised(userIdentity);
406438

407439
createEnabledUserAccountEntity(user);
@@ -440,7 +472,7 @@ void patchUserShouldFailIfUserIsNotAuthorised() throws Exception {
440472
}
441473

442474
@Test
443-
void patchUserShouldFailIfProvidedUserIsASystemUser() throws Exception {
475+
void patchUser_ShouldFailIfProvidedUserIsASystemUser() throws Exception {
444476
superAdminUserStub.givenSystemAdminIsAuthorised(userIdentity);
445477

446478
UserAccountEntity userAccountEntity = accountStub.getSystemUserAccountEntity();

src/integrationTest/java/uk/gov/hmcts/darts/usermanagement/controller/SecurityGroupControllerIntTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package uk.gov.hmcts.darts.usermanagement.controller;
22

33
import org.junit.jupiter.api.Test;
4+
import org.junit.jupiter.api.parallel.Isolated;
45
import org.skyscreamer.jsonassert.JSONAssert;
56
import org.skyscreamer.jsonassert.JSONCompareMode;
67
import org.springframework.beans.factory.annotation.Autowired;
@@ -25,6 +26,7 @@
2526
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
2627

2728
@AutoConfigureMockMvc
29+
@Isolated
2830
class SecurityGroupControllerIntTest extends IntegrationBase {
2931

3032
public static final String TEST_COURTHOUSE_NAME = "SWANSEA";

src/integrationTest/java/uk/gov/hmcts/darts/usermanagement/controller/UserControllerIntTest.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,15 +88,12 @@ void testDeactivateModifyWithSuperAdmin() throws Exception {
8888
UserAccountEntity userAccountEntity = UserAccountTestData.minimalUserAccount();
8989
userAccountEntity = userAccountRepository.save(userAccountEntity);
9090

91-
9291
Optional<SecurityGroupEntity> groupEntity
9392
= securityGroupRepository.findByGroupNameIgnoreCase(SecurityGroupEnum.SUPER_ADMIN.getName());
9493

95-
9694
userAccountEntity.getSecurityGroupEntities().add(groupEntity.get());
9795
userAccountEntity = dartsDatabaseStub.save(userAccountEntity);
9896

99-
10097
HearingEntity hearingEntity = dartsDatabase.givenTheDatabaseContainsCourtCaseWithHearingAndCourthouseWithRoom(
10198
SOME_CASE_ID,
10299
SOME_COURTHOUSE,
@@ -325,7 +322,7 @@ void testActivateModifyUserWithSuperAdminAndFailWithNoFullNameAndNoEmailAddress(
325322
// add user to the super admin group
326323
Optional<SecurityGroupEntity> groupEntity
327324
= securityGroupRepository.findByGroupNameIgnoreCase(SecurityGroupEnum.SUPER_ADMIN.getName());
328-
325+
329326
userAccountEntity.getSecurityGroupEntities().add(groupEntity.get());
330327
userAccountEntity = dartsDatabaseStub.save(userAccountEntity);
331328

src/main/java/uk/gov/hmcts/darts/authorisation/exception/AuthorisationError.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,11 @@ public enum AuthorisationError implements DartsApiError {
8585
AuthorisationErrorCode.USER_NOT_AUTHORISED_TO_ACTIVATE_USER.getValue(),
8686
HttpStatus.FORBIDDEN,
8787
AuthorisationTitleErrors.USER_NOT_AUTHORISED_TO_ACTIVATE_USER.getValue()
88+
),
89+
USER_NOT_AUTHORISED_TO_MODIFY_OWN_ACCOUNT(
90+
AuthorisationErrorCode.USER_NOT_AUTHORISED_TO_MODIFY_OWN_ACCOUNT.getValue(),
91+
HttpStatus.FORBIDDEN,
92+
AuthorisationTitleErrors.USER_NOT_AUTHORISED_TO_MODIFY_OWN_ACCOUNT.getValue()
8893
);
8994

9095
private static final String ERROR_TYPE_PREFIX = "AUTHORISATION";

src/main/java/uk/gov/hmcts/darts/common/entity/CaseRetentionEntity.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,8 @@
2323
@Table(name = CaseRetentionEntity.TABLE_NAME)
2424
@Getter
2525
@Setter
26-
public class CaseRetentionEntity extends CreatedModifiedBaseEntity
27-
implements HasIntegerId {
28-
29-
26+
public class CaseRetentionEntity extends CreatedModifiedBaseEntity implements HasIntegerId {
27+
3028
public static final String ID = "car_id";
3129
public static final String TABLE_NAME = "case_retention";
3230

src/main/java/uk/gov/hmcts/darts/usermanagement/service/impl/UserManagementServiceImpl.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import uk.gov.hmcts.darts.usermanagement.service.validation.UserAccountExistsValidator;
3030
import uk.gov.hmcts.darts.usermanagement.service.validation.UserTypeValidator;
3131
import uk.gov.hmcts.darts.usermanagement.validator.AuthorisedUserPermissionsValidator;
32+
import uk.gov.hmcts.darts.usermanagement.validator.NotSameUserValidator;
3233
import uk.gov.hmcts.darts.usermanagement.validator.UserActivateValidator;
3334
import uk.gov.hmcts.darts.usermanagement.validator.UserDeactivateNotLastInSuperAdminGroupValidator;
3435
import uk.gov.hmcts.darts.util.DataUtil;
@@ -65,6 +66,7 @@ public class UserManagementServiceImpl implements UserManagementService {
6566
private final TranscriptionService transcriptionService;
6667
private final AuditApi auditApi;
6768
private final UserActivateValidator authoriseValidator;
69+
private final NotSameUserValidator notSameUserValidator;
6870

6971
@Override
7072
@Transactional
@@ -102,8 +104,10 @@ public UserWithIdAndTimestamps modifyUser(Integer userId, UserPatch userPatch) {
102104
userAccountExistsValidator.validate(userId);
103105
userTypeValidator.validate(userId);
104106
userActivationPermissionsValidator.validate(userPatch);
105-
userNotLastSuperAdminValidator.validate(new IdRequest<>(userPatch, userId));
106-
authoriseValidator.validate(new IdRequest<>(userPatch, userId));
107+
IdRequest<UserPatch, Integer> request = new IdRequest<>(userPatch, userId);
108+
userNotLastSuperAdminValidator.validate(request);
109+
authoriseValidator.validate(request);
110+
notSameUserValidator.validate(request);
107111

108112
var userAccountEntity = userAccountRepository.findById(userId)
109113
.orElseThrow(() -> new NoSuchElementException("No value present"));
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package uk.gov.hmcts.darts.usermanagement.validator;
2+
3+
import lombok.RequiredArgsConstructor;
4+
import org.springframework.stereotype.Component;
5+
import uk.gov.hmcts.darts.authorisation.api.AuthorisationApi;
6+
import uk.gov.hmcts.darts.authorisation.exception.AuthorisationError;
7+
import uk.gov.hmcts.darts.common.component.validation.Validator;
8+
import uk.gov.hmcts.darts.common.exception.DartsApiException;
9+
import uk.gov.hmcts.darts.common.validation.IdRequest;
10+
import uk.gov.hmcts.darts.usermanagement.model.UserPatch;
11+
12+
import static java.util.Objects.nonNull;
13+
14+
@Component
15+
@RequiredArgsConstructor
16+
public class NotSameUserValidator implements Validator<IdRequest<UserPatch, Integer>> {
17+
18+
private final AuthorisationApi authorisationApi;
19+
20+
@Override
21+
public void validate(IdRequest<UserPatch, Integer> userPatch) {
22+
if (nonNull(userPatch.getPayload())) {
23+
var currentUser = authorisationApi.getCurrentUser();
24+
if (currentUser.getId().equals(userPatch.getId())) {
25+
throw new DartsApiException(AuthorisationError.USER_NOT_AUTHORISED_TO_MODIFY_OWN_ACCOUNT);
26+
}
27+
}
28+
}
29+
}

src/main/resources/openapi/common.yaml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,11 @@ components:
145145
- "AUTHORISATION_112"
146146
- "AUTHORISATION_113"
147147
- "AUTHORISATION_114"
148+
- "AUTHORISATION_115"
148149
x-enum-varnames: [ USER_NOT_AUTHORISED_FOR_COURTHOUSE, BAD_REQUEST_CASE_ID, BAD_REQUEST_HEARING_ID, BAD_REQUEST_MEDIA_REQUEST_ID, BAD_REQUEST_MEDIA_ID,
149150
BAD_REQUEST_TRANSCRIPTION_ID, USER_DETAILS_INVALID, BAD_REQUEST_ANY_ID, BAD_REQUEST_TRANSFORMED_MEDIA_ID,
150151
USER_NOT_AUTHORISED_FOR_ENDPOINT, BAD_REQUEST_ANNOTATION_ID, USER_NOT_AUTHORISED_TO_USE_PAYLOAD_CONTENT, UNABLE_TO_DEACTIVATE_USER,
151-
USER_NOT_AUTHORISED_TO_ACTIVATE_USER, USER_NOT_ACTIVE ]
152+
USER_NOT_AUTHORISED_TO_ACTIVATE_USER, USER_NOT_ACTIVE, USER_NOT_AUTHORISED_TO_MODIFY_OWN_ACCOUNT ]
152153

153154
AuthorisationTitleErrors:
154155
type: string
@@ -168,10 +169,11 @@ components:
168169
- "Failed to deactivate user"
169170
- "Failed to activate user. User not authorised"
170171
- "User is not active"
172+
- "User is not authorised to modify own account"
171173
x-enum-varnames: [ USER_NOT_AUTHORISED_FOR_COURTHOUSE, BAD_REQUEST_CASE_ID, BAD_REQUEST_HEARING_ID, BAD_REQUEST_MEDIA_REQUEST_ID, BAD_REQUEST_MEDIA_ID,
172174
BAD_REQUEST_TRANSCRIPTION_ID, USER_DETAILS_INVALID, BAD_REQUEST_ANY_ID, BAD_REQUEST_TRANSFORMED_MEDIA_ID,
173175
USER_NOT_AUTHORISED_FOR_ENDPOINT, BAD_REQUEST_ANNOTATION_ID, USER_NOT_AUTHORISED_TO_USE_PAYLOAD_CONTENT, UNABLE_TO_DEACTIVATE_USER,
174-
USER_NOT_AUTHORISED_TO_ACTIVATE_USER, USER_NOT_ACTIVE ]
176+
USER_NOT_AUTHORISED_TO_ACTIVATE_USER, USER_NOT_ACTIVE, USER_NOT_AUTHORISED_TO_MODIFY_OWN_ACCOUNT ]
175177

176178
AutomatedTaskErrorCode:
177179
type: string

0 commit comments

Comments
 (0)