New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Asset link in CP leads to "Access Denied" unless user role has access to all Asset Containers #1419

Closed
iboxifoo opened this Issue May 12, 2017 · 2 comments

Comments

Projects
None yet
4 participants
@iboxifoo

iboxifoo commented May 12, 2017

Expected behaviour

User Roles allow granular access to certain containers. If certain containers are not checked, still expect to have access to those which are.

Actual behaviour

Unless all Asset Containers are given access, the Asset link in CP leads to an Access Denied message in the CP.

Steps to reproduce

  1. Select not all Asset Categories under User Roles
  2. Follow Asset link in CP sidebar
  3. Access denied

Server Details

Operating System:
Laravel Valet

Web Server:
Nginx

PHP Version:
7.0.17

Statamic Version:
2.5.8

Updated from an older Statamic or fresh install:
Y

List of installed addons:
NA

No logs generated

@mydudechris

This comment has been minimized.

mydudechris commented Nov 29, 2017

Experiencing a the issue with the Statamic v.2.7.2. However, I've also found that the error appears if you do not allow a collection that appears before your desired collections.

For example:

  • I have a Press role with users that have access to Press Asset collection
  • I want the role to only access Press assets
  • My asset collection list in the role permissions = Event Assets, Press Assets, Blog Assets
  • Unchecking Event Assets, which appears before Press Assets, triggers the Access Denied message
  • Leaving Event and Press assets checked but unchecking Blog does not throw the error

It appears that the CP is looking for the first item in the list rather than the first with permissions.

@RomainFrancony

This comment has been minimized.

RomainFrancony commented Mar 23, 2018

The problem come from statamic/core/Http/Controllers/AssetsController.php in the index method, as @mydudechris pointed out, the CP is redirecting to the first container.

return redirect()->route('assets.browse', $containers->first()->uuid());

Fix

Update the index method

$this->access('assets:*:view');

$containers = AssetContainer::all();

$currentUser = User::getCurrent();

// User is admin, no need to check
if($currentUser->isSuper()){
    return redirect()->route('assets.browse', $containers->first()->uuid());
}

// Check if the user has access to the container and redirect to the first one found
// can't use $this->access method because it will throw an exception if the user can't access to the container
foreach ($containers as $container){
    $containerUuid = $container->uuid();

    if($currentUser->can("assets:{$containerUuid}:view")){
        return redirect()->route('assets.browse', $containerUuid);
    }
}

// No container found, redirect back with error
return redirect()->back()->withErrors(translate('cp.permission_denied'));

@jasonvarga, @jackmcdade could you please confirm this ?

@jackmcdade jackmcdade self-assigned this Jun 5, 2018

@jackmcdade jackmcdade closed this Jun 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment