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

No fallback when certain Shib headers are not present #99

Open
jdpurdyvi opened this issue Nov 1, 2023 · 4 comments
Open

No fallback when certain Shib headers are not present #99

jdpurdyvi opened this issue Nov 1, 2023 · 4 comments
Assignees
Labels

Comments

@jdpurdyvi
Copy link

d64dd2b breaks configs that don't provide [username][name]. Could the code fallback to using Shib-Session-ID?

@jdpurdyvi
Copy link
Author

Well, I was able to remap the Username attribute to something from our Identity Provider and all is well enough.

@jrchamp
Copy link
Collaborator

jrchamp commented Nov 2, 2023

I'm so sorry @jdpurdyvi! In looking at the code, I was assuming that username was a required mapping. How were you getting around it before? Oh! Do you rely completely on the shibboleth_override_username hook?

@jdpurdyvi
Copy link
Author

I suppose so? I would be happy to share specific env details if that's helpful:

  • Username was mapped to EDUPERSONPRINCIPALNAME, which provided [email protected]
  • Allow Automatic Account Merging (Bypass Username Management) was also enabled
  • Accounts were individually created with unique-name as the WordPress account name
  • To get v2.4.3 to work I had to change the Username mapping to colostateEduPersonEID which provides the matching unique-name without the @colostate.edu

Automatic login continued to work if I had a valid Shib session from a same-domain site that was still on v2.4.2

@jrchamp
Copy link
Collaborator

jrchamp commented Nov 2, 2023

Ah, I see! It looks like you were using the email-matching to have it map to the correct user. Now that you have the Username mapping set to your unique-name which does match the account in WordPress, you can probably switch from Allow Automatic Account Merging (Bypass Username Management) to Allow Automatic Account Merging (it's a slightly more efficient WordPress lookup).

I'll see what I can do to add a fallback check that mirrors the truly required fields instead of only the username.

@jrchamp jrchamp reopened this Nov 2, 2023
@jrchamp jrchamp added the bug label Nov 2, 2023
@jrchamp jrchamp self-assigned this Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants