Skip to content

Conversation

@Ladirico
Copy link
Contributor

@Ladirico Ladirico commented Sep 24, 2025

Short description

This pull request introduces a small fix to the lollipopSamlVerify function in ts/features/lollipop/utils/login.ts to improve compatibility when decoding SAML requests.

  • Changed the input to pako.inflateRaw from a Buffer to a Uint8Array to ensure correct handling of binary data when decoding base64-encoded SAML requests.

How to test

Run the application and try to decode saml request on login flow

@Ladirico Ladirico self-assigned this Sep 24, 2025
@Ladirico Ladirico added the IO-A&I IO - Autenticazione e Identità label Sep 24, 2025
@github-actions
Copy link
Contributor

Jira Pull Request Link

This Pull Request refers to Jira issues:

@github-actions
Copy link
Contributor

PR Title Validation for conventional commit type

All good! PR title follows the conventional commit type.

@codecov
Copy link

codecov bot commented Sep 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.38%. Comparing base (cd9685f) to head (77cb123).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #7397   +/-   ##
=======================================
  Coverage   59.38%   59.38%           
=======================================
  Files        1823     1823           
  Lines       39232    39232           
  Branches     9012     8949   -63     
=======================================
  Hits        23297    23297           
- Misses      15853    15866   +13     
+ Partials       82       69   -13     
Files with missing lines Coverage Δ
ts/features/lollipop/utils/login.ts 90.00% <ø> (ø)

... and 13 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e5173a...77cb123. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

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 fixes a compatibility issue in SAML request decoding by changing the data type passed to pako.inflateRaw from Buffer to Uint8Array.

Key Changes:

  • Modified the lollipopSamlVerify function to wrap the Buffer in Uint8Array.from() before passing to pako.inflateRaw

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

// Result is a base64 encoded string, so decode it to obtain the (server) original XML
const xmlSamlRequest = pako.inflateRaw(
Buffer.from(decodedSamlRequest, "base64"),
Uint8Array.from(Buffer.from(decodedSamlRequest, "base64")),
Copy link

Copilot AI Oct 21, 2025

Choose a reason for hiding this comment

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

The conversion Uint8Array.from(Buffer.from(...)) creates an unnecessary intermediate Buffer object. Since Buffer in Node.js is already a subclass of Uint8Array, you can directly pass the Buffer to pako.inflateRaw. If compatibility issues exist, consider using new Uint8Array(Buffer.from(decodedSamlRequest, "base64")) instead, which is more efficient as it creates a view over the same memory rather than copying the data.

Suggested change
Uint8Array.from(Buffer.from(decodedSamlRequest, "base64")),
new Uint8Array(Buffer.from(decodedSamlRequest, "base64")),

Copilot uses AI. Check for mistakes.
// SAMLRequest is URL encoded, so decode it
try {
const decodedSamlRequest = decodeURIComponent(urlEncodedSamlRequest);
// Result is a base64 encoded string, so decode it to obtain the (server) original XML
Copy link

Copilot AI Oct 21, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'rquest' to 'request' in the PR title.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

IO-A&I IO - Autenticazione e Identità

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants