diff --git a/signup/resources/schemas/signup.xml b/signup/resources/schemas/signup.xml index 10174307..3a512d02 100644 --- a/signup/resources/schemas/signup.xml +++ b/signup/resources/schemas/signup.xml @@ -28,14 +28,26 @@ - + + + UserId + core + UsersData + + - + + + UserId + core + UsersData + + diff --git a/signup/src/org/labkey/signup/SignUpAdmin.jsp b/signup/src/org/labkey/signup/SignUpAdmin.jsp index e073d79f..bd2987de 100644 --- a/signup/src/org/labkey/signup/SignUpAdmin.jsp +++ b/signup/src/org/labkey/signup/SignUpAdmin.jsp @@ -55,7 +55,7 @@

Add new user group rule

> - <%for(Container c: list) { m.put(String.valueOf(c.getRowId()), SecurityManager.getGroups(c.getProject(), false));%> @@ -82,7 +82,7 @@ - + <%} }%> diff --git a/signup/src/org/labkey/signup/SignUpController.java b/signup/src/org/labkey/signup/SignUpController.java index 9ad907c9..1c6e74b9 100644 --- a/signup/src/org/labkey/signup/SignUpController.java +++ b/signup/src/org/labkey/signup/SignUpController.java @@ -17,6 +17,7 @@ package org.labkey.signup; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.validator.routines.EmailValidator; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.labkey.api.action.ApiResponse; @@ -36,34 +37,55 @@ import org.labkey.api.data.DbScope; import org.labkey.api.data.PropertyManager; import org.labkey.api.data.Table; +import org.labkey.api.portal.ProjectUrls; +import org.labkey.api.security.AuthenticationManager; import org.labkey.api.security.Group; +import org.labkey.api.security.PasswordRule; import org.labkey.api.security.RequiresLogin; import org.labkey.api.security.RequiresNoPermission; import org.labkey.api.security.RequiresPermission; import org.labkey.api.security.RequiresSiteAdmin; import org.labkey.api.security.SecurityManager; -import org.labkey.api.security.SecurityMessage; import org.labkey.api.security.User; import org.labkey.api.security.UserManager; import org.labkey.api.security.ValidEmail; import org.labkey.api.security.permissions.ReadPermission; import org.labkey.api.settings.LookAndFeelProperties; +import org.labkey.api.util.Button; +import org.labkey.api.util.DOM; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.URLHelper; +import org.labkey.api.util.element.CsrfInput; import org.labkey.api.view.ActionURL; import org.labkey.api.view.HtmlView; +import org.labkey.api.view.HttpView; import org.labkey.api.view.JspView; import org.labkey.api.view.NavTree; import org.labkey.api.view.WebPartView; import org.springframework.validation.BindException; import org.springframework.validation.Errors; import org.springframework.web.servlet.ModelAndView; -import org.springframework.web.servlet.view.RedirectView; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import static org.labkey.api.security.AuthenticationManager.AuthenticationStatus.Success; +import static org.labkey.api.util.DOM.Attribute.method; +import static org.labkey.api.util.DOM.Attribute.name; +import static org.labkey.api.util.DOM.Attribute.style; +import static org.labkey.api.util.DOM.Attribute.type; +import static org.labkey.api.util.DOM.Attribute.value; +import static org.labkey.api.util.DOM.DIV; +import static org.labkey.api.util.DOM.FORM; +import static org.labkey.api.util.DOM.INPUT; +import static org.labkey.api.util.DOM.LABEL; +import static org.labkey.api.util.DOM.TABLE; +import static org.labkey.api.util.DOM.TD; +import static org.labkey.api.util.DOM.TR; +import static org.labkey.api.util.DOM.at; +import static org.labkey.api.util.DOM.cl; + public class SignUpController extends SpringActionController { private static final Logger _log = LogManager.getLogger(SignUpController.class); @@ -111,10 +133,10 @@ public URLHelper getSuccessURL(AddPropertyForm addPropertyForm) @Override public boolean handlePost(AddPropertyForm addPropertyForm, BindException errors) throws Exception { - Container c = ContainerManager.getForRowId(addPropertyForm.getContainerId()); + Container c = ContainerManager.getForRowId(addPropertyForm.getFolderId()); if(c == null) { - errors.addError(new LabKeyError("No container found for rowId " + addPropertyForm.getContainerId())); + errors.addError(new LabKeyError("No container found for rowId " + addPropertyForm.getFolderId())); return false; } PropertyManager.PropertyMap m = PropertyManager.getWritableProperties(c, SignUpModule.SIGNUP_CATEGORY, true); @@ -148,10 +170,10 @@ public URLHelper getSuccessURL(ContainerIdForm containerIdForm) @Override public boolean handlePost(ContainerIdForm containerIdForm, BindException errors) throws Exception { - Container c = ContainerManager.getForRowId(containerIdForm.getContainerId()); + Container c = ContainerManager.getForRowId(containerIdForm.getFolderId()); if(c == null) { - errors.addError(new LabKeyError("No container found for rowId " + containerIdForm.getContainerId())); + errors.addError(new LabKeyError("No container found for rowId " + containerIdForm.getFolderId())); return false; } PropertyManager.PropertyMap m = PropertyManager.getWritableProperties(c, SignUpModule.SIGNUP_CATEGORY, true); @@ -244,16 +266,16 @@ public void validateCommand(AddGroupChangeForm target, Errors errors) public static class ContainerIdForm { - private int _containerId; + private int _folderId; // Changed to _folderId since containerId is a disallowed parameter name: HasAllowBindParameter.disallowed - public int getContainerId() + public int getFolderId() { - return _containerId; + return _folderId; } - public void setContainerId(int containerId) + public void setFolderId(int folderId) { - _containerId = containerId; + _folderId = folderId; } } @@ -313,109 +335,186 @@ public void setNewgroup(int newgroup) // Class ConfirmAction handles a user trying to confirm an account creation. If the email and confirmation code match // that in our database they will be added to the LabKey user base @RequiresNoPermission - public class ConfirmAction extends SimpleViewAction + public class ConfirmAction extends FormViewAction { + protected URLHelper _successUrl = null; + private TempUser _tempUser = null; + private ValidEmail _email = null; + @Override - public ModelAndView getView(SignupConfirmForm form, BindException errors) throws Exception + public ModelAndView getView(SignupConfirmForm form, boolean reshow, BindException errors) throws Exception + { + if (!validateRequest(form, errors)) + { + return new SimpleErrorView(errors, false); + } + + HtmlView view = new HtmlView("Set Password", DIV( + DIV(DOM.LK.ERRORS(errors)), + FORM(at(method, "POST"), + new CsrfInput(HttpView.currentContext()), + TABLE(cl("lk-fields-table"), + TR(TD(cl("labkey-form-label").at(style, "padding-right:10px;"), LABEL("Password: ")), + TD(INPUT(at(type, "password", name, "password", value, form.getPassword())))), + TR(TD(cl("labkey-form-label").at(style, "padding-right:10px;"), LABEL("Confirm Password: ")), + TD(INPUT(at(type, "password", name, "password2", value, form.getPassword2())))) + ), + DIV(at(style, "margin-top:10px"), + new Button.ButtonBuilder("Submit").submit(true).style("margin-right:10px").build(), + new Button.ButtonBuilder("Cancel").href(PageFlowUtil.urlProvider(ProjectUrls.class).getHomeURL()).build() + ) + ) + )); + view.setFrame(WebPartView.FrameType.PORTAL); + return view; + } + + private boolean validateRequest(SignupConfirmForm form, BindException errors) { - ValidEmail email; try { - email = new ValidEmail(form.getEmail()); + _email = new ValidEmail(form.getEmail()); } catch (ValidEmail.InvalidEmailException iee) { errors.reject(ERROR_MSG, "Invalid email address " + form.getEmail() + " in account confirmation request."); - return new SimpleErrorView(errors, false); + return false; } String key = form.getKey(); // Check parameter email & verification(key) match that in database - TempUser tempUser = SignUpManager.get().verifyUser(email, key); - if (tempUser != null) + _tempUser = SignUpManager.get().verifyUser(_email, key); + if (_tempUser != null) { - if (UserManager.getUser(email) != null) + if (UserManager.getUser(_email) != null) { // First check if user already exists. errors.addError(new LabKeyError(String.format(SignUpManager.USER_ALREADY_EXISTS, form.getEmail()))); - return new SimpleErrorView(errors, false); + return false; } - else + } + else + { + errors.addError(new LabKeyError(SignUpManager.CONFIRMATION_DID_NOT_MATCH)); + return false; + } + return true; + } + + @Override + public boolean handlePost(SignupConfirmForm form, BindException errors) throws Exception + { + if (!validateRequest(form, errors)) + { + return false; + } + + // User creation should be in a transaction + try (DbScope.Transaction transaction = CoreSchema.getInstance().getSchema().getScope().ensureTransaction()) + { + // Add user to LabKey core database + SecurityManager.NewUserStatus newUserStatus = SecurityManager.addUser(_email, null); + User newUser = newUserStatus.getUser(); + // Set user's first, last, and organization name + newUser.setFirstName(_tempUser.getFirstName()); + newUser.setLastName(_tempUser.getLastName()); + newUser.setDescription(StringUtils.isBlank(_tempUser.getOrganization()) ? "" : "Organization: " + _tempUser.getOrganization()); // don't add anything if organization is empty + + // ---------------------------------------------------------------------------------------------- // + // Copied from LoginController.attemptSetPassword(ValidEmail email, URLHelper returnUrlHelper, String auditMessage, boolean clearVerification, BindException errors) + // Can attemptSetPassword() be added to SecurityManager or AuthenticationManager? + // Can DbLoginManager be moved from platform.core.main to platform.api.main? + // ----------------------------------------------------------------------------------------------- + String password = form.getPassword(); + String password2 = form.getPassword2(); + List messages = new ArrayList<>(); + // Cannot use DbLoginManager.getPasswordRule() because DbLoginManager is not an API class + // Can DbLoginManager be moved from platform.core.main to platform.api.main? + // On skyline.ms the selected "Password Strength" is "Weak" + if (!PasswordRule.Weak.isValidToStore(password, password2, newUser, messages)) + { + messages.forEach(message -> errors.reject("setPassword", message)); + return false; + } + try + { + SecurityManager.setPassword(_email, password); + } + catch (SecurityManager.UserManagementException e) { - SecurityManager.NewUserStatus newUserStatus; + errors.reject("setPassword", "Setting password failed: " + e.getMessage() + ". Contact the " + LookAndFeelProperties.getInstance(ContainerManager.getRoot()).getShortName() + " team."); + return false; + } - // User creation should be in a transaction - try (DbScope.Transaction transaction = CoreSchema.getInstance().getSchema().getScope().ensureTransaction()) - { - // Add user to LabKey core database - newUserStatus = SecurityManager.addUser(new ValidEmail(email.getEmailAddress()), null); - User newUser = newUserStatus.getUser(); - // Set user's first, last, and organization name - newUser.setFirstName(tempUser.getFirstName()); - newUser.setLastName(tempUser.getLastName()); - newUser.setDescription(StringUtils.isBlank(tempUser.getOrganization()) ? "" : "Organization: " + tempUser.getOrganization()); // don't add anything if organization is empty - - Container c = tempUser.getContainer(); - PropertyManager.PropertyMap property = PropertyManager.getWritableProperties(c, SignUpModule.SIGNUP_CATEGORY, false); - if (property != null && property.get(SignUpModule.SIGNUP_GROUP_NAME) != null) - { - Integer groupId = SecurityManager.getGroupId(c.getProject(), property.get(SignUpModule.SIGNUP_GROUP_NAME)); - if (groupId != null) - { - final Group group = SecurityManager.getGroup(groupId); - SecurityManager.addMember(group, newUser); - UserManager.updateUser(newUser, newUser); // Update user in LabKey core database. - } - } - tempUser.setLabkeyUserId(newUser.getUserId()); - Table.update(null, SignUpManager.getTableInfoTempUsers(), tempUser, tempUser.getUserId()); - - transaction.commit(); - } + SecurityManager.setVerification(_email, null); + UserManager.addToUserHistory(newUser, "Verified and chose a password."); + + if (getUser().isGuest()) + { + AuthenticationManager.PrimaryAuthenticationResult result = AuthenticationManager.authenticate(getViewContext().getRequest(), + _email.getEmailAddress(), password, PageFlowUtil.urlProvider(ProjectUrls.class).getHomeURL(), true); - // Send email to site admin - ActionURL confirmationUrl = getConfirmationURL(getContainer(), email, tempUser.getKey()); - // _log.info("Confirmation URL: " + confirmationUrl.getLocalURIString()); - String siteAdminEmail = LookAndFeelProperties.getInstance(getContainer()).getSystemEmailAddress(); - // _log.info("Site admin email: " + siteAdminEmail); - User newUser = newUserStatus.getUser(); - // _log.info("New user: " + newUser.getEmail()); - try + if (result.getStatus() == Success) { - SecurityMessage msg = SecurityManager.getRegistrationMessage(null, true); - msg.setTo(email.getEmailAddress()); // This will add the new user's email address to - // the message body and subject. - // newUser is used for auditing purposes, the user who originated the message - SecurityManager.sendEmail(getContainer(), newUser, msg, siteAdminEmail, confirmationUrl); + // This user has passed primary authentication + AuthenticationManager.setPrimaryAuthenticationResult(getViewContext().getRequest(), result); } - catch(Exception e) + } + + AuthenticationManager.AuthenticationResult result = AuthenticationManager.handleAuthentication(getViewContext().getRequest(), getContainer()); + + if (errors.hasErrors()) + return false; + if (result != null) + _successUrl = result.getRedirectURL(); + + _tempUser.setLabkeyUserId(newUser.getUserId()); + Table.update(null, SignUpManager.getTableInfoTempUsers(), _tempUser, _tempUser.getUserId()); + + Container c = _tempUser.getContainer(); + PropertyManager.PropertyMap property = PropertyManager.getWritableProperties(c, SignUpModule.SIGNUP_CATEGORY, false); + if (property != null && property.get(SignUpModule.SIGNUP_GROUP_NAME) != null) + { + Integer groupId = SecurityManager.getGroupId(c.getProject(), property.get(SignUpModule.SIGNUP_GROUP_NAME)); + if (groupId != null) { - _log.error(e); + final Group group = SecurityManager.getGroup(groupId); + SecurityManager.addMember(group, newUser); + UserManager.updateUser(newUser, newUser); // Update user in LabKey core database. } - - // creates and redirects the new user to their setPasswordAction page - ActionURL url = SecurityManager.createVerificationURL(getContainer(), email, newUserStatus.getVerification(), null); - return new ModelAndView(new RedirectView(url.getURIString())); } + + transaction.commit(); } - else - { - errors.addError(new LabKeyError(SignUpManager.CONFIRMATION_DID_NOT_MATCH)); - return new SimpleErrorView(errors, false); - } + + return true; } @Override public void addNavTrail(NavTree root) { } + + @Override + public void validateCommand(SignupConfirmForm target, Errors errors) + { + + } + + @Override + public URLHelper getSuccessURL(SignupConfirmForm signupConfirmForm) + { + return _successUrl; + } } public static class SignupConfirmForm { private String _email; private String _key; + private String _password; + private String _password2; public String getEmail() { @@ -436,6 +535,26 @@ public void setKey(String key) { _key = key; } + + public String getPassword() + { + return _password; + } + + public void setPassword(String password) + { + _password = password; + } + + public String getPassword2() + { + return _password2; + } + + public void setPassword2(String password2) + { + _password2 = password2; + } } @@ -479,6 +598,15 @@ public ModelAndView getView(SignupForm form, boolean reshow, BindException error @Override public boolean handlePost(SignupForm signupForm, BindException errors) throws Exception { + // Validate with EmailValidator first. ValidEmail(email) will not throw an exception if the domain is + // missing from the email. The default domain configured for the server is appended. + EmailValidator validator = EmailValidator.getInstance(); + if(!validator.isValid(signupForm.getEmail())) + { + errors.reject(ERROR_MSG,"'" + signupForm.getEmail() + "' is not a valid email address."); + return false; + } + ValidEmail email; try { diff --git a/signup/src/org/labkey/signup/SignUpSchema.java b/signup/src/org/labkey/signup/SignUpSchema.java index 5de53c56..ca6bbf28 100644 --- a/signup/src/org/labkey/signup/SignUpSchema.java +++ b/signup/src/org/labkey/signup/SignUpSchema.java @@ -34,14 +34,11 @@ import org.labkey.api.query.FieldKey; import org.labkey.api.query.FilteredTable; import org.labkey.api.query.QuerySchema; +import org.labkey.api.query.UserIdQueryForeignKey; import org.labkey.api.query.UserSchema; import org.labkey.api.security.Group; import org.labkey.api.security.User; -import org.labkey.api.security.UserUrls; import org.labkey.api.security.permissions.AdminPermission; -import org.labkey.api.util.PageFlowUtil; -import org.labkey.api.util.StringExpression; -import org.labkey.api.util.StringExpressionFactory; import org.labkey.api.view.ActionURL; import java.io.IOException; @@ -85,12 +82,7 @@ public TableInfo createTable(String name, ContainerFilter containerFilter) ContainerForeignKey.initColumn(result.getMutableColumn(FieldKey.fromParts("Container")), this); var userIdCol = result.getMutableColumn(FieldKey.fromParts("labkeyUserId")); - userIdCol.setDisplayColumnFactory(colInfo -> { - DataColumn col = new DataColumn(colInfo); - StringExpression strExpr = StringExpressionFactory.create(PageFlowUtil.urlProvider(UserUrls.class).getUserDetailsURL(getContainer(), null) + "userId=${labkeyUserId}"); - col.setURLExpression(strExpr); - return col; - }); + userIdCol.setFk(new UserIdQueryForeignKey(this, true)); if(name.equals(TABLE_MOVED_USERS)) {
<%=h(c.getPath())%> <%=h(property.get(SignUpModule.SIGNUP_GROUP_NAME))%><%=link("Remove", urlFor(RemovePropertyAction.class).addParameter("containerId", c.getRowId())).usePost()%><%=link("Remove", urlFor(RemovePropertyAction.class).addParameter("folderId", c.getRowId())).usePost()%>