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

Enable shibboleth for non KIT ilias #98

Merged
merged 20 commits into from
Nov 5, 2024

Conversation

PinieP
Copy link
Contributor

@PinieP PinieP commented Nov 2, 2024

This pull request adds support for shibboleth login to (non KIT) ilias-web crawler.

@PinieP PinieP force-pushed the non-kit-shibboleth branch from 75cb4fc to 959fa8c Compare November 2, 2024 17:08
Copy link
Collaborator

@I-Al-Istannen I-Al-Istannen left a comment

Choose a reason for hiding this comment

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

Thanks!

This looks like a relatively straight-forward change, but you reformatted quite a few files to a smaller width. Can you please revert those changes? They make the text a bit harder, and the diff much harder to read :) I guess your editor just did not pick up on the 110 defined in the pyproject.toml file.

@I-Al-Istannen I-Al-Istannen force-pushed the master branch 2 times, most recently from 51496b6 to f5c4e82 Compare November 2, 2024 21:56
@I-Al-Istannen
Copy link
Collaborator

Hey, it would be cool if you could resolve the comments you have addressed in the UI (and maybe address the remaining two) when you have time :) That makes it a bit easier to see if action from our side is required and prevents us three deadlocking :P

@PinieP
Copy link
Contributor Author

PinieP commented Nov 4, 2024

Should be all done now, thanks. Be sure to tell me if there's anything else in here that I need to address.

@I-Al-Istannen
Copy link
Collaborator

I have no idea why, but the 302 redirect does not work for KIT. It gets rejected with a "message does not meet security requirements", but no further mention why. Opening the same URL in a fresh browser window works. Shib really doesn't like the PFERD request though, even when it looks virtually identical.

I have no real idea why, but I can therefore not merge this. The old POST workflow works for KIT, so maybe just fall back to that one in the KIT case and let the rest fend for themselves.

@I-Al-Istannen
Copy link
Collaborator

@PinieP does this still work on your instance? Please make very sure it is not reusing any cookies. Ideally you would download a folder into a fresh directory in /tmp to ensure no cookies are reused (and maybe have a look at the --explain) output to ensure it is not loading any cookies.

@PinieP
Copy link
Contributor Author

PinieP commented Nov 5, 2024

Still works for me. Thanks!

@I-Al-Istannen I-Al-Istannen merged commit 596b6a7 into Garmelon:master Nov 5, 2024
5 checks passed
@I-Al-Istannen
Copy link
Collaborator

Thanks :)

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.

3 participants