Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ID-1086 Migrate TCL to use Sam v2 User Endpoint #149

Merged
merged 6 commits into from
Feb 21, 2024
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
4 changes: 2 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ dependencies {
runtimeOnly group: 'org.postgresql', name: 'postgresql', version: '42.5.0'

// Terra libraries
implementation group: 'org.broadinstitute.dsde.workbench', name: 'sam-client_2.13', version: '0.1-5281c21'
implementation group: 'org.broadinstitute.dsde.workbench', name: 'sam-client_2.13', version: '0.1-0c4b377'
var stairwayVersion= '0.0.80-SNAPSHOT'
api "bio.terra:stairway-gcp:${stairwayVersion}"
implementation "bio.terra:stairway-azure:${stairwayVersion}"
Expand Down Expand Up @@ -133,7 +133,7 @@ dependencies {
// transitive dependencies.
constraints {
implementation('org.scala-lang:scala-library:2.13.10')
implementation('org.json:json:20230227')
implementation('org.json:json:20240205')
implementation('org.bitbucket.b_c:jose4j:0.9.3')
implementation('io.grpc:grpc-xds:1.53.0')
}
Expand Down
17 changes: 11 additions & 6 deletions src/main/java/bio/terra/common/iam/SamUserFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import org.broadinstitute.dsde.workbench.client.sam.ApiClient;
import org.broadinstitute.dsde.workbench.client.sam.ApiException;
import org.broadinstitute.dsde.workbench.client.sam.api.UsersApi;
import org.broadinstitute.dsde.workbench.client.sam.model.UserStatusInfo;
import org.broadinstitute.dsde.workbench.client.sam.model.SamUserResponse;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpStatus;
import org.springframework.stereotype.Component;
Expand Down Expand Up @@ -50,12 +50,17 @@ public SamUser from(BearerToken bearerToken, String samBasePath) {
UsersApi usersApi = createUsersApi(bearerToken, samBasePath);

try {
UserStatusInfo userStatusInfo = usersApi.getUserStatusInfo();
if (!userStatusInfo.getEnabled()) {
throw new UnauthorizedException("User is disabled, please contact Terra support");
SamUserResponse samUserResponse = usersApi.getSamUserSelf();
if (!samUserResponse.getAllowed()) {
var userAllowanceDetails = usersApi.getSamUserSelfAllowances().getDetails();
if (!userAllowanceDetails.getEnabled()) {
throw new UnauthorizedException("User is disabled, please contact Terra support");
}
if (!userAllowanceDetails.getTermsOfService()) {
throw new UnauthorizedException("User has not accepted the terms of service");
}
}
return new SamUser(
userStatusInfo.getUserEmail(), userStatusInfo.getUserSubjectId(), bearerToken);
return new SamUser(samUserResponse.getEmail(), samUserResponse.getId(), bearerToken);
} catch (final NullPointerException e) {
throw new UnauthorizedException(e.getMessage(), e);
} catch (final ApiException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import bio.terra.common.sam.SamRetry;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import java.util.regex.Pattern;
import org.apache.commons.lang3.StringUtils;
import org.apache.http.HttpStatus;
import org.broadinstitute.dsde.workbench.client.sam.ApiException;
Expand All @@ -27,7 +28,14 @@ public static ErrorReportException create(ApiException apiException) {
public static ErrorReportException create(String messagePrefix, ApiException apiException) {
// Sometimes the sam message is buried one level down inside of the error report object.
// If we find an empty message then we try to deserialize the error report and use that message.
String message = apiException.getMessage();

// The autogenerated client now formats its ApiException message instead of returning the raw
// message.
// We want to extract the actual message from that.
var messagePattern = Pattern.compile("^Message: ([\\S\\s]*)\\nHTTP response code:");
var messageMatcher = messagePattern.matcher(apiException.getMessage());
messageMatcher.find();
String message = messageMatcher.group(1);
if (StringUtils.isEmpty(message)) {
try {
ErrorReport errorReport =
Expand Down
31 changes: 17 additions & 14 deletions src/test/java/bio/terra/common/iam/SamUserFactoryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
import static org.mockito.Mockito.*;

import bio.terra.common.exception.UnauthorizedException;
import java.time.OffsetDateTime;
import java.util.Optional;
import java.util.UUID;
import org.broadinstitute.dsde.workbench.client.sam.ApiException;
import org.broadinstitute.dsde.workbench.client.sam.api.UsersApi;
import org.broadinstitute.dsde.workbench.client.sam.model.UserStatusInfo;
import org.broadinstitute.dsde.workbench.client.sam.model.SamUserResponse;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.springframework.http.HttpStatus;
Expand All @@ -17,19 +19,25 @@ public class SamUserFactoryTest {

private static final SamUser SAM_USER =
new SamUser("[email protected]", "Subject", new BearerToken("0123.456-789AbCd"));

private static final SamUserResponse SAM_USER_RESPONSE =
new SamUserResponse()
.id(SAM_USER.getSubjectId())
.email(SAM_USER.getEmail())
.allowed(true)
.azureB2CId(UUID.randomUUID().toString())
.googleSubjectId("12345678")
.createdAt(OffsetDateTime.now())
.updatedAt(OffsetDateTime.now())
.registeredAt(OffsetDateTime.now());
private static final String SAM_BASE_PATH = "not_real";

@Test
public void enabledUser() throws ApiException {
SamUserFactory factory = spy(new SamUserFactory(new BearerTokenFactory(), Optional.empty()));
UsersApi usersApi = mock(UsersApi.class);
when(factory.createUsersApi(SAM_USER.getBearerToken(), SAM_BASE_PATH)).thenReturn(usersApi);
when(usersApi.getUserStatusInfo())
.thenReturn(
new UserStatusInfo()
.userEmail(SAM_USER.getEmail())
.userSubjectId(SAM_USER.getSubjectId())
.enabled(true));
when(usersApi.getSamUserSelf()).thenReturn(SAM_USER_RESPONSE);

SamUser outReq = factory.from(SAM_USER.getBearerToken(), SAM_BASE_PATH);
assertEquals(SAM_USER, outReq);
Expand All @@ -40,12 +48,7 @@ public void disabledUser() throws ApiException {
SamUserFactory factory = spy(new SamUserFactory(new BearerTokenFactory(), Optional.empty()));
UsersApi usersApi = mock(UsersApi.class);
when(factory.createUsersApi(SAM_USER.getBearerToken(), SAM_BASE_PATH)).thenReturn(usersApi);
when(usersApi.getUserStatusInfo())
.thenReturn(
new UserStatusInfo()
.userEmail(SAM_USER.getEmail())
.userSubjectId(SAM_USER.getSubjectId())
.enabled(false));
when(usersApi.getSamUserSelf()).thenReturn(SAM_USER_RESPONSE.allowed(false));

assertThrows(
UnauthorizedException.class, () -> factory.from(SAM_USER.getBearerToken(), SAM_BASE_PATH));
Expand All @@ -56,7 +59,7 @@ public void notFoundUser() throws ApiException {
SamUserFactory factory = spy(new SamUserFactory(new BearerTokenFactory(), Optional.empty()));
UsersApi usersApi = mock(UsersApi.class);
when(factory.createUsersApi(SAM_USER.getBearerToken(), SAM_BASE_PATH)).thenReturn(usersApi);
when(usersApi.getUserStatusInfo())
when(usersApi.getSamUserSelf())
.thenThrow(new ApiException(HttpStatus.NOT_FOUND.value(), "not found"));

assertThrows(
Expand Down
Loading