http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10988 --- Comment #57 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Comment on attachment 46016 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46016 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: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=10988&attachment=46016) ----------------------------------------------------------------- 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&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. -- You are receiving this mail because: You are watching all bug changes.