fixed checks for inresponseto and validateExpiration

[deepakprabhakara]

Summary by CodeRabbit

• New Features

  • Added stronger assertion replay protection support for sign-in responses.
  • Improved handling of assertion timing information, including expiration checks with clock skew tolerance.

• Bug Fixes

  • Fixed cases where response validation could hang or run twice after parsing errors.
  • Tightened response-to-request matching so missing values no longer bypass validation.
  • Corrected expiration handling to reject assertions without a valid upper time bound.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ab1ee9dd-bed9-422d-bb6f-a49e670f4f7d

📥 Commits

Reviewing files that changed from the base of the PR and between 7b57c0a and a1b1999.

📒 Files selected for processing (4)

• lib/response.ts
• lib/saml20.ts
• lib/typings.ts
• test/lib/saml20.inResponseTo.attacks.spec.ts

🚧 Files skipped from review as they are similar to previous changes (4)

• test/lib/saml20.inResponseTo.attacks.spec.ts
• lib/response.ts
• lib/typings.ts
• lib/saml20.ts

---

📝 Walkthrough

Walkthrough

The PR adds replay-related assertion metadata, rewrites expiration validation to evaluate Conditions and bearer subject-confirmation windows with clock skew, and updates response validation to use signed InResponseTo data with optional replay protection. Tests cover expiration, tampering, and replay cases.

Changes

SAML validation hardening

Layer / File(s)	Summary
Assertion metadata and expiration lib/typings.ts, lib/saml20.ts, test/lib/saml20.spec.ts, test/lib/saml20.inResponseTo.attacks.spec.ts	SAMLProfile adds assertionId and notOnOrAfter, parse() returns those fields, saml20 exports getSubjectConfirmationInResponseTo, getAssertionId, and getNotOnOrAfter, and validateExpiration() now evaluates Conditions and bearer SubjectConfirmationData windows with clock skew; the expiration tests cover the new outcomes.
Signed InResponseTo enforcement lib/response.ts, test/lib/saml20.inResponseTo.attacks.spec.ts, test/assets/certificates/testIdpCert.crt	parseResponseAndVersion() now reads assertion.inResponseTo from signed content only, validateInternal() rejects missing or mismatched options.inResponseTo values and awaits assertionReplayValidator, and the attack suite covers wrapper tampering, replay reuse, and validator failures.

Sequence Diagram(s)

sequenceDiagram
  participant validateInternal
  participant parseResponseAndVersion
  participant validateExpiration
  participant assertionReplayValidator
  validateInternal->>parseResponseAndVersion: await onParse(responseObj)
  parseResponseAndVersion-->>validateInternal: assertionId, notOnOrAfter, inResponseTo
  validateInternal->>validateExpiration: assertion
  validateExpiration-->>validateInternal: valid / invalid
  validateInternal->>assertionReplayValidator: assertionId, sessionIndex, notOnOrAfter, inResponseTo
  assertionReplayValidator-->>validateInternal: used / unused

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.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 highlights the two main behavioral fixes: InResponseTo validation and validateExpiration.
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.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch security/inresponseto-check

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details

{}

[coderabbitai]

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details

{}