Skip to content

Commit

Permalink
Merge pull request #12 from mitydigital/feature/optional-via-env
Browse files Browse the repository at this point in the history
Add support for role-specific enforcement
  • Loading branch information
martyf authored Jun 5, 2024
2 parents 5556572 + f2228cd commit 4368f45
Show file tree
Hide file tree
Showing 7 changed files with 326 additions and 38 deletions.
21 changes: 21 additions & 0 deletions config/statamic-two-factor.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,27 @@

'enabled' => env('STATAMIC_TWO_FACTOR_ENABLED', false),

/*
|--------------------------------------------------------------------------
| Role-specific enforcement
|--------------------------------------------------------------------------
|
| Super admins will always require two factor.
|
| Provide an array of Role handles that should have two factor enforced,
| such as:
| 'enforced_roles' => [
| 'content_publisher',
| 'users_admin',
| ],
|
| An empty array will mean that no roles are enforced.
| Set to null to enforce for all roles.
|
*/

'enforced_roles' => null,

/*
|--------------------------------------------------------------------------
| Blueprint field
Expand Down
10 changes: 10 additions & 0 deletions src/Facades/StatamicTwoFactorUser.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,17 @@
namespace MityDigital\StatamicTwoFactor\Facades;

use Illuminate\Support\Facades\Facade;
use Statamic\Contracts\Auth\User;

/**
* @method static ?string getLastChallenged()
* @method static User get()
* @method static static setLastChallenged()
* @method static static clearLastChallenged()
* @method static bool isTwoFactorEnforceable()
*
* @see \MityDigital\StatamicTwoFactor\Support\StatamicTwoFactorUser
*/
class StatamicTwoFactorUser extends Facade
{
protected static function getFacadeAccessor()
Expand Down
66 changes: 35 additions & 31 deletions src/Http/Middleware/EnforceTwoFactor.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,41 +18,45 @@ public function handle(Request $request, Closure $next)
// get the user
$user = StatamicTwoFactorUser::get();

// is two factor NOT set up?
if (! $user->two_factor_completed) {
// go to setup
return redirect(cp_route('statamic-two-factor.setup'));
}
// is two factor enforceable for this user?
if (StatamicTwoFactorUser::isTwoFactorEnforceable()) {

// is two factor NOT set up?
if (! $user->two_factor_completed) {
// go to setup
return redirect(cp_route('statamic-two-factor.setup'));
}

// when were we last challenged?
$lastChallenge = StatamicTwoFactorUser::getLastChallenged();

// when were we last challenged?
$lastChallenge = StatamicTwoFactorUser::getLastChallenged();

// do we use validity?
// if so, we need to check if we have a challenge, and if it hasn't expired
// if not, we just need to check we have a challenge
if (config('statamic-two-factor.validity', null)) {
// if the request is a POST or PATCH, or a "cp/preferences/js" request, ignore it
// this is so that if you're in the middle of editing when it does in fact expire, you can still save
// changes. This may be a bit controversal, but any other requests would trigger the challenge,
// and provides the better UX. The next GET or DELETE would require the challenge.
//
// Ultimately, it probably doesn't matter if the challenge is a bit longer than the "validity" minutes
// as it doesn't need to be *exact* but at least reminding them roughly after that time in a non-
// obtrusive way for their workflow is a happy approach.
if (! in_array(strtoupper($request->method()), ['PATCH', 'POST']) &&
! Str::startsWith($request->path(), config('statamic.cp.route').'/preferences/js')) {

// if we have no challenge token, it has expired
if (! $lastChallenge || Carbon::parse($lastChallenge)->addMinutes(config('statamic-two-factor.validity'))->isPast()) {
// not yet challenged, or expired, so yes, let's challenge
// do we use validity?
// if so, we need to check if we have a challenge, and if it hasn't expired
// if not, we just need to check we have a challenge
if (config('statamic-two-factor.validity', null)) {
// if the request is a POST or PATCH, or a "cp/preferences/js" request, ignore it
// this is so that if you're in the middle of editing when it does in fact expire, you can still
// save changes. This may be a bit controversial, but any other requests would trigger the
// challenge, and provides the better UX. The next GET or DELETE would require the challenge.
//
// Ultimately, it probably doesn't matter if the challenge is a bit longer than the "validity"
// minutes as it doesn't need to be *exact* but at least reminding them roughly after that time in
// a non-obtrusive way for their workflow is a happy approach.
if (! in_array(strtoupper($request->method()), ['PATCH', 'POST']) &&
! Str::startsWith($request->path(), config('statamic.cp.route').'/preferences/js')) {

// if we have no challenge token, it has expired
if (! $lastChallenge || Carbon::parse($lastChallenge)->addMinutes(config('statamic-two-factor.validity'))->isPast()) {
// not yet challenged, or expired, so yes, let's challenge
return redirect(cp_route('statamic-two-factor.challenge'));
}
}
} else {
// we don't care about expiry dates - we just need to know if we have been challenged at all
if (! $lastChallenge) {
return redirect(cp_route('statamic-two-factor.challenge'));
}
}
} else {
// we don't care about expiry dates - we just need to know if we have been challenged at all
if (! $lastChallenge) {
return redirect(cp_route('statamic-two-factor.challenge'));
}
}
}

Expand Down
39 changes: 37 additions & 2 deletions src/Support/StatamicTwoFactorUser.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

class StatamicTwoFactorUser
{
public function getLastChallenged()
public function getLastChallenged(): ?string
{
// get the user
$user = $this->get();
Expand All @@ -34,7 +34,7 @@ public function getLastChallenged()
return $lastChallenged;
}

public function get()
public function get(): ?\Statamic\Contracts\Auth\User
{
return User::current();
}
Expand Down Expand Up @@ -78,4 +78,39 @@ public function clearLastChallenged(): static

return $this;
}

public function isTwoFactorEnforceable(): bool
{
$user = static::get();

// no user - so not enforceable
if (! $user) {
return false;
}

// super admin are always enforced

if ($user->isSuper()) {
return true;
}

// get configured enforced roles
$enforcedRoles = config('statamic-two-factor.enforced_roles', null);

// null means all roles are enforced
if ($enforcedRoles === null) {
return true;
}

// if an array of roles check if the user contains ANY of them
if (is_array($enforcedRoles)) {
foreach ($enforcedRoles as $role) {
if ($user->hasRole($role)) {
return true;
}
}
}

return false; // this far, not enforced
}
}
126 changes: 126 additions & 0 deletions tests/Http/Middleware/EnforceTwoFactorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use Illuminate\Http\Request;
use MityDigital\StatamicTwoFactor\Facades\StatamicTwoFactorUser;
use MityDigital\StatamicTwoFactor\Http\Middleware\EnforceTwoFactor;
use Statamic\Facades\Role;

use function Pest\Laravel\get;
use function Pest\Laravel\post;
Expand Down Expand Up @@ -155,3 +156,128 @@ function () {
expect($response)
->status()->toBe(200);
});

it('redirects to the challenge when super admin', function () {
$user = createUserWithTwoFactor(true);
$this->actingAs($user);

// ALL roles
config()->set('statamic-two-factor.enforced_roles', null);

//
// Middleware setup
//
$request = Request::create(cp_route('dashboard'));
$next = function () {
return response('No enforcement');
};

$middleware = app(EnforceTwoFactor::class);

// Standard - should redirect
$response = $middleware->handle($request, $next);
expect($response->isRedirect(cp_route('statamic-two-factor.challenge')))
->toBeTrue();

// SPECIFIC roles
config()->set('statamic-two-factor.enforced_roles', []);

// Expect roles, should redirect
$response = $middleware->handle($request, $next);
expect($response->isRedirect(cp_route('statamic-two-factor.challenge')))
->toBeTrue();
});

it('redirects to the challenge when no enforced roles provided', function () {
$user = createUserWithTwoFactor(false);
$this->actingAs($user);

//
// Middleware setup
//
$request = Request::create(cp_route('dashboard'));
$next = function () {
return response('No enforcement');
};

$middleware = app(EnforceTwoFactor::class);

// ALL roles
config()->set('statamic-two-factor.enforced_roles', null);

// Standard - should redirect
$response = $middleware->handle($request, $next);
expect($response->isRedirect(cp_route('statamic-two-factor.challenge')))
->toBeTrue();

// EXPLICIT roles - none provided meaning not enforced
config()->set('statamic-two-factor.enforced_roles', []);

// Standard - should redirect
$response = $middleware->handle($request, $next);
expect($response->isRedirect(cp_route('statamic-two-factor.challenge')))
->toBeFalse();

// create some roles
$enforceableRole = Role::make('enforceable_role')->save();
$standardRole = Role::make('standard_role')->save();

// assign role to user
$user->assignRole($enforceableRole);
expect($user->hasRole($enforceableRole))->toBeTrue()
->and($user->hasRole($standardRole))->toBeFalse();

$response = $middleware->handle($request, $next);
expect($response->isRedirect(cp_route('statamic-two-factor.challenge')))
->toBeFalse();

//
// EXPLICIT roles - one provided meaning enforced
//
config()->set('statamic-two-factor.enforced_roles', [
$enforceableRole->handle(),
]);

$user->removeRole($enforceableRole);
expect($user->roles())->toHaveCount(0);

// no roles
$response = $middleware->handle($request, $next);
expect($response->isRedirect(cp_route('statamic-two-factor.challenge')))
->toBeFalse();

// assign role to user
$user->assignRole($enforceableRole);

expect($user->hasRole($enforceableRole))->toBeTrue()
->and($user->hasRole($standardRole))->toBeFalse();

// Standard - should redirect
$response = $middleware->handle($request, $next);
expect($response->isRedirect(cp_route('statamic-two-factor.challenge')))
->toBeTrue();

// assign both roles
$user->assignRole($enforceableRole);
$user->assignRole($standardRole);

expect($user->hasRole($enforceableRole))->toBeTrue()
->and($user->hasRole($standardRole))->toBeTrue();

// Standard - should redirect
$response = $middleware->handle($request, $next);
expect($response->isRedirect(cp_route('statamic-two-factor.challenge')))
->toBeTrue();

// remove enforceable role
$user->removeRole($enforceableRole);
$user->assignRole($standardRole);

expect($user->hasRole($enforceableRole))->toBeFalse()
->and($user->hasRole($standardRole))->toBeTrue();

// Standard - should redirect
$response = $middleware->handle($request, $next);
expect($response->isRedirect(cp_route('statamic-two-factor.challenge')))
->toBeFalse();
});
15 changes: 10 additions & 5 deletions tests/Pest.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,19 +47,24 @@
|
*/

function createUser(): Statamic\Auth\File\User|Statamic\Auth\Eloquent\User
function createUser(bool $isSuper = true): Statamic\Auth\File\User|Statamic\Auth\Eloquent\User
{
return User::make()
->makeSuper()
$user = User::make()
->set('name', 'Peter Parker')
->email('peter.parker@spiderman.com')
->set('password', 'secret')
->save();

if ($isSuper) {
$user->makeSuper()->save();
}

return $user;
}

function createUserWithTwoFactor(): Statamic\Auth\File\User|Statamic\Auth\Eloquent\User
function createUserWithTwoFactor(bool $isSuper = true): Statamic\Auth\File\User|Statamic\Auth\Eloquent\User
{
$user = createUser();
$user = createUser($isSuper);

$user->set('two_factor_locked', false);
$user->set('two_factor_confirmed_at', now());
Expand Down
Loading

0 comments on commit 4368f45

Please sign in to comment.