Bug 9587 - Allow login via mozilla persona - https://www.mozilla.org/en-US/persona/
Summary: Allow login via mozilla persona - https://www.mozilla.org/en-US/persona/
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Authentication (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Assignee: Galen Charlton
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks: 9704 17486
  Show dependency treegraph
 
Reported: 2013-02-11 09:28 UTC by Chris Cormack
Modified: 2016-12-14 17:33 UTC (History)
4 users (show)

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


Attachments
Bug 9587 : Mozilla Persona login (40.14 KB, patch)
2013-02-11 09:47 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 9587 : Follow up, fixing tabs in C4/Auth.pm (17.14 KB, patch)
2013-02-11 18:43 UTC, Chris Cormack
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9587 : Mozilla Persona login (40.30 KB, patch)
2013-02-11 19:30 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9587 : Follow up, fixing tabs in C4/Auth.pm (16.29 KB, patch)
2013-02-11 19:30 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 9587 : Followup removing a commented out warn that was annoying the qa tools (809 bytes, patch)
2013-02-12 01:23 UTC, Chris Cormack
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9587 : Followup removing a commented out warn that was annoying the qa tools (926 bytes, patch)
2013-02-12 02:07 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 9587 Follow up to fix the problem Katrin identified (5.31 KB, patch)
2013-02-17 09:45 UTC, Chris Cormack
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9587 Follow up to fix the problem Katrin identified (5.46 KB, patch)
2013-02-17 12:17 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[PASSED-QA] Bug 9587 : Mozilla Persona login (40.37 KB, patch)
2013-02-17 15:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED-QA] Bug 9587 : Follow up, fixing tabs in C4/Auth.pm (16.36 KB, patch)
2013-02-17 15:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED-QA] Bug 9587 : Followup removing a commented out warn that was annoying the qa tools (983 bytes, patch)
2013-02-17 15:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9587 Follow up to fix the problem Katrin identified (5.82 KB, patch)
2013-02-17 15:24 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 9587 : QA follow up - fix one tabulation character (756 bytes, patch)
2013-02-17 15:24 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 9587 : Handling mismatched emails better (2.02 KB, patch)
2013-02-20 22:41 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 9587 : Handling mismatched emails better (2.02 KB, patch)
2013-02-20 22:43 UTC, Chris Cormack
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9587 : Handling mismatched emails better (2.13 KB, patch)
2013-02-21 14:16 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cormack 2013-02-11 09:28:08 UTC

    
Comment 1 Chris Cormack 2013-02-11 09:47:41 UTC Comment hidden (obsolete)
Comment 2 Bernardo Gonzalez Kriegel 2013-02-11 11:35:19 UTC
1) Test

a) Applied patch, updatedatabase, set preferences
b) Create patron with Persona account
c) Tried to login... enter a endless loop of login page reloads every 3 secs

Is anything more to do/enable?

2) Errors
koha-qa gives
* C4/Auth.pm                                                               FAIL
* koha-tmpl/opac-tmpl/prog/en/includes/masthead.inc                        FAIL
Comment 3 Chris Cormack 2013-02-11 17:44:01 UTC
Yes did you set the opacbaseurl? That needs to be set to the url you are using, because that is what the assertion persona sets is tied to. 

Does the qa script give any more useful messages than that?
Comment 4 Bernardo Gonzalez Kriegel 2013-02-11 18:05:07 UTC
Hi Chris,
yes, I set opacbaseurl, and have same behavior using chrome, safari, etc.
koha-qa -c 1 -v 2

* C4/Auth.pm                                                               FAIL
        pod                         OK
        forbidden patterns          FAIL
                The patch introduces a forbidden pattern: tabulation character (746)
                The patch introduces a forbidden pattern: tabulation character (821)
                The patch introduces a forbidden pattern: tabulation character (816)

* koha-tmpl/opac-tmpl/prog/en/includes/masthead.inc                        FAIL
        forbidden patterns          OK
        tt_valid                    FAIL
                lines 13
        valid_template              OK

I'll try on a clean master.
Comment 5 Chris Cormack 2013-02-11 18:30:37 UTC
Ill sort out the tab issues, and resubmit. Im 99% those were there and a cut and paste moved them so now the QA script is picking them up, but easily fixed.
Comment 6 Chris Cormack 2013-02-11 18:42:38 UTC
Is the OpacBaseURL exactly the same as the url you are hitting the site on? I am not getting the redirect, but if i change it to not match the site i am hitting i get that redirect. Perhaps it was wrong before and you fixed it? The assertion from Persona will be wrong then and it will continue to keep looping. To break the loop go to login.persona.org and log out.
Comment 7 Chris Cormack 2013-02-11 18:43:05 UTC Comment hidden (obsolete)
Comment 8 Chris Cormack 2013-02-11 18:51:48 UTC
I'll do a follow up so it won't loop in that case, but it still can't log in, so I'll make it throw an error.
Comment 9 Bernardo Gonzalez Kriegel 2013-02-11 19:05:01 UTC
Well, now (pre and post followup) works :)
Clean master, cleaned cookies, etc. from browser.
Tested with chrome, firefox, safari, opera.
New and old autentication works.

Old errors gone.
New error from koha-qa :(
* C4/Auth.pm                                                               FAIL
        pod                         OK
        forbidden patterns          FAIL
                The patch introduces a forbidden pattern: Data::Dumper::Dumper (1031)
Comment 10 Chris Cormack 2013-02-11 19:14:48 UTC
Well that is definitely not me. That qa script is a bit broken, since all the follow up does is fix tabs to spaces. That file was full of them, suddenly now any line that had tabs becomes my fault :)
Comment 11 Bernardo Gonzalez Kriegel 2013-02-11 19:30:41 UTC Comment hidden (obsolete)
Comment 12 Bernardo Gonzalez Kriegel 2013-02-11 19:30:51 UTC Comment hidden (obsolete)
Comment 13 Chris Cormack 2013-02-12 01:23:36 UTC Comment hidden (obsolete)
Comment 14 Bernardo Gonzalez Kriegel 2013-02-12 02:07:40 UTC Comment hidden (obsolete)
Comment 15 Katrin Fischer 2013-02-17 08:41:04 UTC
Hi Chris,

it works wonderfully, but I found one problem I have no explanation for:

1) Persona is activated
2) Logged in with my normal password in OPAC (front page or opac-user)
3) It shows the detail page from the patron account for a second, then logs me out again. The page I end up at is: http://localhost/cgi-bin/koha/opac-main.pl?logout.x=1

Could you check please?
Comment 16 Chris Cormack 2013-02-17 08:51:31 UTC
It is because I have it set to automatically log you in if it spots you logged into persona. Conversely if you are logged into Koha but not persona it is trying to log you, fixing this now.
Comment 17 Chris Cormack 2013-02-17 09:45:10 UTC Comment hidden (obsolete)
Comment 18 Bernardo Gonzalez Kriegel 2013-02-17 12:17:25 UTC Comment hidden (obsolete)
Comment 19 Katrin Fischer 2013-02-17 15:23:24 UTC
Created attachment 15474 [details] [review]
[PASSED-QA] Bug 9587 : Mozilla Persona login

Working on Mozilla Persona support (browser id)

    This will let a user log into Koha using browser id, if their email
    address used matches the email address inside Koha.

    Once an assertion is received, we simply need to find the user that
    matches that email address, and create a session for them.

    opac/svc/login handles this part.

    The nice thing about it is, the user doesn't have to do anything, like
    linking their account. As long as the email address they are using to
    identify themselves in browserid is the same as the one in Koha it
    will just work.

    This is covered by a systempreference, to allow people to do it, and
    is of course totally opt in, it works alongside normal Koha (or any
    other method) of login. So only those choosing to use it, need use it

Test Plan

1/ Make sure OPACBaseURL is set correctly
2/ Switch on the Persona syspref
3/ Make a borrower (or edit one) to have the email you plan to use as
the primary email
4/ Click sign in with email, make or use a persona account
5/ Logout
6/ Check you can still login and logout the normal way

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Comment: Works great.
It's not browser dependent, but tested with chrome, firefox, opera and safari.
Old an new login system works.
Minor errors, addresed in follow-up.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 20 Katrin Fischer 2013-02-17 15:23:45 UTC
Created attachment 15475 [details] [review]
[PASSED-QA] Bug 9587 : Follow up, fixing tabs in C4/Auth.pm

And translation problem in masthead.inc

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 21 Katrin Fischer 2013-02-17 15:23:54 UTC
Created attachment 15476 [details] [review]
[PASSED-QA] Bug 9587 : Followup removing a commented out warn that was annoying the qa tools

Not introduced by this work but no reason not to clean it

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Comment: no more complains from koha-qa

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 22 Katrin Fischer 2013-02-17 15:24:06 UTC
Created attachment 15477 [details] [review]
[PASSED QA] Bug 9587 Follow up to fix the problem Katrin identified

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Comment: Solve that problem, but now koha-qa complains about tabs
in C4/Context.pm.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Tests done:
1) System preference 'Persona' added correctly.
2) Persona off, normal login still possible
3) Persona on, Persona login works
4) Persona logout works
5) normal login still possible
6) normal logout still possible

Persona is off by default and uses the primary email address
from the patron account.
Comment 23 Katrin Fischer 2013-02-17 15:24:14 UTC
Created attachment 15478 [details] [review]
Bug 9587 : QA follow up - fix one tabulation character

Makes QA script happy.
Comment 24 Katrin Fischer 2013-02-17 15:24:43 UTC
Cool new feature!
Comment 25 Jared Camins-Esakov 2013-02-20 15:14:00 UTC
I am having problems getting this to work. I enabled Persona, and Koha seems to be recognizing me correctly, but I'm not logged in at the end. I get this in my logs (with $ENV{'DEBUG'} set):

Initiating an anonymous session... at /home/jcamins/kohaclone/C4/Auth.pm line 952.
Use of uninitialized value in concatenation (.) or string at /home/jcamins/kohaclone/C4/Auth.pm line 394.
Checking Auth at /home/jcamins/kohaclone/C4/Auth.pm line 618.
kohaversion : 3.1100018
Use of uninitialized value $cardnumber in concatenation (.) or string at /home/jcamins/kohaclone/C4/Auth.pm line 864.
Use of uninitialized value $userflags in concatenation (.) or string at /home/jcamins/kohaclone/C4/Auth.pm line 864.
AUTH_3 results: ,60,nicelynicely.johnson,Nicely Nicely,Johnson,,CPL,jcamins@gmail.com
Use of uninitialized value in printf at /home/jcamins/kohaclone/C4/Auth.pm line 912.
AUTH_4: ()      Nicely Nicely Johnson - CPL

Any idea what could be going on?
Comment 26 Chris Cormack 2013-02-20 15:55:05 UTC
The email you authenticated to persona is the primary email for your borrower in Koha right?
Comment 27 Jared Camins-Esakov 2013-02-20 16:51:06 UTC
(In reply to comment #26)
> The email you authenticated to persona is the primary email for your
> borrower in Koha right?

Yup. And the system recognized the patron, it just didn't log me in. Do I need to have a password set for the user? It strikes me that I do not.
Comment 28 Chris Cormack 2013-02-20 22:41:14 UTC Comment hidden (obsolete)
Comment 29 Chris Cormack 2013-02-20 22:43:41 UTC Comment hidden (obsolete)
Comment 30 Bernardo Gonzalez Kriegel 2013-02-21 14:16:37 UTC
Created attachment 15585 [details] [review]
[SIGNED-OFF] Bug 9587 : Handling mismatched emails better

To test:

Sign in to Koha via persona using an email that doesn't exist in Koha

Before the patch you will get into an infinite redirect loop

After the patch it will give you an error message

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Comment: Work as described. No errors.
Comment 31 Bernardo Gonzalez Kriegel 2013-02-21 14:40:09 UTC
Chris, I found another two anomalous situations that could be handled 
and reported as the missing email

1) 2+ users with same email, only one of them logs in, presume the first with matching email. 'email' field in borrorwers is not required to be unique. 
Is a strange situation but...

2) user without userid but correct email. Logs in, but enter endless loop. 
Not so strange if users are bulk-inserted, but without assigning userid.

Both should preclude a successful 'persona' login.
Comment 32 Jared Camins-Esakov 2013-02-24 15:19:14 UTC
This patch has been pushed to master.