Skip to content

Commit

Permalink
clients with client_secret cannot use implicit grant
Browse files Browse the repository at this point in the history
because by implication the client_secret means they must use the
Authorization Code Grant, so if a response type of "token" is passed
we must check that the client is allowed to use the Implicit Grant
flow (i.e. they don't have a client_secret set)
  • Loading branch information
leejo committed Aug 31, 2016
1 parent 8d7727f commit 52a9ab5
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 9 deletions.
6 changes: 6 additions & 0 deletions Changes
@@ -1,5 +1,11 @@
Revision history for Net-OAuth2-AuthorizationServer

0.09 2016-08-31
- Fix clients with a client_secret must use Authorization Code
flow and not Implicit Grant flow
- Fix pass redirect_uri and response_type to verify_client cb
so correct validation can be done for above fix

0.08 2016-08-31
- Add Net::OAuth2::AuthorizationServer::ImplicitGrant

Expand Down
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -11,7 +11,7 @@ Authorization Server

# VERSION

0.08
0.09

# SYNOPSIS

Expand Down
4 changes: 2 additions & 2 deletions lib/Net/OAuth2/AuthorizationServer.pm
Expand Up @@ -11,7 +11,7 @@ Authorization Server
=head1 VERSION
0.08
0.09
=head1 SYNOPSIS
Expand Down Expand Up @@ -44,7 +44,7 @@ use Net::OAuth2::AuthorizationServer::AuthorizationCodeGrant;
use Net::OAuth2::AuthorizationServer::ImplicitGrant;
use Net::OAuth2::AuthorizationServer::PasswordGrant;

our $VERSION = '0.08';
our $VERSION = '0.09';

=head1 GRANT TYPES
Expand Down
2 changes: 1 addition & 1 deletion lib/Net/OAuth2/AuthorizationServer/Defaults.pm
Expand Up @@ -135,7 +135,7 @@ sub _delegate_to_cb_or_private {
for ( $method ) {

/login_resource_owner|confirm_by_resource_owner|verify_client/ && do {
return $cb->( $obj, @args{ qw/ client_id scopes redirect_uri / } );
return $cb->( $obj, @args{ qw/ client_id scopes redirect_uri response_type / } );
};

$self->_uses_user_passwords && /verify_user_password/ && do {
Expand Down
7 changes: 7 additions & 0 deletions lib/Net/OAuth2/AuthorizationServer/ImplicitGrant.pm
Expand Up @@ -141,6 +141,13 @@ sub _verify_client {
return ( 0, 'invalid_request' );
}

if (
# implies Authorization Code Grant, not Implicit Grant
$self->clients->{ $client_id }{ client_secret }
) {
return ( 0, 'unauthorized_client' );
}

return ( 1 );
}

Expand Down
11 changes: 7 additions & 4 deletions lib/Net/OAuth2/AuthorizationServer/Manual.pod
Expand Up @@ -32,6 +32,9 @@ A hashref of client details keyed like so:

clients => {
$client_id => {
# note that setting a client_secret implies the "Authorization Code Grant"
# if this is not set then "Implicit Grant" is implied (that has an optional
# redirect_uri parameter)
client_secret => $client_secret
scopes => {
eat => 1,
Expand Down Expand Up @@ -93,8 +96,8 @@ it should call the redirect_to method on the controller and return undef:
my $resource_owner_confirm_scopes_sub = sub {
my ( %args ) = @_;

my ( $obj,$client_id,$scopes_ref )
= @args{ qw/ mojo_controller client_id scopes / };
my ( $obj,$client_id,$scopes_ref,$redirect_uri,$response_type )
= @args{ qw/ mojo_controller client_id scopes redirect_uri response_type / };

my $is_allowed = $obj->flash( "oauth_${client_id}" );

Expand Down Expand Up @@ -169,8 +172,8 @@ element should be the error message in the case of the client being disallowed:
my $verify_client_sub = sub {
my ( %args ) = @_;

my ( $obj,$client_id,$scopes_ref )
= @args{ qw/ mojo_controller client_id scopes / };
my ( $obj,$client_id,$scopes_ref,$redirect_uri,$response_type )
= @args{ qw/ mojo_controller client_id scopes redirect_uri response_type / };

if (
my $client = $obj->db->get_collection( 'clients' )
Expand Down
20 changes: 19 additions & 1 deletion t/net/oauth2/authorizationserver/implicitgrant_tests.pm
Expand Up @@ -33,6 +33,14 @@ sub clients {
sleep => 1,
},
},
test_client_must_use_auth_code => {
client_secret => 'weeee',
scopes => {
eat => 1,
drink => 0,
sleep => 1,
},
},
};
}

Expand All @@ -51,12 +59,22 @@ sub run_tests {
note( "verify_client" );

my %invalid_client = (
client_id => 'test_client_with_redirect_uri',
client_id => 'test_client_must_use_auth_code',
scopes => [ qw/ eat sleep / ],
);

my ( $res,$error ) = $Grant->verify_client( %invalid_client );

ok( ! $res,'->verify_client, client cannot use implicit grant' );
is( $error,'unauthorized_client','has error' );

%invalid_client = (
client_id => 'test_client_with_redirect_uri',
scopes => [ qw/ eat sleep / ],
);

( $res,$error ) = $Grant->verify_client( %invalid_client );

ok( ! $res,'->verify_client, lacks redirect_uri' );
is( $error,'invalid_request','has error' );

Expand Down

0 comments on commit 52a9ab5

Please sign in to comment.