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.

Sign up for GitHub

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

Jump to bottom

Remove obsolete DSM user create code from AuthenticationRoute #2641

Merged
merged 8 commits into from
Aug 2, 2023
Merged

Remove obsolete DSM user create code from AuthenticationRoute #2641

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
eb26587
WIP
denniscunningham Jul 26, 2023
9e4ac8a
Merge branch 'develop' into pepper-987
denniscunningham Jul 26, 2023
c69d626
WIP
denniscunningham Jul 26, 2023
60766b6
WIP
denniscunningham Jul 27, 2023
e27b6d4
Merge branch 'develop' into pepper-987
denniscunningham Aug 1, 2023
5ae7970
Remove obsolete method
denniscunningham Aug 1, 2023
97465c6
Merge branch 'develop' into pepper-987
denniscunningham Aug 1, 2023
851527a
Merge branch 'develop' into pepper-987
denniscunningham Aug 2, 2023
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
3 changes: 2 additions & 1 deletion pepper-apis/dsm-core/src/main/java/org/broadinstitute/dsm/DSMServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,6 @@ protected void setupCustomRouting(@NonNull Config cfg) {
setupDDPConfigurationLookup(cfg.getString(ApplicationConfigConstants.DDP));

AuthenticationRoute authenticationRoute = new AuthenticationRoute(auth0Util,
userUtil,
cfg.getString(ApplicationConfigConstants.AUTH0_DOMAIN),
cfg.getString(ApplicationConfigConstants.AUTH0_MGT_SECRET),
cfg.getString(ApplicationConfigConstants.AUTH0_MGT_KEY),
Expand Down Expand Up @@ -1060,6 +1059,8 @@ private void setupRouteGenericErrorHandlers() {
response.body(exception.getMessage());
});
exception(DsmInternalError.class, (exception, request, response) -> {
logger.error("Internal error {}", exception.toString());
exception.printStackTrace();
response.status(500);
response.body(exception.getMessage());
});
Expand Down
108 changes: 53 additions & 55 deletions pepper-apis/dsm-core/src/main/java/org/broadinstitute/dsm/route/AuthenticationRoute.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,19 @@

import com.google.gson.Gson;
import com.google.gson.JsonObject;
import com.google.gson.JsonParseException;
import com.google.gson.JsonParser;
import com.google.gson.JsonSyntaxException;
import lombok.NonNull;
import org.apache.commons.lang3.StringUtils;
import org.apache.http.entity.ContentType;
import org.broadinstitute.dsm.db.UserSettings;
import org.broadinstitute.dsm.db.dao.user.UserDao;
import org.broadinstitute.dsm.db.dto.user.UserDto;
import org.broadinstitute.dsm.exception.AuthenticationException;
import org.broadinstitute.dsm.util.UserUtil;
import org.broadinstitute.dsm.exception.DSMBadRequestException;
import org.broadinstitute.dsm.exception.DsmInternalError;
import org.broadinstitute.dsm.security.Auth0Util;
import org.broadinstitute.dsm.util.UserUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import spark.Request;
Expand All @@ -37,18 +39,16 @@ public class AuthenticationRoute implements Route {

private final Auth0Util auth0Util;

private final UserUtil userUtil;
private final String auth0Domain;
private final String clientSecret;
private final String auth0ClientId;
private final String auth0MgmntAudience;
private final String audienceNameSpace;

public AuthenticationRoute(@NonNull Auth0Util auth0Util, @NonNull UserUtil userUtil, @NonNull String auth0Domain,
public AuthenticationRoute(@NonNull Auth0Util auth0Util, @NonNull String auth0Domain,
@NonNull String clientSecret, @NonNull String auth0ClientId, @NonNull String auth0MgmntAudience,
@NonNull String audienceNameSpace) {
this.auth0Util = auth0Util;
this.userUtil = userUtil;
this.auth0Domain = auth0Domain;
this.clientSecret = clientSecret;
this.auth0ClientId = auth0ClientId;
Expand All @@ -58,65 +58,59 @@ public AuthenticationRoute(@NonNull Auth0Util auth0Util, @NonNull UserUtil userU

@Override
public Object handle(Request request, Response response) {
logger.info("Check user...");
try {
JsonObject jsonObject = JsonParser.parseString(request.body()).getAsJsonObject();
String auth0Token = jsonObject.get(payloadToken).getAsString();
if (StringUtils.isNotBlank(auth0Token)) {
// checking if Auth0 knows that token
try {
Auth0Util.Auth0UserInfo auth0UserInfo = auth0Util.getAuth0UserInfo(auth0Token, auth0Domain);
if (auth0UserInfo != null) {
String email = auth0UserInfo.getEmail();
logger.info("User (" + email + ") was found ");
Gson gson = new Gson();
Map<String, String> claims = new HashMap<>();
UserDao userDao = new UserDao();
UserDto userDto =
userDao.getUserByEmail(email).orElseThrow(() -> new RuntimeException("User " + email + " not found!"));
if (userDto == null) {
userUtil.insertUser(email, email);
userDto = userDao.getUserByEmail(email)
.orElseThrow(() -> new RuntimeException("new inserted user " + email + " not found!"));
claims.put(userAccessRoles, "user needs roles and groups");
} else {
String userSetting = gson.toJson(userUtil.getUserAccessRoles(email), ArrayList.class);
claims.put(userAccessRoles, userSetting);
logger.info(userSetting);
claims.put(userSettings, gson.toJson(UserSettings.getUserSettings(email), UserSettings.class));
}
claims.put(authUserId, String.valueOf(userDto.getId()));
claims.put(authUserName, userDto.getName().orElse(""));
claims.put(authUserEmail, email);

try {
String dsmToken = auth0Util.getNewAuth0TokenWithCustomClaims(claims, clientSecret, auth0ClientId, auth0Domain,
auth0MgmntAudience, audienceNameSpace);
if (dsmToken != null) {
return new DSMToken(dsmToken);
} else {
haltWithErrorMsg(401, response, "DSMToken was null! Not authorized user");
}
} catch (AuthenticationException e) {
haltWithErrorMsg(401, response, "DSMToken was null! Not authorized user", e);
}
} else {
haltWithErrorMsg(400, response, "user was null");
}
} catch (AuthenticationException e) {
haltWithErrorMsg(400, response, "Problem getting user info from Auth0 token", e);
}
} else {
haltWithErrorMsg(400, response, "There was no token in the payload");
if (StringUtils.isBlank(auth0Token)) {
haltWithErrorMsg(400, response, "There was no Auth0 token in the payload");
}
} catch (JsonSyntaxException e) {
haltWithErrorMsg(400, response, "The provided JSON in the request was malformed", e);
return new DSMToken(updateToken(auth0Token));
} catch (AuthenticationException e) {
haltWithErrorMsg(400, response, "Unable to get user information from Auth0 token", e);
} catch (JsonParseException e) {
haltWithErrorMsg(400, response, "Unable to get Auth0 token from request", e);
}
// DSMInternalError and DSMBadRequestException are handled via Spark
return response;
}

private String updateToken(String auth0Token) {
Auth0Util.Auth0UserInfo auth0UserInfo = auth0Util.getAuth0UserInfo(auth0Token, auth0Domain);
String email = auth0UserInfo.getEmail();

logger.info("Authenticating user {}", email);
UserDao userDao = new UserDao();
UserDto userDto = userDao.getUserByEmail(email).orElseThrow(() ->
new DSMBadRequestException("User not found: " + email));

Map<String, String> claims = updateClaims(userDto);
String dsmToken = auth0Util.getNewAuth0TokenWithCustomClaims(claims, clientSecret, auth0ClientId, auth0Domain,
auth0MgmntAudience, audienceNameSpace);
if (dsmToken == null) {
throw new DsmInternalError("Assert: Auth token should not be null");
}
return dsmToken;
}

private Map<String, String> updateClaims(UserDto userDto) {
Map<String, String> claims = new HashMap<>();
try {
Gson gson = new Gson();
String email = userDto.getEmail().orElseThrow(() -> new DsmInternalError("User email cannot be null"));
String roles = gson.toJson(UserUtil.getUserAccessRoles(email), ArrayList.class);
claims.put(userAccessRoles, roles);
claims.put(userSettings, gson.toJson(UserSettings.getUserSettings(email), UserSettings.class));
claims.put(authUserId, String.valueOf(userDto.getId()));
claims.put(authUserName, userDto.getName().orElse(""));
claims.put(authUserEmail, email);
} catch (JsonParseException e) {
throw new DsmInternalError("Error converting class to JSON", e);
}
return claims;
}

private static class DSMToken {
private String dsmToken;
private final String dsmToken;

public DSMToken(String token) {
this.dsmToken = token;
Expand All @@ -128,13 +122,17 @@ public DSMToken(String token) {
*/
public static void haltWithErrorMsg(int responseStatus, Response response, String message) {
response.type(ContentType.APPLICATION_JSON.getMimeType());
// TODO: this is currently called for bad request status. Do we want to log that at error level?
// Or perhaps we could use the return status to determine the log level? -DC
logger.error(message);
String errorMsgJson = new Gson().toJson(new Error(message));
halt(responseStatus, errorMsgJson);
}

public static void haltWithErrorMsg(int responseStatus, Response response, String message, Throwable t) {
if (t != null) {
// TODO: this is currently called for bad request status. Do we want to log that at error level?
// Or perhaps we could use the return status to determine the log level? -DC
logger.error("Authentication Error", t);
}
haltWithErrorMsg(responseStatus, response, message);
Expand Down
3 changes: 2 additions & 1 deletion pepper-apis/dsm-core/src/main/java/org/broadinstitute/dsm/route/KitDiscardRoute.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;

import com.google.gson.Gson;
import lombok.NonNull;
Expand Down Expand Up @@ -129,7 +130,7 @@ public Object processRequest(Request request, Response response, String userId)
String email = auth0UserInfo.getEmail();
UserDto userDto = new UserDao().getUserByEmail(email).orElseThrow();
if (userDto != null && userDto.getId() > 0) {
ArrayList<String> userSetting = userUtil.getUserAccessRoles(email);
List<String> userSetting = userUtil.getUserAccessRoles(email);
if (userSetting.contains(DBConstants.KIT_SHIPPING) || userSetting.contains(DBConstants.DISCARD_SAMPLE)) {
KitDiscard kit = KitDiscard.getKitDiscard(kitAction.getKitDiscardId());
if (kit.getChangedById() != userDto.getId()) {
Expand Down
15 changes: 7 additions & 8 deletions pepper-apis/dsm-core/src/main/java/org/broadinstitute/dsm/security/Auth0Util.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import org.apache.http.NameValuePair;
import org.apache.http.message.BasicNameValuePair;
import org.broadinstitute.dsm.exception.AuthenticationException;
import org.broadinstitute.dsm.exception.DSMBadRequestException;
import org.broadinstitute.dsm.exception.DsmInternalError;
import org.broadinstitute.dsm.model.auth0.Auth0M2MResponse;
import org.broadinstitute.dsm.util.DDPRequestUtil;
import org.slf4j.Logger;
Expand Down Expand Up @@ -70,8 +72,8 @@ public Auth0UserInfo getAuth0UserInfo(@NonNull String idToken, String auth0Domai
verifyUserConnection(auth0Claims.get("sub").asString(), userInfo.getEmail());

return userInfo;
} catch (AuthenticationException e) {
throw new AuthenticationException("couldn't get Auth0 user info", e);
} catch (Exception e) {
throw new AuthenticationException("Could not get Auth0 user info", e);
}
}

Expand Down Expand Up @@ -104,7 +106,7 @@ private String findUserConnection(List<Identity> list) {
}

if (connection == null) {
throw new RuntimeException("User does not have an approved connection.");
throw new DSMBadRequestException("User does not have an approved connection.");
}
return connection;
}
Expand All @@ -116,20 +118,17 @@ private void verifyUserConnection(@NonNull String userId, @NonNull String email)
User user = userRequest.execute();
findUserConnection(user.getIdentities());
} catch (Exception ex) {
throw new RuntimeException("User connection verification failed for user " + email, ex);
throw new DsmInternalError("User connection verification failed for user " + email, ex);
}
}

public static Map<String, Claim> verifyAndParseAuth0TokenClaims(String auth0Token, String auth0Domain) throws AuthenticationException {
Map<String, Claim> auth0Claims = new HashMap<>();
try {
Optional<DecodedJWT> maybeToken = verifyAuth0Token(auth0Token, auth0Domain);
maybeToken.orElseThrow();
auth0Claims = maybeToken.get().getClaims();
return maybeToken.orElseThrow().getClaims();
} catch (Exception e) {
throw new AuthenticationException("Could not verify auth0 token.", e);
}
return auth0Claims;
}

/**
Expand Down
45 changes: 8 additions & 37 deletions pepper-apis/dsm-core/src/main/java/org/broadinstitute/dsm/util/UserUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
Expand All @@ -15,9 +14,9 @@

import lombok.NonNull;
import org.apache.commons.lang3.StringUtils;
import org.broadinstitute.dsm.db.UserSettings;
import org.broadinstitute.dsm.db.dao.user.UserDao;
import org.broadinstitute.dsm.db.dto.user.UserDto;
import org.broadinstitute.dsm.exception.DsmInternalError;
import org.broadinstitute.dsm.model.NameValue;
import org.broadinstitute.dsm.model.patch.Patch;
import org.broadinstitute.dsm.statics.ApplicationConfigConstants;
Expand All @@ -40,7 +39,6 @@ public class UserUtil {
public static final String SHIPPING_MENU = "shipping";
private static final Logger logger = LoggerFactory.getLogger(UserUtil.class);
private static final String SQL_SELECT_USER = "SELECT user_id, name FROM access_user";
private static final String SQL_INSERT_USER = "INSERT INTO access_user (name, email) VALUES (?,?)";
private static final String SQL_SELECT_USER_ACCESS_ROLE =
"SELECT role.name FROM access_user_role_group roleGroup, access_user user, access_role role "
+ "WHERE roleGroup.user_id = user.user_id AND roleGroup.role_id = role.role_id AND user.is_active = 1";
Expand Down Expand Up @@ -283,11 +281,13 @@ public static boolean checkUserAccessForPatch(String realm, String userId, Strin
} else {
// for now, let's do what DSM did previously and let them change the data.
// Still we need to log this and fix the patch from frontend
logger.error("The id in patch is not a number and also not an email, id in patch is "+ userEmailOrIdInPatch + "and id in token is "+ userId);
logger.error("The id in patch is not a number and also not an email, id in patch is " + userEmailOrIdInPatch +
"and id in token is " + userId);
return checkUserAccess(realm, userId, role, userIdRequest);
}
if (!userId.equals(userIdFromPatch)){
String msg = "User id in patch did not match the one in token, user Id in patch is " + userIdFromPatch + " user Id in token " + userIdRequest;
if (!userId.equals(userIdFromPatch)) {
String msg = "User id in patch did not match the one in token, user Id in patch is " + userIdFromPatch + " user Id in token " +
userIdRequest;
logger.warn(msg);
throw new RuntimeException(msg);
}
Expand Down Expand Up @@ -329,7 +329,7 @@ public static boolean checkKitShippingAccessForPatch(String realm, String userId
&& DBConstants.DDP_KIT_ALIAS.equals(patch.getTableAlias());
}

public ArrayList<String> getUserAccessRoles(@NonNull String email) {
public static List<String> getUserAccessRoles(@NonNull String email) {
ArrayList<String> roles = new ArrayList<>();
SimpleResult results = inTransaction((conn) -> {
SimpleResult dbVals = new SimpleResult();
Expand All @@ -347,37 +347,8 @@ public ArrayList<String> getUserAccessRoles(@NonNull String email) {
});

if (results.resultException != null) {
throw new RuntimeException("Error getting list of roles ", results.resultException);
throw new DsmInternalError("Error getting roles for " + email, results.resultException);
}
return roles;
}

public int insertUser(@NonNull String name, @NonNull String email) {
SimpleResult results = inTransaction((conn) -> {
SimpleResult dbVals = new SimpleResult();
try (PreparedStatement insertStmt = conn.prepareStatement(SQL_INSERT_USER, Statement.RETURN_GENERATED_KEYS)) {
insertStmt.setString(1, name);
insertStmt.setString(2, email);
insertStmt.executeUpdate();
try (ResultSet rs = insertStmt.getGeneratedKeys()) {
if (rs.next()) {
UserSettings.insertUserSetting(conn, rs.getInt(1));
dbVals.resultValue = rs.getInt(1);
}
} catch (Exception e) {
throw new RuntimeException("Error getting id of new kit request ", e);
}
} catch (SQLException ex) {
logger.error(
"User " + name + ", " + email + " already exists but doesn't have any access roles or is set to is_active=0...");
}
return dbVals;
});

if (results.resultException != null) {
throw new RuntimeException("Error getting list of realms ", results.resultException);
}

return (int) results.resultValue;
}
}
Loading