ES-2739 - Updated the CSRF token changes in commons#1878
ES-2739 - Updated the CSRF token changes in commons#1878mohanachandran-s merged 10 commits intomosip:developfrom
Conversation
Signed-off-by: Prathmesh Jadhav <[email protected]>
WalkthroughCentralizes CSRF handling: adds static CSRF storage in BaseTestCase, adds AdminTestUtil helpers to fetch/extract CSRF token and cookie from server responses, and updates RestClient and other callers to use the centralized CSRF values. Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant AdminUtil as AdminTestUtil
participant Server
participant Base as BaseTestCase
participant Rest as RestClient
TestRunner->>AdminUtil: fetchAndStoreCsrfToken()
AdminUtil->>Server: GET /csrf-endpoint
Server-->>AdminUtil: 2xx response with token body and Set-Cookie
AdminUtil->>Base: extractAndStoreCsrfToken(response)\nset CSRF_TOKEN, CSRF_COOKIE
TestRunner->>Rest: invoke API call
Rest->>Base: read CSRF_TOKEN & CSRF_COOKIE
Rest->>Server: request with Cookie (CSRF_COOKIE) and header X-XSRF-TOKEN (CSRF_TOKEN)
Server-->>Rest: response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.40.5)apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/AdminTestUtil.javaThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/AdminTestUtil.java (2)
452-487: Use the CSRF cookie value for cookies, not the token.
tokenis used as the cookie value in both the cookie map and the cookieName branch. It should beCSRF_COOKIE; otherwise the cookie carries the header token and CSRF validation can fail.🐛 Proposed fix
- token = GlobalConstants.CSRF_TOKEN; + token = GlobalConstants.CSRF_COOKIE;
642-663: Use CSRF_COOKIE for the cookie value in this flow too.This path currently passes
CSRF_TOKENas the cookie value, which mirrors the earlier bug and can break CSRF checks.🐛 Proposed fix
- token = GlobalConstants.CSRF_TOKEN; + token = GlobalConstants.CSRF_COOKIE;
🤖 Fix all issues with AI agents
In
`@apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/AdminTestUtil.java`:
- Around line 7548-7561: The extractAndStoreCsrfToken and fetchAndStoreCsrfToken
currently store nulls silently; modify them so fetchAndStoreCsrfToken checks the
HTTP response status from RestClient.getRequest and if not 2xx throws an
exception or logs and fails fast, and then have extractAndStoreCsrfToken
validate that response.jsonPath().getString("token") and
response.getCookie(GlobalConstants.XSRF_TOKEN) are non‑empty before assigning
GlobalConstants.CSRF_TOKEN and GlobalConstants.CSRF_COOKIE; if either value is
missing, throw a descriptive runtime exception (or return a failure) that
includes the response status and body to aid debugging. Ensure you reference
extractAndStoreCsrfToken, fetchAndStoreCsrfToken, RestClient.getRequest,
GlobalConstants.CSRF_TOKEN and GlobalConstants.CSRF_COOKIE when locating where
to add the checks.
In
`@apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/GlobalConstants.java`:
- Around line 283-287: Global mutable CSRF_TOKEN and CSRF_COOKIE in
GlobalConstants risk cross-test leakage; replace these public static mutable
fields with per-thread or per-session storage and accessor methods: convert
CSRF_TOKEN and CSRF_COOKIE into private ThreadLocal<String> variables (or move
into a session-scoped context object) and add get/set methods (e.g.,
getCsrfToken(), setCsrfToken(), getCsrfCookie(), setCsrfCookie()) so tests
explicitly set/read tokens for their own thread/session rather than sharing
globals; update usages to call the new accessors.
In `@apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/RestClient.java`:
- Around line 739-742: Add a guard that fails fast when the CSRF cookie is not
initialized: create a private helper (e.g., ensureCsrfCookieInitialized()) in
RestClient that checks GlobalConstants.CSRF_COOKIE for null/blank and throws an
IllegalStateException with a clear message like "CSRF cookie not initialized.
Call AdminTestUtil.fetchAndStoreCsrfToken() first."; then call this helper at
the start of every request path that currently uses GlobalConstants.CSRF_COOKIE
(e.g., the code building postResponse with .cookie(GlobalConstants.XSRF_TOKEN,
GlobalConstants.CSRF_COOKIE) and the other call sites you flagged) so requests
never proceed without a valid CSRF cookie.
apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/AdminTestUtil.java
Show resolved
Hide resolved
apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/GlobalConstants.java
Outdated
Show resolved
Hide resolved
apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/RestClient.java
Show resolved
Hide resolved
Signed-off-by: Prathmesh Jadhav <[email protected]>
Signed-off-by: Prathmesh Jadhav <[email protected]>
apitest-commons/src/main/java/io/mosip/testrig/apirig/utils/AdminTestUtil.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Prathmesh Jadhav <[email protected]>
Signed-off-by: Prathmesh Jadhav <[email protected]>
Signed-off-by: Prathmesh Jadhav <[email protected]>
Signed-off-by: Prathmesh Jadhav <[email protected]>
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.