Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix an issue with request id deletion happening too early #35

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

adamretter
Copy link
Contributor

@adamretter adamretter commented Aug 20, 2024

The request id was being deleted at the wrong stage in the process. Previously, as the exsaml:check-authnreqid function is called twice, it was unfortunately deleting the SAML request id during the first call, and so the second call always returns an error which interrupts the authentication process. This commit (b0eff7a) fixes the issue by ensuring the request id is only deleted when it is finished with.

NOTE this PR is stacked atop PR #34 (which was used to diagnose the issue), and so please merge PR #34 BEFORE merging this PR.

@adamretter adamretter added the bug Something isn't working label Aug 20, 2024
@adamretter adamretter requested review from dizzzz and marmoure August 20, 2024 11:41
@BernardDebord
Copy link

Without this fix, authentication process fails systematically.
I am using this code which is working correctly.

Copy link

@marmoure marmoure left a comment

Choose a reason for hiding this comment

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

LGTM.

…nce, and so calling empty(xmldb:remove(...)) makes no sense as the result is always fn:true()
…rocess. Previously, as the exsaml:check-authnreqid function is called twice, it was unfortunately deleting the SAML request id during the first call, and so the second call always returns an error which interrupts the authentication process. This commit fixes the issue by ensuring the request id is only deleted when it is finished with.
@adamretter adamretter force-pushed the hotfix/request-id-deletion branch from b0eff7a to a71b384 Compare August 30, 2024 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants