Skip to content

[ES-1943G] Removed method calls for nonce caching - #2016

Merged
anushasunkada merged 5 commits into
mosip:developfrom
Infosys:ES-1943G
Jun 22, 2026
Merged

anushasunkada merged 5 commits into
mosip:developfrom
Infosys:ES-1943G

Conversation

@KashiwalHarsh

@KashiwalHarsh KashiwalHarsh commented Jun 18, 2026 •

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • Refactor
    • Updated OAuth/OIDC authorization flows to remove server-side nonce validation (including cache/Redis-based nonce checks). Redirect URI allowlist validation remains enforced, and DPoP thumbprint validation continues when a DPoP header is provided.
  • Tests
    • Simplified OIDC authorization and flow tests by removing Redis/cache mocking and nonce-check stubbing, and trimmed cache utility test coverage to focus on remaining cache behaviors.

Signed-off-by: Harsh Kashiwal <harsh.kashiwal@infosys.com>
@coderabbitai

coderabbitai Bot commented Jun 18, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c3688f0e-4c2e-40a0-b35c-f4bbbac81d7d

📥 Commits

Reviewing files that changed from the base of the PR and between 42c8590 and 8e10b59.

📒 Files selected for processing (1)
  • oidc-service-impl/src/main/java/io/mosip/esignet/services/AuthorizationServiceImpl.java

Walkthrough

This PR removes nonce validation from the OIDC authorization flows and eliminates the supporting infrastructure entirely. Changes span six layers: (1) four ranges across AuthorizationServiceImpl and OAuthServiceImpl that replace nonce validation calls with direct redirect URI validation and logging; (2) removal of the nonce validation helper methods and Environment dependency from AuthorizationHelperService; (3) elimination of the Redis-backed checkNonce implementation from CacheUtilService; (4) cleanup of nonce mock expectations across authorization service test suites; (5) simplification of flow test setup by removing cache/redis autowiring and initialization; and (6) deletion of all nonce-specific test methods from cache service tests.

Changes

Nonce Validation Removal

Layer / File(s) Summary
Remove nonce validation calls from authorization services
oidc-service-impl/.../AuthorizationServiceImpl.java, oidc-service-impl/.../OAuthServiceImpl.java
getOauthDetails, getOauthDetailsV2, and getOauthDetailsV3 in AuthorizationServiceImpl replace validateRedirectURIAndNonce(...) calls with direct nonce logging and IdentityProviderUtil.validateRedirectURI(...) invocation. OAuthServiceImpl.authorize(...) removes the explicit authorizationHelperService.validateNonce(...) call while retaining nonce logging and redirect URI validation.
Remove nonce validation helper methods and Environment dependency
oidc-service-impl/.../AuthorizationHelperService.java
Removes Environment import and autowired field. Deletes validateNonce(...) and isLocalEnvironment() methods that previously enforced nonce presence via cacheUtilService with environment-based bypass logic for local profiles.
Remove Redis-backed nonce check implementation from cache service
oidc-service-impl/.../CacheUtilService.java
Removes the public checkNonce(String nonce) method and private Redis script helper (isScriptNotLoaded(...)) that tracked nonce existence via Lua script evaluation with TTL. CacheManager remains for other cache operations (sessions, PARs, rate limits, JTI tracking).
Remove nonce validation mocking from authorization service tests
oidc-service-impl/.../AuthorizationServiceTest.java
Removes cacheUtilService.checkNonce(anyString()).thenReturn(1L) mock setup across getOauthDetails V1/V2/V3 test variants (prompt/claims/ACR/PKCE/id_token_hint scenarios), leaving request preparation and assertions intact.
Simplify flow test setup by removing cache/redis dependencies
esignet-service/.../AuthCodeFlowTest.java, esignet-service/.../AuthorizationAPIFlowTest.java
Removes CacheUtilService autowiring, Redis/Mockito imports, and ReflectionTestUtils cache initialization from both test classes, eliminating Redis connection/script mocking while retaining tokenService discovery-map setup.
Remove nonce validation test coverage from cache service tests
oidc-service-impl/.../CacheUtilServiceTest.java
Removes Redis connection/scripting imports and mocks. Deletes six checkNonce test methods (simpleCacheType_returnsOneWithoutRedis, scriptAlreadyLoaded_reusesHashAndClosesConnection, scriptNotLoaded_loadsScriptAndClosesConnection, scriptExistsReturnsFalse_reloadsScriptAndClosesConnection, redisThrowsException_connectionStillClosed, singleConnectionUsedForAllOperations), leaving only cache set/get/remove tests for OIDCTransaction and LinkTransactionMetadata.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • mosip/esignet#1904: Updates nonce handling in id_token_hint flow validation and test expectations for cacheUtilService.checkNonce error codes, directly affected by removal of the underlying nonce-check implementation.

Suggested reviewers

  • sacrana0
  • anushasunkada
  • zesu22

Poem

🐇 The nonce validator hops away today,
Redis scripts and helpers fade to gray,
Redirect checks and logging still remain,
But deep nonce enforcement? Gone, no pain!
Six layers of cleanup, tests refreshed with glee,
The authorization flow now flows so free! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: removal of nonce caching method calls across multiple service and test files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Signed-off-by: Harsh Kashiwal <harsh.kashiwal@infosys.com>
Signed-off-by: Harsh Kashiwal <harsh.kashiwal@infosys.com>
Signed-off-by: Harsh Kashiwal <harsh.kashiwal@infosys.com>
Signed-off-by: Harsh Kashiwal <harsh.kashiwal@infosys.com>
@rachik-hue rachik-hue linked an issue Jun 22, 2026 that may be closed by this pull request
@anushasunkada
anushasunkada merged commit b9377b8 into mosip:develop Jun 22, 2026
8 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nonce cache grows linear

2 participants