Bug 25405 - Make separation of Shibboleth and Local login clearer in modal login on OPAC
Summary: Make separation of Shibboleth and Local login clearer in modal login on OPAC
Status: CLOSED INVALID
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Lucas Gass
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 27318
  Show dependency treegraph
 
Reported: 2020-05-06 22:29 UTC by Lucas Gass
Modified: 2022-06-29 08:49 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 25405: Seperate Shib and Local log in for OPAC modal (4.22 KB, patch)
2020-05-06 22:38 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 25405: Seperate Shib and Local log in for OPAC modal (122.05 KB, patch)
2020-05-06 22:47 UTC, Lucas Gass
Details | Diff | Splinter Review
Screenshot of the revised interface (45.91 KB, image/png)
2020-05-07 14:38 UTC, Owen Leonard
Details
Bug 25405: Seperate Shib and Local log in for OPAC modal (4.23 KB, patch)
2020-05-07 15:14 UTC, Lucas Gass
Details | Diff | Splinter Review
Modal with updated CSS (27.99 KB, image/png)
2020-05-07 15:29 UTC, Lucas Gass
Details
Gitlab Login (29.19 KB, image/jpeg)
2020-05-08 00:19 UTC, David Cook
Details
Bug 25405: clearly define local login and shib login sections (7.71 KB, patch)
2020-12-30 21:50 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 25405: clearly define local login and shib login sections (8.96 KB, patch)
2021-02-04 23:47 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 25405: clearly define local login and shib login sections (9.02 KB, patch)
2021-02-05 00:31 UTC, David Cook
Details | Diff | Splinter Review
Bug 25405: Make it easier to add additional identity providers (3.04 KB, patch)
2021-02-05 00:31 UTC, David Cook
Details | Diff | Splinter Review
Bug 25405: clean up some CSS (1.24 KB, patch)
2021-02-05 01:09 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 25405: clean up some CSS (1.30 KB, patch)
2021-02-05 02:34 UTC, David Cook
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas Gass 2020-05-06 22:29:10 UTC
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.
Comment 1 Lucas Gass 2020-05-06 22:38:10 UTC
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.
Comment 2 Lucas Gass 2020-05-06 22:47:57 UTC Comment hidden (obsolete)
Comment 3 Owen Leonard 2020-05-07 14:38:42 UTC
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!
Comment 4 Lucas Gass 2020-05-07 15:14:52 UTC
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.
Comment 5 Lucas Gass 2020-05-07 15:16:21 UTC
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.
Comment 6 Lucas Gass 2020-05-07 15:29:19 UTC
Created attachment 104522 [details]
Modal with updated CSS
Comment 7 David Cook 2020-05-08 00:19:30 UTC
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...
Comment 8 Fridolin Somers 2020-09-29 13:23:16 UTC
> 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.
Comment 9 Lucas Gass 2020-09-29 15:02:35 UTC
I also like the GitLab login, I'll have another go at this patch ASAP
Comment 10 David Cook 2020-09-29 23:18:53 UTC
(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.
Comment 11 Lucas Gass 2020-12-30 21:50:40 UTC
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.
Comment 12 David Cook 2021-01-04 01:08:24 UTC
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.)
Comment 13 David Cook 2021-01-04 01:17:40 UTC
Of course, this actually reminds me of work I want to do for adding identity providers via the staff admin...
Comment 14 Lucas Gass 2021-02-04 23:47:52 UTC
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
Comment 15 David Cook 2021-02-05 00:23:09 UTC
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...
Comment 16 David Cook 2021-02-05 00:31:12 UTC
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>
Comment 17 David Cook 2021-02-05 00:31:16 UTC
Created attachment 116349 [details] [review]
Bug 25405: Make it easier to add additional identity providers

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 18 David Cook 2021-02-05 00:32:01 UTC
I've signed off your patch, Lucas. So if you sign off mine, I think we can move this to Signed Off.

:D
Comment 19 David Cook 2021-02-05 00:36:58 UTC
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...

😬
Comment 20 Lucas Gass 2021-02-05 01:09:12 UTC
Created attachment 116350 [details] [review]
Bug 25405: clean up some CSS
Comment 21 Lucas Gass 2021-02-05 01:13:19 UTC
> 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...
> 
> 😬

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.
Comment 22 David Cook 2021-02-05 02:34:30 UTC
Created attachment 116351 [details] [review]
Bug 25405: clean up some CSS

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 23 David Cook 2021-02-05 02:34:43 UTC
(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
Comment 24 Katrin Fischer 2021-02-17 20:53:01 UTC
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?
Comment 25 David Cook 2021-06-03 02:20:30 UTC
Still working on this one, Lucas?
Comment 26 Lucas Gass 2021-07-15 21:19:43 UTC
(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
Comment 27 David Cook 2021-07-15 23:51:00 UTC
(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...
Comment 28 Lucas Gass 2021-07-19 21:52:13 UTC
> 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.
Comment 29 David Cook 2022-06-29 08:49:33 UTC
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.