Skip to content

fix: Saml response multiple signatures check#865

Merged
giuseppe-gangemi merged 16 commits intomainfrom
fix/oi-478-saml-response-multiple-signatures-check
Sep 30, 2025
Merged

fix: Saml response multiple signatures check#865
giuseppe-gangemi merged 16 commits intomainfrom
fix/oi-478-saml-response-multiple-signatures-check

Conversation

@giuseppe-gangemi
Copy link
Contributor

@giuseppe-gangemi giuseppe-gangemi commented Sep 23, 2025

What type of PR is this?

  • 🔨 Refactor
  • ✨ Feature
  • ⛑️ Bug Fix
  • 🚀 Optimization
  • 📄 Documentation Update
  • ⚠️ Breaking change ℹ️

List of Changes

  • Introduce check on the Samlresponse containing more than one Signature inside the Assertion or Response element.
    The invalid Samlresponse with multiple signatures will be stored inside the SAML session record on dynamoDB, but the authetication flow will be interrupted.
  • Add cloudwatch metrics for new idp error

NB: this pr doesn't activate the constraint, in this stage in case of SAMLResponse with multiple signature, a metric is sent to cloudwatch. The blocking of auth flow will be introduced after a period of monitoring.

Related Task

  • OI-478

How Has This Been Tested?

  • Local tests

Screenshots (if appropriate)

BenitoVisone
BenitoVisone previously approved these changes Sep 24, 2025
Copy link
Contributor

@BenitoVisone BenitoVisone left a comment

Choose a reason for hiding this comment

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

lgtm!

sebbalex
sebbalex previously approved these changes Sep 24, 2025
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
69.6% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@giuseppe-gangemi giuseppe-gangemi merged commit 9621a14 into main Sep 30, 2025
10 of 11 checks passed
@giuseppe-gangemi giuseppe-gangemi deleted the fix/oi-478-saml-response-multiple-signatures-check branch September 30, 2025 08:57
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