https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23146 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | CC| |josef.moravec@gmail.com, | |julian.maurice@biblibre.com | |, | |kyle@bywatersolutions.com, | |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23146 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 90758 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90758&action=edit Bug 23146: Add support for Basic auth on the OAuth2 token endpoint The original implementation only contemplated the option to pass the client_id and client_secret on the request body. It is very common that clients expect to be able to pass them as a Basic authorization header: Authorization: Basic encode_base64(client_id:client_secret) This patch introduces support for this, by: - Adding a check for the presence of the Authorization header in the OAuth token request handling code and making that case extract the client_id and client_secret from the header instead of the original implementation. No behaviour changes. - The Auth#under sub is changed so it doesn't go through the authenticate_api_request chain step, as it would be in conflict with general Basic authentication. - Original tests are generalized so they are run in both ways, with the same expected results. To test: - Apply the unit tests patch - Run: $ kshell k$ prove t/db_dependent/api/v1/oauth.t => FAIL: Tests fail because the current API doesn't support the feature - Apply this patch - Run: k$ prove t/db_dependent/api/v1/oauth.t => SUCCESS: Tests pass! - Sign off :-D Sponsored-by: ByWater Solutions -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23146 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23146 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #90757|0 |1 is obsolete| | Attachment #90758|0 |1 is obsolete| | --- Comment #3 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 90777 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90777&action=edit Bug 23146: Unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23146 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #90777|0 |1 is obsolete| | --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 90779 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90779&action=edit Bug 23146: Unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23146 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Tests pass, code looks sane and works. Passing QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23146 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 90801 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90801&action=edit Bug 23146: (QA follow-up) Make sure we use the absolute path When making a request using any tool (like cUrl or Postman) you get a 'Basic authentication disabled' error (if it is actually disabled) or an 'invalid password' error if it is disabled. This is because the comparisson of the path we do passes on oauth.t but fails on external tools. This is probably related to our stack including Apache URL mappings and then in the plack.psgi file. The safest way is to just ask Mojo::URL the absolute path to be sure. To test: - Having the rest of the patches applied and plack restarted, run: [1] $ curl -X POST -H 'Authorization: Basic ZGQ2NjlmNGUtZmI1NS00Y2YzLWE4ZmYtYmFiYzJiNDIwNWY1OmM0ZDJmYmYzLWYwOWMtNGJkZi1iNWE4LTgxMDJmNjcwYTI1Mw' -i 'http://kohadev.myDNSname.org:8081/api/v1/oauth/token' --data grant_type=client_credentials => FAIL: It fails saying Basic auth is disabled - Run: $ kshell k$ prove t/db_dependent/api/v1/oauth.t => SUCCESS: Tests pass - Apply this patch - Replicate your curl/postman test => SUCCESS: It now works as expected - Run: k$ prove t/db_dependent/api/v1/oauth.t => SUCCESS: Tests still pass! - Sign off :-D [1] You need to generate a client_id and client_secret, and encode them using: encode_base64url( "$client_id:$client_secret" ); Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23146 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work! Pushed to master for 19.11.00 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23146 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #91558|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23146 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |release-notes-needed -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23146 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This patchset adds release notes| |flexibility to the OAuth2 | |implementation regarding | |how the parameters are | |passed on the request. The | |original implementation of | |OAuth2 only contemplated | |the option to pass the | |client_id and client_secret | |parameters on the request | |body. It is very common | |that clients expect to be | |able to pass them as a | |Basic authorization header. -- You are receiving this mail because: You are watching all bug changes.