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 CILogon redirect #18962

Closed
wants to merge 1 commit into from
Closed

Conversation

laperlej
Copy link
Collaborator

@laperlej laperlej commented Oct 9, 2024

When galaxy redirects to CILogon, it passes a query parameter called idphint which is usually a url starting with https://.

Starting at 24.1 the axios calls in the ExternalLogin.vue were changed to use withPrefix from utils/redirect.ts. The problem is that this strips all double slashes, invalidating any protocol passed as parameters (https:// becomes http:/)

I added a test to ensure that CILogon redirects are done correctly and changed the regex to only match duplicate slashes that are not preceded by a colon.

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@github-actions github-actions bot added this to the 24.2 milestone Oct 9, 2024
@laperlej laperlej changed the base branch from dev to release_23.2 October 9, 2024 17:17
@laperlej laperlej changed the base branch from release_23.2 to dev October 9, 2024 17:17
@davelopez
Copy link
Contributor

Thank you @laperlej!

Can you target the release_24.1 branch since this is a bug fix for that version?

@laperlej laperlej changed the base branch from dev to release_24.1 October 10, 2024 11:45
@laperlej laperlej force-pushed the fix_cilogon branch 3 times, most recently from b6f0d1a to 7fa9314 Compare October 10, 2024 11:54
@laperlej laperlej changed the base branch from release_24.1 to dev October 10, 2024 11:58
@laperlej laperlej closed this Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants