Skip to content

Conversation

@fisx
Copy link
Contributor

@fisx fisx commented May 9, 2025

https://wearezeta.atlassian.net/browse/WPB-17585

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@fisx fisx marked this pull request as ready for review May 9, 2025 14:47
@fisx fisx requested review from a team as code owners May 9, 2025 14:47
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label May 9, 2025
-- do not use `resp` as a result of `parseAuthnResponseBody`! only use what comes back
-- from `simpleVerifyAuthnResponse`!
either (throwError . BadSamlResponseXmlError . cs) pure $
either (throwError . BadSamlResponseXmlError . cs . (<> extraErrInfo)) pure $
Copy link
Member

Choose a reason for hiding this comment

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

It could be hard to tell where the error stops and where the response starts. We should perhaps merge the strings with identifier text like parseError: <parseError>, base64Response: <response>

@fisx fisx requested a review from akshaymankar May 12, 2025 12:06
Copy link
Member

@akshaymankar akshaymankar left a comment

Choose a reason for hiding this comment

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

Assuming its safe to print such details.

@fisx fisx merged commit 35ce21d into develop May 12, 2025
8 checks passed
@fisx fisx deleted the WPB-17585-hotpatch-more-error-info branch May 12, 2025 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants