Skip to content

Commit

Permalink
Merge pull request #35 from sfelix-martins/hotfix-empty-guards
Browse files Browse the repository at this point in the history
Fixed handling empty guards on MultiAuthenticate middleware. Let Authenticate handle it
  • Loading branch information
sfelix-martins authored Jul 29, 2018
2 parents 4687a4c + b152778 commit 2e7df14
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 18 deletions.
3 changes: 2 additions & 1 deletion src/Guards/GuardChecker.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use Illuminate\Support\Str;
use Illuminate\Http\Request;
use Illuminate\Support\Collection;

class GuardChecker
{
Expand Down Expand Up @@ -33,7 +34,7 @@ public static function getAuthGuards(Request $request)
* guard on value.
*
* @param array $guards
* @return array
* @return Collection
*/
public static function getGuardsProviders($guards)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Http/Middleware/AddCustomProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
class AddCustomProvider
{
/**
* The default provder of api guard.
* The default provider of api guard.
*
* @var string
*/
Expand Down
23 changes: 13 additions & 10 deletions src/Http/Middleware/MultiAuthenticate.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
namespace SMartins\PassportMultiauth\Http\Middleware;

use Closure;
use Illuminate\Http\Request;
use League\OAuth2\Server\ResourceServer;
use Illuminate\Auth\AuthenticationException;
use Illuminate\Auth\Middleware\Authenticate;
Expand All @@ -29,17 +28,21 @@ class MultiAuthenticate extends Authenticate
protected $providers;

/**
* The authentication factory instance.
* Create a new middleware instance.
*
* @var \Illuminate\Contracts\Auth\Factory
* @param ResourceServer $server
* @param ProviderRepository $providers
* @param Auth $auth
*/
protected $auth;
public function __construct(
ResourceServer $server,
ProviderRepository $providers,
Auth $auth
) {
parent::__construct($auth);

public function __construct(ResourceServer $server, ProviderRepository $providers, Auth $auth)
{
$this->server = $server;
$this->providers = $providers;
$this->auth = $auth;
}

/**
Expand All @@ -57,7 +60,7 @@ public function handle($request, Closure $next, ...$guards)
{
// If don't has any guard follow the flow
if (empty($guards)) {
return $next($request);
return $this->authenticate($guards);
}

$psrRequest = ServerRequest::createRequest($request);
Expand Down Expand Up @@ -93,7 +96,7 @@ public function handle($request, Closure $next, ...$guards)
/**
* Check if user acting has the required guards and scopes on request.
*
* @param \Illuminate\Foundation\Auth\User $user
* @param Authenticatable $user
* @param array $guards
* @return bool
*/
Expand All @@ -109,7 +112,7 @@ public function canBeAuthenticated(Authenticatable $user, $guards)
*
* @param \SMartins\PassportMultiauth\Provider $token
* @param array $guards
* @return void
* @return mixed
*
* @throws \Illuminate\Auth\AuthenticationException
*/
Expand Down
5 changes: 3 additions & 2 deletions src/PassportMultiauth.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ class PassportMultiauth
/**
* Set the current user for the application with the given scopes.
*
* @param \Illuminate\Contracts\Auth\Authenticatable $user
* @param array $scopes
* @param \Illuminate\Contracts\Auth\Authenticatable $user
* @param array $scopes
* @return void
* @throws Exception
*/
public static function actingAs($user, $scopes = [])
{
Expand Down
11 changes: 11 additions & 0 deletions tests/Feature/MultiauthTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,17 @@ public function setUpRoutes()
Route::middleware('auth:api,company')->get('/users', function (Request $request) {
return get_class($request->user());
});

Route::middleware('auth')->get('/no_guards', function (Request $request) {
return $request->user();
});
}

public function testAuthenticateOnRouteWithoutGuardsWithInvalidToken()
{
$response = $this->json('GET', 'no_guards', [], ['Authorization' => 'Bearer token']);

$this->assertInstanceOf(AuthenticationException::class, $response->exception);
}

public function testGetLoggedUserAsUser()
Expand Down
3 changes: 2 additions & 1 deletion tests/Fixtures/Http/Kernel.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace SMartins\PassportMultiauth\Tests\Fixtures\Http;

use Orchestra\Testbench\Http\Kernel as HttpKernel;
use Orchestra\Testbench\Http\Middleware\RedirectIfAuthenticated;

class Kernel extends HttpKernel
{
Expand All @@ -19,7 +20,7 @@ class Kernel extends HttpKernel
'auth.basic' => \Illuminate\Auth\Middleware\AuthenticateWithBasicAuth::class,
'bindings' => \Illuminate\Routing\Middleware\SubstituteBindings::class,
'can' => \Illuminate\Auth\Middleware\Authorize::class,
'guest' => Middleware\RedirectIfAuthenticated::class,
'guest' => RedirectIfAuthenticated::class,
'throttle' => \Illuminate\Routing\Middleware\ThrottleRequests::class,
];
}
6 changes: 3 additions & 3 deletions tests/Unit/MultiAuthenticateMiddlewareTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,18 @@ public function tearDown()

public function testTryAuthWithoutGuards()
{
$this->expectException(AuthenticationException::class);

$resourceServer = Mockery::mock('League\OAuth2\Server\ResourceServer');

$repository = Mockery::mock('SMartins\PassportMultiauth\ProviderRepository');

$request = $this->createRequest();

$middleware = new MultiAuthenticate($resourceServer, $repository, $this->auth);
$response = $middleware->handle($request, function () {
$middleware->handle($request, function () {
return 'response';
});

$this->assertEquals('response', $response);
}

public function testTryAuthWithoutAccessTokenId()
Expand Down

0 comments on commit 2e7df14

Please sign in to comment.