prevent refresh_token being used as an access_token

in examples and tests - there is nothing to prevent users of this module doing so, but in the basic usage this will no longer happen and documentation and examples are updated to show how this should be done bump Changes and VERSION for CPAN release
payprop · Mar 16, 2015 · 944d9e6 · 944d9e6
1 parent f45babd
commit 944d9e6
Show file tree

Hide file tree

Showing 7 changed files with 59 additions and 34 deletions.
diff --git a/Changes b/Changes
@@ -1,5 +1,10 @@
 Revision history for Mojolicious-Plugin-OAuth2-Server
 
+0.09     2015-03-16
+        - fix refresh_token check to prevent it being used as an access token.
+          this adds an extra argument ($is_refresh_token) to the method that
+          is called to _verify_access_token
+
 0.08     2015-02-12
         - stipulate CryptX in the Makefile.PL rather than Crypt::PRNG, as the
           latter doesn't have a VERSION number so causes dependency check to

diff --git a/README.md b/README.md
@@ -11,7 +11,7 @@ Authorization Server / Resource Server with Mojolicious
 
 # VERSION
 
-0.08
+0.09
 
 # SYNOPSIS
 
@@ -502,9 +502,11 @@ the verify\_access\_token callback for verification:
 Reference: [http://tools.ietf.org/html/rfc6749#section-7](http://tools.ietf.org/html/rfc6749#section-7)
 
 A callback to verify the access token. The callback is passed the Mojolicious
-controller object, the access token, and an optional reference to a list of the
-scopes. Note that the access token could be the refresh token, as this method is
-also called when the Client uses the refresh token to get a new access token.
+controller object, the access token, an optional reference to a list of the
+scopes and if the access\_token is actually a refresh token. Note that the access
+token could be the refresh token, as this method is also called when the Client
+uses the refresh token to get a new access token (in which case the value of the
+$is\_refresh\_token variable will be true).
 
 The callback should verify the access code using the rules defined in the
 reference RFC above, and return false if the access token is not valid otherwise
@@ -517,13 +519,13 @@ return a list where the first element is 0 and the second contains the error
 message (almost certainly 'invalid\_grant' in this case)
 
     my $verify_access_token_sub = sub {
-      my ( $c,$access_token,$scopes_ref ) = @_;
+      my ( $c,$access_token,$scopes_ref,$is_refresh_token ) = @_;
 
-      if (
-        my $rt = $c->db->get_collection( 'refresh_tokens' )->find_one({
-          refresh_token => $access_token
-        })
-      ) {
+      my $rt = $c->db->get_collection( 'refresh_tokens' )->find_one({
+        refresh_token => $access_token
+      });
+
+      if ( $is_refresh_token && $rt ) {
 
         if ( $scopes_ref ) {
           foreach my $scope ( @{ $scopes_ref // [] } ) {

diff --git a/examples/oauth2_server_db.pl b/examples/oauth2_server_db.pl
@@ -243,13 +243,13 @@
 };
 
 my $verify_access_token_sub = sub {
-  my ( $c,$access_token,$scopes_ref ) = @_;
+  my ( $c,$access_token,$scopes_ref,$is_refresh_token ) = @_;
 
-  if (
-    my $rt = $c->db->get_collection( 'refresh_tokens' )->find_one({
-      refresh_token => $access_token
-    })
-  ) {
+  my $rt = $c->db->get_collection( 'refresh_tokens' )->find_one({
+    refresh_token => $access_token
+  });
+
+  if ( $is_refresh_token && $rt ) {
 
     if ( $scopes_ref ) {
       foreach my $scope ( @{ $scopes_ref // [] } ) {

diff --git a/examples/oauth2_server_realistic.pl b/examples/oauth2_server_realistic.pl
@@ -229,11 +229,14 @@ sub load_oauth2_data {
 };
 
 my $verify_access_token_sub = sub {
-  my ( $c,$access_token,$scopes_ref ) = @_;
+  my ( $c,$access_token,$scopes_ref,$is_refresh_token ) = @_;
 
   my $oauth2_data = load_oauth2_data();
 
-  if ( exists( $oauth2_data->{refresh_tokens}{$access_token} ) ) {
+  if (
+    $is_refresh_token
+	&& exists( $oauth2_data->{refresh_tokens}{$access_token} )
+  ) {
 
     if ( $scopes_ref ) {
       foreach my $scope ( @{ $scopes_ref // [] } ) {

diff --git a/lib/Mojolicious/Plugin/OAuth2/Server.pm b/lib/Mojolicious/Plugin/OAuth2/Server.pm
@@ -11,7 +11,7 @@ Authorization Server / Resource Server with Mojolicious
 
 =head1 VERSION
 
-0.08
+0.09
 
 =head1 SYNOPSIS
 
@@ -197,7 +197,7 @@ use MIME::Base64 qw/ encode_base64 decode_base64 /;
 use Carp qw/ croak /;
 use Crypt::PRNG qw/ random_string /;
 
-our $VERSION = '0.08';
+our $VERSION = '0.09';
 
 my %CLIENTS;
 my %AUTH_CODES;
@@ -481,7 +481,7 @@ sub _verify_access_token_and_scope {
     $access_token = $refresh_token;
   }
 
-  return $verify_access_token_sub->( $c,$access_token,\@scopes );
+  return $verify_access_token_sub->( $c,$access_token,\@scopes,$refresh_token );
 }
 
 sub _revoke_access_token {
@@ -927,9 +927,11 @@ sub _store_access_token {
 Reference: L<http://tools.ietf.org/html/rfc6749#section-7>
 
 A callback to verify the access token. The callback is passed the Mojolicious
-controller object, the access token, and an optional reference to a list of the
-scopes. Note that the access token could be the refresh token, as this method is
-also called when the Client uses the refresh token to get a new access token.
+controller object, the access token, an optional reference to a list of the
+scopes and if the access_token is actually a refresh token. Note that the access
+token could be the refresh token, as this method is also called when the Client
+uses the refresh token to get a new access token (in which case the value of the
+$is_refresh_token variable will be true).
 
 The callback should verify the access code using the rules defined in the
 reference RFC above, and return false if the access token is not valid otherwise
@@ -942,13 +944,13 @@ return a list where the first element is 0 and the second contains the error
 message (almost certainly 'invalid_grant' in this case)
 
   my $verify_access_token_sub = sub {
-    my ( $c,$access_token,$scopes_ref ) = @_;
+    my ( $c,$access_token,$scopes_ref,$is_refresh_token ) = @_;
 
-    if (
-      my $rt = $c->db->get_collection( 'refresh_tokens' )->find_one({
-        refresh_token => $access_token
-      })
-    ) {
+    my $rt = $c->db->get_collection( 'refresh_tokens' )->find_one({
+      refresh_token => $access_token
+    });
+
+    if ( $is_refresh_token && $rt ) {
 
       if ( $scopes_ref ) {
         foreach my $scope ( @{ $scopes_ref // [] } ) {
@@ -993,9 +995,12 @@ message (almost certainly 'invalid_grant' in this case)
 =cut
 
 sub _verify_access_token {
-  my ( $c,$access_token,$scopes_ref ) = @_;
+  my ( $c,$access_token,$scopes_ref,$is_refresh_token ) = @_;
 
-  if ( exists( $REFRESH_TOKENS{$access_token} ) ) {
+  if (
+    $is_refresh_token
+    && exists( $REFRESH_TOKENS{$access_token} )
+  ) {
 
     if ( $scopes_ref ) {
       foreach my $scope ( @{ $scopes_ref // [] } ) {

diff --git a/t/015_overrides.t b/t/015_overrides.t
@@ -78,11 +78,11 @@ my $store_access_token_sub = sub {
 };
 
 my $verify_access_token_sub = sub {
-  my ( $c,$access_token,$scopes_ref ) = @_;
+  my ( $c,$access_token,$scopes_ref,$is_refresh_token ) = @_;
 
   # and here we should check the access code is valid, not expired, and the
   # passed scopes are allowed for the access token
-  return 1 if $access_token eq $VALID_REFRESH_TOKEN;
+  return 1 if $is_refresh_token and $access_token eq $VALID_REFRESH_TOKEN;
   return 0 if $ACCESS_REVOKED;
   return 0 if grep { $_ eq 'sleep' } @{ $scopes_ref // [] };
 

diff --git a/t/AllTests.pm b/t/AllTests.pm
@@ -150,6 +150,16 @@ sub run {
   $t->get_ok('/api/eat')->status_is( 200 );
   $t->get_ok('/api/sleep')->status_is( 401 );
 
+  note( "refresh token cannot access routes" );
+
+  $t->ua->on(start => sub {
+    my ( $ua,$tx ) = @_;
+    $tx->req->headers->header( 'Authorization' => "Bearer $refresh_token" );
+  });
+
+  $t->get_ok('/api/eat')->status_is( 401 );
+  $t->get_ok('/api/sleep')->status_is( 401 );
+
   note( "get a new access token using refresh token" );
 
   my %valid_refresh_token_params = (