From 944d9e6249434ba4fb390d2f59caad03b8d29869 Mon Sep 17 00:00:00 2001 From: Lee Johnson Date: Mon, 16 Mar 2015 16:08:01 +0100 Subject: [PATCH] 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 --- Changes | 5 ++++ README.md | 22 +++++++++-------- examples/oauth2_server_db.pl | 12 ++++----- examples/oauth2_server_realistic.pl | 7 ++++-- lib/Mojolicious/Plugin/OAuth2/Server.pm | 33 ++++++++++++++----------- t/015_overrides.t | 4 +-- t/AllTests.pm | 10 ++++++++ 7 files changed, 59 insertions(+), 34 deletions(-) diff --git a/Changes b/Changes index efa7287..bd037c8 100644 --- 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 index c9427a7..36e4f47 100644 --- 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 index bf5898f..a5f1dd0 100644 --- 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 index 79a978c..424942f 100644 --- 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 index 721382e..2e105d2 100644 --- 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 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 index 02f1fb9..37f7713 100644 --- 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 index f14e936..872b8b2 100644 --- 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 = (