Framework: [Proposal] Allow Gate to explicitly deny/allow abilities to guests

[jwalton512]

Currently, the Gate immediately denies abilities if there is no authenticated user. There are likely many instances where this is a good default, but there are likely a great number of scenarios where developers would like the option to handle guests.

Yes, it is more than likely possible to write a custom Gate and include this logic, but I feel like it is at least worth discussing to see if this functionality should be included by default in Laravel.

[rkgrep]

You can pass anything into forUser method.

[jwalton512]

@rkgrep but the subsequent allows(...) call would still try to resolve the user.

i'm sure there are ways this can be achieved via hacking. the spirit of the proposal was to make this a default functionality.

[rkgrep]

allows uses internal resolver, which is overwritten with forUser call.

app(Gate::class)->define('something', function ($user) {
    if ($user == 'guest') do_something();
});
app(Gate::class)->forUser('guest')->allows('something');

As for me - it is quite handy to use class hinting in policies and closures. With your proposal it will be impossible as anything could be passed as user variable.
While implicitly calling forUser allows to prevent unexpected variables passed.

[JamesGuthrie]

I agree with @jwalton512, I personally find it somewhat cumbersome that the documentation mentions that I can use the Gate facade to have my policy applied, but in reality if there is no logged-in user then my policy will be skipped completely. The workaround I have is to just call the policy directly:

$user = Auth::user()
if (!policy($myClassInstance)->show($user, $myClassInstance)){
    abort(403);
}

[GrahamCampbell]

We're open to PRs, but don't process proposal issues.

[jwalton512]

@GrahamCampbell, happy to write a PR, but what is the point if it's not something @taylorotwell would be interested in having in the framework.

In fact, @taylorotwell mentioned in a recent Laravel podcast episode that he felt like it wasted the dev's time to create a PR when it wasn't going to be accepted.

[GrahamCampbell]

Feel free to discuss on slack. That's where proposals can be discussed first. ;)

[salarmehr]

@jwalton512 I totally agree with you. Did you follow up your suggestion?

[salarmehr]

@JamesGuthrie it throws an exception:

Type error: Argument 1 passed to App\Policies\FooPolicy::show() must be an instance of App\Models\User, null given

[tdhsmith]

Did anyone ever discuss this? The Slack archives don't go very far.

I'm be willing to contribute a PR on it. I was thinking something paralleling Validator::extendImplicit where there's a special function (Gate::defineImplicit?) for defining abilities that still run when the user resolves to falsy. That way the behavior for normal rule definitions remains intact.

Feel free to get at me on the Larachat under the same username.

[salarmehr]

@tdhsmith Currently I use the following for checking Guest permissions in actions:

  public function certify($ability, $arguments = [])
  {
    if (auth()->user()) {
      return $this->authorize($ability, $arguments);
    }
    $result = Gate::forUser(new User)->allows($ability, $arguments);

    return $result ? $this->allow() : $this->deny();

  }

[tdhsmith]

Yes, but I think that's better contained within the ability's definition. Plus, calling forUser un-constrains the kinds of $user arguments the callback gets. It would be safer (in a code maintenance/readability sense) to only change the behavior of the user resolver when the ability says it's ok.

Also FYI, you can use $this->authorizeForUser($user, $ability, $arguments) instead of Gate::forUser($user)->allows($ability, $arguments) if you want your calls to look similar / both be sourced from AuthorizesRequests.

[salarmehr]

@tdhsmith, yes, it can be conciser.

  /**
   * Authorizes a given action for a guest.
   * @param       $ability
   * @param array $arguments
   * @return \Illuminate\Auth\Access\Response|void
   */
  public function certify($ability, $arguments = [])
  {
    $user = auth()->user() ? auth()->user() : new User;

    return $this->authorizeForUser($user, $ability, $arguments);
  }

[jsphpl]

Use the NullObject pattern and set a Guest extends User from the Authenticate middleware if the user is not authenticated:

# Http/Middleware/Authenticate
protected function authenticate(array $guards)
{
    if (empty($guards)) {
        return $this->auth->authenticate();
    }

    foreach ($guards as $guard) {
        if ($this->auth->guard($guard)->check()) {
            return $this->auth->shouldUse($guard);
        }
    }

    $this->auth->setUser(new Guest());
}

In your Roles you can then check $user instanceof Guest

[malhal]

A simpler way:

Put this somewhere, perhaps in AuthServiceProvider boot()

if(!Auth::check()) {
    $userClass = config('auth.providers.users.model');
    Auth::setUser(new $userClass());
}

Then in your policy check for guest by doing:

if (is_null($user->getKey())) {
    // guest
}

[mmghv]

Implemented here https://github.com/laravel/framework/pull/16239 :+1:

[malhal]

@mmghv thanks, just to let you know in my particular case I require the policy method to still be called when it's a guest.

[mmghv]

@malhal That's what the PR allows you to do, You add the ability name in the allowGuest attribute in the policy and it will be called for guests.

[malhal]

@mmghv ah sorry I misread it, all good then!

[mmghv]

Who wants this PR to be merged please review and approve or give a thumb-up on the PR because there's a possibility that Taylor may reject it, Thanks :)