Skip to content

Commit fb2a402

Browse files
hafezdivandarihafezdivandari
authored
Let OAuth2 server handle denying the request (#1572)
1 parent 721df11 commit fb2a402

4 files changed

+41
-138
lines changed

‎src/Http/Controllers/ApproveAuthorizationController.php

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
class ApproveAuthorizationController
1010
{
11-
use ConvertsPsrResponses, RetrievesAuthRequestFromSession;
11+
use ConvertsPsrResponses, HandlesOAuthErrors, RetrievesAuthRequestFromSession;
1212

1313
/**
1414
* The authorization server.
@@ -40,8 +40,12 @@ public function approve(Request $request)
4040

4141
$authRequest = $this->getAuthRequestFromSession($request);
4242

43-
return $this->convertResponse(
44-
$this->server->completeAuthorizationRequest($authRequest, new Psr7Response)
45-
);
43+
$authRequest->setAuthorizationApproved(true);
44+
45+
return $this->withErrorHandling(function () use ($authRequest) {
46+
return $this->convertResponse(
47+
$this->server->completeAuthorizationRequest($authRequest, new Psr7Response)
48+
);
49+
});
4650
}
4751
}

‎src/Http/Controllers/DenyAuthorizationController.php

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,30 +2,30 @@
22

33
namespace Laravel\Passport\Http\Controllers;
44

5-
use Illuminate\Contracts\Routing\ResponseFactory;
65
use Illuminate\Http\Request;
7-
use Illuminate\Support\Arr;
6+
use League\OAuth2\Server\AuthorizationServer;
7+
use Nyholm\Psr7\Response as Psr7Response;
88

99
class DenyAuthorizationController
1010
{
11-
use RetrievesAuthRequestFromSession;
11+
use ConvertsPsrResponses, HandlesOAuthErrors, RetrievesAuthRequestFromSession;
1212

1313
/**
14-
* The response factory implementation.
14+
* The authorization server.
1515
*
16-
* @var \Illuminate\Contracts\Routing\ResponseFactory
16+
* @var \League\OAuth2\Server\AuthorizationServer
1717
*/
18-
protected $response;
18+
protected $server;
1919

2020
/**
2121
* Create a new controller instance.
2222
*
23-
* @param \Illuminate\Contracts\Routing\ResponseFactory $response
23+
* @param \League\OAuth2\Server\AuthorizationServer $server
2424
* @return void
2525
*/
26-
public function __construct(ResponseFactory $response)
26+
public function __construct(AuthorizationServer $server)
2727
{
28-
$this->response = $response;
28+
$this->server = $server;
2929
}
3030

3131
/**
@@ -40,16 +40,12 @@ public function deny(Request $request)
4040

4141
$authRequest = $this->getAuthRequestFromSession($request);
4242

43-
$clientUris = Arr::wrap($authRequest->getClient()->getRedirectUri());
43+
$authRequest->setAuthorizationApproved(false);
4444

45-
if (! in_array($uri = $authRequest->getRedirectUri(), $clientUris)) {
46-
$uri = Arr::first($clientUris);
47-
}
48-
49-
$separator = $authRequest->getGrantTypeId() === 'implicit' ? '#' : (strstr($uri, '?') ? '&' : '?');
50-
51-
return $this->response->redirectTo(
52-
$uri.$separator.'error=access_denied&state='.$request->input('state')
53-
);
45+
return $this->withErrorHandling(function () use ($authRequest) {
46+
return $this->convertResponse(
47+
$this->server->completeAuthorizationRequest($authRequest, new Psr7Response)
48+
);
49+
});
5450
}
5551
}

‎src/Http/Controllers/RetrievesAuthRequestFromSession.php

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,6 @@ protected function getAuthRequestFromSession(Request $request)
4242
}
4343

4444
$authRequest->setUser(new User($request->user()->getAuthIdentifier()));
45-
46-
$authRequest->setAuthorizationApproved(true);
4745
});
4846
}
4947
}

‎tests/Unit/DenyAuthorizationControllerTest.php

Lines changed: 18 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@
22

33
namespace Laravel\Passport\Tests\Unit;
44

5-
use Illuminate\Contracts\Routing\ResponseFactory;
65
use Illuminate\Http\Request;
76
use Laravel\Passport\Http\Controllers\DenyAuthorizationController;
7+
use League\OAuth2\Server\AuthorizationServer;
88
use League\OAuth2\Server\RequestTypes\AuthorizationRequest;
99
use Mockery as m;
1010
use PHPUnit\Framework\TestCase;
11+
use Psr\Http\Message\ResponseInterface;
1112

1213
class DenyAuthorizationControllerTest extends TestCase
1314
{
@@ -18,140 +19,44 @@ protected function tearDown(): void
1819

1920
public function test_authorization_can_be_denied()
2021
{
21-
$response = m::mock(ResponseFactory::class);
22+
$this->expectException('Laravel\Passport\Exceptions\OAuthServerException');
2223

23-
$controller = new DenyAuthorizationController($response);
24+
$server = m::mock(AuthorizationServer::class);
25+
$controller = new DenyAuthorizationController($server);
2426

2527
$request = m::mock(Request::class);
2628

2729
$request->shouldReceive('session')->andReturn($session = m::mock());
2830
$request->shouldReceive('user')->andReturn(new DenyAuthorizationControllerFakeUser);
29-
$request->shouldReceive('input')->with('state')->andReturn('state');
3031
$request->shouldReceive('has')->with('auth_token')->andReturn(true);
3132
$request->shouldReceive('get')->with('auth_token')->andReturn('foo');
3233

3334
$session->shouldReceive('get')->once()->with('authToken')->andReturn('foo');
34-
$session->shouldReceive('get')->once()->with('authRequest')->andReturn($authRequest = m::mock(
35+
$session->shouldReceive('get')
36+
->once()
37+
->with('authRequest')
38+
->andReturn($authRequest = m::mock(
3539
AuthorizationRequest::class
36-
));
40+
));
3741

3842
$authRequest->shouldReceive('setUser')->once();
39-
$authRequest->shouldReceive('getGrantTypeId')->andReturn('authorization_code');
40-
$authRequest->shouldReceive('setAuthorizationApproved')->once()->with(true);
41-
$authRequest->shouldReceive('getRedirectUri')->andReturn('http://localhost');
42-
$authRequest->shouldReceive('getClient->getRedirectUri')->andReturn('http://localhost');
43+
$authRequest->shouldReceive('setAuthorizationApproved')->once()->with(false);
4344

44-
$response->shouldReceive('redirectTo')->once()->andReturnUsing(function ($url) {
45-
return $url;
46-
});
45+
$server->shouldReceive('completeAuthorizationRequest')
46+
->with($authRequest, m::type(ResponseInterface::class))
47+
->andThrow('League\OAuth2\Server\Exception\OAuthServerException');
4748

48-
$this->assertSame('http://localhost?error=access_denied&state=state', $controller->deny($request));
49-
}
50-
51-
public function test_authorization_can_be_denied_with_multiple_redirect_uris()
52-
{
53-
$response = m::mock(ResponseFactory::class);
54-
55-
$controller = new DenyAuthorizationController($response);
56-
57-
$request = m::mock(Request::class);
58-
59-
$request->shouldReceive('session')->andReturn($session = m::mock());
60-
$request->shouldReceive('user')->andReturn(new DenyAuthorizationControllerFakeUser);
61-
$request->shouldReceive('input')->with('state')->andReturn('state');
62-
$request->shouldReceive('has')->with('auth_token')->andReturn(true);
63-
$request->shouldReceive('get')->with('auth_token')->andReturn('foo');
64-
65-
$session->shouldReceive('get')->once()->with('authRequest')->andReturn($authRequest = m::mock(
66-
AuthorizationRequest::class
67-
));
68-
69-
$authRequest->shouldReceive('setUser')->once();
70-
$authRequest->shouldReceive('getGrantTypeId')->andReturn('authorization_code');
71-
$authRequest->shouldReceive('setAuthorizationApproved')->once()->with(true);
72-
$authRequest->shouldReceive('getRedirectUri')->andReturn('http://localhost');
73-
$authRequest->shouldReceive('getClient->getRedirectUri')->andReturn(['http://localhost.localdomain', 'http://localhost']);
74-
75-
$session->shouldReceive('get')->once()->with('authToken')->andReturn('foo');
76-
$response->shouldReceive('redirectTo')->once()->andReturnUsing(function ($url) {
77-
return $url;
78-
});
79-
80-
$this->assertSame('http://localhost?error=access_denied&state=state', $controller->deny($request));
81-
}
82-
83-
public function test_authorization_can_be_denied_implicit()
84-
{
85-
$response = m::mock(ResponseFactory::class);
86-
87-
$controller = new DenyAuthorizationController($response);
88-
89-
$request = m::mock(Request::class);
90-
91-
$request->shouldReceive('session')->andReturn($session = m::mock());
92-
$request->shouldReceive('user')->andReturn(new DenyAuthorizationControllerFakeUser);
93-
$request->shouldReceive('input')->with('state')->andReturn('state');
94-
$request->shouldReceive('has')->with('auth_token')->andReturn(true);
95-
$request->shouldReceive('get')->with('auth_token')->andReturn('foo');
96-
97-
$session->shouldReceive('get')->once()->with('authToken')->andReturn('foo');
98-
$session->shouldReceive('get')->once()->with('authRequest')->andReturn($authRequest = m::mock(
99-
AuthorizationRequest::class
100-
));
101-
102-
$authRequest->shouldReceive('setUser')->once();
103-
$authRequest->shouldReceive('getGrantTypeId')->andReturn('implicit');
104-
$authRequest->shouldReceive('setAuthorizationApproved')->once()->with(true);
105-
$authRequest->shouldReceive('getRedirectUri')->andReturn('http://localhost');
106-
$authRequest->shouldReceive('getClient->getRedirectUri')->andReturn('http://localhost');
107-
108-
$response->shouldReceive('redirectTo')->once()->andReturnUsing(function ($url) {
109-
return $url;
110-
});
111-
112-
$this->assertSame('http://localhost#error=access_denied&state=state', $controller->deny($request));
113-
}
114-
115-
public function test_authorization_can_be_denied_with_existing_query_string()
116-
{
117-
$response = m::mock(ResponseFactory::class);
118-
119-
$controller = new DenyAuthorizationController($response);
120-
121-
$request = m::mock(Request::class);
122-
123-
$request->shouldReceive('session')->andReturn($session = m::mock());
124-
$request->shouldReceive('user')->andReturn(new DenyAuthorizationControllerFakeUser);
125-
$request->shouldReceive('input')->with('state')->andReturn('state');
126-
$request->shouldReceive('has')->with('auth_token')->andReturn(true);
127-
$request->shouldReceive('get')->with('auth_token')->andReturn('foo');
128-
129-
$session->shouldReceive('get')->once()->with('authToken')->andReturn('foo');
130-
$session->shouldReceive('get')->once()->with('authRequest')->andReturn($authRequest = m::mock(
131-
AuthorizationRequest::class
132-
));
133-
134-
$authRequest->shouldReceive('setUser')->once();
135-
$authRequest->shouldReceive('getGrantTypeId')->andReturn('authorization_code');
136-
$authRequest->shouldReceive('setAuthorizationApproved')->once()->with(true);
137-
$authRequest->shouldReceive('getRedirectUri')->andReturn('http://localhost?action=some_action');
138-
$authRequest->shouldReceive('getClient->getRedirectUri')->andReturn('http://localhost?action=some_action');
139-
140-
$response->shouldReceive('redirectTo')->once()->andReturnUsing(function ($url) {
141-
return $url;
142-
});
143-
144-
$this->assertSame('http://localhost?action=some_action&error=access_denied&state=state', $controller->deny($request));
49+
$controller->deny($request);
14550
}
14651

14752
public function test_auth_request_should_exist()
14853
{
14954
$this->expectException('Exception');
15055
$this->expectExceptionMessage('Authorization request was not present in the session.');
15156

152-
$response = m::mock(ResponseFactory::class);
57+
$server = m::mock(AuthorizationServer::class);
15358

154-
$controller = new DenyAuthorizationController($response);
59+
$controller = new DenyAuthorizationController($server);
15560

15661
$request = m::mock(Request::class);
15762

@@ -164,7 +69,7 @@ public function test_auth_request_should_exist()
16469
$session->shouldReceive('get')->once()->with('authToken')->andReturn('foo');
16570
$session->shouldReceive('get')->once()->with('authRequest')->andReturnNull();
16671

167-
$response->shouldReceive('redirectTo')->never();
72+
$server->shouldReceive('completeAuthorizationRequest')->never();
16873

16974
$controller->deny($request);
17075
}

0 commit comments

Comments
 (0)