From adb02ff8408719d9818920b772f91e74c27c44dc Mon Sep 17 00:00:00 2001 From: Dries Vints Date: Tue, 11 Feb 2020 19:54:40 +0100 Subject: [PATCH 1/2] Implement auth token for access requests --- resources/views/authorize.blade.php | 2 ++ src/Exceptions/InvalidAuthTokenException.php | 18 ++++++++++++++++++ .../ApproveAuthorizationController.php | 2 ++ .../Controllers/AuthorizationController.php | 3 +++ .../DenyAuthorizationController.php | 2 ++ .../RetrievesAuthRequestFromSession.php | 16 ++++++++++++++++ tests/ApproveAuthorizationControllerTest.php | 5 +++++ tests/AuthorizationControllerTest.php | 1 + tests/DenyAuthorizationControllerTest.php | 10 ++++++++++ 9 files changed, 59 insertions(+) create mode 100644 src/Exceptions/InvalidAuthTokenException.php diff --git a/resources/views/authorize.blade.php b/resources/views/authorize.blade.php index ab0d4fb31..ecbdcaa58 100644 --- a/resources/views/authorize.blade.php +++ b/resources/views/authorize.blade.php @@ -69,6 +69,7 @@ + @@ -79,6 +80,7 @@ + diff --git a/src/Exceptions/InvalidAuthTokenException.php b/src/Exceptions/InvalidAuthTokenException.php new file mode 100644 index 000000000..6427929e4 --- /dev/null +++ b/src/Exceptions/InvalidAuthTokenException.php @@ -0,0 +1,18 @@ +assertValidAuthToken($request); + $authRequest = $this->getAuthRequestFromSession($request); return $this->convertResponse( diff --git a/src/Http/Controllers/AuthorizationController.php b/src/Http/Controllers/AuthorizationController.php index fe1c03258..df3eede8d 100644 --- a/src/Http/Controllers/AuthorizationController.php +++ b/src/Http/Controllers/AuthorizationController.php @@ -4,6 +4,7 @@ use Illuminate\Contracts\Routing\ResponseFactory; use Illuminate\Http\Request; +use Illuminate\Support\Str; use Laminas\Diactoros\Response as Psr7Response; use Laravel\Passport\Bridge\User; use Laravel\Passport\ClientRepository; @@ -73,6 +74,7 @@ public function authorize(ServerRequestInterface $psrRequest, return $this->approveRequest($authRequest, $user); } + $request->session()->put('authToken', $authToken = Str::random()); $request->session()->put('authRequest', $authRequest); return $this->response->view('passport::authorize', [ @@ -80,6 +82,7 @@ public function authorize(ServerRequestInterface $psrRequest, 'user' => $user, 'scopes' => $scopes, 'request' => $request, + 'authToken' => $authToken, ]); } diff --git a/src/Http/Controllers/DenyAuthorizationController.php b/src/Http/Controllers/DenyAuthorizationController.php index bf35c77a2..613dfd465 100644 --- a/src/Http/Controllers/DenyAuthorizationController.php +++ b/src/Http/Controllers/DenyAuthorizationController.php @@ -36,6 +36,8 @@ public function __construct(ResponseFactory $response) */ public function deny(Request $request) { + $this->assertValidAuthToken($request); + $authRequest = $this->getAuthRequestFromSession($request); $clientUris = Arr::wrap($authRequest->getClient()->getRedirectUri()); diff --git a/src/Http/Controllers/RetrievesAuthRequestFromSession.php b/src/Http/Controllers/RetrievesAuthRequestFromSession.php index 4ab67b36d..e5f31ffb3 100644 --- a/src/Http/Controllers/RetrievesAuthRequestFromSession.php +++ b/src/Http/Controllers/RetrievesAuthRequestFromSession.php @@ -5,9 +5,25 @@ use Exception; use Illuminate\Http\Request; use Laravel\Passport\Bridge\User; +use Laravel\Passport\Exceptions\InvalidAuthTokenException; trait RetrievesAuthRequestFromSession { + /** + * Make sure the auth token matches the one in the session. + * + * @param \Illuminate\Http\Request $request + * @return void + * + * @throws \Laravel\Passport\Exceptions\InvalidAuthTokenException + */ + protected function assertValidAuthToken(Request $request) + { + if ($request->session()->get('authToken') !== $request->get('auth_token')) { + throw InvalidAuthTokenException::different(); + } + } + /** * Get the authorization request from the session. * diff --git a/tests/ApproveAuthorizationControllerTest.php b/tests/ApproveAuthorizationControllerTest.php index 97f42b846..357f7bfd6 100644 --- a/tests/ApproveAuthorizationControllerTest.php +++ b/tests/ApproveAuthorizationControllerTest.php @@ -26,11 +26,16 @@ public function test_complete_authorization_request() $request = m::mock(Request::class); $request->shouldReceive('session')->andReturn($session = m::mock()); + $request->shouldReceive('get')->with('auth_token')->andReturn('foo'); + + $session->shouldReceive('get')->once()->with('authToken')->andReturn('foo'); $session->shouldReceive('get') ->once() ->with('authRequest') ->andReturn($authRequest = m::mock(AuthorizationRequest::class)); + $request->shouldReceive('user')->andReturn(new ApproveAuthorizationControllerFakeUser); + $authRequest->shouldReceive('getClient->getIdentifier')->andReturn(1); $authRequest->shouldReceive('getUser->getIdentifier')->andReturn(2); $authRequest->shouldReceive('setUser')->once(); diff --git a/tests/AuthorizationControllerTest.php b/tests/AuthorizationControllerTest.php index e9f47e4e7..ab5a14287 100644 --- a/tests/AuthorizationControllerTest.php +++ b/tests/AuthorizationControllerTest.php @@ -43,6 +43,7 @@ public function test_authorization_view_is_presented() $request = m::mock(Request::class); $request->shouldReceive('session')->andReturn($session = m::mock()); + $session->shouldReceive('put')->withSomeOfArgs('authToken'); $session->shouldReceive('put')->with('authRequest', $authRequest); $request->shouldReceive('user')->andReturn($user = m::mock()); diff --git a/tests/DenyAuthorizationControllerTest.php b/tests/DenyAuthorizationControllerTest.php index 38362c77f..bcb8ed4d9 100644 --- a/tests/DenyAuthorizationControllerTest.php +++ b/tests/DenyAuthorizationControllerTest.php @@ -27,7 +27,9 @@ public function test_authorization_can_be_denied() $request->shouldReceive('session')->andReturn($session = m::mock()); $request->shouldReceive('user')->andReturn(new DenyAuthorizationControllerFakeUser); $request->shouldReceive('input')->with('state')->andReturn('state'); + $request->shouldReceive('get')->with('auth_token')->andReturn('foo'); + $session->shouldReceive('get')->once()->with('authToken')->andReturn('foo'); $session->shouldReceive('get')->once()->with('authRequest')->andReturn($authRequest = m::mock( AuthorizationRequest::class )); @@ -56,6 +58,7 @@ public function test_authorization_can_be_denied_with_multiple_redirect_uris() $request->shouldReceive('session')->andReturn($session = m::mock()); $request->shouldReceive('user')->andReturn(new DenyAuthorizationControllerFakeUser); $request->shouldReceive('input')->with('state')->andReturn('state'); + $request->shouldReceive('get')->with('auth_token')->andReturn('foo'); $session->shouldReceive('get')->once()->with('authRequest')->andReturn($authRequest = m::mock( AuthorizationRequest::class @@ -67,6 +70,7 @@ public function test_authorization_can_be_denied_with_multiple_redirect_uris() $authRequest->shouldReceive('getRedirectUri')->andReturn('http://localhost'); $authRequest->shouldReceive('getClient->getRedirectUri')->andReturn(['http://localhost.localdomain', 'http://localhost']); + $session->shouldReceive('get')->once()->with('authToken')->andReturn('foo'); $response->shouldReceive('redirectTo')->once()->andReturnUsing(function ($url) { return $url; }); @@ -85,7 +89,9 @@ public function test_authorization_can_be_denied_implicit() $request->shouldReceive('session')->andReturn($session = m::mock()); $request->shouldReceive('user')->andReturn(new DenyAuthorizationControllerFakeUser); $request->shouldReceive('input')->with('state')->andReturn('state'); + $request->shouldReceive('get')->with('auth_token')->andReturn('foo'); + $session->shouldReceive('get')->once()->with('authToken')->andReturn('foo'); $session->shouldReceive('get')->once()->with('authRequest')->andReturn($authRequest = m::mock( AuthorizationRequest::class )); @@ -114,7 +120,9 @@ public function test_authorization_can_be_denied_with_existing_query_string() $request->shouldReceive('session')->andReturn($session = m::mock()); $request->shouldReceive('user')->andReturn(new DenyAuthorizationControllerFakeUser); $request->shouldReceive('input')->with('state')->andReturn('state'); + $request->shouldReceive('get')->with('auth_token')->andReturn('foo'); + $session->shouldReceive('get')->once()->with('authToken')->andReturn('foo'); $session->shouldReceive('get')->once()->with('authRequest')->andReturn($authRequest = m::mock( AuthorizationRequest::class )); @@ -146,7 +154,9 @@ public function test_auth_request_should_exist() $request->shouldReceive('session')->andReturn($session = m::mock()); $request->shouldReceive('user')->never(); $request->shouldReceive('input')->never(); + $request->shouldReceive('get')->with('auth_token')->andReturn('foo'); + $session->shouldReceive('get')->once()->with('authToken')->andReturn('foo'); $session->shouldReceive('get')->once()->with('authRequest')->andReturnNull(); $response->shouldReceive('redirectTo')->never(); From bf6b57ddd572490f04bfd11bf729f1c3ab4ffe76 Mon Sep 17 00:00:00 2001 From: Dries Vints Date: Tue, 11 Feb 2020 20:28:20 +0100 Subject: [PATCH 2/2] Optionally skip if no auth_token is provided --- src/Http/Controllers/RetrievesAuthRequestFromSession.php | 2 +- tests/ApproveAuthorizationControllerTest.php | 1 + tests/DenyAuthorizationControllerTest.php | 5 +++++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Http/Controllers/RetrievesAuthRequestFromSession.php b/src/Http/Controllers/RetrievesAuthRequestFromSession.php index e5f31ffb3..06fec9d14 100644 --- a/src/Http/Controllers/RetrievesAuthRequestFromSession.php +++ b/src/Http/Controllers/RetrievesAuthRequestFromSession.php @@ -19,7 +19,7 @@ trait RetrievesAuthRequestFromSession */ protected function assertValidAuthToken(Request $request) { - if ($request->session()->get('authToken') !== $request->get('auth_token')) { + if ($request->has('auth_token') && $request->session()->get('authToken') !== $request->get('auth_token')) { throw InvalidAuthTokenException::different(); } } diff --git a/tests/ApproveAuthorizationControllerTest.php b/tests/ApproveAuthorizationControllerTest.php index 357f7bfd6..88a8c5c56 100644 --- a/tests/ApproveAuthorizationControllerTest.php +++ b/tests/ApproveAuthorizationControllerTest.php @@ -26,6 +26,7 @@ public function test_complete_authorization_request() $request = m::mock(Request::class); $request->shouldReceive('session')->andReturn($session = m::mock()); + $request->shouldReceive('has')->with('auth_token')->andReturn(true); $request->shouldReceive('get')->with('auth_token')->andReturn('foo'); $session->shouldReceive('get')->once()->with('authToken')->andReturn('foo'); diff --git a/tests/DenyAuthorizationControllerTest.php b/tests/DenyAuthorizationControllerTest.php index bcb8ed4d9..095609a6e 100644 --- a/tests/DenyAuthorizationControllerTest.php +++ b/tests/DenyAuthorizationControllerTest.php @@ -27,6 +27,7 @@ public function test_authorization_can_be_denied() $request->shouldReceive('session')->andReturn($session = m::mock()); $request->shouldReceive('user')->andReturn(new DenyAuthorizationControllerFakeUser); $request->shouldReceive('input')->with('state')->andReturn('state'); + $request->shouldReceive('has')->with('auth_token')->andReturn(true); $request->shouldReceive('get')->with('auth_token')->andReturn('foo'); $session->shouldReceive('get')->once()->with('authToken')->andReturn('foo'); @@ -58,6 +59,7 @@ public function test_authorization_can_be_denied_with_multiple_redirect_uris() $request->shouldReceive('session')->andReturn($session = m::mock()); $request->shouldReceive('user')->andReturn(new DenyAuthorizationControllerFakeUser); $request->shouldReceive('input')->with('state')->andReturn('state'); + $request->shouldReceive('has')->with('auth_token')->andReturn(true); $request->shouldReceive('get')->with('auth_token')->andReturn('foo'); $session->shouldReceive('get')->once()->with('authRequest')->andReturn($authRequest = m::mock( @@ -89,6 +91,7 @@ public function test_authorization_can_be_denied_implicit() $request->shouldReceive('session')->andReturn($session = m::mock()); $request->shouldReceive('user')->andReturn(new DenyAuthorizationControllerFakeUser); $request->shouldReceive('input')->with('state')->andReturn('state'); + $request->shouldReceive('has')->with('auth_token')->andReturn(true); $request->shouldReceive('get')->with('auth_token')->andReturn('foo'); $session->shouldReceive('get')->once()->with('authToken')->andReturn('foo'); @@ -120,6 +123,7 @@ public function test_authorization_can_be_denied_with_existing_query_string() $request->shouldReceive('session')->andReturn($session = m::mock()); $request->shouldReceive('user')->andReturn(new DenyAuthorizationControllerFakeUser); $request->shouldReceive('input')->with('state')->andReturn('state'); + $request->shouldReceive('has')->with('auth_token')->andReturn(true); $request->shouldReceive('get')->with('auth_token')->andReturn('foo'); $session->shouldReceive('get')->once()->with('authToken')->andReturn('foo'); @@ -154,6 +158,7 @@ public function test_auth_request_should_exist() $request->shouldReceive('session')->andReturn($session = m::mock()); $request->shouldReceive('user')->never(); $request->shouldReceive('input')->never(); + $request->shouldReceive('has')->with('auth_token')->andReturn(true); $request->shouldReceive('get')->with('auth_token')->andReturn('foo'); $session->shouldReceive('get')->once()->with('authToken')->andReturn('foo');