-
Notifications
You must be signed in to change notification settings - Fork 6.1k
Description
Expected Behavior
The XOR-based CSRF token encoding and decoding logic should be refactored into a dedicated public class (e.g., XorCsrfTokenEncoder) with publicly accessible methods. This change would:
-
Enable direct unit testing of the encoding mechanism
-
Improve test coverage
-
Allow easier future extensions or replacements of the XOR encoding logic
-
Improve code maintainability and testability
The refactor should preserve the current behavior, without introducing any functional changes.
Current Behavior
Currently, the XOR encoding and decoding logic for CSRF tokens is implemented as private static methods inside the XorCsrfTokenRequestAttributeHandler class:
private static String createXoredCsrfToken(SecureRandom secureRandom, String token) {
byte[] tokenBytes = Utf8.encode(token);
byte[] randomBytes = new byte[tokenBytes.length];
secureRandom.nextBytes(randomBytes);
byte[] xoredBytes = xorCsrf(randomBytes, tokenBytes);
byte[] combinedBytes = new byte[tokenBytes.length + randomBytes.length];
System.arraycopy(randomBytes, 0, combinedBytes, 0, randomBytes.length);
System.arraycopy(xoredBytes, 0, combinedBytes, randomBytes.length, xoredBytes.length);
return Base64.getUrlEncoder().encodeToString(combinedBytes);
}
private static byte[] xorCsrf(byte[] randomBytes, byte[] csrfBytes) {
Assert.isTrue(randomBytes.length == csrfBytes.length, "arrays must be equal length");
int len = csrfBytes.length;
byte[] xoredCsrf = new byte[len];
System.arraycopy(csrfBytes, 0, xoredCsrf, 0, len);
for (int i = 0; i < len; i++) {
xoredCsrf[i] ^= randomBytes[i];
}
return xoredCsrf;
}
This design limits modularity and flexibility in the following ways:
-
The encoding logic is tightly coupled with request-handling responsibilities
-
The methods are private and inaccessible for testing in isolation
-
Reusability is reduced, and no alternative strategies can be introduced
Most importantly:
- The XOR encoding logic is implemented inside a handler class that has a different primary responsibility,
which reduces modularity and violates the Single Responsibility Principle (SRP)
Context
This architectural limitation makes it more difficult to safely maintain or extend the CSRF token encoding mechanism. Additionally, the lack of testable and modular code reduces overall confidence in this critical part of the security pipeline.
By extracting the XOR logic into a separate public class, we gain the following benefits:
-
Focused and isolated unit tests for the XOR logic
-
Improved code organization and separation of concerns
-
Flexibility to introduce alternative encoding mechanisms in the future
-
Better test coverage and code quality
-
Strong alignment with the Single Responsibility Principle (SRP), with potential for future extensibility
There are currently no clean workarounds to test or reuse this logic independently. This refactor would improve overall code health while keeping external behavior unchanged.