Skip to content

Commit 20bdb72

Browse files
Merge 26.6 to develop
2 parents a2eeb19 + e920dad commit 20bdb72

2 files changed

Lines changed: 190 additions & 74 deletions

File tree

signup/src/org/labkey/signup/SignUpController.java

Lines changed: 163 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package org.labkey.signup;
1818

19+
import jakarta.mail.MessagingException;
1920
import org.apache.commons.lang3.StringUtils;
2021
import org.apache.commons.validator.routines.EmailValidator;
2122
import org.apache.logging.log4j.LogManager;
@@ -54,6 +55,7 @@
5455
import org.labkey.api.security.permissions.ReadPermission;
5556
import org.labkey.api.settings.LookAndFeelProperties;
5657
import org.labkey.api.util.ButtonBuilder;
58+
import org.labkey.api.util.ConfigurationException;
5759
import org.labkey.api.util.DOM;
5860
import org.labkey.api.util.PageFlowUtil;
5961
import org.labkey.api.util.URLHelper;
@@ -62,10 +64,12 @@
6264
import org.labkey.api.view.HtmlView;
6365
import org.labkey.api.view.HttpView;
6466
import org.labkey.api.view.JspView;
67+
import org.labkey.api.view.LabKeyKaptchaServlet;
6568
import org.labkey.api.view.NavTree;
6669
import org.labkey.api.view.WebPartView;
6770
import org.springframework.validation.BindException;
6871
import org.springframework.validation.Errors;
72+
import org.springframework.validation.ObjectError;
6973
import org.springframework.web.servlet.ModelAndView;
7074

7175
import java.util.ArrayList;
@@ -527,7 +531,8 @@ public class BeginAction extends FormViewAction<SignupForm>
527531
@Override
528532
public void validateCommand(SignupForm form, Errors errors)
529533
{
530-
validateSignupForm(form, errors);
534+
// All validation happens in handlePost so the order matches SignUpApiAction
535+
// (captcha first, then blank-field checks, then email parsing, etc.).
531536
}
532537

533538
@Override
@@ -559,54 +564,50 @@ public ModelAndView getView(SignupForm form, boolean reshow, BindException error
559564
@Override
560565
public boolean handlePost(SignupForm signupForm, BindException errors) throws Exception
561566
{
562-
// Validate with EmailValidator first. ValidEmail(email) will not throw an exception if the domain is
563-
// missing from the email. The default domain configured for the server is appended.
564-
EmailValidator validator = EmailValidator.getInstance();
565-
if(!validator.isValid(signupForm.getEmail()))
567+
String kaptchaError = verifyCaptcha(signupForm.getKaptchaText(), signupForm.getEmail());
568+
if (kaptchaError != null)
566569
{
567-
errors.reject(ERROR_MSG,"'" + signupForm.getEmail() + "' is not a valid email address.");
570+
errors.reject(ERROR_MSG, kaptchaError);
568571
return false;
569572
}
570573

571-
ValidEmail email;
572-
try
574+
validateSignupForm(signupForm, errors);
575+
if (errors.hasErrors())
573576
{
574-
email = new ValidEmail(signupForm.getEmail());
577+
return false;
575578
}
576-
catch (ValidEmail.InvalidEmailException iee)
579+
580+
ValidEmail email = parseAndValidateEmail(signupForm, errors);
581+
if (email == null)
577582
{
578-
errors.reject(ERROR_MSG, iee.getMessage());
579583
return false;
580584
}
581585

582-
if(UserManager.userExists(email))
586+
if (UserManager.userExists(email))
583587
{
584588
// If the user already exists forward them to a page where they can click on a link to recover their password, if required
585589
signupForm.setAccountExists(true);
586590
signupForm.setNewSignUp(false);
587591
return false;
588592
}
589593

590-
// If the user does not exit in LabKey's core database, check in our temporaryusers table
591-
TempUser tempUser = getTempUser(signupForm, email);
592-
593-
// Send email to the user.
594-
ActionURL confirmationUrl = getConfirmationURL(getContainer(), email, tempUser.getKey());
595594
try
596595
{
597-
User mockUser = new User();
598-
mockUser.setEmail(email.getEmailAddress());
599-
SecurityManager.sendEmail(getContainer(), mockUser, SecurityManager.getRegistrationMessage(null, false), email.getEmailAddress(), confirmationUrl);
596+
createUserAndSendEmail(signupForm, email);
600597
}
601-
catch(Exception e)
598+
catch (MessagingException | ConfigurationException e)
602599
{
603-
String systemEmail = LookAndFeelProperties.getInstance(getContainer()).getSystemEmailAddress();
604-
errors.reject(ERROR_MSG, "Could not send new user registration email. Please contact your server administrator at " + systemEmail);
605-
errors.reject(ERROR_MSG, e.getMessage());
600+
errors.reject(ERROR_MSG, sendEmailErrorMessage(getContainer()));
601+
if (e.getMessage() != null)
602+
{
603+
errors.reject(ERROR_MSG, e.getMessage());
604+
}
606605
return false;
607606
}
608607

608+
clearCaptcha();
609609

610+
// Re-render the JSP with the CONFIRMATION_SENT message.
610611
signupForm.setNewSignUp(false);
611612
return false;
612613
}
@@ -641,6 +642,94 @@ private void validateSignupForm(SignupForm form, Errors errors)
641642
{
642643
errors.reject(ERROR_MSG, "Email cannot be blank.");
643644
}
645+
if(StringUtils.isBlank(form.getEmailConfirm()))
646+
{
647+
errors.reject(ERROR_MSG, "Confirm email cannot be blank.");
648+
}
649+
else if(!StringUtils.isBlank(form.getEmail()) && !form.getEmail().equalsIgnoreCase(form.getEmailConfirm()))
650+
{
651+
errors.reject(ERROR_MSG, "Email addresses do not match.");
652+
}
653+
}
654+
655+
// On success returns null. On failure returns a user-facing error message.
656+
// Does not clear the session attribute — callers clear it after the full operation
657+
// succeeds so the user can retry with the same captcha if a later step fails.
658+
// Logging matches LoginController's RegisterUserAction.
659+
private String verifyCaptcha(String submittedText, String emailForLogging)
660+
{
661+
var session = getViewContext().getRequest().getSession(true);
662+
String expected = (String) session.getAttribute(LabKeyKaptchaServlet.SESSION_KEY_VALUE);
663+
if (expected == null)
664+
{
665+
_log.info("Captcha not initialized for signup attempt");
666+
return "Captcha not initialized, please retry.";
667+
}
668+
if (!expected.equalsIgnoreCase(StringUtils.trimToNull(submittedText)))
669+
{
670+
_log.warn("Captcha text did not match for signup attempt for {}", emailForLogging);
671+
return "Verification text does not match, please retry.";
672+
}
673+
return null;
674+
}
675+
676+
private void clearCaptcha()
677+
{
678+
getViewContext().getRequest().getSession(true).removeAttribute(LabKeyKaptchaServlet.SESSION_KEY_VALUE);
679+
}
680+
681+
// Returns a parsed ValidEmail, or null if the address is invalid (errors populated).
682+
// Uses EmailValidator first because ValidEmail's constructor does not throw on bare
683+
// strings like "foo" - it silently appends the server's default domain.
684+
private ValidEmail parseAndValidateEmail(SignupForm form, Errors errors)
685+
{
686+
EmailValidator validator = EmailValidator.getInstance();
687+
if (!validator.isValid(form.getEmail()))
688+
{
689+
errors.reject(ERROR_MSG, "'" + form.getEmail() + "' is not a valid email address.");
690+
return null;
691+
}
692+
try
693+
{
694+
return new ValidEmail(form.getEmail());
695+
}
696+
catch (ValidEmail.InvalidEmailException iee)
697+
{
698+
errors.reject(ERROR_MSG, iee.getMessage());
699+
return null;
700+
}
701+
}
702+
703+
// Creates (or reuses) a TempUser row and sends the confirmation email in a single
704+
// transaction. On send failure the exception propagates and the transaction rolls back
705+
// automatically so a freshly inserted TempUser row does not persist.
706+
private void createUserAndSendEmail(SignupForm form, ValidEmail email)
707+
throws MessagingException, ConfigurationException, java.sql.SQLException
708+
{
709+
try (DbScope.Transaction transaction = SignUpManager.getSchema().getScope().ensureTransaction())
710+
{
711+
TempUser tempUser = getTempUser(form, email);
712+
ActionURL confirmationUrl = getConfirmationURL(getContainer(), email, tempUser.getKey());
713+
User mockUser = new User();
714+
mockUser.setEmail(email.getEmailAddress());
715+
SecurityManager.sendEmail(getContainer(), mockUser,
716+
SecurityManager.getRegistrationMessage(null, false),
717+
email.getEmailAddress(), confirmationUrl);
718+
transaction.commit();
719+
}
720+
}
721+
722+
private static List<String> errorsToMessages(Errors errors)
723+
{
724+
return errors.getAllErrors().stream()
725+
.map(ObjectError::getDefaultMessage)
726+
.toList();
727+
}
728+
729+
private static String sendEmailErrorMessage(Container container)
730+
{
731+
return "Could not send new user registration email. Please contact your server administrator at "
732+
+ LookAndFeelProperties.getInstance(container).getSystemEmailAddress();
644733
}
645734

646735
public static ActionURL getConfirmationURL(Container c, ValidEmail email, String key)
@@ -657,6 +746,7 @@ public static class SignupForm extends ReturnUrlForm
657746
private String _lastName;
658747
private String _organization;
659748
private String _email;
749+
private String _emailConfirm;
660750
private boolean _accountExists;
661751
private boolean _newSignUp = true;
662752

@@ -700,6 +790,16 @@ public void setEmail(String email)
700790
_email = email;
701791
}
702792

793+
public String getEmailConfirm()
794+
{
795+
return _emailConfirm;
796+
}
797+
798+
public void setEmailConfirm(String emailConfirm)
799+
{
800+
_emailConfirm = emailConfirm;
801+
}
802+
703803
public boolean isAccountExists()
704804
{
705805
return _accountExists;
@@ -719,6 +819,18 @@ public void setNewSignUp(boolean newSignUp)
719819
{
720820
_newSignUp = newSignUp;
721821
}
822+
823+
private String _kaptchaText;
824+
825+
public String getKaptchaText()
826+
{
827+
return _kaptchaText;
828+
}
829+
830+
public void setKaptchaText(String kaptchaText)
831+
{
832+
_kaptchaText = kaptchaText;
833+
}
722834
}
723835

724836
@RequiresLogin
@@ -778,52 +890,56 @@ public ApiResponse execute(SignupForm signupForm, BindException errors) throws E
778890
{
779891
ApiSimpleResponse response = new ApiSimpleResponse();
780892

781-
ValidEmail email;
782-
try
893+
String kaptchaError = verifyCaptcha(signupForm.getKaptchaText(), signupForm.getEmail());
894+
if (kaptchaError != null)
783895
{
784-
email = new ValidEmail(signupForm.getEmail());
896+
response.put("status", "ERROR");
897+
response.put("error_message", List.of(kaptchaError));
898+
return response;
785899
}
786-
catch (ValidEmail.InvalidEmailException iee)
900+
901+
validateSignupForm(signupForm, errors);
902+
if (errors.hasErrors())
787903
{
788-
errors.reject(ERROR_MSG, iee.getMessage());
904+
response.put("status", "ERROR");
905+
response.put("error_message", errorsToMessages(errors));
789906
return response;
790907
}
791908

792-
if(UserManager.userExists(email))
909+
ValidEmail email = parseAndValidateEmail(signupForm, errors);
910+
if (email == null)
793911
{
794-
response.put("status", "USER_EXISTS");
912+
response.put("status", "ERROR");
913+
response.put("error_message", errorsToMessages(errors));
795914
return response;
796915
}
797916

798-
validateSignupForm(signupForm, errors);
799-
if(errors.hasErrors())
917+
if (UserManager.userExists(email))
800918
{
919+
response.put("status", "USER_EXISTS");
801920
return response;
802921
}
803922

804-
TempUser tempUser = getTempUser(signupForm, email);
805-
806-
807-
// Send email to the user.
808-
ActionURL confirmationUrl = getConfirmationURL(getContainer(), email, tempUser.getKey());
809923
try
810924
{
811-
User mockUser = new User();
812-
mockUser.setEmail(email.getEmailAddress());
813-
SecurityManager.sendEmail(getContainer(), mockUser, SecurityManager.getRegistrationMessage(null, false), email.getEmailAddress(), confirmationUrl);
925+
createUserAndSendEmail(signupForm, email);
814926
}
815-
catch(Exception e)
927+
catch (MessagingException | ConfigurationException e)
816928
{
817-
String systemEmail = LookAndFeelProperties.getInstance(getContainer()).getSystemEmailAddress();
929+
response.put("status", "ERROR");
818930
List<String> messages = new ArrayList<>();
819-
messages.add("Could not send new user registration email. Please contact your server administrator at " + systemEmail);
820-
messages.add(e.getMessage());
931+
messages.add(sendEmailErrorMessage(getContainer()));
932+
if (e.getMessage() != null)
933+
{
934+
messages.add(e.getMessage());
935+
}
821936
response.put("error_message", messages);
937+
return response;
822938
}
823939

824-
signupForm.setNewSignUp(false); // TODO: Most likely not needed here
825-
response.put("status", "USER_ADDED");
940+
clearCaptcha();
826941

942+
response.put("status", "USER_ADDED");
827943
return response;
828944
}
829945
}

0 commit comments

Comments
 (0)