spring-projects / spring-projects/spring-security

Refactor XOR CSRF Token Encoding Logic into Public Class to Improve Testability and Maintainability

Open
#17,968 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

status: waiting-for-triage type: enhancement
Dominant language
Java
Stars
9.6k
Forks
6.3k
Avg merge
2d 11h
Merged PRs (30d)
52

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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by locating XorCsrfTokenRequestAttributeHandler and reading its private createXoredCsrfToken and xorCsrf methods. Extract the XOR encoding and decoding responsibilities into a public class such as XorCsrfTokenEncoder, preserve the existing behavior, and add focused unit tests for the extracted logic.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
security
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.