diff --git a/signup/resources/schemas/dbscripts/postgresql/signup-26.000-26.001.sql b/signup/resources/schemas/dbscripts/postgresql/signup-26.000-26.001.sql
new file mode 100644
index 00000000..a6610971
--- /dev/null
+++ b/signup/resources/schemas/dbscripts/postgresql/signup-26.000-26.001.sql
@@ -0,0 +1,19 @@
+/*
+ * Copyright (c) 2026 LabKey Corporation
+ *
+ * Licensed under the Apache License, Version 2.0: http://www.apache.org/licenses/LICENSE-2.0
+ */
+
+-- Room for the sha256: prefix plus a 64-character hash. NULL means the key was used to create an account.
+ALTER TABLE signup.temporaryuser ALTER COLUMN key TYPE VARCHAR(100);
+ALTER TABLE signup.temporaryuser ALTER COLUMN key DROP NOT NULL;
+ALTER TABLE signup.temporaryuser ADD keyTimeout TIMESTAMP;
+
+-- Rows with a LabKey userId already used their key to create the account.
+UPDATE signup.temporaryuser SET key = NULL WHERE labkeyUserId IS NOT NULL;
+
+-- Must match SignUpManager.hashKey(). Pending links expire seven days after upgrade.
+UPDATE signup.temporaryuser
+SET key = 'sha256:' || encode(sha256(convert_to(key, 'UTF8')), 'hex'),
+ keyTimeout = LOCALTIMESTAMP + INTERVAL '7 days'
+WHERE key IS NOT NULL;
diff --git a/signup/resources/schemas/signup.xml b/signup/resources/schemas/signup.xml
index 35e7bd26..943f0c15 100644
--- a/signup/resources/schemas/signup.xml
+++ b/signup/resources/schemas/signup.xml
@@ -27,6 +27,7 @@
+
diff --git a/signup/src/org/labkey/signup/SignUpController.java b/signup/src/org/labkey/signup/SignUpController.java
index a51533fd..54d0c64b 100644
--- a/signup/src/org/labkey/signup/SignUpController.java
+++ b/signup/src/org/labkey/signup/SignUpController.java
@@ -450,6 +450,11 @@ public boolean handlePost(SignupConfirmForm form, BindException errors) throws E
// User creation should be in a transaction
try (DbScope.Transaction transaction = CoreSchema.getInstance().getSchema().getScope().ensureTransaction())
{
+ // Clear the key so it cannot be reused. Returns false if the key was replaced, cleared or expired since
+ // validateRequest. The reshown view reports the error.
+ if (!SignUpManager.get().clearKeyIfValid(_tempUser))
+ return false;
+
// Add user to LabKey core database
SecurityManager.NewUserStatus newUserStatus = SecurityManager.addUser(_email, null);
User newUser = newUserStatus.getUser();
@@ -466,8 +471,7 @@ public boolean handlePost(SignupConfirmForm form, BindException errors) throws E
if (result != null)
_successUrl = result.getRedirectURL();
- _tempUser.setLabkeyUserId(newUser.getUserId());
- Table.update(null, SignUpManager.getTableInfoTempUsers(), _tempUser, _tempUser.getUserId());
+ SignUpManager.get().setLabkeyUserId(_tempUser, newUser.getUserId());
Container c = _tempUser.getContainer();
WritablePropertyMap property = PropertyManager.getWritableProperties(c, SignUpModule.SIGNUP_CATEGORY, false);
@@ -721,21 +725,22 @@ private ValidEmail parseAndValidateEmail(SignupForm form, Errors errors)
}
}
- // Creates (or reuses) a TempUser row and sends the confirmation email in a single
- // transaction. On send failure the exception propagates and the transaction rolls back
- // automatically so a freshly inserted TempUser row does not persist.
+ // Saves the TempUser row with a new key and sends the confirmation email in one transaction. A repeat
+ // signup replaces the key, so the link already sent stops working. If the email cannot be sent, nothing
+ // is saved, and the earlier link keeps working.
private void createUserAndSendEmail(SignupForm form, ValidEmail email)
throws MessagingException, ConfigurationException, java.sql.SQLException
{
try (DbScope.Transaction transaction = SignUpManager.getSchema().getScope().ensureTransaction())
{
- TempUser tempUser = getTempUser(form, email);
- ActionURL confirmationUrl = getConfirmationURL(getContainer(), email, tempUser.getKey());
+ String key = SignUpManager.get().saveWithNewKey(email, form.getFirstName(), form.getLastName(),
+ form.getOrganization(), getContainer());
+ ActionURL confirmationUrl = getConfirmationURL(getContainer(), email, key);
User mockUser = new User();
mockUser.setEmail(email.getEmailAddress());
SecurityManager.sendEmail(getContainer(), mockUser,
SecurityManager.getRegistrationMessage(null, false),
- email.getEmailAddress(), confirmationUrl);
+ email.getEmailAddress(), confirmationUrl, LoginManager.SETUP_LINK_TIMEOUT);
transaction.commit();
}
}
@@ -1029,24 +1034,4 @@ public ApiResponse execute(SignupForm signupForm, BindException errors) throws E
return response;
}
}
-
- private TempUser getTempUser(SignupForm signupForm, ValidEmail email)
- {
- // If the user does not exit in LabKey's core database, check in our temporaryusers table
- TempUser tempUser = SignUpManager.get().getTempUserWithEmail(email.getEmailAddress());
-
- if(tempUser == null)
- { // If tempUser is null create a new TempUser object and populate fields
- tempUser = new TempUser();
- tempUser.setEmail(email.getEmailAddress());
- tempUser.setFirstName(signupForm.getFirstName());
- tempUser.setLastName(signupForm.getLastName());
- tempUser.setOrganization(signupForm.getOrganization());
- tempUser.setKey(LoginManager.createTempPassword());
- tempUser.setContainer(getContainer());
-
- Table.insert(null, SignUpManager.getTableInfoTempUsers(), tempUser);
- }
- return tempUser;
- }
}
\ No newline at end of file
diff --git a/signup/src/org/labkey/signup/SignUpManager.java b/signup/src/org/labkey/signup/SignUpManager.java
index c4655364..b2d816ba 100644
--- a/signup/src/org/labkey/signup/SignUpManager.java
+++ b/signup/src/org/labkey/signup/SignUpManager.java
@@ -16,19 +16,38 @@
package org.labkey.signup;
+import org.jetbrains.annotations.Nullable;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.labkey.api.data.CompareType;
+import org.labkey.api.data.Container;
import org.labkey.api.data.DbSchema;
+import org.labkey.api.data.SQLFragment;
import org.labkey.api.data.SimpleFilter;
+import org.labkey.api.data.SqlExecutor;
import org.labkey.api.data.Table;
import org.labkey.api.data.TableInfo;
import org.labkey.api.data.TableSelector;
import org.labkey.api.query.FieldKey;
+import org.labkey.api.security.Crypt;
+import org.labkey.api.security.LoginManager;
import org.labkey.api.security.ValidEmail;
+import org.labkey.api.util.JunitUtil;
+import org.labkey.api.util.TestContext;
+
+import java.time.Instant;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
public class SignUpManager
{
// Returned messages
public static final String CONFIRMATION_DID_NOT_MATCH = "The email address and confirmation key could not be verified. " +
- "Please make sure that you copied the entire link into the browser's address bar.";
+ "The link may have expired or already been used. Please make sure that you copied the entire link " +
+ "into the browser's address bar, or sign up again to receive a new link.";
// TODO: Add a link to reset password in this message.
public static final String USER_ALREADY_EXISTS = "A user with the email %s already exists. " +
"If you have forgotten your password, you can click the \"Forgot your password?\" " +
@@ -65,16 +84,172 @@ public TempUser getTempUserWithEmail(String email)
}
- public TempUser verifyUser(ValidEmail email, String key)
+ // Returns the pending user whose unexpired, unused key matches
+ public @Nullable TempUser verifyUser(ValidEmail email, @Nullable String key)
{
+ if (key == null)
+ return null;
SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("email"), email.getEmailAddress());
- filter.addCondition(FieldKey.fromParts("key"), key);
+ filter.addCondition(FieldKey.fromParts("key"), hashKey(key));
+ filter.addCondition(FieldKey.fromParts("keyTimeout"), new Date(), CompareType.GT);
return new TableSelector(getTableInfoTempUsers(),filter, null).getObject(TempUser.class);
}
+ // Saves and returns a new key for the pending user. A repeat signup for the same email replaces the key,
+ // so the link sent with the old key stops working.
+ public String saveWithNewKey(ValidEmail email, String firstName, String lastName, String organization, Container container)
+ {
+ TempUser tempUser = getTempUserWithEmail(email.getEmailAddress());
+ if (tempUser != null)
+ {
+ String key = assignKey(tempUser);
+ Map keyFields = new HashMap<>();
+ keyFields.put("key", tempUser.getKey());
+ keyFields.put("keyTimeout", tempUser.getKeyTimeout());
+ Table.update(null, getTableInfoTempUsers(), keyFields, tempUser.getUserId());
+ return key;
+ }
+
+ tempUser = new TempUser();
+ tempUser.setEmail(email.getEmailAddress());
+ tempUser.setFirstName(firstName);
+ tempUser.setLastName(lastName);
+ tempUser.setOrganization(organization);
+ tempUser.setContainer(container);
+ String key = assignKey(tempUser);
+ Table.insert(null, getTableInfoTempUsers(), tempUser);
+ return key;
+ }
+
+ // Clears the key so the link cannot be used again. Returns false if the key has been replaced, used or has
+ // expired since tempUser was read. A concurrent call for the same row waits until this transaction ends.
+ public boolean clearKeyIfValid(TempUser tempUser)
+ {
+ SQLFragment sql = SQLFragment.unsafe("UPDATE ").append(getTableInfoTempUsers())
+ .unsafeAppend(" SET Key = NULL, KeyTimeout = NULL WHERE UserId = ? AND Key = ? AND KeyTimeout > ?",
+ tempUser.getUserId(), tempUser.getKey(), new Date());
+ return new SqlExecutor(getSchema()).execute(sql) == 1;
+ }
+
+ // Stores the id of the LabKey account created from tempUser
+ public void setLabkeyUserId(TempUser tempUser, int labkeyUserId)
+ {
+ Map fields = new HashMap<>();
+ fields.put("labkeyUserId", labkeyUserId);
+ Table.update(null, getTableInfoTempUsers(), fields, tempUser.getUserId());
+ }
+
+ // Sets a new key on tempUser, storing only its hash, and returns the key. The caller saves tempUser.
+ private String assignKey(TempUser tempUser)
+ {
+ String key = LoginManager.createTempPassword();
+ tempUser.setKey(hashKey(key));
+ tempUser.setKeyTimeout(Date.from(Instant.now().plus(LoginManager.SETUP_LINK_TIMEOUT)));
+ return key;
+ }
+
+ // Keys are long and random, so a fast unsalted hash suffices. signup-26.000-26.001.sql hashes existing rows
+ // identically.
+ private static String hashKey(String key)
+ {
+ return Crypt.SHA256.digestWithPrefix(key);
+ }
+
public void deleteUser(String email)
{
if (null != getTableInfoTempUsers())
Table.delete(getTableInfoTempUsers(), new SimpleFilter(FieldKey.fromParts("email"), email));
}
+
+ public static class TestCase extends Assert
+ {
+ private static final String EMAIL = "signup-key-test@signup.test";
+
+ private ValidEmail _email;
+ private TempUser _tempUser;
+ private String _key;
+
+ @Before
+ public void setUp() throws ValidEmail.InvalidEmailException
+ {
+ _email = new ValidEmail(EMAIL);
+ get().deleteUser(EMAIL);
+
+ _key = get().saveWithNewKey(_email, "Signup", "Test", "Signup Test", JunitUtil.getTestContainer());
+ _tempUser = get().getTempUserWithEmail(EMAIL);
+ }
+
+ @After
+ public void tearDown()
+ {
+ get().deleteUser(EMAIL);
+ }
+
+ private void save()
+ {
+ Table.update(null, getTableInfoTempUsers(), _tempUser, _tempUser.getUserId());
+ }
+
+ @Test
+ public void testKeyStoredAsHash()
+ {
+ TempUser stored = get().getTempUserWithEmail(EMAIL);
+ assertEquals("Key not stored as its SHA-256 hash", Crypt.SHA256.digestWithPrefix(_key), stored.getKey());
+ assertNotNull("Key rejected", get().verifyUser(_email, _key));
+ }
+
+ @Test
+ public void testWrongKeyRejected()
+ {
+ assertNull("Wrong key accepted", get().verifyUser(_email, _key + "x"));
+ assertNull("Missing key accepted", get().verifyUser(_email, null));
+ assertNull("Stored hash accepted as a key", get().verifyUser(_email, _tempUser.getKey()));
+ }
+
+ @Test
+ public void testExpiredKeyRejected()
+ {
+ _tempUser.setKeyTimeout(Date.from(Instant.now().minusSeconds(1)));
+ save();
+ assertNull("Expired key accepted", get().verifyUser(_email, _key));
+ assertFalse("Expired key cleared", get().clearKeyIfValid(_tempUser));
+ }
+
+ @Test
+ public void testRepeatSignupReplacesOnlyKey()
+ {
+ String newKey = get().saveWithNewKey(_email, "Other", "Name", "Other Org", JunitUtil.getTestContainer());
+ assertNull("Replaced key accepted", get().verifyUser(_email, _key));
+
+ TempUser stored = get().verifyUser(_email, newKey);
+ assertNotNull("New key rejected", stored);
+ assertEquals("Repeat signup created a second row", _tempUser.getUserId(), stored.getUserId());
+ assertEquals("First name changed by a repeat signup", "Signup", stored.getFirstName());
+ assertEquals("Last name changed by a repeat signup", "Test", stored.getLastName());
+ assertEquals("Organization changed by a repeat signup", "Signup Test", stored.getOrganization());
+ }
+
+ @Test
+ public void testReplacedKeyCannotBeUsed()
+ {
+ String newKey = get().saveWithNewKey(_email, "Signup", "Test", "Signup Test", JunitUtil.getTestContainer());
+ assertFalse("Replaced key cleared", get().clearKeyIfValid(_tempUser));
+ assertNotNull("New key cleared by the replaced key", get().verifyUser(_email, newKey));
+ }
+
+ @Test
+ public void testUsedKeyRejected()
+ {
+ int userId = TestContext.get().getUser().getUserId();
+ assertTrue("Valid key not cleared", get().clearKeyIfValid(_tempUser));
+ get().setLabkeyUserId(_tempUser, userId);
+ assertNull("Used key accepted", get().verifyUser(_email, _key));
+ assertFalse("Key cleared twice", get().clearKeyIfValid(_tempUser));
+
+ TempUser stored = get().getTempUserWithEmail(EMAIL);
+ assertNull("Key not cleared after use", stored.getKey());
+ assertNull("Key timeout not cleared after use", stored.getKeyTimeout());
+ assertEquals("Account userId not saved on the signup row", Integer.valueOf(userId), stored.getLabkeyUserId());
+ }
+ }
}
\ No newline at end of file
diff --git a/signup/src/org/labkey/signup/SignUpModule.java b/signup/src/org/labkey/signup/SignUpModule.java
index c7205424..035c6f8d 100644
--- a/signup/src/org/labkey/signup/SignUpModule.java
+++ b/signup/src/org/labkey/signup/SignUpModule.java
@@ -52,7 +52,7 @@ public String getName()
@Override
public @Nullable Double getSchemaVersion()
{
- return 13.24;
+ return 26.001;
}
@Override
@@ -113,4 +113,10 @@ public boolean getRequireSitePermission()
return true;
}
+ @Override
+ public @NotNull Set> getIntegrationTests()
+ {
+ return Set.of(SignUpManager.TestCase.class);
+ }
+
}
\ No newline at end of file
diff --git a/signup/src/org/labkey/signup/SignUpSchema.java b/signup/src/org/labkey/signup/SignUpSchema.java
index 25026e1b..60a58104 100644
--- a/signup/src/org/labkey/signup/SignUpSchema.java
+++ b/signup/src/org/labkey/signup/SignUpSchema.java
@@ -42,6 +42,7 @@
import org.labkey.api.view.ActionURL;
import org.labkey.api.writer.HtmlWriter;
+import java.util.Date;
import java.util.Set;
public class SignUpSchema extends UserSchema
@@ -118,17 +119,37 @@ public DisplayColumn createRenderer(ColumnInfo colInfo)
{
return new DataColumn(colInfo)
{
+ // Returns the link with the key masked while the key is unused and unexpired,
+ // otherwise null. Only a hash of the key is stored. Exports use this value, so the
+ // hash is not exported.
@Override
- public void renderGridCellContents(RenderContext ctx, HtmlWriter out)
+ public Object getDisplayValue(RenderContext ctx)
{
- Container container = ContainerManager.getForId(String.valueOf(ctx.get(FieldKey.fromParts("Container"))));
- String email = String.valueOf(ctx.get(FieldKey.fromParts("email")));
- String key = String.valueOf(ctx.get(getColumnInfo().getFieldKey()));
+ Date keyTimeout = ctx.get(FieldKey.fromParts("keyTimeout"), Date.class);
+ if (getValue(ctx) == null || keyTimeout == null || !keyTimeout.after(new Date()))
+ return null;
+
+ Container container = ContainerManager.getForId(ctx.get(FieldKey.fromParts("Container"), String.class));
+ String email = ctx.get(FieldKey.fromParts("email"), String.class);
ActionURL url = new ActionURL(SignUpController.ConfirmAction.class, container);
url.addParameter("email", email);
- url.addParameter("key", key);
- out.write(url.getLocalURIString());
+ url.addParameter("key", "**********");
+ return url.getLocalURIString();
+ }
+
+ @Override
+ public Object getJsonValue(RenderContext ctx)
+ {
+ return getDisplayValue(ctx);
+ }
+
+ @Override
+ public void renderGridCellContents(RenderContext ctx, HtmlWriter out)
+ {
+ Object url = getDisplayValue(ctx);
+ if (url != null)
+ out.write(url.toString());
// out.write("Confirmation URL");
}
@@ -138,6 +159,7 @@ public void addQueryFieldKeys(Set keys)
super.addQueryFieldKeys(keys);
keys.add(FieldKey.fromParts("Container"));
keys.add(FieldKey.fromParts("email"));
+ keys.add(FieldKey.fromParts("keyTimeout"));
}
};
}
diff --git a/signup/src/org/labkey/signup/TempUser.java b/signup/src/org/labkey/signup/TempUser.java
index f8cedfce..67c3e4d4 100644
--- a/signup/src/org/labkey/signup/TempUser.java
+++ b/signup/src/org/labkey/signup/TempUser.java
@@ -16,11 +16,14 @@
package org.labkey.signup;
import org.labkey.api.data.Container;
+import java.util.Date;
+
// Class TempUser contains information needed to add a user to the LabKey database as a User later on but has addition
-// field @key for use with confirmation
+// field @key for use with confirmation. The key holds a hash of the emailed key, or null once the key has been used.
public class TempUser
{
private String key;
+ private Date keyTimeout;
private String email;
private String firstName;
private String lastName;
@@ -100,6 +103,16 @@ public void setKey(String key)
this.key = key;
}
+ public Date getKeyTimeout()
+ {
+ return keyTimeout;
+ }
+
+ public void setKeyTimeout(Date keyTimeout)
+ {
+ this.keyTimeout = keyTimeout;
+ }
+
public Integer getLabkeyUserId()
{
return _labkeyUserId;