From 54e5286f9ca2fa4b7bcea2e15d1ec83a29932bce Mon Sep 17 00:00:00 2001 From: Alex Borodin Date: Mon, 30 Apr 2018 15:40:38 +0200 Subject: [PATCH] fix(users): handle external change of user email address by storing previously known email addresses and falling back to search by former addresses when necessary introduce generic ids for users instead of using emails as ids closes eclipse/sw360#242 --- .../sw360/datahandler/db/UserRepository.java | 36 ++++++- .../db/ComponentDatabaseHandlerTest.java | 4 +- .../moderation/TestModerationClient.java | 2 +- .../org/eclipse/sw360/users/UserHandler.java | 20 ++-- .../sw360/users/db/UserDatabaseHandler.java | 11 ++- .../portal/common/CustomFieldHelper.java | 12 +-- .../portal/portlets/admin/UserPortlet.java | 52 +++------- .../portal/portlets/signup/SignupPortlet.java | 2 +- .../portal/users/OrganizationHelper.java | 2 +- .../portal/users/PortletRequestAdapter.java | 6 +- .../sw360/portal/users/SSOAutoLogin.java | 97 +++++++++++-------- .../eclipse/sw360/portal/users/UserCache.java | 24 ++--- .../eclipse/sw360/portal/users/UserUtils.java | 95 ++++++++++-------- .../src/main/webapp/html/preferences/view.jsp | 2 +- .../datahandler/thrift/ThriftValidate.java | 2 - .../src/main/thrift/users.thrift | 13 ++- .../thrift/ThriftValidateTest.java | 5 +- .../core/JacksonCustomizations.java | 4 +- .../core/RestControllerHelper.java | 5 +- .../resourceserver/user/Sw360UserService.java | 9 ++ .../resourceserver/user/UserController.java | 29 ++++-- .../user/UserResourceProcessor.java | 5 +- .../integration/ComponentTest.java | 2 +- .../integration/ProjectTest.java | 2 +- .../resourceserver/integration/UserTest.java | 4 +- .../restdocs/AttachmentSpecTest.java | 2 +- .../restdocs/ComponentSpecTest.java | 2 +- .../restdocs/ProjectSpecTest.java | 2 +- .../restdocs/ReleaseSpecTest.java | 2 +- .../resourceserver/restdocs/UserSpecTest.java | 37 ++++++- .../restdocs/VulnerabilitySpecTest.java | 2 +- 31 files changed, 297 insertions(+), 195 deletions(-) diff --git a/backend/src-common/src/main/java/org/eclipse/sw360/datahandler/db/UserRepository.java b/backend/src-common/src/main/java/org/eclipse/sw360/datahandler/db/UserRepository.java index f676aadd7e..472de7ff0f 100644 --- a/backend/src-common/src/main/java/org/eclipse/sw360/datahandler/db/UserRepository.java +++ b/backend/src-common/src/main/java/org/eclipse/sw360/datahandler/db/UserRepository.java @@ -11,13 +11,16 @@ package org.eclipse.sw360.datahandler.db; import org.eclipse.sw360.components.summary.UserSummary; +import org.eclipse.sw360.datahandler.common.CommonUtils; import org.eclipse.sw360.datahandler.couchdb.DatabaseConnector; import org.eclipse.sw360.datahandler.couchdb.SummaryAwareRepository; import org.eclipse.sw360.datahandler.thrift.users.User; import org.ektorp.support.View; +import org.ektorp.support.Views; import java.util.Collection; import java.util.List; +import java.util.Set; /** * CRUD access for the User class @@ -26,7 +29,24 @@ * @author Johannes.Najjar@tngtech.com */ -@View(name = "all", map = "function(doc) { if (doc.type == 'user') emit(null, doc._id) }") +@Views({ + @View(name = "all", + map = "function(doc) { if (doc.type == 'user') emit(null, doc._id) }"), + @View(name = "byExternalId", + map = "function(doc) { if (doc.type == 'user') emit(doc.externalid, doc._id) }"), + @View(name = "byEmail", + map = "function(doc) { " + + " if (doc.type == 'user') {" + + " emit(doc.email, doc._id); " + + " if (doc.formerEmailAddresses && Array.isArray(doc.formerEmailAddresses)) {" + + " var arr = doc.formerEmailAddresses;" + + " for (var i = 0; i < arr.length; i++){" + + " emit(arr[i], doc._id);" + + " }" + + " }" + + " }" + + "}"), +}) public class UserRepository extends SummaryAwareRepository { public UserRepository(DatabaseConnector databaseConnector) { super(User.class, databaseConnector, new UserSummary()); @@ -37,4 +57,18 @@ public UserRepository(DatabaseConnector databaseConnector) { public List get(Collection ids) { return getConnector().get(User.class, ids, true); } + + public User getByExternalId(String externalId) { + final Set userIds = queryForIdsAsValue("byExternalId", externalId); + if (userIds != null && !userIds.isEmpty()) + return get(CommonUtils.getFirst(userIds)); + return null; + } + + public User getByEmail(String email) { + final Set userIds = queryForIdsAsValue("byEmail", email); + if (userIds != null && !userIds.isEmpty()) + return get(CommonUtils.getFirst(userIds)); + return null; + } } diff --git a/backend/src/src-components/src/test/java/org/eclipse/sw360/components/db/ComponentDatabaseHandlerTest.java b/backend/src/src-components/src/test/java/org/eclipse/sw360/components/db/ComponentDatabaseHandlerTest.java index 70e87c74a7..28b77b2858 100644 --- a/backend/src/src-components/src/test/java/org/eclipse/sw360/components/db/ComponentDatabaseHandlerTest.java +++ b/backend/src/src-components/src/test/java/org/eclipse/sw360/components/db/ComponentDatabaseHandlerTest.java @@ -58,8 +58,8 @@ public class ComponentDatabaseHandlerTest { private static final String email1 = "cedric.bodet@tngtech.com"; private static final String email2 = "johannes.najjar@tngtech.com"; - private static final User user1 = new User().setEmail(email1).setDepartment("AB CD EF").setId(email1); - private static final User user2 = new User().setEmail(email2).setDepartment("AB CD EF").setId(email2); + private static final User user1 = new User().setEmail(email1).setDepartment("AB CD EF").setId("481489458"); + private static final User user2 = new User().setEmail(email2).setDepartment("AB CD EF").setId("4786487647680"); @Rule public final ExpectedException exception = ExpectedException.none(); diff --git a/backend/src/src-moderation/src/test/java/org/eclipse/sw360/moderation/TestModerationClient.java b/backend/src/src-moderation/src/test/java/org/eclipse/sw360/moderation/TestModerationClient.java index 2d039ae47c..fc953ee42c 100644 --- a/backend/src/src-moderation/src/test/java/org/eclipse/sw360/moderation/TestModerationClient.java +++ b/backend/src/src-moderation/src/test/java/org/eclipse/sw360/moderation/TestModerationClient.java @@ -33,7 +33,7 @@ public static void main(String[] args) throws TException, IOException { TProtocol protocol = new TCompactProtocol(thriftClient); ModerationService.Iface client = new ModerationService.Client(protocol); - List requestsByModerator = client.getRequestsByModerator(new User().setId("").setEmail("cedric.bodet@tngtech.com").setDepartment("BB")); + List requestsByModerator = client.getRequestsByModerator(new User().setId("58245y9845").setEmail("cedric.bodet@tngtech.com").setDepartment("BB")); System.out.println("Fetched " + requestsByModerator.size() + " moderation requests from moderation service"); diff --git a/backend/src/src-users/src/main/java/org/eclipse/sw360/users/UserHandler.java b/backend/src/src-users/src/main/java/org/eclipse/sw360/users/UserHandler.java index f443031481..143bd4a48d 100644 --- a/backend/src/src-users/src/main/java/org/eclipse/sw360/users/UserHandler.java +++ b/backend/src/src-users/src/main/java/org/eclipse/sw360/users/UserHandler.java @@ -10,16 +10,15 @@ */ package org.eclipse.sw360.users; +import org.apache.log4j.Logger; +import org.apache.thrift.TException; import org.eclipse.sw360.datahandler.common.DatabaseSettings; import org.eclipse.sw360.datahandler.thrift.RequestStatus; import org.eclipse.sw360.datahandler.thrift.users.User; import org.eclipse.sw360.datahandler.thrift.users.UserService; import org.eclipse.sw360.users.db.UserDatabaseHandler; -import org.apache.log4j.Logger; -import org.apache.thrift.TException; import java.io.IOException; -import java.net.MalformedURLException; import java.util.List; import static org.eclipse.sw360.datahandler.common.SW360Assert.assertNotEmpty; @@ -40,6 +39,11 @@ public UserHandler() throws IOException { db = new UserDatabaseHandler(DatabaseSettings.getConfiguredHttpClient(), DatabaseSettings.COUCH_DB_USERS); } + @Override + public User getUser(String id) throws TException { + return db.getUser(id); + } + @Override public User getByEmail(String email) throws TException { StackTraceElement stackTraceElement = Thread.currentThread().getStackTrace()[2]; @@ -47,10 +51,14 @@ public User getByEmail(String email) throws TException { if (log.isTraceEnabled()) log.trace("getByEmail: " + email); - // Get user from database - User user = db.getByEmail(email); + return db.getByEmail(email); + } + + @Override + public User getByEmailOrExternalId(String email, String externalId) throws TException { + User user = getByEmail(email); if (user == null) { - log.info("User does not exist in DB"); + user = db.getByExternalId(externalId); } return user; } diff --git a/backend/src/src-users/src/main/java/org/eclipse/sw360/users/db/UserDatabaseHandler.java b/backend/src/src-users/src/main/java/org/eclipse/sw360/users/db/UserDatabaseHandler.java index 147ce20f64..49a0161842 100644 --- a/backend/src/src-users/src/main/java/org/eclipse/sw360/users/db/UserDatabaseHandler.java +++ b/backend/src/src-users/src/main/java/org/eclipse/sw360/users/db/UserDatabaseHandler.java @@ -21,7 +21,6 @@ import org.ektorp.http.HttpClient; import java.io.IOException; -import java.net.MalformedURLException; import java.util.List; import java.util.function.Supplier; @@ -49,7 +48,11 @@ public UserDatabaseHandler(Supplier httpClient, String dbName) throw } public User getByEmail(String email) { - return db.get(User.class, email); + return repository.getByEmail(email); + } + + public User getUser(String id) { + return db.get(User.class, id); } private void prepareUser(User user) throws SW360Exception { @@ -87,4 +90,8 @@ public List getAll() { public List searchUsers(String searchText) { return userSearch.searchByNameAndEmail(searchText); } + + public User getByExternalId(String externalId) { + return repository.getByExternalId(externalId); + } } diff --git a/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/common/CustomFieldHelper.java b/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/common/CustomFieldHelper.java index a49c5870a3..12ee75a8ac 100644 --- a/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/common/CustomFieldHelper.java +++ b/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/common/CustomFieldHelper.java @@ -13,15 +13,12 @@ import com.liferay.portal.kernel.exception.PortalException; import com.liferay.portal.kernel.exception.SystemException; -import com.liferay.portal.kernel.util.WebKeys; import com.liferay.portal.model.ResourceConstants; import com.liferay.portal.model.Role; import com.liferay.portal.model.RoleConstants; import com.liferay.portal.security.permission.ActionKeys; import com.liferay.portal.service.ResourcePermissionLocalServiceUtil; import com.liferay.portal.service.RoleLocalServiceUtil; -import com.liferay.portal.service.UserLocalServiceUtil; -import com.liferay.portal.theme.ThemeDisplay; import com.liferay.portlet.expando.model.ExpandoBridge; import com.liferay.portlet.expando.model.ExpandoColumn; import com.liferay.portlet.expando.model.ExpandoColumnConstants; @@ -29,6 +26,7 @@ import com.liferay.portlet.expando.service.ExpandoColumnLocalServiceUtil; import org.apache.log4j.Logger; import org.eclipse.sw360.datahandler.thrift.users.User; +import org.eclipse.sw360.portal.users.UserUtils; import javax.portlet.PortletRequest; @@ -83,19 +81,13 @@ public static Optional loadField(Class type, Port } private static ExpandoBridge getUserExpandoBridge(PortletRequest request, User user) throws PortalException, SystemException { - com.liferay.portal.model.User liferayUser = getLiferayUser(request, user); + com.liferay.portal.model.User liferayUser = UserUtils.findLiferayUser(request, user); ensureUserCustomFieldExists(liferayUser, CUSTOM_FIELD_PROJECT_GROUP_FILTER, ExpandoColumnConstants.STRING); ensureUserCustomFieldExists(liferayUser, CUSTOM_FIELD_COMPONENTS_VIEW_SIZE, ExpandoColumnConstants.INTEGER); ensureUserCustomFieldExists(liferayUser, CUSTOM_FIELD_VULNERABILITIES_VIEW_SIZE, ExpandoColumnConstants.INTEGER); return liferayUser.getExpandoBridge(); } - private static com.liferay.portal.model.User getLiferayUser(PortletRequest request, User user) throws PortalException, SystemException { - ThemeDisplay themeDisplay = (ThemeDisplay) request.getAttribute(WebKeys.THEME_DISPLAY); - long companyId = themeDisplay.getCompanyId(); - return UserLocalServiceUtil.getUserByEmailAddress(companyId, user.email); - } - private static void ensureUserCustomFieldExists(com.liferay.portal.model.User liferayUser, String customFieldName, int customFieldType) throws PortalException, SystemException { ExpandoBridge exp = liferayUser.getExpandoBridge(); if (!exp.hasAttribute(customFieldName)) { diff --git a/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/portlets/admin/UserPortlet.java b/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/portlets/admin/UserPortlet.java index bc39d4b1a0..0bc66124e1 100644 --- a/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/portlets/admin/UserPortlet.java +++ b/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/portlets/admin/UserPortlet.java @@ -19,14 +19,16 @@ import com.liferay.portal.kernel.exception.SystemException; import com.liferay.portal.kernel.portlet.PortletResponseUtil; import com.liferay.portal.kernel.upload.UploadPortletRequest; -import com.liferay.portal.kernel.util.WebKeys; import com.liferay.portal.model.*; -import com.liferay.portal.model.User; import com.liferay.portal.service.*; -import com.liferay.portal.theme.ThemeDisplay; import com.liferay.portal.util.PortalUtil; +import org.apache.commons.csv.CSVFormat; +import org.apache.commons.csv.CSVParser; +import org.apache.commons.csv.CSVPrinter; +import org.apache.commons.csv.CSVRecord; +import org.apache.log4j.Logger; +import org.apache.thrift.TException; import org.eclipse.sw360.datahandler.common.CommonUtils; -import org.eclipse.sw360.datahandler.thrift.SW360Exception; import org.eclipse.sw360.datahandler.thrift.users.UserGroup; import org.eclipse.sw360.datahandler.thrift.users.UserService; import org.eclipse.sw360.portal.common.PortalConstants; @@ -35,12 +37,6 @@ import org.eclipse.sw360.portal.users.UserCSV; import org.eclipse.sw360.portal.users.UserCacheHolder; import org.eclipse.sw360.portal.users.UserUtils; -import org.apache.commons.csv.CSVFormat; -import org.apache.commons.csv.CSVParser; -import org.apache.commons.csv.CSVPrinter; -import org.apache.commons.csv.CSVRecord; -import org.apache.log4j.Logger; -import org.apache.thrift.TException; import javax.portlet.*; import java.io.*; @@ -222,19 +218,13 @@ public void backUpUsers(ResourceRequest request, ResourceResponse response) thro } @UsedAsLiferayAction - public void updateUsers(ActionRequest request, ActionResponse response) throws PortletException, IOException { + public void updateUsers(ActionRequest request, ActionResponse response) throws IOException { - List users; - try { - users = getUsersFromRequest(request, "file"); - } catch (TException e) { - log.error("Error processing csv file", e); - users = Collections.emptyList(); - } + List users = getUsersFromRequest(request, "file"); try { createOrganizations(request, users); - } catch (SW360Exception | SystemException | PortalException e) { + } catch (SystemException | PortalException e) { log.error("Error creating organizations", e); } @@ -251,7 +241,7 @@ private String extractHeadDept(String input) { } - private void createOrganizations(PortletRequest request, List users) throws SW360Exception, SystemException, PortalException { + private void createOrganizations(PortletRequest request, List users) throws SystemException, PortalException { /* Find the departments of the users, create the head departments and then create the organizations */ @@ -260,13 +250,12 @@ private void createOrganizations(PortletRequest request, List users) th createOrganizations(request, departments); } - public void createOrganizations(PortletRequest request, Iterable departments) throws PortalException, SystemException { + private void createOrganizations(PortletRequest request, Iterable departments) throws PortalException, SystemException { ImmutableSet headDepartments = FluentIterable.from(departments).transform(department -> extractHeadDept(department)).toSet(); Map organizationIds = new HashMap<>(); ServiceContext serviceContext = ServiceContextFactory.getInstance(request); - ThemeDisplay themeDisplay = (ThemeDisplay) request.getAttribute(WebKeys.THEME_DISPLAY); - long companyId = themeDisplay.getCompanyId(); + long companyId = UserUtils.getCompanyId(request); for (String headDepartment : headDepartments) { long organizationId; @@ -311,20 +300,7 @@ private Organization createOrganization(ServiceContext serviceContext, String he ); } - private User getCurrentUser(PortletRequest request) throws SW360Exception { - User user; - ThemeDisplay themeDisplay = (ThemeDisplay) request.getAttribute(WebKeys.THEME_DISPLAY); - - if (themeDisplay.isSignedIn()) - user = themeDisplay.getUser(); - else { - throw new SW360Exception("Broken portlet!"); - } - return user; - } - - - private List getUsersFromRequest(PortletRequest request, String fileUploadFormId) throws IOException, TException { + private List getUsersFromRequest(PortletRequest request, String fileUploadFormId) throws IOException { final UploadPortletRequest uploadPortletRequest = PortalUtil.getUploadPortletRequest(request); @@ -362,7 +338,7 @@ private User dealWithUser(PortletRequest request, UserCSV userRec) { try { user = userRec.addLifeRayUser(request); if (user != null) { - UserUtils.synchronizeUserWithDatabase(userRec, thriftClients, userRec::getEmail, UserUtils::fillThriftUserFromUserCSV); + UserUtils.synchronizeUserWithDatabase(userRec, thriftClients, userRec::getEmail, userRec::getGid, UserUtils::fillThriftUserFromUserCSV); } } catch (SystemException | PortalException e) { log.error("Error creating a new user", e); diff --git a/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/portlets/signup/SignupPortlet.java b/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/portlets/signup/SignupPortlet.java index 3ac94fdf67..d071d180f9 100644 --- a/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/portlets/signup/SignupPortlet.java +++ b/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/portlets/signup/SignupPortlet.java @@ -101,7 +101,7 @@ private User createUser(Registrant registrant, PortletRequest request) { try { com.liferay.portal.model.User liferayUser = registrant.addLifeRayUser(request); if (liferayUser != null) { - user = UserUtils.synchronizeUserWithDatabase(registrant, thriftClients, registrant::getEmail, UserUtils::fillThriftUserFromThriftUser); + user = UserUtils.synchronizeUserWithDatabase(registrant, thriftClients, registrant::getEmail, registrant::getExternalid, UserUtils::fillThriftUserFromThriftUser); } } catch (PortalException | SystemException e) { log.error(e); diff --git a/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/users/OrganizationHelper.java b/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/users/OrganizationHelper.java index 5f4e1f2b9f..f54c9d6de6 100644 --- a/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/users/OrganizationHelper.java +++ b/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/users/OrganizationHelper.java @@ -53,7 +53,7 @@ public class OrganizationHelper { public void reassignUserToOrganizationIfNecessary(User user, Organization organization) throws PortalException, SystemException { if (organization != null && userIsNotInOrganization(user, organization.getOrganizationId())) { removeUserFromOtherOrganizations(user); - log.debug("OrganizationHelper adds user " + user.getEmailAddress() + " to the organization " + organization.getName()); + log.info("OrganizationHelper adds user " + user.getEmailAddress() + " to the organization " + organization.getName()); UserLocalServiceUtil.addOrganizationUsers(organization.getOrganizationId(), Collections.singletonList(user)); } } diff --git a/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/users/PortletRequestAdapter.java b/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/users/PortletRequestAdapter.java index 3c21639fec..447ed49540 100644 --- a/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/users/PortletRequestAdapter.java +++ b/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/users/PortletRequestAdapter.java @@ -13,13 +13,10 @@ import com.liferay.portal.kernel.exception.PortalException; import com.liferay.portal.kernel.exception.SystemException; import com.liferay.portal.kernel.servlet.SessionMessages; -import com.liferay.portal.kernel.util.WebKeys; import com.liferay.portal.service.ServiceContext; import com.liferay.portal.service.ServiceContextFactory; -import com.liferay.portal.theme.ThemeDisplay; import javax.portlet.PortletRequest; -import javax.servlet.http.HttpServletRequest; import java.util.Optional; import java.util.function.Consumer; @@ -33,8 +30,7 @@ class PortletRequestAdapter implements RequestAdapter { @Override public long getCompanyId() { - ThemeDisplay themeDisplay = (ThemeDisplay) request.getAttribute(WebKeys.THEME_DISPLAY); - return themeDisplay.getCompanyId(); + return UserUtils.getCompanyId(request); } @Override diff --git a/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/users/SSOAutoLogin.java b/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/users/SSOAutoLogin.java index 459b9a766e..35f8ba6830 100644 --- a/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/users/SSOAutoLogin.java +++ b/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/users/SSOAutoLogin.java @@ -21,6 +21,7 @@ import com.liferay.portal.service.UserLocalServiceUtil; import com.liferay.portal.util.PortalUtil; import org.eclipse.sw360.datahandler.common.CommonUtils; +import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -43,31 +44,29 @@ public class SSOAutoLogin implements AutoLogin { private static final Logger log = LoggerFactory.getLogger(SSOAutoLogin.class); public static final String PROPERTIES_FILE_PATH = "/sw360.properties"; - private Properties props; - + public static final String AUTH_EMAIL_KEY = "key.auth.email"; - public static String authEmailHeader = "EMAIL"; + public static final String AUTH_EMAIL_HEADER; public static final String AUTH_EXTID_KEY = "key.auth.extid"; - public static String authExtidHeader = "EXTID"; + public static final String AUTH_EXTID_HEADER; public static final String AUTH_GIVEN_NAME_KEY = "key.auth.givenname"; - public static String authGivenNameHeader = "GIVENNAME"; + public static final String AUTH_GIVEN_NAME_HEADER; public static final String AUTH_SURNAME_KEY = "key.auth.surname"; - public static String authSurnameHeader = "SURNAME"; + public static final String AUTH_SURNAME_HEADER; public static final String AUTH_DEPARTMENT_KEY = "key.auth.department"; - public static String authDepartmentHeader = "DEPARTMENT"; + public static final String AUTH_DEPARTMENT_HEADER; private static final OrganizationHelper orgHelper = new OrganizationHelper(); - public SSOAutoLogin() { - super(); + static { Properties props = CommonUtils.loadProperties(SSOAutoLogin.class, PROPERTIES_FILE_PATH); - authEmailHeader = props.getProperty(AUTH_EMAIL_KEY, authEmailHeader); - authExtidHeader = props.getProperty(AUTH_EXTID_KEY, authExtidHeader); - authGivenNameHeader = props.getProperty(AUTH_GIVEN_NAME_KEY, authGivenNameHeader); - authSurnameHeader = props.getProperty(AUTH_SURNAME_KEY, authSurnameHeader); - authDepartmentHeader = props.getProperty(AUTH_DEPARTMENT_KEY, authDepartmentHeader); + AUTH_EMAIL_HEADER = props.getProperty(AUTH_EMAIL_KEY, "EMAIL"); + AUTH_EXTID_HEADER = props.getProperty(AUTH_EXTID_KEY, "EXTID"); + AUTH_GIVEN_NAME_HEADER = props.getProperty(AUTH_GIVEN_NAME_KEY, "GIVENNAME"); + AUTH_SURNAME_HEADER = props.getProperty(AUTH_SURNAME_KEY, "SURNAME"); + AUTH_DEPARTMENT_HEADER = props.getProperty(AUTH_DEPARTMENT_KEY, "DEPARTMENT"); log.info(String.format("Expecting the following header values for auto login email: '%s', external ID: '%s', given name: '%s', surname: '%s', group: %s", - authEmailHeader, authExtidHeader, authGivenNameHeader, authSurnameHeader, authDepartmentHeader)); + AUTH_EMAIL_HEADER, AUTH_EXTID_HEADER, AUTH_GIVEN_NAME_HEADER, AUTH_SURNAME_HEADER, AUTH_DEPARTMENT_HEADER)); } @Override @@ -78,18 +77,17 @@ public String[] handleException(HttpServletRequest request, HttpServletResponse @Override public String[] login(HttpServletRequest request, HttpServletResponse response) throws AutoLoginException { - String emailId = request.getHeader(authEmailHeader); - String extid = request.getHeader(authExtidHeader); - String givenName = request.getHeader(authGivenNameHeader); - String surname = request.getHeader(authSurnameHeader); - String department = request.getHeader(authDepartmentHeader); + dumpHeadersToLog(request); + String email = request.getHeader(AUTH_EMAIL_HEADER); + String extId = request.getHeader(AUTH_EXTID_HEADER); + String givenName = request.getHeader(AUTH_GIVEN_NAME_HEADER); + String surname = request.getHeader(AUTH_SURNAME_HEADER); + String department = request.getHeader(AUTH_DEPARTMENT_HEADER); log.info(String.format("Attempting auto login for email: '%s', external ID: '%s', given name: '%s', surname: '%s', group: %s", - emailId, extid, givenName, surname, department)); - - dumpHeadersToLog(request); + email, extId, givenName, surname, department)); - if (isNullEmptyOrWhitespace(emailId)) { + if (isNullEmptyOrWhitespace(email)) { log.error("Empty credentials, auto login impossible."); return new String[]{}; } @@ -100,23 +98,9 @@ public String[] login(HttpServletRequest request, HttpServletResponse response) String organizationName = orgHelper.mapOrganizationName(department); Organization organization = orgHelper.addOrGetOrganization(organizationName, companyId); log.info(String.format("Mapped orgcode %s to %s", department, organizationName)); - User user; - try { - user = UserLocalServiceUtil.getUserByEmailAddress(companyId, emailId); - } catch (NoSuchUserException e) { - log.error("Could not find user with email: '" + emailId + "'. Will create one with a random password."); - String password = UUID.randomUUID().toString(); - - user = UserPortletUtils.addLiferayUser(request, givenName, surname, emailId, - organizationName, RoleConstants.USER, false, extid, password, false, true); - if (user == null) { - throw new AutoLoginException("Couldn't create user for '" + emailId + "' and company id: '" + companyId + "'"); - } - log.info("Created user " + user); - } - + User user = findOrCreateLiferayUser(request, email, extId, givenName, surname, companyId, organizationName); + user = updateLiferayUserEmailIfNecessary(email, user); orgHelper.reassignUserToOrganizationIfNecessary(user, organization); - // Create a return credentials object return new String[]{ String.valueOf(user.getUserId()), @@ -124,11 +108,42 @@ public String[] login(HttpServletRequest request, HttpServletResponse response) Boolean.TRUE.toString() // True: password is encrypted }; } catch (SystemException | PortalException e) { - log.error("Exception during login of user: '" + emailId + "' and company id: '" + companyId + "'", e); + log.error("Exception during login of user: '" + email + "' and company id: '" + companyId + "'", e); throw new AutoLoginException(e); } } + private User updateLiferayUserEmailIfNecessary(String email, User user) throws PortalException, SystemException { + if (!email.equals(user.getEmailAddress())){ + user = UserLocalServiceUtil.updateEmailAddress(user.getUserId(), user.getPasswordUnencrypted(), email, email); + } + return user; + } + + private User findOrCreateLiferayUser(HttpServletRequest request, String email, String extId, String givenName, String surname, long companyId, String organizationName) throws SystemException, PortalException { + User user; + try { + user = UserUtils.findLiferayUser(new HttpServletRequestAdapter(request), email, extId); + } catch (NoSuchUserException e) { + user = createLiferayUser(request, email, extId, givenName, surname, companyId, organizationName); + } + return user; + } + + @NotNull + public User createLiferayUser(HttpServletRequest request, String emailId, String extid, String givenName, String surname, long companyId, String organizationName) throws SystemException, PortalException { + User user; + String password = UUID.randomUUID().toString(); + + user = UserPortletUtils.addLiferayUser(request, givenName, surname, emailId, + organizationName, RoleConstants.USER, false, extid, password, false, true); + if (user == null) { + throw new AutoLoginException("Couldn't create user for '" + emailId + "' and company id: '" + companyId + "'"); + } + log.info("Created user %s", user); + return user; + } + private void dumpHeadersToLog(HttpServletRequest request) { Enumeration headerNames = request.getHeaderNames(); while (headerNames.hasMoreElements()) { diff --git a/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/users/UserCache.java b/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/users/UserCache.java index b5017be0d0..cc58771f73 100644 --- a/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/users/UserCache.java +++ b/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/users/UserCache.java @@ -10,15 +10,14 @@ */ package org.eclipse.sw360.portal.users; -import com.google.common.base.Function; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; import com.google.common.collect.Maps; +import org.apache.thrift.TException; import org.eclipse.sw360.datahandler.thrift.ThriftClients; import org.eclipse.sw360.datahandler.thrift.users.User; import org.eclipse.sw360.datahandler.thrift.users.UserService; -import org.apache.thrift.TException; import java.util.Collections; import java.util.List; @@ -51,13 +50,8 @@ public UserCache() { .expireAfterWrite(1, TimeUnit.DAYS) .build(loader); - if(allUsers.size()>0) { - cache.putAll(Maps.uniqueIndex(allUsers, new Function() { - @Override - public String apply(User input) { - return input.getEmail(); - } - })); + if(!allUsers.isEmpty()) { + cache.putAll(Maps.uniqueIndex(allUsers, User::getEmail)); } } @@ -72,17 +66,19 @@ public User getRefreshed (String email) throws ExecutionException { private static class UserLoader extends CacheLoader { + private UserService.Iface createUserClient() { + ThriftClients thriftClients = new ThriftClients(); + return thriftClients.makeUserClient(); + } + @Override public User load(String email) throws TException { - ThriftClients thriftClients = new ThriftClients(); - UserService.Iface client = thriftClients.makeUserClient(); + UserService.Iface client = createUserClient(); return client.getByEmail(email); } private List getAllUsers() throws TException { - ThriftClients thriftClients = new ThriftClients(); - UserService.Iface client = thriftClients.makeUserClient(); - + UserService.Iface client = createUserClient(); return client.getAllUsers(); } } diff --git a/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/users/UserUtils.java b/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/users/UserUtils.java index 654f343f8a..951b45344f 100644 --- a/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/users/UserUtils.java +++ b/frontend/sw360-portlet/src/main/java/org/eclipse/sw360/portal/users/UserUtils.java @@ -11,6 +11,7 @@ */ package org.eclipse.sw360.portal.users; +import com.liferay.portal.NoSuchUserException; import com.liferay.portal.kernel.exception.PortalException; import com.liferay.portal.kernel.exception.SystemException; import com.liferay.portal.kernel.util.WebKeys; @@ -26,17 +27,16 @@ import org.eclipse.sw360.portal.common.PortalConstants; import org.apache.log4j.Logger; import org.apache.thrift.TException; +import org.jetbrains.annotations.NotNull; import javax.portlet.PortletRequest; import javax.portlet.RenderRequest; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Optional; +import java.util.*; import java.util.function.BiConsumer; import java.util.function.Supplier; import java.util.stream.Collectors; +import static org.eclipse.sw360.datahandler.common.CommonUtils.nullToEmptySet; import static org.eclipse.sw360.datahandler.common.SW360Constants.TYPE_USER; /** @@ -58,32 +58,49 @@ public UserUtils() { thriftClients = new ThriftClients(); } - public static org.eclipse.sw360.datahandler.thrift.users.User synchronizeUserWithDatabase(T source, ThriftClients thriftClients, Supplier emailSupplier, BiConsumer synchronizer) { + public static org.eclipse.sw360.datahandler.thrift.users.User synchronizeUserWithDatabase( + T source, ThriftClients thriftClients, Supplier emailSupplier, + Supplier extIdSupplier, BiConsumer synchronizer) { UserService.Iface client = thriftClients.makeUserClient(); - org.eclipse.sw360.datahandler.thrift.users.User thriftUser = null; + org.eclipse.sw360.datahandler.thrift.users.User existingThriftUser = null; + String email = emailSupplier.get(); try { - thriftUser = client.getByEmail(emailSupplier.get()); + existingThriftUser = client.getByEmailOrExternalId(email, extIdSupplier.get()); } catch (TException e) { //This occurs for every new user, so there is not necessarily something wrong - log.trace("User does not exist in DB yet."); + log.trace("User not found by email or external ID"); } + org.eclipse.sw360.datahandler.thrift.users.User resultUser = null; try { - if (thriftUser == null) { + if (existingThriftUser == null) { log.info("Creating new user."); - thriftUser = new org.eclipse.sw360.datahandler.thrift.users.User(); - synchronizer.accept(thriftUser, source); - client.addUser(thriftUser); + resultUser = new org.eclipse.sw360.datahandler.thrift.users.User(); + synchronizer.accept(resultUser, source); + client.addUser(resultUser); } else { - synchronizer.accept(thriftUser, source); - client.updateUser(thriftUser); + resultUser = existingThriftUser; + if (!existingThriftUser.getEmail().equals(email)) { // email has changed + resultUser.setFormerEmailAddresses(prepareFormerEmailAddresses(existingThriftUser, email)); + } + synchronizer.accept(resultUser, source); + client.updateUser(resultUser); } } catch (TException e) { log.error("Thrift exception when saving the user", e); } - return thriftUser; + return resultUser; + } + + @NotNull + private static Set prepareFormerEmailAddresses(org.eclipse.sw360.datahandler.thrift.users.User thriftUser, String email) { + Set formerEmailAddresses = nullToEmptySet(thriftUser.getFormerEmailAddresses()).stream() + .filter(e -> !e.equals(email)) // make sure the current email is not in the former addresses + .collect(Collectors.toCollection(HashSet::new)); + formerEmailAddresses.add(thriftUser.getEmail()); + return formerEmailAddresses; } public static String displayUser(String email, org.eclipse.sw360.datahandler.thrift.users.User user) { @@ -108,52 +125,56 @@ public static List getOrganizations(RenderRequest request) { return organizations; } - public static void activateLiferayUser(PortletRequest request, org.eclipse.sw360.datahandler.thrift.users.User user){ - Optional liferayUser = findLiferayUser(request, user); + public static void activateLiferayUser(PortletRequest request, org.eclipse.sw360.datahandler.thrift.users.User user) { try { - if (liferayUser.isPresent()) { - UserLocalServiceUtil.updateStatus(liferayUser.get().getUserId(), WorkflowConstants.STATUS_APPROVED); - } + User liferayUser = findLiferayUser(request, user); + UserLocalServiceUtil.updateStatus(liferayUser.getUserId(), WorkflowConstants.STATUS_APPROVED); } catch (SystemException | PortalException e) { log.error("Could not activate Liferay user", e); } } - public static void deleteLiferayUser(PortletRequest request, org.eclipse.sw360.datahandler.thrift.users.User user){ - Optional liferayUser = findLiferayUser(request, user); + public static void deleteLiferayUser(PortletRequest request, org.eclipse.sw360.datahandler.thrift.users.User user) { try { - if (liferayUser.isPresent()){ - UserLocalServiceUtil.deleteUser(liferayUser.get()); - } + User liferayUser = findLiferayUser(request, user); + UserLocalServiceUtil.deleteUser(liferayUser); } catch (PortalException | SystemException e) { log.error("Could not delete Liferay user", e); } } - private static Optional findLiferayUser(PortletRequest request, org.eclipse.sw360.datahandler.thrift.users.User user) { - long companyId = getCompanyId(request); - User liferayUser = null; + public static User findLiferayUser(PortletRequest request, org.eclipse.sw360.datahandler.thrift.users.User user) throws PortalException, SystemException { + return findLiferayUser(new PortletRequestAdapter(request), user.getEmail(), user.getExternalid()); + } + + public static User findLiferayUser(RequestAdapter requestAdapter, String email, String externalId) throws SystemException, PortalException { + long companyId = requestAdapter.getCompanyId(); try { - liferayUser = UserLocalServiceUtil.getUserByEmailAddress(companyId, user.getEmail()); - } catch (PortalException | SystemException e) { - log.error("Could not find Liferay user", e); + return UserLocalServiceUtil.getUserByEmailAddress(companyId, email); + } catch (NoSuchUserException e) { + log.info("Could not find user with email: '" + email + "'. Will try searching by external id."); + try { + return UserLocalServiceUtil.getUserByOpenId(companyId, externalId); + } catch (NoSuchUserException nsue) { + log.info("Could not find user with externalId: '" + externalId); + throw new NoSuchUserException("Couldn't find user either with email or external id", nsue); + } } - return Optional.ofNullable(liferayUser); } - private static long getCompanyId(PortletRequest request) { + + public static long getCompanyId(PortletRequest request) { ThemeDisplay themeDisplay = (ThemeDisplay) request.getAttribute(WebKeys.THEME_DISPLAY); return themeDisplay.getCompanyId(); } public void synchronizeUserWithDatabase(User user) { String userEmailAddress = user.getEmailAddress(); - org.eclipse.sw360.datahandler.thrift.users.User refreshed = UserCacheHolder.getRefreshedUserFromEmail(userEmailAddress); if (!equivalent(refreshed, user)) { - synchronizeUserWithDatabase(user, thriftClients, user::getEmailAddress, UserUtils::fillThriftUserFromLiferayUser); + synchronizeUserWithDatabase(user, thriftClients, user::getEmailAddress, user::getOpenId, UserUtils::fillThriftUserFromLiferayUser); UserCacheHolder.getRefreshedUserFromEmail(userEmailAddress); } } @@ -166,7 +187,6 @@ private boolean equivalent(org.eclipse.sw360.datahandler.thrift.users.User refre public static void fillThriftUserFromUserCSV(final org.eclipse.sw360.datahandler.thrift.users.User thriftUser, final UserCSV userCsv) { thriftUser.setEmail(userCsv.getEmail()); - thriftUser.setId(userCsv.getEmail()); thriftUser.setType(TYPE_USER); thriftUser.setUserGroup(UserGroup.valueOf(userCsv.getGroup())); thriftUser.setExternalid(userCsv.getGid()); @@ -179,7 +199,6 @@ public static void fillThriftUserFromUserCSV(final org.eclipse.sw360.datahandler public static void fillThriftUserFromLiferayUser(final org.eclipse.sw360.datahandler.thrift.users.User thriftUser, final User user) { thriftUser.setEmail(user.getEmailAddress()); - thriftUser.setId(user.getEmailAddress()); thriftUser.setType(TYPE_USER); thriftUser.setUserGroup(getUserGroupFromLiferayUser(user)); thriftUser.setExternalid(user.getOpenId()); @@ -191,7 +210,7 @@ public static void fillThriftUserFromLiferayUser(final org.eclipse.sw360.datahan public static void fillThriftUserFromThriftUser(final org.eclipse.sw360.datahandler.thrift.users.User thriftUser, final org.eclipse.sw360.datahandler.thrift.users.User user) { thriftUser.setEmail(user.getEmail()); - thriftUser.setId(user.getEmail()); + thriftUser.setId(user.getId()); thriftUser.setType(TYPE_USER); thriftUser.setUserGroup(user.getUserGroup()); thriftUser.setExternalid(user.getExternalid()); diff --git a/frontend/sw360-portlet/src/main/webapp/html/preferences/view.jsp b/frontend/sw360-portlet/src/main/webapp/html/preferences/view.jsp index bcf5f5f9a3..5bc29c242e 100644 --- a/frontend/sw360-portlet/src/main/webapp/html/preferences/view.jsp +++ b/frontend/sw360-portlet/src/main/webapp/html/preferences/view.jsp @@ -37,7 +37,7 @@ - Id (E-Mail): + E-mail: diff --git a/libraries/lib-datahandler/src/main/java/org/eclipse/sw360/datahandler/thrift/ThriftValidate.java b/libraries/lib-datahandler/src/main/java/org/eclipse/sw360/datahandler/thrift/ThriftValidate.java index 72494d10b1..a6dc7f542f 100644 --- a/libraries/lib-datahandler/src/main/java/org/eclipse/sw360/datahandler/thrift/ThriftValidate.java +++ b/libraries/lib-datahandler/src/main/java/org/eclipse/sw360/datahandler/thrift/ThriftValidate.java @@ -143,8 +143,6 @@ public static void prepareLicense(License license) throws SW360Exception { public static void prepareUser(User user) throws SW360Exception { // Check required fields assertNotEmpty(user.getEmail()); - // Set id to email, in order to have human readable database - user.setId(user.getEmail()); // Set type user.setType(TYPE_USER); // guarantee that `CommentMadeDuringModerationRequest` is never stored in the database as this is intended to be a transient field diff --git a/libraries/lib-datahandler/src/main/thrift/users.thrift b/libraries/lib-datahandler/src/main/thrift/users.thrift index aabe931217..af2f65ee5b 100644 --- a/libraries/lib-datahandler/src/main/thrift/users.thrift +++ b/libraries/lib-datahandler/src/main/thrift/users.thrift @@ -56,15 +56,26 @@ struct User { 11: optional bool wantsMailNotification, 12: optional string commentMadeDuringModerationRequest, 13: optional map notificationPreferences, + 14: optional set formerEmailAddresses, } service UserService { /** - * returns SW360-user with id equal to email + * returns SW360-user with given id + **/ + User getUser(1:string id); + + /** + * returns SW360-user with given email **/ User getByEmail(1:string email); + /** + * searches for a SW360 user by email, or, if no such user is found, by externalId + **/ + User getByEmailOrExternalId(1:string email, 2:string externalId); + /** * get list of all SW360-users in database with name equal to parameter name **/ diff --git a/libraries/lib-datahandler/src/test/java/org/eclipse/sw360/datahandler/thrift/ThriftValidateTest.java b/libraries/lib-datahandler/src/test/java/org/eclipse/sw360/datahandler/thrift/ThriftValidateTest.java index 307e8c6824..d7c93740b6 100644 --- a/libraries/lib-datahandler/src/test/java/org/eclipse/sw360/datahandler/thrift/ThriftValidateTest.java +++ b/libraries/lib-datahandler/src/test/java/org/eclipse/sw360/datahandler/thrift/ThriftValidateTest.java @@ -17,6 +17,7 @@ import static org.eclipse.sw360.datahandler.thrift.ThriftValidate.prepareUser; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; public class ThriftValidateTest { final String DUMMY_EMAIL_ADDRESS = "dummy.name@dummy.domain.tld"; @@ -36,8 +37,8 @@ public void testPrepareUser() throws Exception { user.setCommentMadeDuringModerationRequest(DUMMY_MODERATION_COMMENT); prepareUser(user); - assertEquals(user.getEmail(), user.getId()); - assertEquals(TYPE_USER,user.getType()); + assertNull(user.getId()); + assertEquals(TYPE_USER, user.getType()); assertFalse(user.isSetCommentMadeDuringModerationRequest()); } } diff --git a/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/core/JacksonCustomizations.java b/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/core/JacksonCustomizations.java index 5c9b9489ac..bdc7ba2224 100644 --- a/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/core/JacksonCustomizations.java +++ b/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/core/JacksonCustomizations.java @@ -166,7 +166,6 @@ static abstract class ProjectMixin extends Project { @JsonInclude(JsonInclude.Include.NON_NULL) @JsonIgnoreProperties({ - "id", "revision", "externalid", "wantsMailNotification", @@ -183,6 +182,9 @@ static abstract class ProjectMixin extends Project { "setDepartment", "notificationPreferencesSize", "setNotificationPreferences", + "formerEmailAddressesSize", + "formerEmailAddressesIterator", + "setFormerEmailAddresses", "setCommentMadeDuringModerationRequest" }) static abstract class UserMixin extends User { diff --git a/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/core/RestControllerHelper.java b/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/core/RestControllerHelper.java index 22b374cba3..811f5c2684 100644 --- a/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/core/RestControllerHelper.java +++ b/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/core/RestControllerHelper.java @@ -40,7 +40,7 @@ import org.springframework.security.oauth2.provider.OAuth2Authentication; import org.springframework.stereotype.Service; -import java.util.Base64; +import java.net.URLEncoder; import java.util.List; import java.util.Set; @@ -97,8 +97,7 @@ public void addEmbeddedUser(HalResource halResource, User user, String relation) User embeddedUser = convertToEmbeddedUser(user); Resource embeddedUserResource = new Resource<>(embeddedUser); try { - String userUUID = Base64.getEncoder().encodeToString(user.getEmail().getBytes("utf-8")); - Link userLink = linkTo(UserController.class).slash("api/users/" + userUUID).withSelfRel(); + Link userLink = linkTo(UserController.class).slash("api/users/" + URLEncoder.encode(user.getId(), "UTF-8")).withSelfRel(); embeddedUserResource.add(userLink); } catch (Exception e) { LOGGER.error("cannot create embedded user with email: " + user.getEmail()); diff --git a/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/user/Sw360UserService.java b/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/user/Sw360UserService.java index 36f802a004..e0fc1ae049 100644 --- a/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/user/Sw360UserService.java +++ b/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/user/Sw360UserService.java @@ -46,6 +46,15 @@ public User getUserByEmail(String email) { } } + public User getUser(String id) { + try { + UserService.Iface sw360UserClient = getThriftUserClient(); + return sw360UserClient.getUser(id); + } catch (TException e) { + throw new RuntimeException(e); + } + } + private UserService.Iface getThriftUserClient() throws TTransportException { THttpClient thriftClient = new THttpClient(thriftServerUrl + "/users/thrift"); TProtocol protocol = new TCompactProtocol(thriftClient); diff --git a/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/user/UserController.java b/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/user/UserController.java index 3418dd21c1..ff0d6d5624 100644 --- a/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/user/UserController.java +++ b/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/user/UserController.java @@ -30,8 +30,8 @@ import org.springframework.web.bind.annotation.RequestMethod; import java.io.UnsupportedEncodingException; +import java.net.URLDecoder; import java.util.ArrayList; -import java.util.Base64; import java.util.List; import static org.springframework.hateoas.mvc.ControllerLinkBuilder.linkTo; @@ -66,18 +66,29 @@ public ResponseEntity>> getUsers() { return new ResponseEntity<>(resources, HttpStatus.OK); } - @RequestMapping(value = USERS_URL + "/{id:.+}", method = RequestMethod.GET) - public ResponseEntity> getUser( - @PathVariable("id") String id) { - byte[] base64decodedBytes = Base64.getDecoder().decode(id); - String decodedId; + // '/users/{xyz}' searches by email, as opposed to by id, as is customary, + // for compatibility with older version of the REST API + @RequestMapping(value = USERS_URL + "/{email:.+}", method = RequestMethod.GET) + public ResponseEntity> getUserByEmail( + @PathVariable("email") String email) { + + String decodedEmail; try { - decodedId = new String(base64decodedBytes, "utf-8"); + decodedEmail = URLDecoder.decode(email, "UTF-8"); } catch (UnsupportedEncodingException e) { - throw (new RuntimeException(e)); + throw new RuntimeException(e); } - User sw360User = userService.getUserByEmail(decodedId); + User sw360User = userService.getUserByEmail(decodedEmail); + HalResource halResource = createHalUser(sw360User); + return new ResponseEntity<>(halResource, HttpStatus.OK); + } + + // unusual URL mapping for compatibility with older version of the REST API (see getUserByEmail()) + @RequestMapping(value = USERS_URL + "/byid/{id:.+}", method = RequestMethod.GET) + public ResponseEntity> getUser( + @PathVariable("id") String id) { + User sw360User = userService.getUser(id); HalResource halResource = createHalUser(sw360User); return new ResponseEntity<>(halResource, HttpStatus.OK); } diff --git a/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/user/UserResourceProcessor.java b/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/user/UserResourceProcessor.java index 024770d456..cdb5740300 100644 --- a/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/user/UserResourceProcessor.java +++ b/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/user/UserResourceProcessor.java @@ -18,7 +18,7 @@ import org.springframework.hateoas.ResourceProcessor; import org.springframework.stereotype.Component; -import java.util.Base64; +import java.net.URLEncoder; import static org.springframework.hateoas.mvc.ControllerLinkBuilder.linkTo; @@ -31,8 +31,7 @@ class UserResourceProcessor implements ResourceProcessor> { public Resource process(Resource resource) { try { User user = resource.getContent(); - String userUUID = Base64.getEncoder().encodeToString(user.getEmail().getBytes("utf-8")); - Link selfLink = linkTo(UserController.class).slash("api/users/" + userUUID).withSelfRel(); + Link selfLink = linkTo(UserController.class).slash("api/users/" + URLEncoder.encode(user.getId(), "UTF-8")).withSelfRel(); resource.add(selfLink); return resource; } catch (Exception e) { diff --git a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/ComponentTest.java b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/ComponentTest.java index f3fa81223d..5b7763cab3 100644 --- a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/ComponentTest.java +++ b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/ComponentTest.java @@ -55,7 +55,7 @@ public void before() throws TException { given(this.componentServiceMock.getComponentsForUser(anyObject())).willReturn(componentList); User user = new User(); - user.setId("admin@sw360.org"); + user.setId("123456789"); user.setEmail("admin@sw360.org"); user.setFullname("John Doe"); diff --git a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/ProjectTest.java b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/ProjectTest.java index cecb7c1455..ed7a617296 100644 --- a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/ProjectTest.java +++ b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/ProjectTest.java @@ -55,7 +55,7 @@ public void before() throws TException { given(this.projectServiceMock.getProjectsForUser(anyObject())).willReturn(projectList); User user = new User(); - user.setId("admin@sw360.org"); + user.setId("123456789"); user.setEmail("admin@sw360.org"); user.setFullname("John Doe"); diff --git a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/UserTest.java b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/UserTest.java index 31c09c6132..e5ede3d6b8 100644 --- a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/UserTest.java +++ b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/UserTest.java @@ -43,13 +43,13 @@ public void before() { List userList = new ArrayList<>(); User user = new User(); - user.setId("admin@sw360.org"); + user.setId("123456789"); user.setEmail("admin@sw360.org"); user.setFullname("John Doe"); userList.add(user); user = new User(); - user.setId("jane@sw360.org"); + user.setId("987654321"); user.setEmail("jane@sw360.org"); user.setFullname("Jane Doe"); userList.add(user); diff --git a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/AttachmentSpecTest.java b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/AttachmentSpecTest.java index bf5f37acc1..ef880f5cec 100644 --- a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/AttachmentSpecTest.java +++ b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/AttachmentSpecTest.java @@ -96,7 +96,7 @@ public void before() throws TException { given(this.attachmentServiceMock.getAttachmentByIdForUser(eq(attachment.getAttachmentContentId()), anyObject())).willReturn(attachmentInfo); User user = new User(); - user.setId("admin@sw360.org"); + user.setId("123456789"); user.setEmail("admin@sw360.org"); user.setFullname("John Doe"); diff --git a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/ComponentSpecTest.java b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/ComponentSpecTest.java index ff9aea0978..8d43bcff83 100644 --- a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/ComponentSpecTest.java +++ b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/ComponentSpecTest.java @@ -110,7 +110,7 @@ public void before() throws TException { given(this.componentServiceMock.searchComponentByName(eq(angularComponent.getName()))).willReturn(componentListByName); User user = new User(); - user.setId("admin@sw360.org"); + user.setId("123456789"); user.setEmail("admin@sw360.org"); user.setFullname("John Doe"); user.setDepartment("sw360"); diff --git a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/ProjectSpecTest.java b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/ProjectSpecTest.java index 9c992c2cf3..44a862d71e 100644 --- a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/ProjectSpecTest.java +++ b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/ProjectSpecTest.java @@ -164,7 +164,7 @@ public void before() throws TException { given(this.releaseServiceMock.getReleaseForUserById(eq(release2.getId()), anyObject())).willReturn(release2); User user = new User(); - user.setId("admin@sw360.org"); + user.setId("123456789"); user.setEmail("admin@sw360.org"); user.setFullname("John Doe"); user.setDepartment("sw360"); diff --git a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/ReleaseSpecTest.java b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/ReleaseSpecTest.java index 83ec7dd9e6..b2bde5bd85 100644 --- a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/ReleaseSpecTest.java +++ b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/ReleaseSpecTest.java @@ -104,7 +104,7 @@ public void before() throws TException { given(this.releaseServiceMock.getReleaseForUserById(eq(release.getId()), anyObject())).willReturn(release); User user = new User(); - user.setId("admin@sw360.org"); + user.setId("123456789"); user.setEmail("admin@sw360.org"); user.setFullname("John Doe"); diff --git a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/UserSpecTest.java b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/UserSpecTest.java index 9fb8077295..ef88e458ad 100644 --- a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/UserSpecTest.java +++ b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/UserSpecTest.java @@ -8,6 +8,7 @@ */ package org.eclipse.sw360.rest.resourceserver.restdocs; +import com.google.common.collect.Sets; import org.eclipse.sw360.datahandler.thrift.users.User; import org.eclipse.sw360.datahandler.thrift.users.UserGroup; import org.eclipse.sw360.rest.resourceserver.TestHelper; @@ -49,25 +50,27 @@ public class UserSpecTest extends TestRestDocsSpecBase { private User user; @Before - public void before() throws UnsupportedEncodingException { + public void before() { List userList = new ArrayList<>(); user = new User(); user.setEmail("admin@sw360.org"); - user.setId(Base64.getEncoder().encodeToString(user.getEmail().getBytes("utf-8"))); + user.setId("4784587578e87989"); user.setUserGroup(UserGroup.ADMIN); user.setFullname("John Doe"); user.setGivenname("John"); user.setLastname("Doe"); user.setDepartment("SW360 Administration"); user.setWantsMailNotification(true); + user.setFormerEmailAddresses(Sets.newHashSet("admin_bachelor@sw360.org")); userList.add(user); given(this.userServiceMock.getUserByEmail("admin@sw360.org")).willReturn(user); + given(this.userServiceMock.getUser("4784587578e87989")).willReturn(user); User user2 = new User(); user2.setEmail("jane@sw360.org"); - user2.setId(Base64.getEncoder().encodeToString(user.getEmail().getBytes("utf-8"))); + user2.setId("frwey45786rwe"); user2.setUserGroup(UserGroup.USER); user2.setFullname("Jane Doe"); user2.setGivenname("Jane"); @@ -97,10 +100,34 @@ public void should_document_get_users() throws Exception { ))); } + @Test + public void should_document_get_user_by_id() throws Exception { + String accessToken = TestHelper.getAccessToken(mockMvc, testUserId, testUserPassword); + mockMvc.perform(get("/api/users/byid/" + user.getId()) + .header("Authorization", "Bearer " + accessToken) + .accept(MediaTypes.HAL_JSON)) + .andExpect(status().isOk()) + .andDo(this.documentationHandler.document( + links( + linkWithRel("self").description("The <>") + ), + responseFields( + fieldWithPath("id").description("The user's id"), + fieldWithPath("email").description("The user's email"), + fieldWithPath("userGroup").description("The user group, possible values are: " + Arrays.asList(UserGroup.values())), + fieldWithPath("fullName").description("The users's full name"), + fieldWithPath("givenName").description("The user's given name"), + fieldWithPath("lastName").description("The user's last name"), + fieldWithPath("department").description("The user's company department"), + fieldWithPath("formerEmailAddresses").description("The user's former email addresses"), + fieldWithPath("_links").description("<> to other resources") + ))); + } + @Test public void should_document_get_user() throws Exception { String accessToken = TestHelper.getAccessToken(mockMvc, testUserId, testUserPassword); - mockMvc.perform(get("/api/users/" + user.getId()) + mockMvc.perform(get("/api/users/" + user.getEmail()) .header("Authorization", "Bearer " + accessToken) .accept(MediaTypes.HAL_JSON)) .andExpect(status().isOk()) @@ -109,12 +136,14 @@ public void should_document_get_user() throws Exception { linkWithRel("self").description("The <>") ), responseFields( + fieldWithPath("id").description("The user's id"), fieldWithPath("email").description("The user's email"), fieldWithPath("userGroup").description("The user group, possible values are: " + Arrays.asList(UserGroup.values())), fieldWithPath("fullName").description("The users's full name"), fieldWithPath("givenName").description("The user's given name"), fieldWithPath("lastName").description("The user's last name"), fieldWithPath("department").description("The user's company department"), + fieldWithPath("formerEmailAddresses").description("The user's former email addresses"), fieldWithPath("_links").description("<> to other resources") ))); } diff --git a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/VulnerabilitySpecTest.java b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/VulnerabilitySpecTest.java index fec2e2cd18..1da4bf159d 100644 --- a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/VulnerabilitySpecTest.java +++ b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/VulnerabilitySpecTest.java @@ -86,7 +86,7 @@ public void before() { vulnerabilityList.add(vulnerability2); User user = new User(); - user.setId("admin@sw360.org"); + user.setId("123456789"); user.setEmail("admin@sw360.org"); user.setFullname("John Doe");