Bug 23146: Add support for Basic auth on the OAuth2 token endpoint
authorTomas Cohen Arazi <tomascohen@theke.io>
Tue, 18 Jun 2019 19:09:01 +0000 (16:09 -0300)
committerMartin Renvoize <martin.renvoize@ptfs-europe.com>
Mon, 24 Jun 2019 15:05:31 +0000 (16:05 +0100)
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
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Koha/REST/V1/Auth.pm
Koha/REST/V1/OAuth.pm

index 5e505cb..8daa1ad 100644 (file)
@@ -71,7 +71,13 @@ sub under {
                 "Configuration prevents the usage of this endpoint by unprivileged users");
         }
 
-        $status = authenticate_api_request($c);
+        if ( $c->req->url->to_string eq '/api/v1/oauth/token' ) {
+            # Requesting a token shouldn't go through the API authenticaction chain
+            $status = 1;
+        }
+        else {
+            $status = authenticate_api_request($c);
+        }
 
     } catch {
         unless (blessed($_)) {
index b75c892..06cb83c 100644 (file)
@@ -21,6 +21,7 @@ use Module::Load::Conditional;
 
 use C4::Context;
 use Koha::OAuth;
+use MIME::Base64;
 
 use Mojo::Base 'Mojolicious::Controller';
 
@@ -53,8 +54,25 @@ sub token {
         return $c->render(status => 400, openapi => {error => 'Unimplemented grant type'});
     }
 
-    my $client_id = $c->validation->param('client_id');
-    my $client_secret = $c->validation->param('client_secret');
+    my $client_id;
+    my $client_secret;
+
+    my $authorization_header = $c->req->headers->authorization;
+
+    if ( $authorization_header and $authorization_header =~ /^Basic / ) {
+        my ( $type, $credentials ) = split / /, $authorization_header;
+
+        unless ($credentials) {
+            Koha::Exceptions::Authentication::Required->throw( error => 'Authentication failure.' );
+        }
+
+        my $decoded_credentials = decode_base64( $credentials );
+        ( $client_id, $client_secret ) = split( /:/, $decoded_credentials, 2 );
+    }
+    else {
+        $client_id = $c->validation->param('client_id');
+        $client_secret = $c->validation->param('client_secret');
+    }
 
     my $cb = "${grant_type}_grant";
     my $server = Net::OAuth2::AuthorizationServer->new;