Skip to content

Conversation

labkey-adam
Copy link
Contributor

@labkey-adam labkey-adam commented Aug 4, 2025

Rationale

Slightly stricter isLocalUri() check allows redirect to SAML test IDP (same host, but different servlet) as an "external" URI.

Related Pull Requests

@labkey-adam labkey-adam marked this pull request as draft August 4, 2025 22:29
@labkey-adam labkey-adam changed the title Temporary logging to investigate redirect problems Fix test SAML IDP redirect Aug 5, 2025
@labkey-adam labkey-adam marked this pull request as ready for review August 5, 2025 15:27
@labkey-adam labkey-adam requested review from a team and labkey-tchad August 5, 2025 15:28
@labkey-adam labkey-adam self-assigned this Aug 5, 2025
@labkey-adam labkey-adam changed the title Fix test SAML IDP redirect Fix SAML test IDP redirect Aug 5, 2025
|| currentContext == null
|| currentContext.getActionURL() == null
// Scheme check is here for test SAML redirect, which is to the same host ("localhost") but a different scheme
|| getHost().equalsIgnoreCase(currentContext.getActionURL().getHost()) && getScheme().equalsIgnoreCase(currentContext.getActionURL().getScheme());
Copy link
Contributor

Choose a reason for hiding this comment

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

I know operator precedence makes this work but I typically prefer explicit parens.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

@labkey-adam labkey-adam merged commit 3c033b8 into develop Aug 5, 2025
9 of 10 checks passed
@labkey-adam labkey-adam deleted the fb_redirect_logging branch August 5, 2025 21:48
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.

2 participants