The latest patch is a fully functional version of GoogleOAuth2, being used in production here at http://library.pbc.wa.edu.au/

Test Plan:
1) Apply this patch
2) Update the database (this is the bit I'm not sure about - I've added the SQL into the atomicupdates directory - is this right?)
3) Create a web app in the google cloud console:
     - Go to https://cloud.google.com/console
     - Create a project, and give it some details
     - Open the Project by clicking on it
     - Under APIs & auth menu, open "Registered Apps" and click "Register App"
     - Give it a name and make sure you select "Web app", click ok
     - Under OAuth 2.0 Client ID,
         - under web origin, type <your_opac_address>
         - in the redirect uri enter <your_opac_address>/cgi-bin/koha/svc/googleoauth2
         - click Generate
4) Set the GoogleOAuth2ClientID and GoogleOAuth2ClientSecret according to the values generated above
5) Test that clicking login intiates an oauth2 login
6) Try logging in with an email that is not registered to a library user. It should fail and offer either to retry or to login manually.
7) Try logging in with an email that is registered to a library user. You should be logged in.

When using this in a public computer, there are some nice javascripts that can be injected to make this a whole lot more secure. Basically, the following userscripts disable the google account chooser, and log a person out of their google accounts when they log out of Koha. This is not very useful or polite outside of a public terminal, hence my thinking to do it with userscripts.

Just putting it here as a reference for anyone thinking to use this.

--------------------------------------------------------------------------------
// ==UserScript==
// @id             accounts.google.com-forcenocookie@scriptish
// @name           Disable persistent google login
// @version        1.0
// @namespace      
// @author         
// @description    
// @include        https://accounts.google.com/*
// @run-at         document-end
// ==/UserScript==
document.getElementsByName("PersistentCookie")[0].value='no';

--------------------------------------------------------------------------------
// ==UserScript==
// @id             koha-full-oauth2-logout@scriptish
// @name           Force Full Logout
// @version        1.0
// @namespace
// @author
// @description
// @include        http://OPACURLHERE/*
// @run-at         document-end
// ==/UserScript==
document.getElementById('logoutmenu').href="https://www.google.com/accounts/Logout?continue=https://appengine.google.com/_ah/logout?continue="+document.getElementById('logoutmenu').href;
document.getElementById('logout').href="https://www.google.com/accounts/Logout?continue=https://appengine.google.com/_ah/logout?continue="+document.getElementById('logoutmenu').href;

Ping on this. Anybody interested in  signing this one off?

Patch did not apply. Merge conflict in 
koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc

Oh no! I'm on holiday for the next week, but will fix it next Tuesday.

Ran a test on this patch. And I had some issues. I'm no good at programming. But a friend of mine helped me. I thought I could post this to help out others.

I hope you'll understand what I mean. I'm new here and more of a librarian than a programmer.

So... Here's what we found out:

In the file: usr/share/koha/opac/cgi-bin/opac/svc/googleoauth2
there appeared to be some lines missing in the patch at the very beginning of the file. Though the code is available here in this bug. So that was easy to fix.

And then we still had some issues. At that time I still had an error 400 straight from google this time. We had to set up the opac URL in: /etc/apache2/conf.d/

I'm not sure if this was supposed to be done before or not. But I had to do it at that time.

Also: in the opac-main there is no hint where to log in with the google account. It would be a nice feature that once you use Google Oauth it would say which log in link to use to do that. Maybe even with a google logo?

Other than that the patch is working perfectly though I didn't test it extensively. It's only running on my test server.

(In reply to Marc Véron from comment #10)
> Patch did not apply. Merge conflict in 
> koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc

Should apply cleanly with the latest patch...

I tried to apply the second patch: Allow Google OAuth2 Login

It did not apply with:
error: patch failed: koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc:74
error: koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc: patch does not apply
Did you hand edit your patch?
It does not apply to blobs recorded in its index.
Cannot fall back to three-way merge.

--------------

Additionaly, I think in installer/data/mysql/updatedatabase.pl line 8573 should read:

$DBversion = "XXX";

See: http://wiki.koha-community.org/wiki/System_Preferences

Interesting work.. could we have this rebased for testing Nicholas?

It depends on 5010, because I cut out the hard coded 'http://' part of the code in the new googleoauth2 script. Technically, you can test it without 5010, but you need the protocol in your OPACBaseURL. :)

Thanks so much to M. Tompsett for doing this. I've been busy with other jobs (including upgrading to 3.18!), and just haven't got around to sending in our patch. :-)
(In reply to M. Tompsett from comment #22)
> It depends on 5010, because I cut out the hard coded 'http://' part of the
> code in the new googleoauth2 script. Technically, you can test it without
> 5010, but you need the protocol in your OPACBaseURL. :)

I'm just wrapping up a local project on using OpenID Connect, which is basically a wrapper around OAuth2.0 with a few extras thrown in on top, so I'd be interested in taking a look at this at some point.

Could we look at making this more extensible though? There are lots of other OAuth2.0 providers than just Google. 

Have yo looked at http://search.cpan.org/~kgrennan/Net-OAuth2-0.06/lib/Net/OAuth2.pm? I believe Chris Cormack took a look at that recently.

Anywho, I could be interested in testing this out down the road!

Hmm, it might be a better idea to add a parameter instead of using the $persona parameter in C4::Auth::checkauth().

C4::Auth needs a clean-up in general... which Martin (ashimema) is working on, I believe.

In any case, locally I added a $external_authen hashref as a parameter after $persona. I use $external_authen->{OpenIDConnect} to store the data I need to pass to checkauth(), add some handling in the appropriate spots, and that's it.

You might want to look at RFC 6749 (OAuth 2.0) http://tools.ietf.org/html/rfc6749 as well, as it might give you some more information.

OpenID Connect has a lot of validation steps, which may or may not also exist in OAuth2.0... not sure.

(In reply to David Cook from comment #25)
> Hmm, it might be a better idea to add a parameter instead of using the
> $persona parameter in C4::Auth::checkauth().

I would think the parameter would be better renamed to $AuthenticatedEmail, because persona and this OAuth patch both authenticate against an Email address (the primary email address is used to find the user).


> C4::Auth needs a clean-up in general... which Martin (ashimema) is working
> on, I believe.

True, it does need clean-up.


> In any case, locally I added a $external_authen hashref as a parameter after
> $persona. I use $external_authen->{OpenIDConnect} to store the data I need
> to pass to checkauth(), add some handling in the appropriate spots, and
> that's it.

NOOOOOO! Please hashref the parameter!
If there are more than 3 parameters, you probably should use a hashref.

(In reply to David Cook from comment #24)
> I'm just wrapping up a local project on using OpenID Connect, which is
> basically a wrapper around OAuth2.0 with a few extras thrown in on top, so
> I'd be interested in taking a look at this at some point.
> 
> Could we look at making this more extensible though? There are lots of other
> OAuth2.0 providers than just Google. 
> 
> Have yo looked at
> http://search.cpan.org/~kgrennan/Net-OAuth2-0.06/lib/Net/OAuth2.pm? I
> believe Chris Cormack took a look at that recently.
> 
> Anywho, I could be interested in testing this out down the road!

Hi David,

I like the idea of making it more extensible, and when I get a chance (probably in a few weeks from now) I'll see about checking out OAuth2.pm . Since we run Google Apps for Education for our students (at library.pbc.wa.edu.au), Google's oauth2 was our priority. It's been quite a while since I wrote this, but I think google has some google-specific functionality available - like the hd parameter. (Actually, the hd parameter isn't all that useful for us, seeing as it doesn't seem to like subdomains)

(In reply to M. Tompsett from comment #26)

> I would think the parameter would be better renamed to $AuthenticatedEmail,
> because persona and this OAuth patch both authenticate against an Email
> address (the primary email address is used to find the user).
> 

Yeah, I thought about that a bit, but it also sets a "persona" key in the session, which I think might be used for persona session management, so re-using it without careful checking could lead to weirdness with Persona. 

> NOOOOOO! Please hashref the parameter!
> If there are more than 3 parameters, you probably should use a hashref.

...

Yes, a single hashref would be nicer. If I were designing a new sub, I would use a single hashref. However, I'm personally not planning on refactoring C4::Auth::checkauth() and testing every use of it. Nor am I going to add checking to checkauth() so that it can use a single hashref OR a bunch of scalars. That just leads to more technical debt.

Of course, I'm not planning on upstreaming my OpenID Connect work at the moment anyway, so it doesn't really matter. I'm sure whoever refactors checkauth() someday will use a single hashref instead.

Comment on attachment 36355 [details] [review]
Bug 10988 - Allow login via Google OAuth2

Review of attachment 36355 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc
@@ +58,4 @@
>                                                  <li role="presentation"><a href="#" tabindex="-1" class="menu-inactive" role="menuitem">No private lists</a></li>
>                                                  <li role="presentation"><a href="/cgi-bin/koha/opac-shelves.pl?display=privateshelves" tabindex="-1" role="menuitem" class="listmenulink">New list</a></li>
>                                              [% END %]
> +                                        [% ELSIF ( GoogleOAuth2 ) %]

I'd be surprised if there wasn't a [% USE Koha %] at the top of the file. Why pass preferences? Koha.Preference('GoogleOAuth2')

Comment on attachment 40087 [details] [review]
Bug 10988 - Allow login via Google OAuth

Review of attachment 40087 [details] [review]:
-----------------------------------------------------------------

::: C4/Auth.pm
@@ +37,4 @@
>  use Encode qw( encode is_utf8);
>  
>  # use utf8;
> +use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS $debug $googleoauth2 $ldap $cas $caslogout $shib $shib_login);

No need to add this.

@@ +55,4 @@
>        &get_all_subpermissions &get_user_subpermissions
>      );
>      %EXPORT_TAGS = ( EditPermissions => [qw(get_all_subpermissions get_user_subpermissions)] );
> +    $googleoauth2 = C4::Context->preference('GoogleOAuth2');

Or this.

@@ +471,4 @@
>              BranchesLoop                          => GetBranchesLoop($opac_name),
>              BranchCategoriesLoop                  => GetBranchCategories( 'searchdomain', 1, $opac_name ),
>              CalendarFirstDayOfWeek                => ( C4::Context->preference("CalendarFirstDayOfWeek") eq "Sunday" ) ? 0 : 1,
> +            GoogleOAuth2                          => (C4::Context->preference("GoogleOAuth2")),

Or even this.

@@ +1247,4 @@
>          );
>      }
>  
> +    if ($googleoauth2) {

Though you will have to grab the preference directly here.

::: koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc
@@ +58,4 @@
>                                                  <li role="presentation"><a href="#" tabindex="-1" class="menu-inactive" role="menuitem">No private lists</a></li>
>                                                  <li role="presentation"><a href="/cgi-bin/koha/opac-shelves.pl?display=privateshelves" tabindex="-1" role="menuitem" class="listmenulink">New list</a></li>
>                                              [% END %]
> +                                        [% ELSIF ( KOHA.Preference('GoogleOAuth2') == 1 ) %]

Haven't tested yet, but is Template Toolkit case sensitive? Why KOHA? The top has [% Use Koha %] right?

@@ +78,4 @@
>                                          [% IF Koha.Preference('casAuthentication') %]
>                                              [%# CAS authentication is too complicated for modal window %]
>                                              <li><a href="/cgi-bin/koha/opac-user.pl">Log in to your account</a></li>
> +                                        [% ELSIF ( GoogleOAuth2 ) %]

Wasn't just the one location. I was talking about all of them. That way you could minimize touching Auth.pm in the first place.

::: opac/svc/googleoauth2
@@ +114,5 @@
> +            my ( $userid, $cookie, $sessionID ) =
> +              checkauth( $query, 1, { borrow => 1 }, 'opac', $email );
> +            if ($userid) {    # A valid user has logged in
> +		if (   ($restricttodomain ne '' )
> +                    && (index($email, $restricttodomain) == -1) ) 

Pink spots at the end (at least that's the colour on my screen), usually indicate spaces at the end of a line. That fails koha qa test tools.

I have yet to test this, but I've been thinking about it, and the comments made in comment #24 in terms of Net::OAuth2 or something similar seem to be a concern for me.

Comment on attachment 40252 [details] [review]
Bug 10988 - Allow login via Google OAuth2

Review of attachment 40252 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc
@@ +78,4 @@
>                                          [% IF Koha.Preference('casAuthentication') %]
>                                              [%# CAS authentication is too complicated for modal window %]
>                                              <li><a href="/cgi-bin/koha/opac-user.pl">Log in to your account</a></li>
> +                                        [% ELSIF ( Koha.Preference(' GoogleOAuth2') == 1 ) %]

Space! Oops.

Comment on attachment 40442 [details] [review]
Bug 10988: Allow login via Google OAuth2

Review of attachment 40442 [details] [review]:
-----------------------------------------------------------------

I don't have an installation with a public ip, so I cannot test this patch.

Following a code review:
1/ Fix the patch's author (From: root <root@beta.pbcstudents.pbc.wa.edu.au>)
2/ perltidy the new file (opac/svc/googleoauth2)
3/ Maybe opac/svc/auth/googleoauth2 is more appropriate?
4/ the following:

::: opac/svc/googleoauth2
@@ +1,1 @@
> +#!/usr/bin/perl -w

Remove -w

@@ +36,5 @@
> +# means caching some information and updating it daily. But that
> +# would make things a lot faster
> +
> +use Modern::Perl;
> +use CGI qw/escape/;

and -utf8, even if not needed.

@@ +63,5 @@
> +    $query->delete('code');
> +    $query->param( 'OAuth2Failed' => $reason );
> +    my ( $template, $borrowernumber, $cookie ) = get_template_and_user(
> +        {
> +            template_name   => 'opac-user.tmpl',

Should be opac-user.tt

@@ +75,5 @@
> +    $template->param( 'loginprompt'        => 1 );
> +    output_html_with_http_headers $query, $cookie, $template->output;
> +}
> +
> +#die $query->param('code');

To remove.

(In reply to Jonathan Druart from comment #36)
> I don't have an installation with a public ip,
> so I cannot test this patch.

You don't need a public ip, as far as I recall, since the redirect is done at the browser level.

(In reply to M. Tompsett from comment #38)
> (In reply to Jonathan Druart from comment #36)
> > I don't have an installation with a public ip,
> > so I cannot test this patch.
> 
> You don't need a public ip, as far as I recall, since the redirect is done
> at the browser level.

I'm not familiar with this patch at all, but it's true about not needing a public IP. 

I have a local app where I send "localhost:8888/callback" as the return uri to the OAuth 2.0 server and it works well. Not sure if that's feasible within the context of this patch, but...

I can take a look again with a detailed test plan using a non-public IP.

FYI: CALYX came up with a technique to force a google-auth session logout when a user logs out of Koha. (note: this may be considered optional behaviour)

Unfortunately, it is a solution that exists within a Google account, therefore is outside of Koha

More info to come...

Patch does not apply - please rebase!

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 10988: Allow login via Google OAuth2
Using index info to reconstruct a base tree...
M	C4/Auth.pm
M	installer/data/mysql/sysprefs.sql
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref
M	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-auth.tt
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-auth.tt
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-auth.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref
Auto-merging installer/data/mysql/sysprefs.sql
Auto-merging C4/Auth.pm
CONFLICT (content): Merge conflict in C4/Auth.pm
Failed to merge in the changes.
Patch failed at 0001 Bug 10988: Allow login via Google OAuth2
The copy of the patch that failed is found in:
   /home/katrin/kohaclone/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-10988-Allow-login-via-Google-OAuth2-fFUW8a.patch

Setting back to Signed Off after this rebasing.

I have a follow-up with some suggestions for the wording - but can't test on my local Koha installation (localhost...)

Also: the svc script asks for the 'borrow' permission, that has since be removed. Please take a look and fix.
Bug 7976 - "Borrow Permission" is not used and can be removed

Test plan is the same:
Test Plan:
 0) Back up your database
 1) Apply this patch
 2) In your mysql client use your Koha database and execute:
    > DELETE FROM systempreferences;
    > SOURCE ~/kohaclone/installer/data/mysql/sysprefs.sql;
    -- Should be no errors.
    > SELECT * FROM systempreferences LIKE 'GoogleOAuth%';
    -- Should see 4 entries.
    > QUIT;
 3) Restore your database
 4) Run ./installer/data/mysql/updatedatabase.pl;
 5) In your mysql client use your Koha database and execute:
    > SELECT * FROM systempreferences LIKE 'GoogleOAuth%';
    -- Should see the same 4 entries.
 6) Log into the staff client
 7) Home -> Koha administration -> Global system preferences
 8) -> OPAC
    -- make sure your OPACBaseURL is set (e.g. https://opac.koha.ca)
 9) -> Administration
    -- There should be a 'Google OAuth2' section with the ability
       to set those 4 system preferences.
10) In a new tab, go to https://console.developers.google.com/project
11) Click 'Create Project'
12) Type in a project name that won't freak users out, like your
    library name (e.g. South Pole Library).
13) Click the 'Create' button.
14) Click the 'APIs & auth' in the left frame.
15) Click 'Credentials'
16) Click 'Create new Client ID'
17) Select 'Web application' and click 'Configure consent screen'.
18) Select the Email Address.
19) Put it a meaningful string into the Product Name
    (e.g. South Pole Library Authentication)
20) Fill in the other fields as desired (or not)
21) Click 'Save'
22) Change the 'AUTHORIZED JAVASCRIPT ORIGINS' to your OPACBaseURL.
    (http://library.yourDNS.org)
23) Change the 'AUTHORIZED REDIRECT URIS' to point to the new
    googleoauth2 script
    (http://library.yourDNS.org/cgi-bin/koha/svc/auth/googleoauth2)
24) Click 'Create Client ID'
25) Copy and paste the 'CLIENT ID' into the GoogleOAuth2ClientID
    system preference.
26) Copy and paste the 'CLIENT SECRET' into the GoogleOAuth2ClientSecret
    system preference.
27) Change the GoogleOAuth2 preference to 'Use'.
28) Click 'Save all Administration preferences'
29) In the OPAC, click 'Log in to your account'.
    -- You should get a confirmation request, if you are
        already logged in, OR a login screen if you are not.
    -- You need to have the primary email address set to one
       authenticated by Google in order to log in.
30) Run koha qa test tools

There are no differences between the single commit and the multi-commit versions? What was the status before the commit? I'll retest anyways. *sigh* Seems like a waste of 15 minutes.

(In reply to M. Tompsett from comment #53)
> There are no differences between the single commit and the multi-commit
> versions? What was the status before the commit? I'll retest anyways. *sigh*
> Seems like a waste of 15 minutes.

Did this a while ago, but from memory the multiple patches didn't apply cleanly to master....

Just reviewing the code here.. one thing partially stands out.. What your actually implimenting here is OpenID Connect (Which is OpenID over Auth2.0) which is great, as I was a bit suspicious of using OAuth 2.0 for authentication as it's an api Authorization layer, not an Authentication medium out of the box (with OpenID Connect, one can use it for Authentication too).

Could we reword/rename the prefs to reflect that this is Google's implimentation of OpenID Connect, and not OAuth.. i think that will make it more clear that this is only for authentication and does not expand to using the range of api's available via OAuth.

I'll get back to reading the code now

Comment on attachment 46016 [details] [review]
Bug 10988 - Allow for Google OAuth2 logins Combined all of the patches above into one, making them apply to master again.

Review of attachment 46016 [details] [review]:
-----------------------------------------------------------------

In general I feel this is a good start, but it's just that.. a start.  We need a more thorough handling of login fallbacks and we need to add state tokens into the mix to protect our users from CRSF attacks.

::: koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc
@@ +65,5 @@
>                                              [% IF some_private_shelves > 10 %]
>                                                  <li role="presentation"><a href="/cgi-bin/koha/opac-shelves.pl?op=list&amp;category=1" tabindex="-1" role="menuitem" class="listmenulink">View All</a></li>
>                                              [% END %]
> +                                        [% ELSIF ( Koha.Preference('GoogleOAuth2') == 1 ) %]
> +                                            <li role="presentation"><a href="/cgi-bin/koha/svc/auth/googleoauth2" tabindex="-1" class="menu-inactive" role="menuitem">Log in to create your own lists</a></li>

I disagree with this change. A) it looks to me like it won't do what your expecting (it looks like it's a level too deep in the nested IF's) but B) I don't feel adding a login link here is appropriate unless your going to add it for all available authentication mechanisms.. it's just make the interface inconsistent.

::: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-auth.tt
@@ +138,2 @@
>  
>                              [% END # / IF casAuthentication %]

There's not enough added to this file.  There should be a 'login with your google id' block somewhere which appears to be missing.

@@ +139,5 @@
>                              [% END # / IF casAuthentication %]
>  
> +                            [% IF ( invalidOAuth2Login ) %]
> +                                <h4>Automatic login</h4>
> +                                <p>Sorry, your automatic login failed. <span class="error">[% invalidOAuth2Login %]</span></p>

I think this needs rewording, it's a Google Login.. it's not automagic.. it's a shared login using the email claim from a google openid connect id token.  I feel the text is a little misleading.

::: opac/svc/auth/googleoauth2
@@ +147,5 @@
> +    }
> +
> +}
> +else {
> +    my $prompt = $query->param('reauthenticate') // q{};

I'm not seeing an state tokens in use anywhere in this Flow.. without them we are wide open to cross-site request forgery (CSRF) attacks.. we likely need to create a nice randomised string and store it between invocations of the script.

Comment on attachment 46016 [details] [review]
Bug 10988 - Allow for Google OAuth2 logins Combined all of the patches above into one, making them apply to master again.

Review of attachment 46016 [details] [review]:
-----------------------------------------------------------------

I hadn't realized that this was OpenID Connect until Martin pointed it out and until I saw the "openid" value in the scope of the Authorization Request. 

I actually wrote an OpenID Connect feature for Koha for a client in 2014, but due to a lack of time and an incorrectly implemented third party OpenID Connect server, I never got around to upstreaming it to the community codebase. Perhaps I should try and remedy that one day. In any case, I've included some comments in the code review below based on my experience.

Nicholas, what documentation did you use for this patch? When I look at https://developers.google.com/identity/protocols/OpenIDConnect, it specifies different endpoints than you've used here. I'd recommend consulting that webpage and http://openid.net/specs/openid-connect-core-1_0.html. The latter is what I used for making an OpenID Connect compliant server app.

::: opac/svc/auth/googleoauth2
@@ +53,5 @@
> +# protocol is assumed in OPACBaseURL see bug 5010.
> +my $redirecturl  = $host . '/cgi-bin/koha/svc/auth/googleoauth2';
> +my $issuer       = 'accounts.google.com';
> +my $clientid     = C4::Context->preference('GoogleOAuth2ClientID');
> +my $clientsecret = C4::Context->preference('GoogleOAuth2ClientSecret');

Alternatively, you could put the ClientID and the ClientSecret in koha-conf.xml. While it would give less control to libraries, it would keep this information on a more "need to know" basis.

@@ +86,5 @@
> +elsif ( defined $query->param('code') ) {
> +    my $code    = $query->param('code');
> +    my $ua      = LWP::UserAgent->new();
> +    my $request = POST(
> +        'https://accounts.google.com/o/oauth2/token',

How did you choose this endpoint? Both https://developers.google.com/identity/protocols/OpenIDConnect?hl=en#discovery and https://accounts.google.com/.well-known/openid-configuration provide versioned endpoints.

@@ +99,5 @@
> +    );
> +    my $response = $ua->request($request)->decoded_content;
> +    my $json     = decode_json($response);
> +    if ( exists( $json->{'id_token'} ) ) {
> +        $request = POST( 'https://www.googleapis.com/oauth2/v1/tokeninfo',

tokeninfo isn't a standard OpenID Connect endpoint. It's a debugging tool that Google has available, but they discourage its use in production: https://developers.google.com/identity/protocols/OpenIDConnect?hl=en#validatinganidtoken

Use of tokeninfo also makes it harder for the rest of us to read the code since a decrypted token has a standard layout, but I haven't found what a tokeninfo response contains.

@@ +105,5 @@
> +        $response = $ua->request($request)->decoded_content;
> +        $json     = decode_json($response);
> +
> +# Confirm (as google suggests) that the issuer and audience are what we expect them to be
> +        if (   ( $json->{'issuer'} eq $issuer )

As noted above, a standard response should be $json->{'iss'}, but otherwise this is good as per #2 at http://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation.

@@ +106,5 @@
> +        $json     = decode_json($response);
> +
> +# Confirm (as google suggests) that the issuer and audience are what we expect them to be
> +        if (   ( $json->{'issuer'} eq $issuer )
> +            && ( $json->{'audience'} eq $clientid )

As noted in #3 at http://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation, $json->{'aud'} (the standard claim), "may" be an array. I'm not familiar with Google's responses... it might always return a string, but the spec points out that you should check for either a string or an array (and to reject the token if it doesn't contain the expected audience or if it includes untrusted audiences).

@@ +107,5 @@
> +
> +# Confirm (as google suggests) that the issuer and audience are what we expect them to be
> +        if (   ( $json->{'issuer'} eq $issuer )
> +            && ( $json->{'audience'} eq $clientid )
> +            && exists( $json->{'email'} ) )

This is bizarre... the token shouldn't contain the email. That must be for debugging purposes in tokeninfo. You should have to get it from https://www.googleapis.com/oauth2/v3/userinfo or a similar endpoint, which also need its "sub" claim to be validated.

@@ +108,5 @@
> +# Confirm (as google suggests) that the issuer and audience are what we expect them to be
> +        if (   ( $json->{'issuer'} eq $issuer )
> +            && ( $json->{'audience'} eq $clientid )
> +            && exists( $json->{'email'} ) )
> +        {

At this point, you must also verify that the token type is "Bearer" (case insensitive as per RFC 6749 Section 5.1).

At this point, you must also be verifying that the token isn't expired. 

You must also validate the "azp" claim if it's present in the response. 

For more on validation, consult http://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation and RFC 6749 (https://tools.ietf.org/html/rfc6749)

@@ +148,5 @@
> +
> +}
> +else {
> +    my $prompt = $query->param('reauthenticate') // q{};
> +

I agree with Martin. Even on Google's page, it states "You must protect the security of your users by preventing request forgery attacks." (https://developers.google.com/identity/protocols/OpenIDConnect?hl=en#createxsrftoken).

As developers, we have a responsibility to the users of Koha to ensure their security and their privacy.

All insightful stuff David, thanks :)

Just to make sure I wasn't coming accross too negative; I'm really pleased someone is working on adding this functionality.. nice start Nicholas.

https://developers.google.com/identity/protocols/OpenIDConnect is the documentation I would likely use to implement this authentication.

Nice work Nick! 

I will be eagerly looking at this, hopefully this afternoon.  Thanks for putting in all the hard work, so glad you followed through.

:-D

I wasn't attempting to gauge the prettiness of the UI for this sign off. :)

Created attachment 49785 [details] [review]
Bug 10988 - Fixes for comments 57 and 58

    Test Plan (remains the same):
     0) Back up your database
     1) Apply all these patches
     2) In your mysql client use your Koha database and execute:
        > DELETE FROM systempreferences;
        > SOURCE ~/kohaclone/installer/data/mysql/sysprefs.sql;
        -- Should be no errors.
        > SELECT * FROM systempreferences LIKE 'GoogleO%';
        -- Should see 4 entries.
        > QUIT;
     3) Restore your database
     4) Run ./installer/data/mysql/updatedatabase.pl;
     5) In your mysql client use your Koha database and execute:
        > SELECT * FROM systempreferences LIKE 'GoogleO%';
        -- Should see the same 4 entries.
     6) Log into the staff client
     7) Home -> Koha administration -> Global system preferences
     8) -> OPAC
        -- make sure your OPACBaseURL is set (e.g. https://opac.koha.ca)
     9) -> Administration
        -- There should be a 'Google OAuth2' section with the ability
           to set those 4 system preferences.
    10) In a new tab, go to https://console.developers.google.com/project
    11) Click 'Create Project'
    12) Type in a project name that won't freak users out, like your
        library name (e.g. South Pole Library).
    13) Click the 'Create' button.
    14) Click the 'APIs & auth' in the left frame.
    15) Click 'Credentials'
    16) Click 'Create new Client ID'
    17) Select 'Web application' and click 'Configure consent screen'.
    18) Select the Email Address.
    19) Put it a meaningful string into the Product Name
        (e.g. South Pole Library Authentication)
    20) Fill in the other fields as desired (or not)
    21) Click 'Save'
    22) Change the 'AUTHORIZED JAVASCRIPT ORIGINS' to your OPACBaseURL.
        (http://library.yourDNS.org)
    23) Change the 'AUTHORIZED REDIRECT URIS' to point to the new
        googleoauth2 script
        (http://library.yourDNS.org/cgi-bin/koha/svc/auth/googleopenidconnect)
    24) Click 'Create Client ID'
    25) Copy and paste the 'CLIENT ID' into the GoogleOAuth2ClientID
        system preference.
    26) Copy and paste the 'CLIENT SECRET' into the GoogleOAuth2ClientSecret
        system preference.
    27) Change the GoogleOpenIDConnect preference to 'Use'.
    28) Click 'Save all Administration preferences'
    29) In the OPAC, click 'Log in to your account'.
        -- You should get a confirmation request, if you are
            already logged in, OR a login screen if you are not.
        -- You need to have the primary email address set to one
           authenticated by Google in order to log in.
    30) Run koha qa test tools

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 49786 [details] [review]
Bug 10988 - Fix some wording Fix some outdated wording in googleopenidconnect

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 49787 [details] [review]
Bug 10988: Tabs vs Spaces qa failure patch

Run Koha QA Test tools and discovered this failed because of tabs.
Rather than fail this and wait forever for it to get fixed, this
patches it, and I'll mark it as signed off anyways.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Looks great, works great, good enhancement.. well done!

Pushed to Master - Should be in the May 2016 release.  Thanks!

Thanks Martin, Mark and Brendan! It feels great to have this done! ﷐[U+1F603]﷑

(In reply to Brendan Gallagher from comment #72)
> Pushed to Master - Should be in the May 2016 release.  Thanks!

Neato! I should take a look at this again. I haven't had time/money to upstream my generic OpenID Connect stuff, but maybe I can use this to replace some of my homespun code...

Does this solution is still working on Koha 17, I have tried to apply the patches but I receive the fallowing error
 Hunk #1 FAILED at 137.
1 out of 1 hunk FAILED -- saving rejects to file /usr/share/koha/opac/htdocs/opac-tmpl/bootstrap/en/modules/opac-auth.tt.rej
patching file opac/svc/auth/googleopenidconnect
patch unexpectedly ends in middle of line

(In reply to Marian from comment #75)
> Does this solution is still working on Koha 17, I have tried to apply the
> patches but I receive the fallowing error
>  Hunk #1 FAILED at 137.
> 1 out of 1 hunk FAILED -- saving rejects to file
> /usr/share/koha/opac/htdocs/opac-tmpl/bootstrap/en/modules/opac-auth.tt.rej
> patching file opac/svc/auth/googleopenidconnect
> patch unexpectedly ends in middle of line

It's already in Koha 17.05, so no need to apply. It was pushed (see comment #72). Changing status.

When bug 30988 is ready, which will be very soon, I think that it could replace this Google-specific functionality. 

Then we could just maintain the 1 generic OpenID Connect client.

In the past, I've had libraries that need multiple Open ID Connect clients supported and I used bug 21586 for that, but I could develop that functionality later for bug 30988 if necessary. 

Actually, if I did that, then we could seamlessly migrate the googleopenidconnect to the generic model just by adding it as a generic provider. 

Something to think about at some point...

Just noting that I've signed off Bug 30988 so it would be great to get some QA eyes on it.

I suspect eventually this will be deprecated and removed now that we have bug 31378 in master.