Skip to content

Replace Random with SecureRandom for improved randomness in SAML2SSOTestBase#26929

Open
HarishRock0 wants to merge 1 commit intowso2:masterfrom
HarishRock0:linear-congruential-generator-Fix
Open

Replace Random with SecureRandom for improved randomness in SAML2SSOTestBase#26929
HarishRock0 wants to merge 1 commit intowso2:masterfrom
HarishRock0:linear-congruential-generator-Fix

Conversation

@HarishRock0
Copy link
Copy Markdown

@HarishRock0 HarishRock0 commented Mar 6, 2026

Problem
The createID() method used java.util.Random, which relies on a linear congruential generator (LCG). LCGs are not cryptographically secure — an attacker who observes a sufficient number of generated SAML request IDs can predict future values, enabling:

SAML request ID spoofing
Replay attacks against the SAML authentication flow
This is a violation of OWASP Top 10 — A02: Cryptographic Failures.

Changes
SAML2SSOTestBase.java: Replaced import java.util.Random with import java.security.SecureRandom
SAML2SSOTestBase.java: Changed field declaration from private static Random random = new Random() to private static SecureRandom random = new SecureRandom()
Impact
SecureRandom is backed by OS-level entropy sources and satisfies cryptographic unpredictability requirements. The nextBytes() call site at createID() requires no changes — the API is identical.

No functional behaviour changes; this is a security hardening fix.

Summary by CodeRabbit

  • Bug Fixes
    • Improved ID generation in SAML2SSO test infrastructure.

Copilot AI review requested due to automatic review settings March 6, 2026 14:34
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 6, 2026

CLA assistant check
All committers have signed the CLA.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 6, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a66df125-ca9f-4b7f-86dd-21c4c00488a3

📥 Commits

Reviewing files that changed from the base of the PR and between 5f04fac and fedd9f6.

📒 Files selected for processing (1)
  • product-scenarios/scenarios-commons/src/main/java/org/wso2/identity/scenarios/commons/SAML2SSOTestBase.java

Walkthrough

A security improvement was made to switch the random number generation in SAML2 SSO testing from a non-cryptographic Random instance to SecureRandom, enhancing the randomness quality used for ID generation in test scenarios.

Changes

Cohort / File(s) Summary
SecureRandom Migration
product-scenarios/scenarios-commons/src/main/java/org/wso2/identity/scenarios/commons/SAML2SSOTestBase.java
Replaced java.util.Random import with java.security.SecureRandom and updated the static field initialization to use SecureRandom instead of Random for improved cryptographic randomness in SAML2 ID generation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A hop towards security's light,
Random now wears armor bright,
SecureRandom takes the stage,
IDs safer, age by age! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 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—replacing Random with SecureRandom in SAML2SSOTestBase—and is concise, clear, and specific enough for teammates to understand the primary purpose of the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens SAML AuthnRequest ID generation in the product scenario test utilities by switching from java.util.Random to java.security.SecureRandom in SAML2SSOTestBase, improving unpredictability of generated request IDs.

Changes:

  • Replace Random import with SecureRandom.
  • Update the static RNG field to use SecureRandom while keeping the existing createID() logic intact.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Mar 6, 2026

@HarishRock0
Copy link
Copy Markdown
Author

Hi @splinter , @gnudeep , @hevayo ,
I’ve submitted this PR to WSO2 Identity Server and all automated checks have passed.
Could you please review it when you have some time?
Thank you for your guidance and support!

Regards,
Hariksan Thulasithas

@HarishRock0
Copy link
Copy Markdown
Author

Hi @NipunaMadhushan ,

I’ve submitted this PR to WSO2 Identity Server and all automated checks have passed.
Could you please review it when you have some time?
Thank you for your guidance and support!

Regards,
Hariksan Thulasithas

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.

3 participants