fix #16 - handle inconsistencies between various grant types

and the return data from ->verify_token_and_scope sometimes returning a hash ref and sometimes returning a string - now they always return a hash ref in the case of a succesful authentication (GH #16) note that this may be a BREAKING CHANGE if you are using password grant in your app thanks to sillitoe for the above find + suggestions on a fix bump VERSION and Changes for CPAN release
payprop · Apr 16, 2018 · 44c01f9 · 44c01f9
1 parent fd68982
commit 44c01f9
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 21 deletions.
diff --git a/Changes b/Changes
@@ -1,5 +1,16 @@
 Revision history for Net-OAuth2-AuthorizationServer
 
+0.17     2018-04-16
+        - Handle inconsistencies between various grant types and the return
+          data from ->verify_token_and_scope sometimes returning a hash ref
+          and sometimes returning a string - now they always return a hash
+          ref in the case of a successful authentication (GH #16)
+
+        - Note that this may be a BREAKING CHANGE if you are using password
+          grant in your app
+
+        - Thanks to sillitoe for the above find + suggestions on a fix
+
 0.16     2017-09-01
         - Correct return type from verification of refresh token when
           the refresh token is a JWT (GH #12, thanks to pierre-vigier)

diff --git a/README.md b/README.md
@@ -11,7 +11,7 @@ Authorization Server
 
 # VERSION
 
-0.16
+0.17
 
 # SYNOPSIS
 
@@ -72,7 +72,9 @@ With contributions from:
 
 Martin Renvoize - `martin.renvoize@ptfs-europe.com`
 
-Pierre VIGIER `pierre.vigier@gmail.com`
+Pierre VIGIER - `pierre.vigier@gmail.com`
+
+Ian Sillitoe - [https://github.com/sillitoe](https://github.com/sillitoe)
 
 # LICENSE
 

diff --git a/lib/Net/OAuth2/AuthorizationServer.pm b/lib/Net/OAuth2/AuthorizationServer.pm
@@ -11,7 +11,7 @@ Authorization Server
 
 =head1 VERSION
 
-0.16
+0.17
 
 =head1 SYNOPSIS
 
@@ -45,7 +45,7 @@ use Net::OAuth2::AuthorizationServer::ImplicitGrant;
 use Net::OAuth2::AuthorizationServer::PasswordGrant;
 use Net::OAuth2::AuthorizationServer::ClientCredentialsGrant;
 
-our $VERSION = '0.16';
+our $VERSION = '0.17';
 
 =head1 GRANT TYPES
 
@@ -115,7 +115,9 @@ With contributions from:
 
 Martin Renvoize - C<martin.renvoize@ptfs-europe.com>
 
-Pierre VIGIER C<pierre.vigier@gmail.com>
+Pierre VIGIER - C<pierre.vigier@gmail.com>
+
+Ian Sillitoe - L<https://github.com/sillitoe>
 
 =head1 LICENSE
 

diff --git a/lib/Net/OAuth2/AuthorizationServer/Defaults.pm b/lib/Net/OAuth2/AuthorizationServer/Defaults.pm
@@ -79,7 +79,7 @@ sub confirm_by_resource_owner {
 sub verify_token_and_scope {
     my ( $self, %args ) = @_;
 
-    my ( $refresh_token, $scopes_ref, $auth_header, $is_legacy_caller ) =
+    my ( $refresh_token, $scopes_ref, $auth_header ) =
         @args{ qw/ refresh_token scopes auth_header / };
 
     my $access_token;
@@ -237,7 +237,7 @@ sub _verify_access_token {
             }
         }
 
-        return ( $self->refresh_tokens->{ $a_token }{ client_id }, undef );
+        return ( $self->refresh_tokens->{ $a_token }, undef );
     }
     elsif ( exists( $self->access_tokens->{ $a_token } ) ) {
 
@@ -254,7 +254,7 @@ sub _verify_access_token {
 
         }
 
-        return ( $self->access_tokens->{ $a_token }{ client_id }, undef );
+        return ( $self->access_tokens->{ $a_token }, undef );
     }
 
     return ( 0, 'invalid_grant' );
@@ -295,7 +295,7 @@ sub _verify_access_token_jwt {
             }
         }
 
-        return ( $access_token_payload->{client}, undef, $access_token_payload->{scopes} );
+        return ( $access_token_payload, undef, $access_token_payload->{scopes} );
     }
 
     return ( 0, 'invalid_grant' );

diff --git a/lib/Net/OAuth2/AuthorizationServer/Manual.pod b/lib/Net/OAuth2/AuthorizationServer/Manual.pod
@@ -530,10 +530,9 @@ client_id, client_secret, username, password, an optional reference to a list of
 the scopes.
 
 The callback should verify client details and username + password and return a
-a list with 4 elements. The first element should be the client id if the client
-details + username is valid. The second element should be the error message in
-the case of bad credentials. The third element should be an array reference of
-the required scopes. The fourth should be the username.
+a hash list with 2 elements. The first element should a hash containing the
+client id if the client details + username is valid + scopes + username. The
+second element should be the error message in the case of bad credentials.
 
   my $verify_user_password_sub = sub {
     my ( $self, %args ) = @_;
@@ -558,7 +557,11 @@ the required scopes. The fourth should be the username.
       return ( 0, 'invalid_grant' );
     }
     else {
-      return ( $client_id, undef, $scopes, $username );
+      return ({
+          client_id => $client_id,
+          scopes    => $scopes,
+          username  => $username,
+      });
     }
 
   };

diff --git a/lib/Net/OAuth2/AuthorizationServer/PasswordGrant.pm b/lib/Net/OAuth2/AuthorizationServer/PasswordGrant.pm
@@ -69,7 +69,7 @@ Net::OAuth2::AuthorizationServer::PasswordGrant - OAuth2 Resource Owner Password
   );
 
   # or:
-  my ( $client,$error,$scope,$user_id ) = $Grant->verify_token_and_scope(
+  my ( $oauth_details,$error ) = $Grant->verify_token_and_scope(
     refresh_token    => $refresh_token,
     auth_header      => $http_authorization_header,
   );
@@ -176,7 +176,16 @@ sub _verify_user_password {
         return ( 0, 'invalid_grant' );
     }
     else {
-        return ( $client_id, undef, $scopes, $username );
+        return (
+			{
+				client_id => $client_id,
+				scopes    => $scopes,
+				username  => $username
+			},
+			undef,
+			$scopes,   # here for back compat
+			$username, # here for back compat
+		);
     }
 
 }

diff --git a/t/net/oauth2/authorizationserver/passwordgrant_tests.pm b/t/net/oauth2/authorizationserver/passwordgrant_tests.pm
@@ -73,11 +73,11 @@ sub run_tests {
 		[ { username => 'i_do_not_exist' },'invalid_grant','bad username' ],
 		[ { password => 'bad_password' },'invalid_grant','bad password' ],
 	) {
-		my ( $client,$vac_error,$scopes ) = $Grant->verify_user_password(
+		my ( $rt_client,$vac_error,$scopes ) = $Grant->verify_user_password(
 			%valid_user_password,%{ $t->[0] },
 		);
 
-		ok( ! $client,'->verify_user_password, ' . $t->[2] );
+		ok( ! $rt_client,'->verify_user_password, ' . $t->[2] );
 		is( $vac_error,$t->[1],'has error' );
 		ok( ! $scopes,'has no scopes' );
 	}
@@ -87,7 +87,7 @@ sub run_tests {
 	note( "store_access_token" );
 
 	ok( my $access_token = $Grant->token(
-		client_id    => $client,
+		client_id    => 'test_client',
 		scopes       => $scopes_ref,
 		type         => 'access',
 		user_id      => $user_id,
@@ -97,7 +97,7 @@ sub run_tests {
 		if $args->{token_format_tests};
 
 	ok( my $refresh_token = $Grant->token(
-		client_id    => $client,
+		client_id    => 'test_client',
 		scopes       => $scopes_ref,
 		type         => 'refresh',
 		user_id      => $user_id,
@@ -107,7 +107,7 @@ sub run_tests {
 		if $args->{token_format_tests};
 
 	ok( $Grant->store_access_token(
-		client_id     => $client,
+		client_id     => 'test_client',
 		access_token  => $access_token,
 		refresh_token => $refresh_token,
 		scopes       => $scopes_ref,