Currently in the OPAC if you have Shibboleth enabled the loginModal is a bit confusing when differentiating between local and shib logins. It would be nice if this was a bit clearer. I'm proposing the idea of using jQuery UI tabs to split them into separate tabs.
Created attachment 104464 [details] [review] Bug 25405: Seperate Shib and Local log in for OPAC modal 1. Enable shibboleth in your koha-conf.xml by changng the value of <useshibboleth> to 1. 2. I also need to add the following: <shibboleth> <matchpoint>email</matchpoint> <mapping> <email is="SHIB_ATTR_email"></email> </mapping> </shibboleth> 3. Look at the loginModal in the OPAC by clicking 'Log in to your account' in the top right corner. 4. Apply patch, restart all, and regenerate CSS. 5. Look at the loginModal again and you should be able to toggle Shibboleth/Local. 6. Make sure it all works. 7. Turn Shibboleth back off and restart all. 8. Make sure everything still works without Shibboleth on.
Created attachment 104465 [details] [review] Bug 25405: Seperate Shib and Local log in for OPAC modal 1. Enable shibboleth in your koha-conf.xml by changng the value of <useshibboleth> to 1. 2. I also need to add the following: <shibboleth> <matchpoint>email</matchpoint> <mapping> <email is="SHIB_ATTR_email"></email> </mapping> </shibboleth> 3. Look at the loginModal in the OPAC by clicking 'Log in to your account' in the top right corner. 4. Apply patch, restart all, and regenerate CSS. 5. Look at the loginModal again and you should be able to toggle Shibboleth/Local. 6. Make sure it all works. 7. Turn Shibboleth back off and restart all. 8. Make sure everything still works without Shibboleth on.
Created attachment 104516 [details] Screenshot of the revised interface I don't think this is working well. The login form is too narrow for the tabs to fit comfortably. I think it might work better if we used something besides tabs to switch between the views. Or, perhaps the Shibboleth message could be reformatted to make it clearer, without showing and hiding forms? I don't really understand what is triggering the confusion, so it's hard to say. Personally I find the word "Shibboleth" to be kind of scary so I would never click that link unless I knew what it was!
Created attachment 104520 [details] [review] Bug 25405: Seperate Shib and Local log in for OPAC modal 1. Enable shibboleth in your koha-conf.xml by changng the value of <useshibboleth> to 1. 2. I also need to add the following: <shibboleth> <matchpoint>email</matchpoint> <mapping> <email is="SHIB_ATTR_email"></email> </mapping> </shibboleth> 3. Look at the loginModal in the OPAC by clicking 'Log in to your account' in the top right corner. 4. Apply patch, restart all, and regenerate CSS. 5. Look at the loginModal again and you should be able to toggle Shibboleth/Local. 6. Make sure it all works. 7. Turn Shibboleth back off and restart all. 8. Make sure everything still works without Shibboleth on.
I think so problems can be addressed by changing some CSS and I supplied a new patch that does. But if this idea is generally disliked I'm 100% willing to abandon the idea.
Created attachment 104522 [details] Modal with updated CSS
Created attachment 104542 [details] Gitlab Login Personally, I would prefer something like this what Gitlab does for their (admittedly non-modal) login. I have a OpenID Connect module (which I swear one of these days I will upstream), and I've had library clients with multiple sign in options, so I have to provide them multiple choices at login. Honestly, I think this is how most apps work these days, so I think this styling would be better. But that's just my 2 cents. Said as someone who would love to do it but hasn't prioritized writing that patch yet...
> Personally, I would prefer something like this what Gitlab does for their (admittedly non-modal) login. I agree. And surely a vertical display will be more responsive.
I also like the GitLab login, I'll have another go at this patch ASAP
(In reply to Lucas Gass from comment #9) > I also like the GitLab login, I'll have another go at this patch ASAP Champion! Looking forward to it.
Created attachment 114754 [details] [review] Bug 25405: clearly define local login and shib login sections This patch attempts to clearly seperate local login from shibboleth login in the OPAC loginModal. It includes seperate id's for local and shibboleth to make it easier to customize this login To test: 1. Enable shibboleth in your koha-conf.xml by changng the value of <useshibboleth> to 1. 2. I also add the following to my koha-conf: <shibboleth> <matchpoint>email</matchpoint> <mapping> <email is="SHIB_ATTR_email"></email> </mapping> </shibboleth> 3. Look at the loginModal in the OPAC by clicking 'Log in to your account' in the top right corner. 4. Apply patch, restart all, and regenerate CSS. (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client) 5. Look at the loginModal again the shibboleth section should be underneath the local login stuff. 6. Inscept the loginModal and make sure there is a <div> for both local_login and shibboleth_login 7. Make sure it all works. 8. Turn Shibboleth back off and restart all. 9. Make sure everything still works and looks good without Shibboleth off.
Cheers, Lucas. I think that this is looking quite nice. I do have a few suggestions though. 1. I'd change "shibboleth_login" to something more generic like "external_login" (or idp_login or remote_login), so that we can add additional methods more easily in the future. (I think it's a fairly easy change to make for future proofing.) 2. Wrap the word "Shibboleth" in a span with an ID like "shibboleth_name". I like the sound of Bug 27318, but I suspect that rewriting the word Shibboleth with some Javascript would be sufficient for many cases. Alternatively, we replace the existing text with a new default news item that just says "Shibboleth" and maybe includes a generic Shibboleth icon? (As you can tell, I really like how Gitlab does multi provider auth heh.) I agree that Shibboleth is meaningless to most users, and honestly it's actually incorrect for most of my clients, since their actual Identity Provider is not Shibboleth and is instead something like Microsoft ADFS with institutional branding (like "Department of X"). 3. Wrap the Shibboleth text in another border box. (This would be to differentiate it from additional methods added later.)
Of course, this actually reminds me of work I want to do for adding identity providers via the staff admin...
Created attachment 116346 [details] [review] Bug 25405: clearly define local login and shib login sections This patch attempts to clearly seperate local login from shibboleth login in the OPAC loginModal. It includes seperate id's for local and shibboleth to make it easier to customize this login To test: 1. Enable shibboleth in your koha-conf.xml by changng the value of <useshibboleth> to 1. 2. I also add the following to my koha-conf: <shibboleth> <matchpoint>email</matchpoint> <mapping> <email is="SHIB_ATTR_email"></email> </mapping> </shibboleth> 3. Look at the loginModal in the OPAC by clicking 'Log in to your account' in the top right corner. 4. Apply patch, restart all, and regenerate CSS. (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client) 5. Look at the loginModal again the shibboleth section should be underneath the local login stuff. 6. Inscept the loginModal and make sure there is a <div> for both local_login and external_login 7. Make sure it all works. 8. Turn Shibboleth back off and restart all. 9. Make sure everything still works and looks good without Shibboleth off. 10. Check HTML elements #external_login, #external_login_button, #external_login_name, and #external_login_icon. There were added to allow for further customization with css/js
This is looking so good! I'm sorry but I just have a couple more things. This is so so close. In "div#external_login", the "a" element should be encapsulated within "div#external_login_button". I think that might just be a typo in the current patch. I think that "external_login_button" should be "shibboleth_login_button" and "external_login_icon" should be "shibboleth_login_icon" and "external_login_name" should be "shibboleth_login_name". My reasoning here is that we could add additional external login buttons for different external auth providers. I have an OpenID Connect local extension that I would add here, for instance. This would also be a good place for authentication plugins to advertise. In fact, if you don't mind, I might just provide a little follow-up and see what you think...
Created attachment 116348 [details] [review] Bug 25405: clearly define local login and shib login sections This patch attempts to clearly seperate local login from shibboleth login in the OPAC loginModal. It includes seperate id's for local and shibboleth to make it easier to customize this login To test: 1. Enable shibboleth in your koha-conf.xml by changng the value of <useshibboleth> to 1. 2. I also add the following to my koha-conf: <shibboleth> <matchpoint>email</matchpoint> <mapping> <email is="SHIB_ATTR_email"></email> </mapping> </shibboleth> 3. Look at the loginModal in the OPAC by clicking 'Log in to your account' in the top right corner. 4. Apply patch, restart all, and regenerate CSS. (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client) 5. Look at the loginModal again the shibboleth section should be underneath the local login stuff. 6. Inscept the loginModal and make sure there is a <div> for both local_login and external_login 7. Make sure it all works. 8. Turn Shibboleth back off and restart all. 9. Make sure everything still works and looks good without Shibboleth off. 10. Check HTML elements #external_login, #external_login_button, #external_login_name, and #external_login_icon. There were added to allow for further customization with css/js Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 116349 [details] [review] Bug 25405: Make it easier to add additional identity providers Signed-off-by: David Cook <dcook@prosentient.com.au>
I've signed off your patch, Lucas. So if you sign off mine, I think we can move this to Signed Off. :D
With my patch, I tested out relabeling the option in the console: $(".shibboleth_login_name").text('Keycloak'); Worked so nicely. -- Oh, although opac-auth.tt still shows Shibboleth without the span, and it could use the same HTML construct as the modal... mmm maybe that can just be a follow-up bug, as it would be nice to get this one in... [U+1F62C]
Created attachment 116350 [details] [review] Bug 25405: clean up some CSS
> Oh, although opac-auth.tt still shows Shibboleth without the span, and it > could use the same HTML construct as the modal... mmm maybe that can just be > a follow-up bug, as it would be nice to get this one in... > > [U+1F62C] Yes, we need to add auth.tt next and there is a bit more to untangle with cas, shib, and GoogleOpenIDConnect. I file another bug to do so.
Created attachment 116351 [details] [review] Bug 25405: clean up some CSS Signed-off-by: David Cook <dcook@prosentient.com.au>
(In reply to Lucas Gass from comment #21) > Yes, we need to add auth.tt next and there is a bit more to untangle with > cas, shib, and GoogleOpenIDConnect. I file another bug to do so. Sounds good to me
Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 25405: clearly define local login and shib login sections Using index info to reconstruct a base tree... M koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc error: Failed to merge in the changes. Can you please add the other bug (comment#21) in See also or as blocks?
Still working on this one, Lucas?
(In reply to David Cook from comment #25) > Still working on this one, Lucas? I'm not sure if this bug is still valid now that we have the 'OPACShibOnly' pref from Bug 18506
(In reply to Lucas Gass from comment #26) > (In reply to David Cook from comment #25) > > Still working on this one, Lucas? > > I'm not sure if this bug is still valid now that we have the 'OPACShibOnly' > pref from Bug 18506 I think it's still technically valid but I see the merit of Bug 18506. I know 3 of our biggest Shibboleth users only want the SAML login. I have a local customization that does the same thing as Bug 18506 but in a very different way (for reasons). Admittedly, my interest in this is personal rather than business. I suppose having multiple auth options doesn't really make sense for a library OPAC. Maybe we should mark this one as invalid...
> Admittedly, my interest in this is personal rather than business. I suppose > having multiple auth options doesn't really make sense for a library OPAC. > Maybe we should mark this one as invalid... I agree here and I am setting this to invalid, thanks David.
With bug 31064, it might be a bit easier. Admittedly I'm targeting a scenario where I insert a button using OpacUserJS to point to custom/plugin code.