From b0566302380d8460e16eaef843bf901cd6784877 Mon Sep 17 00:00:00 2001 From: rushtong Date: Wed, 17 Jan 2024 11:34:16 -0500 Subject: [PATCH 1/7] feat: cache request headers and make available to authenticator --- .../consent/http/ConsentApplication.java | 2 ++ .../authentication/OAuthAuthenticator.java | 5 +++ .../http/filters/RequestHeaderCache.java | 36 +++++++++++++++++++ .../filters/RequestHeaderCacheFilter.java | 25 +++++++++++++ 4 files changed, 68 insertions(+) create mode 100644 src/main/java/org/broadinstitute/consent/http/filters/RequestHeaderCache.java create mode 100644 src/main/java/org/broadinstitute/consent/http/filters/RequestHeaderCacheFilter.java diff --git a/src/main/java/org/broadinstitute/consent/http/ConsentApplication.java b/src/main/java/org/broadinstitute/consent/http/ConsentApplication.java index 63a4a83515..3dfba8e52e 100644 --- a/src/main/java/org/broadinstitute/consent/http/ConsentApplication.java +++ b/src/main/java/org/broadinstitute/consent/http/ConsentApplication.java @@ -40,6 +40,7 @@ import org.broadinstitute.consent.http.cloudstore.GCSService; import org.broadinstitute.consent.http.configurations.ConsentConfiguration; import org.broadinstitute.consent.http.db.UserRoleDAO; +import org.broadinstitute.consent.http.filters.RequestHeaderCacheFilter; import org.broadinstitute.consent.http.filters.ResponseServerFilter; import org.broadinstitute.consent.http.health.ElasticSearchHealthCheck; import org.broadinstitute.consent.http.health.GCSHealthCheck; @@ -262,6 +263,7 @@ public void run(ConsentConfiguration config, Environment env) { List filters = List.of( defaultAuthFilter, new OAuthCustomAuthFilter(authenticator, userRoleDAO)); + env.jersey().register(RequestHeaderCacheFilter.class); env.jersey().register(new AuthDynamicFeature(new ChainedAuthFilter(filters))); env.jersey().register(RolesAllowedDynamicFeature.class); env.jersey().register(new AuthValueFactoryProvider.Binder<>(AuthUser.class)); diff --git a/src/main/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticator.java b/src/main/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticator.java index 9075b7cfa6..5cecb7e667 100644 --- a/src/main/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticator.java +++ b/src/main/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticator.java @@ -9,6 +9,7 @@ import jakarta.ws.rs.core.Response; import java.util.Objects; import java.util.Optional; +import org.broadinstitute.consent.http.filters.RequestHeaderCache; import org.broadinstitute.consent.http.models.AuthUser; import org.broadinstitute.consent.http.models.sam.UserStatus; import org.broadinstitute.consent.http.models.sam.UserStatusInfo; @@ -21,16 +22,20 @@ public class OAuthAuthenticator implements Authenticator, Cons private static final String USER_INFO_URL = "https://www.googleapis.com/oauth2/v3/userinfo?access_token="; private final Client client; private final SamService samService; + private final RequestHeaderCache requestHeaderCache; @Inject public OAuthAuthenticator(Client client, SamService samService) { this.client = client; this.samService = samService; + this.requestHeaderCache = RequestHeaderCache.getInstance(); } @Override public Optional authenticate(String bearer) { try { + // TODO: Populate AuthUser from header values + var headers = requestHeaderCache.cache.getIfPresent(bearer); GenericUser genericUser = getUserProfileInfo(bearer); AuthUser user = Objects.nonNull(genericUser) ? new AuthUser(genericUser).setAuthToken(bearer) : diff --git a/src/main/java/org/broadinstitute/consent/http/filters/RequestHeaderCache.java b/src/main/java/org/broadinstitute/consent/http/filters/RequestHeaderCache.java new file mode 100644 index 0000000000..e25c6fe4db --- /dev/null +++ b/src/main/java/org/broadinstitute/consent/http/filters/RequestHeaderCache.java @@ -0,0 +1,36 @@ +package org.broadinstitute.consent.http.filters; + +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; +import jakarta.inject.Singleton; +import jakarta.ws.rs.core.MultivaluedMap; +import java.util.concurrent.TimeUnit; + +@Singleton +public class RequestHeaderCache { + + private static RequestHeaderCache INSTANCE; + + private RequestHeaderCache() { + } + + public static RequestHeaderCache getInstance() { + if (INSTANCE == null) { + INSTANCE = new RequestHeaderCache(); + } + return INSTANCE; + } + + public Cache> cache = CacheBuilder + .newBuilder() + .expireAfterWrite(5, TimeUnit.MINUTES) + .build(); + + public void loadCache(String token, MultivaluedMap headers) { + if (this.cache.getIfPresent(token) == null) { + this.cache.put(token, headers); + this.cache.cleanUp(); + } + } + +} diff --git a/src/main/java/org/broadinstitute/consent/http/filters/RequestHeaderCacheFilter.java b/src/main/java/org/broadinstitute/consent/http/filters/RequestHeaderCacheFilter.java new file mode 100644 index 0000000000..cf0c703b66 --- /dev/null +++ b/src/main/java/org/broadinstitute/consent/http/filters/RequestHeaderCacheFilter.java @@ -0,0 +1,25 @@ +package org.broadinstitute.consent.http.filters; + +import jakarta.annotation.Priority; +import jakarta.ws.rs.container.ContainerRequestContext; +import jakarta.ws.rs.container.ContainerRequestFilter; +import jakarta.ws.rs.core.HttpHeaders; +import jakarta.ws.rs.ext.Provider; +import java.io.IOException; + +@Provider +@Priority(Integer.MIN_VALUE) +public class RequestHeaderCacheFilter implements ContainerRequestFilter { + + private final RequestHeaderCache headerCache = RequestHeaderCache.getInstance(); + + @Override + public void filter(ContainerRequestContext containerRequestContext) throws IOException { + var headers = containerRequestContext.getHeaders(); + var token = headers.getFirst(HttpHeaders.AUTHORIZATION); + if (token != null) { + var bearer = token.replace("Bearer ", ""); + headerCache.loadCache(bearer, headers); + } + } +} From 87e3e33b7b4b6036be0c5d161e59c30b223b91c9 Mon Sep 17 00:00:00 2001 From: rushtong Date: Wed, 17 Jan 2024 15:53:33 -0500 Subject: [PATCH 2/7] feat: populate auth user with headers --- .../consent/http/ConsentModule.java | 2 +- .../http/authentication/GenericUser.java | 129 ------------------ .../authentication/OAuthAuthenticator.java | 61 ++++----- .../http/filters/RequestHeaderCache.java | 18 ++- .../consent/http/models/AuthUser.java | 27 ++-- .../consent/http/models/User.java | 6 - .../http/resources/DatasetResource.java | 4 +- .../consent/http/resources/UserResource.java | 23 ++-- .../OAuthAuthenticatorTest.java | 84 ++++++------ .../OAuthCustomAuthFilterTest.java | 8 +- .../http/resources/DatasetResourceTest.java | 16 --- .../http/resources/UserResourceTest.java | 8 +- .../http/service/ResearcherServiceTest.java | 18 ++- 13 files changed, 114 insertions(+), 290 deletions(-) delete mode 100644 src/main/java/org/broadinstitute/consent/http/authentication/GenericUser.java diff --git a/src/main/java/org/broadinstitute/consent/http/ConsentModule.java b/src/main/java/org/broadinstitute/consent/http/ConsentModule.java index e3c4a3fe7f..3bf37d11f4 100644 --- a/src/main/java/org/broadinstitute/consent/http/ConsentModule.java +++ b/src/main/java/org/broadinstitute/consent/http/ConsentModule.java @@ -197,7 +197,7 @@ OntologyService providesOntologyService() { @Provides OAuthAuthenticator providesOAuthAuthenticator() { - return new OAuthAuthenticator(providesClient(), providesSamService()); + return new OAuthAuthenticator(providesSamService()); } @Provides diff --git a/src/main/java/org/broadinstitute/consent/http/authentication/GenericUser.java b/src/main/java/org/broadinstitute/consent/http/authentication/GenericUser.java deleted file mode 100644 index 4f6a0375da..0000000000 --- a/src/main/java/org/broadinstitute/consent/http/authentication/GenericUser.java +++ /dev/null @@ -1,129 +0,0 @@ -package org.broadinstitute.consent.http.authentication; - -import com.google.gson.FieldNamingPolicy; -import com.google.gson.Gson; -import com.google.gson.GsonBuilder; - -public class GenericUser { - - private String sub; - private String name; - private String givenName; - private String familyName; - private String profile; - private String picture; - private String email; - private Boolean emailVerified; - private String locale; - private String hd; - - public GenericUser() { - } - - /** - * Convenience method to deserialize from Google's userinfo API, e.g.: { "sub": "...", "name": - * "Gregory Rushton", "given_name": "Gregory", "family_name": "Rushton", "profile": - * "https://plus.google.com/...", "picture": "https://lh3.googleusercontent.com/...", "email": - * "grushton@broadinstitute.org", "email_verified": true, "locale": "en", "hd": - * "broadinstitute.org" } - * - * @param json The JSON string to deserialize - */ - GenericUser(String json) { - Gson gson = new GsonBuilder() - .setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES) - .create(); - GenericUser u = gson.fromJson(json, GenericUser.class); - this.setSub(u.getSub()); - this.setName(u.getName()); - this.setGivenName(u.getGivenName()); - this.setFamilyName(u.getFamilyName()); - this.setProfile(u.getProfile()); - this.setPicture(u.getPicture()); - this.setEmail(u.getEmail()); - this.setEmailVerified(u.getEmailVerified()); - this.setLocale(u.getLocale()); - this.setHd(u.getHd()); - } - - private String getSub() { - return sub; - } - - private void setSub(String sub) { - this.sub = sub; - } - - public String getName() { - return name; - } - - public void setName(String name) { - this.name = name; - } - - private String getGivenName() { - return givenName; - } - - private void setGivenName(String givenName) { - this.givenName = givenName; - } - - private String getFamilyName() { - return familyName; - } - - private void setFamilyName(String familyName) { - this.familyName = familyName; - } - - private String getProfile() { - return profile; - } - - private void setProfile(String profile) { - this.profile = profile; - } - - private String getPicture() { - return picture; - } - - private void setPicture(String picture) { - this.picture = picture; - } - - public String getEmail() { - return email; - } - - public void setEmail(String email) { - this.email = email; - } - - private Boolean getEmailVerified() { - return emailVerified; - } - - private void setEmailVerified(Boolean emailVerified) { - this.emailVerified = emailVerified; - } - - private String getLocale() { - return locale; - } - - private void setLocale(String locale) { - this.locale = locale; - } - - private String getHd() { - return hd; - } - - private void setHd(String hd) { - this.hd = hd; - } - -} diff --git a/src/main/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticator.java b/src/main/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticator.java index 5cecb7e667..6bf0c1675d 100644 --- a/src/main/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticator.java +++ b/src/main/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticator.java @@ -4,9 +4,8 @@ import com.google.inject.Inject; import io.dropwizard.auth.Authenticator; import jakarta.ws.rs.NotFoundException; -import jakarta.ws.rs.client.Client; -import jakarta.ws.rs.core.MediaType; -import jakarta.ws.rs.core.Response; +import jakarta.ws.rs.ServerErrorException; +import jakarta.ws.rs.core.MultivaluedMap; import java.util.Objects; import java.util.Optional; import org.broadinstitute.consent.http.filters.RequestHeaderCache; @@ -19,14 +18,11 @@ public class OAuthAuthenticator implements Authenticator, ConsentLogger { - private static final String USER_INFO_URL = "https://www.googleapis.com/oauth2/v3/userinfo?access_token="; - private final Client client; private final SamService samService; private final RequestHeaderCache requestHeaderCache; @Inject - public OAuthAuthenticator(Client client, SamService samService) { - this.client = client; + public OAuthAuthenticator(SamService samService) { this.samService = samService; this.requestHeaderCache = RequestHeaderCache.getInstance(); } @@ -36,18 +32,35 @@ public Optional authenticate(String bearer) { try { // TODO: Populate AuthUser from header values var headers = requestHeaderCache.cache.getIfPresent(bearer); - GenericUser genericUser = getUserProfileInfo(bearer); - AuthUser user = Objects.nonNull(genericUser) ? - new AuthUser(genericUser).setAuthToken(bearer) : - new AuthUser().setAuthToken(bearer); - AuthUser userWithStatus = getUserWithStatusInfo(user); - return Optional.of(userWithStatus); + if (headers != null) { + AuthUser user = buildAuthUserFromHeaders(headers); + AuthUser userWithStatus = getUserWithStatusInfo(user); + return Optional.of(userWithStatus); + } + logException(new ServerErrorException("Error reading request headers", 500)); + return Optional.empty(); } catch (Exception e) { logException("Error authenticating credentials", e); return Optional.empty(); } } + private AuthUser buildAuthUserFromHeaders(MultivaluedMap headers) { + String aud = headers.getFirst(RequestHeaderCache.OAUTH2_CLAIM_aud); + String token = headers.getFirst(RequestHeaderCache.OAUTH2_CLAIM_access_token); + String email = headers.getFirst(RequestHeaderCache.OAUTH2_CLAIM_email); + String name = headers.getFirst(RequestHeaderCache.OAUTH2_CLAIM_name); + // Name is not a guaranteed header + if (name == null) { + name = email; + } + return new AuthUser() + .setAud(aud) + .setAuthToken(token) + .setEmail(email) + .setName(name); + } + /** * Attempt to get the registration status of the current user and set the value on AuthUser * @@ -89,26 +102,4 @@ private AuthUser getUserWithStatusInfo(AuthUser authUser) { return authUser; } - /** - * This method is currently google-centric. When we fully support B2C authentication, we should - * ensure that we can look up user info from a MS service. - * - * @param bearer Bearer Token - * @return GenericUser - */ - private GenericUser getUserProfileInfo(String bearer) { - GenericUser u = null; - try { - Response response = this.client. - target(USER_INFO_URL + bearer). - request(MediaType.APPLICATION_JSON_TYPE). - get(Response.class); - String result = response.readEntity(String.class); - u = new GenericUser(result); - } catch (Exception e) { - logWarn("Error getting Google user info from token: " + e.getMessage()); - } - return u; - } - } diff --git a/src/main/java/org/broadinstitute/consent/http/filters/RequestHeaderCache.java b/src/main/java/org/broadinstitute/consent/http/filters/RequestHeaderCache.java index e25c6fe4db..db886a6df1 100644 --- a/src/main/java/org/broadinstitute/consent/http/filters/RequestHeaderCache.java +++ b/src/main/java/org/broadinstitute/consent/http/filters/RequestHeaderCache.java @@ -6,12 +6,25 @@ import jakarta.ws.rs.core.MultivaluedMap; import java.util.concurrent.TimeUnit; +/** + * Manage a cache of bearer token to map of headers for every request. This is useful in cases + * where components need, but do not have, access to the complete request context. + */ @Singleton public class RequestHeaderCache { private static RequestHeaderCache INSTANCE; + public final Cache> cache; + public final static String OAUTH2_CLAIM_email = "OAUTH2_CLAIM_email"; + public final static String OAUTH2_CLAIM_name = "OAUTH2_CLAIM_name"; + public final static String OAUTH2_CLAIM_access_token = "OAUTH2_CLAIM_access_token"; + public final static String OAUTH2_CLAIM_aud = "OAUTH2_CLAIM_aud"; private RequestHeaderCache() { + cache = CacheBuilder + .newBuilder() + .expireAfterWrite(5, TimeUnit.MINUTES) + .build(); } public static RequestHeaderCache getInstance() { @@ -21,11 +34,6 @@ public static RequestHeaderCache getInstance() { return INSTANCE; } - public Cache> cache = CacheBuilder - .newBuilder() - .expireAfterWrite(5, TimeUnit.MINUTES) - .build(); - public void loadCache(String token, MultivaluedMap headers) { if (this.cache.getIfPresent(token) == null) { this.cache.put(token, headers); diff --git a/src/main/java/org/broadinstitute/consent/http/models/AuthUser.java b/src/main/java/org/broadinstitute/consent/http/models/AuthUser.java index aa7be0babd..abbdf334d8 100644 --- a/src/main/java/org/broadinstitute/consent/http/models/AuthUser.java +++ b/src/main/java/org/broadinstitute/consent/http/models/AuthUser.java @@ -2,16 +2,14 @@ import com.google.gson.Gson; import java.security.Principal; -import java.util.Objects; -import org.broadinstitute.consent.http.authentication.GenericUser; import org.broadinstitute.consent.http.models.sam.UserStatusInfo; public class AuthUser implements Principal { private String authToken; private String email; - private GenericUser genericUser; private String name; + private String aud; private UserStatusInfo userStatusInfo; public AuthUser() { @@ -26,16 +24,6 @@ public AuthUser deepCopy() { return gson.fromJson(gson.toJson(this), AuthUser.class); } - public AuthUser(GenericUser genericUser) { - if (Objects.nonNull(genericUser) && Objects.nonNull(genericUser.getName())) { - this.name = genericUser.getName(); - } - if (Objects.nonNull(genericUser) && Objects.nonNull(genericUser.getEmail())) { - this.email = genericUser.getEmail(); - } - this.genericUser = genericUser; - } - public String getAuthToken() { return authToken; } @@ -54,10 +42,6 @@ public AuthUser setEmail(String email) { return this; } - public GenericUser getGenericUser() { - return genericUser; - } - @Override public String getName() { return name; @@ -68,6 +52,15 @@ public AuthUser setName(String name) { return this; } + public String getAud() { + return aud; + } + + public AuthUser setAud(String aud) { + this.aud = aud; + return this; + } + public UserStatusInfo getUserStatusInfo() { return userStatusInfo; } diff --git a/src/main/java/org/broadinstitute/consent/http/models/User.java b/src/main/java/org/broadinstitute/consent/http/models/User.java index b1f0a0a792..d846c1b721 100644 --- a/src/main/java/org/broadinstitute/consent/http/models/User.java +++ b/src/main/java/org/broadinstitute/consent/http/models/User.java @@ -15,7 +15,6 @@ import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.builder.EqualsBuilder; -import org.broadinstitute.consent.http.authentication.GenericUser; import org.broadinstitute.consent.http.enumeration.UserRoles; public class User { @@ -87,11 +86,6 @@ public User(Integer userId, String email, String displayName, Date createDate, this.roles = roles; } - public User(GenericUser genericUser) { - this.displayName = genericUser.getName(); - this.email = genericUser.getEmail(); - } - /** * Convenience method for backwards compatibility support for older clients. This method is * intended to reconstruct a User object from a supplied JSON string for primary fields and roles. diff --git a/src/main/java/org/broadinstitute/consent/http/resources/DatasetResource.java b/src/main/java/org/broadinstitute/consent/http/resources/DatasetResource.java index 38121be2c2..7a6d0bd45b 100644 --- a/src/main/java/org/broadinstitute/consent/http/resources/DatasetResource.java +++ b/src/main/java/org/broadinstitute/consent/http/resources/DatasetResource.java @@ -133,7 +133,7 @@ public Response createDataset(@Auth AuthUser authUser, @Context UriInfo info, St throw new ClientErrorException("Dataset name: " + name + " is already in use", Status.CONFLICT); } - User dacUser = userService.findUserByEmail(authUser.getGenericUser().getEmail()); + User dacUser = userService.findUserByEmail(authUser.getEmail()); Integer userId = dacUser.getUserId(); try { DatasetDTO createdDatasetWithConsent = datasetService.createDatasetFromDatasetDTO( @@ -291,7 +291,7 @@ public Response updateDataset(@Auth AuthUser authUser, @Context UriInfo info, if (duplicateProperties.size() > 0) { throw new BadRequestException("Dataset contains multiple values for the same property."); } - User user = userService.findUserByEmail(authUser.getGenericUser().getEmail()); + User user = userService.findUserByEmail(authUser.getEmail()); // Validate that the admin/chairperson has edit access to this dataset validateDatasetDacAccess(user, datasetExists); Integer userId = user.getUserId(); diff --git a/src/main/java/org/broadinstitute/consent/http/resources/UserResource.java b/src/main/java/org/broadinstitute/consent/http/resources/UserResource.java index d0cb57eac3..d1a1d2db09 100644 --- a/src/main/java/org/broadinstitute/consent/http/resources/UserResource.java +++ b/src/main/java/org/broadinstitute/consent/http/resources/UserResource.java @@ -35,7 +35,6 @@ import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; -import org.broadinstitute.consent.http.authentication.GenericUser; import org.broadinstitute.consent.http.enumeration.UserRoles; import org.broadinstitute.consent.http.models.Acknowledgement; import org.broadinstitute.consent.http.models.ApprovedDataset; @@ -387,17 +386,15 @@ private Response doDelete(AuthUser authUser, Integer userId, Integer roleId, Use @Consumes("application/json") @Produces("application/json") @PermitAll - public Response createResearcher(@Context UriInfo info, @Auth AuthUser user) { - GenericUser genericUser = user.getGenericUser(); - if (genericUser == null || genericUser.getEmail() == null || genericUser.getName() == null) { + public Response createResearcher(@Context UriInfo info, @Auth AuthUser authUser) { + if (authUser == null || authUser.getEmail() == null || authUser.getName() == null) { return Response. status(Response.Status.BAD_REQUEST). entity(new Error("Unable to verify google identity", Response.Status.BAD_REQUEST.getStatusCode())). build(); - } - try { - if (userService.findUserByEmail(genericUser.getEmail()) != null) { + } try { + if (userService.findUserByEmail(authUser.getEmail()) != null) { return Response. status(Response.Status.CONFLICT). entity(new Error("Registered user exists", Response.Status.CONFLICT.getStatusCode())). @@ -406,15 +403,17 @@ public Response createResearcher(@Context UriInfo info, @Auth AuthUser user) { } catch (NotFoundException nfe) { // no-op, we expect to not find the new user in this case. } - User dacUser = new User(genericUser); + User user = new User(); + user.setEmail(authUser.getEmail()); + user.setDisplayName(authUser.getName()); UserRole researcher = new UserRole(UserRoles.RESEARCHER.getRoleId(), UserRoles.RESEARCHER.getRoleName()); - dacUser.setRoles(Collections.singletonList(researcher)); + user.setRoles(Collections.singletonList(researcher)); try { URI uri; - dacUser = userService.createUser(dacUser); - uri = info.getRequestUriBuilder().path("{email}").build(dacUser.getEmail()); - return Response.created(new URI(uri.toString().replace("user", "dacuser"))).entity(dacUser) + user = userService.createUser(user); + uri = info.getRequestUriBuilder().path("{email}").build(user.getEmail()); + return Response.created(new URI(uri.toString().replace("user", "dacuser"))).entity(user) .build(); } catch (Exception e) { return Response.status(Response.Status.INTERNAL_SERVER_ERROR) diff --git a/src/test/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticatorTest.java b/src/test/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticatorTest.java index 45bf05f731..9bc9ded4ba 100644 --- a/src/test/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticatorTest.java +++ b/src/test/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticatorTest.java @@ -1,23 +1,23 @@ package org.broadinstitute.consent.http.authentication; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import com.google.gson.Gson; import jakarta.ws.rs.NotFoundException; -import jakarta.ws.rs.client.Client; -import jakarta.ws.rs.client.Invocation; -import jakarta.ws.rs.client.WebTarget; -import jakarta.ws.rs.core.MediaType; -import jakarta.ws.rs.core.Response; +import jakarta.ws.rs.core.MultivaluedHashMap; +import jakarta.ws.rs.core.MultivaluedMap; +import java.util.List; import java.util.Optional; +import org.apache.commons.lang3.RandomStringUtils; +import org.broadinstitute.consent.http.filters.RequestHeaderCache; import org.broadinstitute.consent.http.models.AuthUser; import org.broadinstitute.consent.http.service.sam.SamService; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; @@ -26,61 +26,52 @@ @ExtendWith(MockitoExtension.class) class OAuthAuthenticatorTest { - @Mock - private Client client; - @Mock private SamService samService; - - @Mock - private WebTarget target; - - @Mock - private Invocation.Builder builder; - - @Mock - private Response response; - private OAuthAuthenticator oAuthAuthenticator; + private final RequestHeaderCache headerCache = RequestHeaderCache.getInstance(); + + @BeforeEach + void setUp() { + headerCache.cache.invalidateAll(); + } @Test void testAuthenticateWithToken() { - oAuthAuthenticator = new OAuthAuthenticator(client, samService); - Optional authUser = oAuthAuthenticator.authenticate("bearer-token"); + String bearerToken = RandomStringUtils.randomAlphabetic(100); + MultivaluedMap headerMap = new MultivaluedHashMap<>(); + headerMap.put(RequestHeaderCache.OAUTH2_CLAIM_email, List.of("email")); + headerCache.loadCache(bearerToken, headerMap); + oAuthAuthenticator = new OAuthAuthenticator(samService); + Optional authUser = oAuthAuthenticator.authenticate(bearerToken); assertTrue(authUser.isPresent()); } @Test void testAuthenticateGetUserInfoSuccess() { - String bearerToken = "bearer-token"; - Gson gson = new Gson(); - GenericUser user = new GenericUser(); - user.setEmail("email"); - user.setName("name"); - when(client.target(anyString())).thenReturn(target); - when(target.request(MediaType.APPLICATION_JSON_TYPE)).thenReturn(builder); - when(builder.get(Response.class)).thenReturn(response); - when(response.readEntity(String.class)).thenReturn(gson.toJson(user)); - oAuthAuthenticator = new OAuthAuthenticator(client, samService); - + String bearerToken = RandomStringUtils.randomAlphabetic(100); + MultivaluedMap headerMap = new MultivaluedHashMap<>(); + headerMap.put(RequestHeaderCache.OAUTH2_CLAIM_access_token, List.of(bearerToken)); + headerMap.put(RequestHeaderCache.OAUTH2_CLAIM_email, List.of("email")); + headerMap.put(RequestHeaderCache.OAUTH2_CLAIM_name, List.of("name")); + headerCache.loadCache(bearerToken, headerMap); + oAuthAuthenticator = new OAuthAuthenticator(samService); Optional authUser = oAuthAuthenticator.authenticate(bearerToken); assertTrue(authUser.isPresent()); - assertEquals(user.getEmail(), authUser.get().getEmail()); - assertEquals(user.getName(), authUser.get().getName()); - assertEquals(authUser.get().getAuthToken(), bearerToken); + assertNotNull(authUser.get().getEmail()); + assertNotNull(authUser.get().getAuthToken()); } /** - * Test that in the case of a token lookup failure, we don't fail the overall request. + * Test that in the case of a header lookup failure, we don't fail the overall request. */ @Test void testAuthenticateGetUserInfoFailure() { - String bearerToken = "bearer-token"; - when(client.target(anyString())).thenReturn(target); - when(target.request(MediaType.APPLICATION_JSON_TYPE)).thenReturn(builder); - when(builder.get(Response.class)).thenReturn(response); - when(response.readEntity(String.class)).thenReturn("Bad Request"); - oAuthAuthenticator = new OAuthAuthenticator(client, samService); + String bearerToken = RandomStringUtils.randomAlphabetic(100); + MultivaluedMap headerMap = new MultivaluedHashMap<>(); + headerMap.put(RequestHeaderCache.OAUTH2_CLAIM_access_token, List.of(bearerToken)); + headerCache.loadCache(bearerToken, headerMap); + oAuthAuthenticator = new OAuthAuthenticator(samService); Optional authUser = oAuthAuthenticator.authenticate(bearerToken); assertTrue(authUser.isPresent()); @@ -92,9 +83,12 @@ void testAuthenticateGetUserInfoFailure() { */ @Test void testAuthenticateGetUserWithStatusInfoFailurePostUserSuccess() throws Exception { - String bearerToken = "bearer-token"; + String bearerToken = RandomStringUtils.randomAlphabetic(100); + MultivaluedMap headerMap = new MultivaluedHashMap<>(); + headerMap.put(RequestHeaderCache.OAUTH2_CLAIM_access_token, List.of(bearerToken)); + headerCache.loadCache(bearerToken, headerMap); when(samService.getRegistrationInfo(any())).thenThrow(new NotFoundException()); - oAuthAuthenticator = new OAuthAuthenticator(client, samService); + oAuthAuthenticator = new OAuthAuthenticator(samService); Optional authUser = oAuthAuthenticator.authenticate(bearerToken); assertTrue(authUser.isPresent()); diff --git a/src/test/java/org/broadinstitute/consent/http/authentication/OAuthCustomAuthFilterTest.java b/src/test/java/org/broadinstitute/consent/http/authentication/OAuthCustomAuthFilterTest.java index 0759791b9e..0401076fa5 100644 --- a/src/test/java/org/broadinstitute/consent/http/authentication/OAuthCustomAuthFilterTest.java +++ b/src/test/java/org/broadinstitute/consent/http/authentication/OAuthCustomAuthFilterTest.java @@ -40,9 +40,6 @@ class OAuthCustomAuthFilterTest { @Mock private AuthUser user; - @Mock - private GenericUser genericUser; - private final String token = "0cx2G9gKm4XZdK8BFxoWy7AE025tvq"; @BeforeEach @@ -52,10 +49,7 @@ void setUp() { when(headers.getFirst("Authorization")).thenReturn("Bearer %s".formatted(token)); when(authenticator.authenticate(notNull())).thenReturn(Optional.of(user)); filter = Mockito.spy(new OAuthCustomAuthFilter<>(authenticator, userRoleDAO)); - genericUser = new GenericUser(); - genericUser.setName("Test User"); - genericUser.setEmail("test@gmail.com"); - user = new AuthUser(genericUser); + user = new AuthUser().setName("Test User").setEmail("test@gmail.com"); } @Test diff --git a/src/test/java/org/broadinstitute/consent/http/resources/DatasetResourceTest.java b/src/test/java/org/broadinstitute/consent/http/resources/DatasetResourceTest.java index 981e0b8118..0b9c062992 100644 --- a/src/test/java/org/broadinstitute/consent/http/resources/DatasetResourceTest.java +++ b/src/test/java/org/broadinstitute/consent/http/resources/DatasetResourceTest.java @@ -34,7 +34,6 @@ import java.util.Set; import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.RandomUtils; -import org.broadinstitute.consent.http.authentication.GenericUser; import org.broadinstitute.consent.http.enumeration.PropertyType; import org.broadinstitute.consent.http.enumeration.UserRoles; import org.broadinstitute.consent.http.models.AuthUser; @@ -84,9 +83,6 @@ class DatasetResourceTest { @Mock private AuthUser authUser; - @Mock - private GenericUser genericUser; - @Mock private User user; @@ -137,8 +133,6 @@ void testCreateDatasetSuccess() throws Exception { when(datasetService.getDatasetByName("test")).thenReturn(null); when(datasetService.createDatasetFromDatasetDTO(any(), any(), anyInt())).thenReturn(result); when(datasetService.getDatasetDTO(any())).thenReturn(result); - when(authUser.getGenericUser()).thenReturn(genericUser); - when(genericUser.getEmail()).thenReturn("email@email.com"); when(userService.findUserByEmail(any())).thenReturn(user); when(user.getUserId()).thenReturn(1); when(uriInfo.getRequestUriBuilder()).thenReturn(uriBuilder); @@ -249,8 +243,6 @@ void testCreateDatasetError() throws Exception { when(datasetService.getDatasetByName("test")).thenReturn(null); doThrow(new RuntimeException()).when(datasetService) .createDatasetFromDatasetDTO(any(), any(), anyInt()); - when(authUser.getGenericUser()).thenReturn(genericUser); - when(genericUser.getEmail()).thenReturn("email@email.com"); when(userService.findUserByEmail(any())).thenReturn(user); when(user.getUserId()).thenReturn(1); initResource(); @@ -266,8 +258,6 @@ void testUpdateDatasetSuccess() { when(datasetService.findDatasetById(anyInt())).thenReturn(preexistingDataset); when(datasetService.updateDataset(any(), any(), any())).thenReturn( Optional.of(preexistingDataset)); - when(authUser.getGenericUser()).thenReturn(genericUser); - when(genericUser.getEmail()).thenReturn("email@email.com"); when(userService.findUserByEmail(any())).thenReturn(user); when(user.getUserId()).thenReturn(1); when(user.hasUserRole(any())).thenReturn(true); @@ -340,8 +330,6 @@ void testUpdateDatasetNoContent() { String json = createPropertiesJson("Dataset Name", "test"); when(datasetService.findDatasetById(anyInt())).thenReturn(preexistingDataset); when(datasetService.updateDataset(any(), any(), any())).thenReturn(Optional.empty()); - when(authUser.getGenericUser()).thenReturn(genericUser); - when(genericUser.getEmail()).thenReturn("email@email.com"); when(userService.findUserByEmail(any())).thenReturn(user); when(user.getUserId()).thenReturn(1); when(user.hasUserRole(any())).thenReturn(true); @@ -1010,8 +998,6 @@ void testupdateDatasetByDatasetIntakeSuccess() throws SQLException, IOException when(datasetService.findDatasetById(anyInt())).thenReturn(preexistingDataset); when(datasetRegistrationService.updateDataset(any(), any(), any(), any())).thenReturn( preexistingDataset); - when(authUser.getGenericUser()).thenReturn(genericUser); - when(genericUser.getEmail()).thenReturn("email@email.com"); when(userService.findUserByEmail(any())).thenReturn(user); when(user.getUserId()).thenReturn(1); when(user.hasUserRole(any())).thenReturn(true); @@ -1119,8 +1105,6 @@ void testUpdateDatasetInvalidFileName() throws SQLException, IOException { when(datasetService.findDatasetById(anyInt())).thenReturn(preexistingDataset); when(datasetRegistrationService.updateDataset(any(), any(), any(), any())).thenReturn( preexistingDataset); - when(authUser.getGenericUser()).thenReturn(genericUser); - when(genericUser.getEmail()).thenReturn("email@email.com"); when(userService.findUserByEmail(any())).thenReturn(user); when(user.getUserId()).thenReturn(1); when(user.hasUserRole(any())).thenReturn(true); diff --git a/src/test/java/org/broadinstitute/consent/http/resources/UserResourceTest.java b/src/test/java/org/broadinstitute/consent/http/resources/UserResourceTest.java index ed77b7c546..e163bfdbc6 100644 --- a/src/test/java/org/broadinstitute/consent/http/resources/UserResourceTest.java +++ b/src/test/java/org/broadinstitute/consent/http/resources/UserResourceTest.java @@ -38,7 +38,6 @@ import java.util.stream.Collectors; import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.RandomUtils; -import org.broadinstitute.consent.http.authentication.GenericUser; import org.broadinstitute.consent.http.enumeration.UserFields; import org.broadinstitute.consent.http.enumeration.UserRoles; import org.broadinstitute.consent.http.models.Acknowledgement; @@ -98,11 +97,10 @@ public class UserResourceTest { @BeforeEach public void setUp() throws URISyntaxException { - GenericUser genericUser = new GenericUser(); - genericUser.setName("Test User"); - genericUser.setEmail(TEST_EMAIL); - authUser = new AuthUser(genericUser) + authUser = new AuthUser() .setAuthToken("auth-token") + .setName("Test User") + .setEmail(TEST_EMAIL) .setUserStatusInfo(userStatusInfo); openMocks(this); when(uriInfo.getRequestUriBuilder()).thenReturn(uriBuilder); diff --git a/src/test/java/org/broadinstitute/consent/http/service/ResearcherServiceTest.java b/src/test/java/org/broadinstitute/consent/http/service/ResearcherServiceTest.java index 73a4ee55c6..07eeb866c7 100644 --- a/src/test/java/org/broadinstitute/consent/http/service/ResearcherServiceTest.java +++ b/src/test/java/org/broadinstitute/consent/http/service/ResearcherServiceTest.java @@ -12,7 +12,6 @@ import java.util.Map; import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.RandomUtils; -import org.broadinstitute.consent.http.authentication.GenericUser; import org.broadinstitute.consent.http.db.UserDAO; import org.broadinstitute.consent.http.db.UserPropertyDAO; import org.broadinstitute.consent.http.enumeration.UserFields; @@ -21,9 +20,13 @@ import org.broadinstitute.consent.http.models.UserProperty; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.mockito.junit.jupiter.MockitoExtension; -public class ResearcherServiceTest { +@ExtendWith(MockitoExtension.class) +class ResearcherServiceTest { @Mock private UserPropertyDAO userPropertyDAO; @@ -38,16 +41,12 @@ public class ResearcherServiceTest { private User user; @BeforeEach - public void setUp() { - GenericUser genericUser = new GenericUser(); - genericUser.setName("Test User"); - genericUser.setEmail("test@gmail.com"); - authUser = new AuthUser(genericUser); + void setUp() { + authUser = new AuthUser().setEmail("test@gmail.com").setName("Test User"); user = new User(); user.setEmail(authUser.getEmail()); user.setUserId(RandomUtils.nextInt(1, 10)); user.setDisplayName(RandomStringUtils.randomAlphabetic(10)); - openMocks(this); } private void initService() { @@ -55,12 +54,11 @@ private void initService() { } @Test - public void testUpdateProperties() { + void testUpdateProperties() { when(userDAO.findUserById(any())).thenReturn(user); when(userDAO.findUserByEmail(any())).thenReturn(user); when(userPropertyDAO.findUserPropertiesByUserIdAndPropertyKeys(any(), any())).thenReturn( List.of()); - doNothing().when(userPropertyDAO).deleteAllPropertiesByUser(anyInt()); doNothing().when(userPropertyDAO).insertAll(any()); initService(); Map props = new HashMap<>(); From 3e628c85497049c4e96beac41be58f514840cda0 Mon Sep 17 00:00:00 2001 From: rushtong Date: Thu, 18 Jan 2024 06:24:58 -0500 Subject: [PATCH 3/7] doc: rm comment --- .../consent/http/authentication/OAuthAuthenticator.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticator.java b/src/main/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticator.java index 6bf0c1675d..141d513853 100644 --- a/src/main/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticator.java +++ b/src/main/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticator.java @@ -30,7 +30,6 @@ public OAuthAuthenticator(SamService samService) { @Override public Optional authenticate(String bearer) { try { - // TODO: Populate AuthUser from header values var headers = requestHeaderCache.cache.getIfPresent(bearer); if (headers != null) { AuthUser user = buildAuthUserFromHeaders(headers); From 42b88b2eaa35aa136002d74c7323a11ad3267a29 Mon Sep 17 00:00:00 2001 From: rushtong Date: Thu, 18 Jan 2024 08:45:36 -0500 Subject: [PATCH 4/7] feat: simplify header map --- .../authentication/OAuthAuthenticator.java | 20 ++++++------ ...questHeaderCache.java => ClaimsCache.java} | 31 ++++++++++++++----- .../filters/RequestHeaderCacheFilter.java | 4 +-- .../OAuthAuthenticatorTest.java | 16 +++++----- 4 files changed, 44 insertions(+), 27 deletions(-) rename src/main/java/org/broadinstitute/consent/http/filters/{RequestHeaderCache.java => ClaimsCache.java} (55%) diff --git a/src/main/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticator.java b/src/main/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticator.java index 141d513853..1504168e76 100644 --- a/src/main/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticator.java +++ b/src/main/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticator.java @@ -5,10 +5,10 @@ import io.dropwizard.auth.Authenticator; import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.ServerErrorException; -import jakarta.ws.rs.core.MultivaluedMap; +import java.util.Map; import java.util.Objects; import java.util.Optional; -import org.broadinstitute.consent.http.filters.RequestHeaderCache; +import org.broadinstitute.consent.http.filters.ClaimsCache; import org.broadinstitute.consent.http.models.AuthUser; import org.broadinstitute.consent.http.models.sam.UserStatus; import org.broadinstitute.consent.http.models.sam.UserStatusInfo; @@ -19,18 +19,18 @@ public class OAuthAuthenticator implements Authenticator, ConsentLogger { private final SamService samService; - private final RequestHeaderCache requestHeaderCache; + private final ClaimsCache claimsCache; @Inject public OAuthAuthenticator(SamService samService) { this.samService = samService; - this.requestHeaderCache = RequestHeaderCache.getInstance(); + this.claimsCache = ClaimsCache.getInstance(); } @Override public Optional authenticate(String bearer) { try { - var headers = requestHeaderCache.cache.getIfPresent(bearer); + var headers = claimsCache.cache.getIfPresent(bearer); if (headers != null) { AuthUser user = buildAuthUserFromHeaders(headers); AuthUser userWithStatus = getUserWithStatusInfo(user); @@ -44,11 +44,11 @@ public Optional authenticate(String bearer) { } } - private AuthUser buildAuthUserFromHeaders(MultivaluedMap headers) { - String aud = headers.getFirst(RequestHeaderCache.OAUTH2_CLAIM_aud); - String token = headers.getFirst(RequestHeaderCache.OAUTH2_CLAIM_access_token); - String email = headers.getFirst(RequestHeaderCache.OAUTH2_CLAIM_email); - String name = headers.getFirst(RequestHeaderCache.OAUTH2_CLAIM_name); + private AuthUser buildAuthUserFromHeaders(Map headers) { + String aud = headers.get(ClaimsCache.OAUTH2_CLAIM_aud); + String token = headers.get(ClaimsCache.OAUTH2_CLAIM_access_token); + String email = headers.get(ClaimsCache.OAUTH2_CLAIM_email); + String name = headers.get(ClaimsCache.OAUTH2_CLAIM_name); // Name is not a guaranteed header if (name == null) { name = email; diff --git a/src/main/java/org/broadinstitute/consent/http/filters/RequestHeaderCache.java b/src/main/java/org/broadinstitute/consent/http/filters/ClaimsCache.java similarity index 55% rename from src/main/java/org/broadinstitute/consent/http/filters/RequestHeaderCache.java rename to src/main/java/org/broadinstitute/consent/http/filters/ClaimsCache.java index db886a6df1..0b3382c7c7 100644 --- a/src/main/java/org/broadinstitute/consent/http/filters/RequestHeaderCache.java +++ b/src/main/java/org/broadinstitute/consent/http/filters/ClaimsCache.java @@ -4,39 +4,56 @@ import com.google.common.cache.CacheBuilder; import jakarta.inject.Singleton; import jakarta.ws.rs.core.MultivaluedMap; +import java.util.AbstractMap; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; /** * Manage a cache of bearer token to map of headers for every request. This is useful in cases * where components need, but do not have, access to the complete request context. */ @Singleton -public class RequestHeaderCache { +public class ClaimsCache { - private static RequestHeaderCache INSTANCE; - public final Cache> cache; + private static ClaimsCache INSTANCE; + public final Cache> cache; public final static String OAUTH2_CLAIM_email = "OAUTH2_CLAIM_email"; public final static String OAUTH2_CLAIM_name = "OAUTH2_CLAIM_name"; public final static String OAUTH2_CLAIM_access_token = "OAUTH2_CLAIM_access_token"; public final static String OAUTH2_CLAIM_aud = "OAUTH2_CLAIM_aud"; - private RequestHeaderCache() { + private ClaimsCache() { cache = CacheBuilder .newBuilder() .expireAfterWrite(5, TimeUnit.MINUTES) .build(); } - public static RequestHeaderCache getInstance() { + public static ClaimsCache getInstance() { if (INSTANCE == null) { - INSTANCE = new RequestHeaderCache(); + INSTANCE = new ClaimsCache(); } return INSTANCE; } + private String getFirst(List headerValues) { + if (headerValues == null) { + return null; + } + return headerValues.stream().findFirst().orElse(null); + } + public void loadCache(String token, MultivaluedMap headers) { if (this.cache.getIfPresent(token) == null) { - this.cache.put(token, headers); + Map claimsMap = headers.entrySet() + .stream() + .filter(e -> e.getKey().startsWith("OAUTH2_CLAIM")) + .map(e -> new AbstractMap.SimpleEntry<>(e.getKey(), getFirst(e.getValue()))) + .collect(Collectors.toMap(Entry::getKey, Entry::getValue)); + this.cache.put(token, claimsMap); this.cache.cleanUp(); } } diff --git a/src/main/java/org/broadinstitute/consent/http/filters/RequestHeaderCacheFilter.java b/src/main/java/org/broadinstitute/consent/http/filters/RequestHeaderCacheFilter.java index cf0c703b66..cda6222787 100644 --- a/src/main/java/org/broadinstitute/consent/http/filters/RequestHeaderCacheFilter.java +++ b/src/main/java/org/broadinstitute/consent/http/filters/RequestHeaderCacheFilter.java @@ -11,7 +11,7 @@ @Priority(Integer.MIN_VALUE) public class RequestHeaderCacheFilter implements ContainerRequestFilter { - private final RequestHeaderCache headerCache = RequestHeaderCache.getInstance(); + private final ClaimsCache claimsCache = ClaimsCache.getInstance(); @Override public void filter(ContainerRequestContext containerRequestContext) throws IOException { @@ -19,7 +19,7 @@ public void filter(ContainerRequestContext containerRequestContext) throws IOExc var token = headers.getFirst(HttpHeaders.AUTHORIZATION); if (token != null) { var bearer = token.replace("Bearer ", ""); - headerCache.loadCache(bearer, headers); + claimsCache.loadCache(bearer, headers); } } } diff --git a/src/test/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticatorTest.java b/src/test/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticatorTest.java index 9bc9ded4ba..e5baf348dd 100644 --- a/src/test/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticatorTest.java +++ b/src/test/java/org/broadinstitute/consent/http/authentication/OAuthAuthenticatorTest.java @@ -14,7 +14,7 @@ import java.util.List; import java.util.Optional; import org.apache.commons.lang3.RandomStringUtils; -import org.broadinstitute.consent.http.filters.RequestHeaderCache; +import org.broadinstitute.consent.http.filters.ClaimsCache; import org.broadinstitute.consent.http.models.AuthUser; import org.broadinstitute.consent.http.service.sam.SamService; import org.junit.jupiter.api.BeforeEach; @@ -29,7 +29,7 @@ class OAuthAuthenticatorTest { @Mock private SamService samService; private OAuthAuthenticator oAuthAuthenticator; - private final RequestHeaderCache headerCache = RequestHeaderCache.getInstance(); + private final ClaimsCache headerCache = ClaimsCache.getInstance(); @BeforeEach void setUp() { @@ -40,7 +40,7 @@ void setUp() { void testAuthenticateWithToken() { String bearerToken = RandomStringUtils.randomAlphabetic(100); MultivaluedMap headerMap = new MultivaluedHashMap<>(); - headerMap.put(RequestHeaderCache.OAUTH2_CLAIM_email, List.of("email")); + headerMap.put(ClaimsCache.OAUTH2_CLAIM_email, List.of("email")); headerCache.loadCache(bearerToken, headerMap); oAuthAuthenticator = new OAuthAuthenticator(samService); Optional authUser = oAuthAuthenticator.authenticate(bearerToken); @@ -51,9 +51,9 @@ void testAuthenticateWithToken() { void testAuthenticateGetUserInfoSuccess() { String bearerToken = RandomStringUtils.randomAlphabetic(100); MultivaluedMap headerMap = new MultivaluedHashMap<>(); - headerMap.put(RequestHeaderCache.OAUTH2_CLAIM_access_token, List.of(bearerToken)); - headerMap.put(RequestHeaderCache.OAUTH2_CLAIM_email, List.of("email")); - headerMap.put(RequestHeaderCache.OAUTH2_CLAIM_name, List.of("name")); + headerMap.put(ClaimsCache.OAUTH2_CLAIM_access_token, List.of(bearerToken)); + headerMap.put(ClaimsCache.OAUTH2_CLAIM_email, List.of("email")); + headerMap.put(ClaimsCache.OAUTH2_CLAIM_name, List.of("name")); headerCache.loadCache(bearerToken, headerMap); oAuthAuthenticator = new OAuthAuthenticator(samService); Optional authUser = oAuthAuthenticator.authenticate(bearerToken); @@ -69,7 +69,7 @@ void testAuthenticateGetUserInfoSuccess() { void testAuthenticateGetUserInfoFailure() { String bearerToken = RandomStringUtils.randomAlphabetic(100); MultivaluedMap headerMap = new MultivaluedHashMap<>(); - headerMap.put(RequestHeaderCache.OAUTH2_CLAIM_access_token, List.of(bearerToken)); + headerMap.put(ClaimsCache.OAUTH2_CLAIM_access_token, List.of(bearerToken)); headerCache.loadCache(bearerToken, headerMap); oAuthAuthenticator = new OAuthAuthenticator(samService); @@ -85,7 +85,7 @@ void testAuthenticateGetUserInfoFailure() { void testAuthenticateGetUserWithStatusInfoFailurePostUserSuccess() throws Exception { String bearerToken = RandomStringUtils.randomAlphabetic(100); MultivaluedMap headerMap = new MultivaluedHashMap<>(); - headerMap.put(RequestHeaderCache.OAUTH2_CLAIM_access_token, List.of(bearerToken)); + headerMap.put(ClaimsCache.OAUTH2_CLAIM_access_token, List.of(bearerToken)); headerCache.loadCache(bearerToken, headerMap); when(samService.getRegistrationInfo(any())).thenThrow(new NotFoundException()); oAuthAuthenticator = new OAuthAuthenticator(samService); From 4c662ba67a179eb50d4143d40d9c99895a0cb02e Mon Sep 17 00:00:00 2001 From: rushtong Date: Thu, 18 Jan 2024 10:39:50 -0500 Subject: [PATCH 5/7] feat: simplify cache --- .../org/broadinstitute/consent/http/filters/ClaimsCache.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/broadinstitute/consent/http/filters/ClaimsCache.java b/src/main/java/org/broadinstitute/consent/http/filters/ClaimsCache.java index 0b3382c7c7..722ebe630a 100644 --- a/src/main/java/org/broadinstitute/consent/http/filters/ClaimsCache.java +++ b/src/main/java/org/broadinstitute/consent/http/filters/ClaimsCache.java @@ -15,7 +15,6 @@ * Manage a cache of bearer token to map of headers for every request. This is useful in cases * where components need, but do not have, access to the complete request context. */ -@Singleton public class ClaimsCache { private static ClaimsCache INSTANCE; From 8af4695157503af3de276a849c838ca0bf3e25e2 Mon Sep 17 00:00:00 2001 From: rushtong Date: Thu, 18 Jan 2024 10:40:41 -0500 Subject: [PATCH 6/7] feat: cleanup --- .../org/broadinstitute/consent/http/filters/ClaimsCache.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/broadinstitute/consent/http/filters/ClaimsCache.java b/src/main/java/org/broadinstitute/consent/http/filters/ClaimsCache.java index 722ebe630a..d916245f29 100644 --- a/src/main/java/org/broadinstitute/consent/http/filters/ClaimsCache.java +++ b/src/main/java/org/broadinstitute/consent/http/filters/ClaimsCache.java @@ -2,7 +2,6 @@ import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; -import jakarta.inject.Singleton; import jakarta.ws.rs.core.MultivaluedMap; import java.util.AbstractMap; import java.util.List; From 1b15386b95ecc231d867fc82175af966b777c93f Mon Sep 17 00:00:00 2001 From: rushtong Date: Thu, 18 Jan 2024 10:44:19 -0500 Subject: [PATCH 7/7] doc: cleanup --- .../org/broadinstitute/consent/http/filters/ClaimsCache.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/broadinstitute/consent/http/filters/ClaimsCache.java b/src/main/java/org/broadinstitute/consent/http/filters/ClaimsCache.java index d916245f29..c57187cf0c 100644 --- a/src/main/java/org/broadinstitute/consent/http/filters/ClaimsCache.java +++ b/src/main/java/org/broadinstitute/consent/http/filters/ClaimsCache.java @@ -11,8 +11,8 @@ import java.util.stream.Collectors; /** - * Manage a cache of bearer token to map of headers for every request. This is useful in cases - * where components need, but do not have, access to the complete request context. + * Manage a cache of bearer token to map of `OAUTH2_CLAIM` headers for every request. This is + * useful in cases where components need, but do not have access to, the full request context. */ public class ClaimsCache {