https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30988
Martin Renvoize <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |martin.renvoize@ptfs-europe | |.com --- Comment #42 from Martin Renvoize <[email protected]> --- This is looking good.. a few comments of where I'd love to see it headed and one that is unfortunately a QA fail at the moment. QA Fail * We're missing Unit Tests for the new module you introduce, I'm afraid that's a hard fail for now. It also looks like you have a note to do validation here which is missing... little confused as it looks like you are passing in a json structure rather than the token string.. so I'd have expected that to already be verified? We could perhaps just use an existing library for this Mojo::JWT for instance? Future 1. I'd love to see this moved out of preferences and into it's own management page.. I could see multiple OpenIDConnect providers being configured at the same time and allowing the end user to 'Pick their poison' at login. 2. If we went the above route, I'd love to see the Google stuff pulled out and the preferences it uses also migrated into the above management area. 3. The controller/svc script is pretty heavy right now.. I'd love to have seen more of this done 'in module' and have the controller lighter and perhaps in the REST side of things rather than a new /svc.. but I'm honestly not sure how it would fit in REST as yet so have no issue with it in /svc for now. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] https://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/
