http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10988
--- Comment #57 from Martin Renvoize <[email protected]> --- 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. _______________________________________________ Koha-bugs mailing list [email protected] http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
