Skip to content

Commit 724b1e0

Browse files
arcshiftsolutionsarcshiftsolutions
authored
Merge pull request #9 from bcgov/feature/GUID
feat: added IDIR GUID lookup
2 parents e6fd23c + feec16f commit 724b1e0

6 files changed

Lines changed: 253 additions & 6 deletions

File tree

‎backend/src/main/java/ca/bc/gov/nrs/userlookup/controller/v1/UserLookupController.java‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import ca.bc.gov.nrs.userlookup.dto.SearchIdirUsersQuery;
66
import ca.bc.gov.nrs.userlookup.dto.SearchIdirUsersResponse;
77
import ca.bc.gov.nrs.userlookup.dto.SearchUserParameterType;
8+
import ca.bc.gov.nrs.userlookup.exception.InvalidRequestException;
89
import ca.bc.gov.nrs.userlookup.security.ApiScopes;
910
import ca.bc.gov.nrs.userlookup.service.UserLookupService;
1011
import io.swagger.v3.oas.annotations.Operation;
@@ -13,6 +14,7 @@
1314
import jakarta.validation.Valid;
1415
import lombok.RequiredArgsConstructor;
1516
import org.springframework.security.access.prepost.PreAuthorize;
17+
import org.springframework.util.StringUtils;
1618
import org.springframework.web.bind.annotation.GetMapping;
1719
import org.springframework.web.bind.annotation.PostMapping;
1820
import org.springframework.web.bind.annotation.RequestMapping;
@@ -39,12 +41,39 @@ public class UserLookupController {
3941

4042
private final UserLookupService userLookupService;
4143

44+
/**
45+
* Look up one IDIR account by {@code userId} or {@code userGuid}.
46+
*
47+
* <p>Both parameters are optional individually and exactly one must be given.
48+
* {@code userId} was the only form originally, so it stays optional-but-alone
49+
* rather than becoming a {@code searchUserBy}/{@code searchValue} pair like the
50+
* Business BCeID endpoint - that would have broken every existing caller for a
51+
* cosmetic gain.
52+
*
53+
* <p>Same {@code idir:read} scope either way: it is the same account detail
54+
* from the same source, reached by a different key.
55+
*/
4256
@GetMapping("/idir-account-detail")
4357
@PreAuthorize(ApiScopes.IDIR_READ)
44-
@Operation(summary = "Get IDIR user account detail by userId (exact match)")
58+
@Operation(summary = "Get IDIR user account detail by userId or userGuid (exact match)",
59+
description = "Supply exactly one of userId or userGuid.")
4560
public IdirUserResponse verifyIdirUserByAccountDetail(
46-
@RequestParam("userId") String userId) {
47-
return userLookupService.verifyIdirUserByAccountDetail(userId);
61+
@RequestParam(value = "userId", required = false) String userId,
62+
@RequestParam(value = "userGuid", required = false) String userGuid) {
63+
64+
boolean hasUserId = StringUtils.hasText(userId);
65+
boolean hasUserGuid = StringUtils.hasText(userGuid);
66+
67+
if (hasUserId == hasUserGuid) {
68+
// Both or neither. Rejected rather than resolved by precedence: silently
69+
// ignoring one of two identifiers that disagree would answer a question
70+
// the caller did not ask.
71+
throw new InvalidRequestException("Supply exactly one of userId or userGuid.");
72+
}
73+
74+
return hasUserGuid
75+
? userLookupService.verifyIdirUser(SearchUserParameterType.userGuid, userGuid)
76+
: userLookupService.verifyIdirUser(SearchUserParameterType.userId, userId);
4877
}
4978

5079
@PostMapping("/idir-users/search")

‎backend/src/main/java/ca/bc/gov/nrs/userlookup/exception/InvalidRequestException.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package ca.bc.gov.nrs.userlookup.exception;
2+
3+
/**
4+
* The caller's request is not answerable as asked - a combination of parameters
5+
* that cannot be resolved, rather than a value the upstream directory rejected.
6+
*
7+
* <p>Distinct from {@link UpstreamBusinessException}, which also becomes a 400
8+
* but reports the directory's verdict on a well-formed request. Keeping them
9+
* apart means a log line says whether the caller or the directory refused.
10+
*
11+
* <p>A dedicated type rather than {@code IllegalArgumentException}: mapping that
12+
* to 400 globally would also turn an internal one - {@code NumberFormatException}
13+
* is a subclass - into an apparent client error, hiding a real fault.
14+
*/
15+
public class InvalidRequestException extends RuntimeException {
16+
17+
public InvalidRequestException(String message) {
18+
super(message);
19+
}
20+
}

‎backend/src/main/java/ca/bc/gov/nrs/userlookup/exception/RestExceptionHandler.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,13 @@ protected ResponseEntity<Object> handleUpstreamBusiness(UpstreamBusinessExceptio
3939
return new ResponseEntity<>(apiError, apiError.getStatus());
4040
}
4141

42+
@ExceptionHandler(InvalidRequestException.class)
43+
protected ResponseEntity<Object> handleInvalidRequest(InvalidRequestException ex) {
44+
ApiError apiError = new ApiError(HttpStatus.BAD_REQUEST, ex.getMessage());
45+
log.info("Invalid request: {}", ex.getMessage());
46+
return new ResponseEntity<>(apiError, apiError.getStatus());
47+
}
48+
4249
@ExceptionHandler(UpstreamServiceException.class)
4350
protected ResponseEntity<Object> handleUpstreamService(UpstreamServiceException ex) {
4451
ApiError apiError = new ApiError(HttpStatus.INTERNAL_SERVER_ERROR, ex.getMessage());

‎backend/src/main/java/ca/bc/gov/nrs/userlookup/service/UserLookupService.java‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,34 @@ public class UserLookupService {
6666

6767
/** Scenario: an IDIR requester looks up an IDIR user by exact userId. */
6868
public IdirUserResponse verifyIdirUserByAccountDetail(String userId) {
69+
return verifyIdirUser(SearchUserParameterType.userId, userId);
70+
}
71+
72+
/**
73+
* Scenario: look up an IDIR user by exact userId or userGuid.
74+
*
75+
* <p>The GUID form exists because a caller does not always hold a userId. A
76+
* consumer reading identities out of Keycloak has the GUID and nothing else -
77+
* a federated user who has never signed in is stored as {@code <guid>@azureidir}
78+
* with no name or email against it - so without this there is no way to turn
79+
* that back into a person.
80+
*
81+
* <p>Both forms are the same SOAP call: {@code AccountDetailRequest} takes
82+
* either property, exactly as the Business BCeID lookup already does.
83+
*/
84+
public IdirUserResponse verifyIdirUser(SearchUserParameterType searchUserBy,
85+
String searchValue) {
6986
checkRequiredCredentials();
7087

7188
AccountDetailRequest detail = new AccountDetailRequest();
7289
detail.setOnlineServiceId(properties.getOnlineServiceId());
7390
detail.setRequesterAccountTypeCode(RequesterAccountTypeCode.Internal.name());
7491
detail.setRequesterUserGuid(properties.getRequesterUserGuid());
75-
detail.setUserId(userId);
92+
if (searchUserBy == SearchUserParameterType.userGuid) {
93+
detail.setUserGuid(searchValue);
94+
} else {
95+
detail.setUserId(searchValue);
96+
}
7697
detail.setAccountTypeCode(RequesterAccountTypeCode.Internal.name());
7798

7899
GetAccountDetailRequest request = new GetAccountDetailRequest();
@@ -83,8 +104,14 @@ public IdirUserResponse verifyIdirUserByAccountDetail(String userId) {
83104

84105
IdirUserResponse response = new IdirUserResponse();
85106
if (isNoResults(result.getCode(), result.getFailureCode())) {
107+
// Echo back whichever identifier was asked about, so the caller can tell
108+
// which of a batch of lookups came back empty.
86109
response.setFound(false);
87-
response.setUserId(userId);
110+
if (searchUserBy == SearchUserParameterType.userGuid) {
111+
response.setGuid(searchValue);
112+
} else {
113+
response.setUserId(searchValue);
114+
}
88115
return response;
89116
}
90117

‎backend/src/test/java/ca/bc/gov/nrs/userlookup/controller/UserLookupControllerSecurityTest.java‎

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,18 @@
11
package ca.bc.gov.nrs.userlookup.controller;
22

3+
import static org.assertj.core.api.Assertions.assertThatCode;
34
import static org.mockito.ArgumentMatchers.any;
5+
import static org.mockito.Mockito.verify;
6+
import static org.mockito.Mockito.verifyNoInteractions;
47
import static org.mockito.Mockito.when;
58
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.jwt;
69
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
710
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
11+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
812
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
913

1014
import ca.bc.gov.nrs.userlookup.dto.IdirUserResponse;
15+
import ca.bc.gov.nrs.userlookup.dto.SearchUserParameterType;
1116
import ca.bc.gov.nrs.userlookup.dto.SearchIdirUsersResponse;
1217
import ca.bc.gov.nrs.userlookup.service.UserLookupService;
1318
import org.junit.jupiter.api.Test;
@@ -80,7 +85,7 @@ void searchWithNoSearchFieldReturns400() throws Exception {
8085

8186
@Test
8287
void accountDetailWithRequiredScopeReturns200() throws Exception {
83-
when(userLookupService.verifyIdirUserByAccountDetail(any())).thenReturn(new IdirUserResponse());
88+
when(userLookupService.verifyIdirUser(any(), any())).thenReturn(new IdirUserResponse());
8489

8590
mockMvc.perform(get(DETAIL_URL)
8691
.with(jwt().authorities(scope("user-lookup:idir:read")))
@@ -95,4 +100,72 @@ void accountDetailWithWrongScopeReturns403() throws Exception {
95100
.param("userId", "jdoe"))
96101
.andExpect(status().isForbidden());
97102
}
103+
104+
@Test
105+
void accountDetailByGuidUsesTheSameScope() {
106+
// Same account detail from the same source, reached by a different key, so
107+
// it is not a separate permission.
108+
when(userLookupService.verifyIdirUser(any(), any())).thenReturn(new IdirUserResponse());
109+
110+
assertThatCode(() -> mockMvc.perform(get(DETAIL_URL)
111+
.with(jwt().authorities(scope("user-lookup:idir:read")))
112+
.param("userGuid", "0C93B7EB34654CB2B5F6EC399990D466"))
113+
.andExpect(status().isOk()))
114+
.doesNotThrowAnyException();
115+
}
116+
117+
@Test
118+
void accountDetailByGuidWithWrongScopeReturns403() throws Exception {
119+
mockMvc.perform(get(DETAIL_URL)
120+
.with(jwt().authorities(scope("user-lookup:business-bceid:read")))
121+
.param("userGuid", "0C93B7EB34654CB2B5F6EC399990D466"))
122+
.andExpect(status().isForbidden());
123+
}
124+
125+
@Test
126+
void accountDetailByGuidLooksUpByGuid() throws Exception {
127+
// Guards the wiring: passing the GUID through as a userId would silently
128+
// look up a login name and find nobody.
129+
when(userLookupService.verifyIdirUser(any(), any())).thenReturn(new IdirUserResponse());
130+
131+
mockMvc.perform(get(DETAIL_URL)
132+
.with(jwt().authorities(scope("user-lookup:idir:read")))
133+
.param("userGuid", "GUID1"))
134+
.andExpect(status().isOk());
135+
136+
verify(userLookupService).verifyIdirUser(SearchUserParameterType.userGuid, "GUID1");
137+
}
138+
139+
@Test
140+
void accountDetailWithBothIdentifiersReturns400() throws Exception {
141+
// Two identifiers that disagree have no single right answer, so neither is
142+
// silently preferred.
143+
mockMvc.perform(get(DETAIL_URL)
144+
.with(jwt().authorities(scope("user-lookup:idir:read")))
145+
.param("userId", "jdoe")
146+
.param("userGuid", "GUID1"))
147+
.andExpect(status().isBadRequest());
148+
149+
verifyNoInteractions(userLookupService);
150+
}
151+
152+
@Test
153+
void accountDetailWithNeitherIdentifierReturns400() throws Exception {
154+
mockMvc.perform(get(DETAIL_URL)
155+
.with(jwt().authorities(scope("user-lookup:idir:read"))))
156+
.andExpect(status().isBadRequest());
157+
158+
verifyNoInteractions(userLookupService);
159+
}
160+
161+
@Test
162+
void accountDetailBadRequestSaysWhy() throws Exception {
163+
// The service does not expose exception messages by default, so the reason
164+
// has to be carried in the response body deliberately.
165+
mockMvc.perform(get(DETAIL_URL)
166+
.with(jwt().authorities(scope("user-lookup:idir:read"))))
167+
.andExpect(status().isBadRequest())
168+
.andExpect(content().string(org.hamcrest.Matchers.containsString(
169+
"exactly one of userId or userGuid")));
170+
}
98171
}

‎backend/src/test/java/ca/bc/gov/nrs/userlookup/service/UserLookupServiceTest.java‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@
88

99
import ca.bc.gov.nrs.userlookup.client.BceidSoapClient;
1010
import ca.bc.gov.nrs.userlookup.client.BceidProperties;
11+
import ca.bc.gov.nrs.userlookup.client.soap.AccountDetailRequest;
1112
import ca.bc.gov.nrs.userlookup.client.soap.AccountList;
1213
import ca.bc.gov.nrs.userlookup.client.soap.BceidAccount;
1314
import ca.bc.gov.nrs.userlookup.client.soap.Contact;
15+
import ca.bc.gov.nrs.userlookup.client.soap.GetAccountDetailRequest;
1416
import ca.bc.gov.nrs.userlookup.client.soap.GetAccountDetailResult;
1517
import ca.bc.gov.nrs.userlookup.client.soap.IndividualIdentity;
1618
import ca.bc.gov.nrs.userlookup.client.soap.PersonName;
@@ -22,6 +24,7 @@
2224
import ca.bc.gov.nrs.userlookup.dto.SearchIdirUsersQuery;
2325
import ca.bc.gov.nrs.userlookup.dto.SearchIdirUsersResponse;
2426
import ca.bc.gov.nrs.userlookup.dto.SearchMatchMode;
27+
import ca.bc.gov.nrs.userlookup.dto.SearchUserParameterType;
2528
import ca.bc.gov.nrs.userlookup.exception.UpstreamBusinessException;
2629
import ca.bc.gov.nrs.userlookup.exception.UpstreamServiceException;
2730
import java.util.List;
@@ -203,4 +206,92 @@ void missingCredentialsThrows500() {
203206
.isInstanceOf(UpstreamServiceException.class)
204207
.hasMessageContaining("not configured");
205208
}
209+
210+
// ------------------------------------------------ IDIR lookup by GUID
211+
212+
private GetAccountDetailRequest captureAccountDetailRequest() {
213+
ArgumentCaptor<GetAccountDetailRequest> captor =
214+
ArgumentCaptor.forClass(GetAccountDetailRequest.class);
215+
org.mockito.Mockito.verify(soapClient).getAccountDetail(captor.capture());
216+
return captor.getValue();
217+
}
218+
219+
@Test
220+
void accountDetailByGuidAsksTheDirectoryByGuid() {
221+
// The GUID has to travel as userGuid. Sent as userId it would be looked up
222+
// as a login name, match nothing, and read as "no such user".
223+
GetAccountDetailResult result = new GetAccountDetailResult();
224+
result.setCode("Success");
225+
result.setAccount(account());
226+
when(soapClient.getAccountDetail(any())).thenReturn(result);
227+
228+
service.verifyIdirUser(SearchUserParameterType.userGuid, "guid1");
229+
230+
AccountDetailRequest sent = captureAccountDetailRequest().getAccountDetailRequest();
231+
assertThat(sent.getUserGuid()).isEqualTo("guid1");
232+
assertThat(sent.getUserId()).isNull();
233+
}
234+
235+
@Test
236+
void accountDetailByGuidStaysAnInternalLookup() {
237+
// Internal is what makes this an IDIR account rather than a Business BCeID
238+
// one; the same GUID could exist in either directory.
239+
GetAccountDetailResult result = new GetAccountDetailResult();
240+
result.setCode("Success");
241+
result.setAccount(account());
242+
when(soapClient.getAccountDetail(any())).thenReturn(result);
243+
244+
service.verifyIdirUser(SearchUserParameterType.userGuid, "guid1");
245+
246+
AccountDetailRequest sent = captureAccountDetailRequest().getAccountDetailRequest();
247+
assertThat(sent.getAccountTypeCode()).isEqualTo("Internal");
248+
assertThat(sent.getRequesterAccountTypeCode()).isEqualTo("Internal");
249+
assertThat(sent.getRequesterUserGuid()).isEqualTo(REQUESTER_GUID);
250+
}
251+
252+
@Test
253+
void accountDetailByGuidFoundMapsAccount() {
254+
GetAccountDetailResult result = new GetAccountDetailResult();
255+
result.setCode("Success");
256+
result.setAccount(account());
257+
when(soapClient.getAccountDetail(any())).thenReturn(result);
258+
259+
IdirUserResponse response = service.verifyIdirUser(SearchUserParameterType.userGuid, "guid1");
260+
261+
assertThat(response.isFound()).isTrue();
262+
assertThat(response.getUserId()).isEqualTo("jdoe");
263+
assertThat(response.getFirstName()).isEqualTo("John");
264+
assertThat(response.getLastName()).isEqualTo("Doe");
265+
}
266+
267+
@Test
268+
void accountDetailByGuidNoResultsEchoesTheGuid() {
269+
// A caller resolving several GUIDs needs to know which one came back empty.
270+
GetAccountDetailResult result = new GetAccountDetailResult();
271+
result.setCode("Failed");
272+
result.setFailureCode("NoResults");
273+
when(soapClient.getAccountDetail(any())).thenReturn(result);
274+
275+
IdirUserResponse response = service.verifyIdirUser(SearchUserParameterType.userGuid, "guid1");
276+
277+
assertThat(response.isFound()).isFalse();
278+
assertThat(response.getGuid()).isEqualTo("guid1");
279+
assertThat(response.getUserId()).isNull();
280+
}
281+
282+
@Test
283+
void accountDetailByUserIdStillAsksByUserId() {
284+
// The original form has to keep working; it is what every existing caller
285+
// sends.
286+
GetAccountDetailResult result = new GetAccountDetailResult();
287+
result.setCode("Success");
288+
result.setAccount(account());
289+
when(soapClient.getAccountDetail(any())).thenReturn(result);
290+
291+
service.verifyIdirUserByAccountDetail("jdoe");
292+
293+
AccountDetailRequest sent = captureAccountDetailRequest().getAccountDetailRequest();
294+
assertThat(sent.getUserId()).isEqualTo("jdoe");
295+
assertThat(sent.getUserGuid()).isNull();
296+
}
206297
}

0 commit comments

Comments
 (0)