Skip to content
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

Any user with site access can access protected page media #132

Closed
jackbrycesmith opened this issue Sep 27, 2017 · 3 comments
Closed

Any user with site access can access protected page media #132

jackbrycesmith opened this issue Sep 27, 2017 · 3 comments
Labels

Comments

@jackbrycesmith
Copy link

Hi there,

Came across some undesirable behaviour in a certain scenario with the following config:

protect_protected_page_media: true
parent_acl: true

Scenario

  • user1 & user2 both have site access
  • /restricted has defined that only user1 can access the page
  • user1 can access /restricted
  • if user2 tries to access /restricted, they will see the contents of unauthorized.md (as expected)
  • if user2 tries to access media files, e.g. /restricted/media.jpg, they can see them (undesirable)

Note: if a guest user tries to access /resticted/media.jpg, they will be redirected to a login page as expected.

So any user with site access will be able to access media files of a protected page, regardless of that protected page excluding certain users based on a group e.g. whether they have paid.

Did a quick fix locally by doing a 302 redirect if the uri extension is a media type, near the end of setUnauthorizedPage()

grav-plugin-login/login.php

Lines 415 to 434 in 3f9560a

public function setUnauthorizedPage()
{
$route = $this->config->get('plugins.login.route_unauthorized');
/** @var Pages $pages */
$pages = $this->grav['pages'];
$page = $pages->dispatch($route);
if (!$page) {
$page = new Page;
$page->init(new \SplFileInfo(__DIR__ . '/pages/unauthorized.md'));
$page->template('default');
$page->slug(basename($route));
$pages->addPage($page, $route);
}
unset($this->grav['page']);
$this->grav['page'] = $page;
}

Wanted to see whether there was a better way of doing this?

@rhukster
Copy link
Member

can you create a pull request so i can easily test this? Cheers.

@rhukster
Copy link
Member

This actually turned out to be a bigger problem. You can't redirect non-HTML requests to HTML pages, so moved this up into authorizePage() and simply throw an 403 error if unauthorized.

4e6199e

Also this uncovered an issue with page's that were not found under protected pages. That's fixed in Grav develop branch.

getgrav/grav@a4ab5d9

Please test if possible :)

@rhukster rhukster added the fixed label Sep 28, 2017
@jackbrycesmith
Copy link
Author

Cheers - tested the latest release & works as expected 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants