Address PR feedback

Author: DJAndries

controllers/accounts_test.go

@@ -303,11 +303,6 @@ func (suite *AccountsTestSuite) TestRegistration() {
 	suite.Require().NoError(err)
 	suite.NotNil(account.LastEmailVerifiedAt) // Email should now be verified
 
-	// Check that account is now verified
-	account, err = suite.ds.GetAccount(nil, email)
-	suite.Require().NoError(err)
-	suite.NotNil(account.LastEmailVerifiedAt) // Email should now be verified
-
 	// Check that the session created has the correct version (PasswordAuthSessionVersion)
 	sessionID, _, err := suite.jwtService.ValidateAuthToken(*verificationResult.AuthToken)
 	suite.Require().NoError(err)

services/verification.go

@@ -133,7 +133,7 @@ func (vs *VerificationService) CompleteVerification(verification *datastore.Veri
 		}
 	}
 
-	if util.NormalizeVerificationCode(code) != verification.Code {
+	if !util.VerificationCodeEquals(code, verification.Code) {
 		return nil, util.ErrInvalidCode
 	}
 

util/util.go

@@ -312,19 +312,20 @@ func (k *KeyServiceClient) MakeRequest(method string, path string, body interfac
 	return nil
 }
 
-// NormalizeVerificationCode applies canonical cleanup rules to a verification
-// code to tolerate common user input mistakes before comparison or storage.
-func NormalizeVerificationCode(code string) string {
-	code = strings.ToUpper(code)
-	code = strings.ReplaceAll(code, " ", "")
-	code = strings.ReplaceAll(code, "\t", "")
-	code = strings.ReplaceAll(code, "\n", "")
-	code = strings.ReplaceAll(code, "\r", "")
-	code = strings.ReplaceAll(code, "-", "")
-	code = strings.ReplaceAll(code, "1", "I")
-	code = strings.ReplaceAll(code, "8", "B")
-	code = strings.ReplaceAll(code, "0", "O")
-	return code
+// VerificationCodeEquals reports whether the user-supplied code matches the
+// stored normalized code, using a constant-time comparison to prevent timing
+// attacks.
+func VerificationCodeEquals(userInput, expected string) bool {
+	userInput = strings.ToUpper(userInput)
+	userInput = strings.ReplaceAll(userInput, " ", "")
+	userInput = strings.ReplaceAll(userInput, "\t", "")
+	userInput = strings.ReplaceAll(userInput, "\n", "")
+	userInput = strings.ReplaceAll(userInput, "\r", "")
+	userInput = strings.ReplaceAll(userInput, "-", "")
+	userInput = strings.ReplaceAll(userInput, "1", "I")
+	userInput = strings.ReplaceAll(userInput, "8", "B")
+	userInput = strings.ReplaceAll(userInput, "0", "O")
+	return subtle.ConstantTimeCompare([]byte(userInput), []byte(expected)) == 1
 }
 
 func GetRequestLocale(explicitLocale string, r *http.Request) string {

util/util_test.go

@@ -62,22 +62,24 @@ func (suite *UtilTest) TestCanonicalizeEmail() {
 	suite.Equal("\"test@Foo\"@example.com", result)
 }
 
-func (suite *UtilTest) TestNormalizeVerificationCode() {
+func (suite *UtilTest) TestVerificationCodeEquals() {
 	// Uppercase
-	suite.Equal("ABCDEF", util.NormalizeVerificationCode("abcdef"))
+	suite.True(util.VerificationCodeEquals("abcdef", "ABCDEF"))
 	// Whitespace removal
-	suite.Equal("ABCDEF", util.NormalizeVerificationCode("ABC DEF"))
-	suite.Equal("ABCDEF", util.NormalizeVerificationCode("AB\tCD\nEF"))
+	suite.True(util.VerificationCodeEquals("ABC DEF", "ABCDEF"))
+	suite.True(util.VerificationCodeEquals("AB\tCD\nEF", "ABCDEF"))
 	// Hyphen removal
-	suite.Equal("ABCDEF", util.NormalizeVerificationCode("ABC-DEF"))
+	suite.True(util.VerificationCodeEquals("ABC-DEF", "ABCDEF"))
 	// 1 -> I
-	suite.Equal("IABCDE", util.NormalizeVerificationCode("1ABCDE"))
+	suite.True(util.VerificationCodeEquals("1ABCDE", "IABCDE"))
 	// 8 -> B
-	suite.Equal("BABCDE", util.NormalizeVerificationCode("8ABCDE"))
+	suite.True(util.VerificationCodeEquals("8ABCDE", "BABCDE"))
 	// 0 -> O
-	suite.Equal("OABCDE", util.NormalizeVerificationCode("0ABCDE"))
+	suite.True(util.VerificationCodeEquals("0ABCDE", "OABCDE"))
 	// Combined
-	suite.Equal("IOBBCD", util.NormalizeVerificationCode("1 0-8-8cd"))
+	suite.True(util.VerificationCodeEquals("1 0-8-8cd", "IOBBCD"))
+	// Mismatch
+	suite.False(util.VerificationCodeEquals("ABCDEF", "XYZXYZ"))
 }
 
 func TestUtil(t *testing.T) {