Skip to content

Commit

Permalink
Refactor UserRepository and improve user metadata generation
Browse files Browse the repository at this point in the history
Updated UserRepository.java to streamline the user data querying and creation process, and added explicit save(user) method for better clarity. In OktaOAuthAuthenticationService.java, a refactoring was done to simplify the process of updating user metadata. A unit test for generating user metadata was also added in a new OktaOAuthAuthenticationServiceTest.java file for improved code coverage and reliability.
  • Loading branch information
Gcolon021 committed Jan 31, 2024
1 parent af59d26 commit 7a8563d
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ public User findByEmailAndConnection(String email, String connectionId) {
query.select(queryRoot);
CriteriaBuilder cb = cb();
return em.createQuery(query
.where(
cb.equal(queryRoot.join("connection")
.get("id"), connectionId),
eq(cb, queryRoot, "email", email)))
.getSingleResult();
.where(
cb.equal(queryRoot.join("connection")
.get("id"), connectionId),
eq(cb, queryRoot, "email", email)))
.getSingleResult();
}

public User findBySubjectAndConnection(String subject, String connectionId) {
Expand Down Expand Up @@ -101,19 +101,19 @@ public User findOrCreate(User inputUser) {
} catch (NoResultException e) {
logger.debug("findOrCreate() subject " + subject +
" could not be found by `entityManager`, checking by email and connection");
try {
// If the user isn't found by subject then check by email and connection just
// in case they were created by jenkins
user = findByEmailAndConnection(inputUser.getEmail(), inputUser.getConnection().getId());
if (StringUtils.isEmpty(user.getSubject())) {
user.setSubject(inputUser.getSubject());
user.setGeneralMetadata(inputUser.getGeneralMetadata());
}
} catch (NoResultException ex) {
logger.debug("findOrCreate() email " + inputUser.getEmail() +
" could not be found by `entityManager`, creating a new user");
user = createUser(inputUser);
}
try {
// If the user isn't found by subject then check by email and connection just
// in case they were created by jenkins
user = findByEmailAndConnection(inputUser.getEmail(), inputUser.getConnection().getId());
if (StringUtils.isEmpty(user.getSubject())) {
user.setSubject(inputUser.getSubject());
user.setGeneralMetadata(inputUser.getGeneralMetadata());
}
} catch (NoResultException ex) {
logger.debug("findOrCreate() email " + inputUser.getEmail() +
" could not be found by `entityManager`, creating a new user");
user = createUser(inputUser);
}
} catch (NonUniqueResultException e) {
logger.error("findOrCreate() " + e.getClass().getSimpleName() + ": " + e.getMessage());
}
Expand Down Expand Up @@ -229,4 +229,13 @@ public User createOpenAccessUser(Role openAccessRole) {
+ ", email: " + user.getEmail());
return user;
}

/**
* Saves the given user to the database
*
* @param user the user to save
*/
public void save(User user) {
em().merge(user);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -131,22 +131,14 @@ private User loadUser(JsonNode introspectResponse) {
userRepository.changeRole(user, roles);
}

// Update the user's metadata with the information from the introspect response
// We do this every time the user logs in, because the user's information may have changed
ObjectNode objectNode = generateUserMetadata(introspectResponse, user);
String userMetadata = JAXRSConfiguration.objectMapper.writeValueAsString(objectNode);
logger.info("Adding metadata to user: " + user.getUuid() + " ___ " + userMetadata);
// Set the general metadata to the objectNode
user.setGeneralMetadata(userMetadata);

userRepository.persist(user);
user.setGeneralMetadata(generateUserMetadata(introspectResponse, user).toString());

userRepository.save(user);
logger.info("LOGIN SUCCESS ___ USER DATA: " + user);
return user;
} catch (NoResultException ex) {
logger.info("LOGIN FAILED ___ USER NOT FOUND ___ " + userEmail + " ___");
return null;
} catch (JsonProcessingException e) {
throw new RuntimeException(e);
}
}

Expand All @@ -158,7 +150,7 @@ private User loadUser(JsonNode introspectResponse) {
* @param user The user
* @return The user metadata as an ObjectNode
*/
private ObjectNode generateUserMetadata(JsonNode introspectResponse, User user) {
protected ObjectNode generateUserMetadata(JsonNode introspectResponse, User user) {
// JsonNode is immutable, so we need to convert it to an ObjectNode
ObjectNode objectNode = JAXRSConfiguration.objectMapper.createObjectNode();
ObjectNode authzNode = objectNode.putObject("authz");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package edu.harvard.hms.dbmi.avillach.auth.service.auth;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ObjectNode;
import edu.harvard.hms.dbmi.avillach.auth.data.entity.User;
import org.junit.Test;
import org.mockito.Mock;

import java.util.UUID;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.Mockito.when;

public class OktaOAuthAuthenticationServiceTest {

private OktaOAuthAuthenticationService service;

@Test
public void testAuthenticate() {
}

@Test
public void testGenerateUserMetadata() {
/**
* unit test for generateUserMetadata
*protected ObjectNode generateUserMetadata (JsonNode introspectResponse, User user){
* // JsonNode is immutable, so we need to convert it to an ObjectNode
*ObjectNode objectNode = JAXRSConfiguration.objectMapper.createObjectNode();
*ObjectNode authzNode = objectNode.putObject("authz");
*ObjectNode tagsNode = authzNode.putObject("tags");
*
*authzNode.put("role", "user");
*authzNode.put("sub", introspectResponse.get("sub").asText());
*authzNode.put("user_id", user.getUuid().toString());
*authzNode.put("username", user.getEmail());
*tagsNode.put("email", user.getEmail());
*
*return objectNode;
*}
*
*/

//mock user
UUID uuid = UUID.randomUUID();
String email = "[email protected]";
User user = org.mockito.Mockito.mock(User.class);
when(user.getUuid()).thenReturn(uuid);
when(user.getEmail()).thenReturn(email);

//mock introspectResponse
ObjectMapper objectMapper = new ObjectMapper();
ObjectNode objectNode = objectMapper.createObjectNode();
objectNode.put("sub", "test_sub");
JsonNode introspectResponse = org.mockito.Mockito.mock(JsonNode.class);
when(introspectResponse.get("sub")).thenReturn(objectNode.get("sub"));

//test
service = new OktaOAuthAuthenticationService();
ObjectNode result = service.generateUserMetadata(introspectResponse, user);

assertEquals(result.get("authz").get("tags").get("email").asText(), email);
assertEquals(result.get("authz").get("sub").asText(), "test_sub");
assertEquals(result.get("authz").get("user_id").asText(), uuid.toString());
assertEquals(result.get("authz").get("username").asText(), email);
assertEquals(result.get("authz").get("role").asText(), "user");

// print result
System.out.println(result);
}

}

0 comments on commit 7a8563d

Please sign in to comment.