Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions src/main/java/org/broadinstitute/consent/http/db/SamDAO.java
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,7 @@ public UserStatusInfo getCombinedUserStatusInfo(AuthUser authUser) throws Except
String body = response.parseAsString();
if (!response.isSuccessStatusCode()) {
if (body.toLowerCase().contains("cannot update azureb2cid for user")) {
throw new SamAzureB2CException(
String.format(
"AzureB2C authentication Error for user %s. Please contact support for help with this error.",
authUser.getEmail()));
throw new SamAzureB2CException(getAzureB2CErrorMessage(authUser.getEmail()));
}
String errorMsg =
String.format(
Expand Down Expand Up @@ -170,16 +167,20 @@ public static String getErrorMessage(DuosUser duosUser, String body) {
JsonElement messageElement = JsonParser.parseString(body).getAsJsonObject().get("message");
String message = messageElement != null ? messageElement.getAsString() : body;
if (message.contains("Cannot update azureB2cId")) {
return String.format(
"Email: %s. You may have previously signed in with a different authentication provider (Google or Microsoft). Please sign in with that provider. For more information visit: https://support.terra.bio/hc/en-us/community/posts/24089648317467-Cannot-update-azureB2cId-for-user",
duosUser.getEmail());
return getAzureB2CErrorMessage(duosUser.getEmail());
}
return String.format(errorMsg + " %s.", message);
} catch (JsonSyntaxException e) { // If the body is not a valid JSON
return String.format(errorMsg + " %s.", body);
return String.format("%s %s.", errorMsg, message);
} catch (JsonSyntaxException _) { // If the body is not a valid JSON
return String.format("%s %s.", errorMsg, body);
}
}

public static String getAzureB2CErrorMessage(String email) {
return String.format(
"Email: %s. You may have previously signed in with a different authentication provider (Google or Microsoft). Please sign in with that provider. For more information visit: https://support.terra.bio/hc/en-us/community/posts/24089648317467-Cannot-update-azureB2cId-for-user",
email);
}

public void asyncPostRegistrationInfo(DuosUser duosUser) {
ListeningExecutorService listeningExecutorService =
MoreExecutors.listeningDecorator(executorService);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.broadinstitute.consent.http.exceptions.ConsentConflictException;
import org.broadinstitute.consent.http.exceptions.LibraryCardRequiredException;
import org.broadinstitute.consent.http.exceptions.NIHComplianceRuleException;
import org.broadinstitute.consent.http.exceptions.SamAzureB2CException;
import org.broadinstitute.consent.http.exceptions.SubmittedDARCannotBeEditedException;
import org.broadinstitute.consent.http.exceptions.UnknownIdentifierException;
import org.broadinstitute.consent.http.exceptions.UnprocessableEntityException;
Expand Down Expand Up @@ -106,6 +107,13 @@ public abstract class Resource implements ConsentLogger {
.type(MediaType.APPLICATION_JSON)
.entity(new Error(e.getMessage(), Response.Status.CONFLICT.getStatusCode()))
.build());
DISPATCH.put(
SamAzureB2CException.class,
e ->
Response.status(Response.Status.CONFLICT)
.type(MediaType.APPLICATION_JSON)
.entity(new Error(e.getMessage(), Response.Status.CONFLICT.getStatusCode()))
.build());
DISPATCH.put(
UnprocessableEntityException.class,
e ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,13 @@ public Response getUser(@Auth AuthUser authUser) {
DuosUser duosUser = new DuosUser(authUser, findRegisteredUser(authUser));
UserStatusInfo userStatusInfo = duosUser.getUserStatusInfo();
if (userStatusInfo == null) {
samService.asyncPostRegistrationInfo(duosUser);
// Refresh the user status info after posting registration info to Sam
// Query Sam before the registration post: a B2C identity conflict throws a
// mapped exception here, and the registration post always answers 409 for
// those users, so we must not send it.
userStatusInfo = getUserStatusInfo(duosUser);
if (userStatusInfo == null) {
samService.asyncPostRegistrationInfo(duosUser);
}
}
User user = nihService.syncAccount(duosUser);
if (userStatusInfo != null) {
Expand All @@ -142,23 +146,28 @@ public Response getUser(@Auth AuthUser authUser) {
* Resolve the DUOS account for an authenticated caller. Unlike the {@code @Auth DuosUser}
* endpoints, /me does its own lookup so that a valid token with no account answers 404 instead of
* the 401 an unresolved principal produces — callers need those distinguishable to know whether
* to start registration or drop the session.
* to start registration or drop the session. A 404 sends the client into registration, which
* cannot succeed for a user whose Sam record has a B2C identity conflict, so when the auth filter
* got no Sam status we ask Sam again and let a B2C conflict answer 409 instead of 404.
*/
private User findRegisteredUser(AuthUser authUser) {
private User findRegisteredUser(AuthUser authUser) throws SamAzureB2CException {
try {
return userService.findUserByEmail(authUser.getEmail());
} catch (NotFoundException e) {
} catch (NotFoundException _) {
if (authUser.getUserStatusInfo() == null) {
getUserStatusInfo(authUser);
}
throw new NotFoundException("Authenticated user is not registered");
}
}

private UserStatusInfo getUserStatusInfo(DuosUser duosUser) throws SamAzureB2CException {
private UserStatusInfo getUserStatusInfo(AuthUser authUser) throws SamAzureB2CException {
try {
return samService.getCombinedUserStatusInfo(duosUser);
return samService.getCombinedUserStatusInfo(authUser);
} catch (SamAzureB2CException e) {
logWarn(
"Sam azure b2c exception: %s for user: %s"
.formatted(e.getMessage(), duosUser.getEmail()));
.formatted(e.getMessage(), authUser.getEmail()));
throw e;
} catch (Exception ex) {
logWarn("Unable to retrieve user status info from Sam: " + ex.getMessage());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.google.inject.Inject;
import java.util.List;
import org.broadinstitute.consent.http.db.SamDAO;
import org.broadinstitute.consent.http.models.AuthUser;
import org.broadinstitute.consent.http.models.DuosUser;
import org.broadinstitute.consent.http.models.sam.ResourceType;
import org.broadinstitute.consent.http.models.sam.TosResponse;
Expand Down Expand Up @@ -40,8 +41,8 @@ public void asyncPostRegistrationInfo(DuosUser duosUser) {
samDAO.asyncPostRegistrationInfo(duosUser);
}

public UserStatusInfo getCombinedUserStatusInfo(DuosUser duosUser) throws Exception {
return samDAO.getCombinedUserStatusInfo(duosUser);
public UserStatusInfo getCombinedUserStatusInfo(AuthUser authUser) throws Exception {
return samDAO.getCombinedUserStatusInfo(authUser);
}

public String getToSText() throws Exception {
Expand Down
8 changes: 8 additions & 0 deletions src/main/resources/assets/api-docs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -913,6 +913,14 @@ paths:
description: Unauthorized - missing or unusable credentials
404:
description: Authenticated, but no DUOS account exists for this user
409:
description: The Sam record for this user has an azureB2cId conflict. The user
may have previously signed in with a different authentication provider. The
message explains how to resolve the conflict; registration cannot succeed.
content:
application/json:
schema:
$ref: '#/components/schemas/ErrorResponse'
429:
description: Too Many Requests - rate limit exceeded
content:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,45 @@ void testGetMeUnregisteredUserReturnsNotFound() throws Exception {
verify(nihService, never()).syncAccount(any());
}

/**
* A caller with no DUOS account whose Sam record has a B2C identity conflict gets 409, not 404. A
* 404 would send them into registration, which cannot succeed for them.
*/
@Test
void testGetMeUnregisteredUserWithB2CConflictReturns409() throws Exception {
when(userService.findUserByEmail(TEST_EMAIL))
.thenThrow(new NotFoundException("Unable to find user with email: " + TEST_EMAIL));
when(samService.getCombinedUserStatusInfo(any(AuthUser.class)))
.thenThrow(new SamAzureB2CException("AzureB2C error for user " + TEST_EMAIL));

Response response = userResource.getUser(authUser);

assertEquals(Status.CONFLICT.getStatusCode(), response.getStatus());
verify(samService, never()).asyncPostRegistrationInfo(any());
verify(nihService, never()).syncAccount(any());
}

/**
* When the auth filter already has a Sam status, an unregistered caller gets 404 with no second
* Sam query.
*/
@Test
void testGetMeUnregisteredUserWithStatusSkipsSamQuery() throws Exception {
AuthUser withStatus =
new AuthUser()
.setAuthToken("auth-token")
.setName("Test User")
.setEmail(TEST_EMAIL)
.setUserStatusInfo(new UserStatusInfo().setUserEmail(TEST_EMAIL));
when(userService.findUserByEmail(TEST_EMAIL))
.thenThrow(new NotFoundException("Unable to find user with email: " + TEST_EMAIL));

Response response = userResource.getUser(withStatus);

assertEquals(Status.NOT_FOUND.getStatusCode(), response.getStatus());
verify(samService, never()).getCombinedUserStatusInfo(any(AuthUser.class));
}

@Test
void testGetMeWithUserStatusInfo() throws Exception {
User user = createUserWithRole();
Expand Down Expand Up @@ -195,15 +234,17 @@ void testGetMeSamFailure() throws Exception {
}

@Test
void testGetMe_SamAzureB2CException_Returns500() throws Exception {
// SamAzureB2CException should NOT be silently swallowed - it propagates and returns a 500
void testGetMe_SamAzureB2CException_Returns409() throws Exception {
// SamAzureB2CException is not swallowed - it propagates and maps to a 409 Conflict
User user = createUserWithRole();
when(userService.findUserByEmail(TEST_EMAIL)).thenReturn(user);
when(samService.getCombinedUserStatusInfo(any(DuosUser.class)))
.thenThrow(new SamAzureB2CException("AzureB2C error for user test@test.org"));

Response response = userResource.getUser(authUser);
assertEquals(Status.INTERNAL_SERVER_ERROR.getStatusCode(), response.getStatus());
assertEquals(Status.CONFLICT.getStatusCode(), response.getStatus());
// The registration post always answers 409 for these users - it must not fire
verify(samService, never()).asyncPostRegistrationInfo(any(DuosUser.class));
}

@Test
Expand All @@ -217,7 +258,7 @@ void testGetMe_SamAzureB2CException_ErrorMessageContainsDetails() throws Excepti
.thenThrow(new SamAzureB2CException(errorMessage));

Response response = userResource.getUser(authUser);
assertEquals(Status.INTERNAL_SERVER_ERROR.getStatusCode(), response.getStatus());
assertEquals(Status.CONFLICT.getStatusCode(), response.getStatus());
assertNotNull(response.getEntity());
assertTrue(response.getEntity().toString().contains(errorMessage));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,8 @@ void testGetCombinedUserStatusInfoAzureB2CError(String errorBody) {
assertThrows(SamAzureB2CException.class, () -> samDAO.getCombinedUserStatusInfo(duosUser));
assertNotNull(ex.getMessage());
assertTrue(ex.getMessage().contains("test@test.org"));
assertTrue(ex.getMessage().contains("different authentication provider"));
assertTrue(ex.getMessage().contains("https://support.terra.bio"));
}

@Test
Expand Down
Loading