Hash SignUp confirmation keys and expire signup links

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;

signup/resources/schemas/signup.xml

@@ -27,6 +27,7 @@
             <column columnName="lastname"/>
             <column columnName="organization"/>
             <column columnName="key"/>
+            <column columnName="keytimeout"/>
             <column columnName="container"/>
             <column columnName="labkeyuserid">
                 <fk>

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;
-    }
 }

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<String, Object> 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<String, Object> 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());
+        }
+    }
 }

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<Class<?>> getIntegrationTests()
+    {
+        return Set.of(SignUpManager.TestCase.class);
+    }
+
 }