Jump to content

PagefileSecure and Page::isPublic() hook not working


thetuningspoon
 Share

Recommended Posts

I have a secure area of my site where I need protection against direct access of files in the /site/assets/ folder. I have enabled $config->pagefileSecure. After some research, I discovered that the Page::isPublic() method is what controls whether ProcessWire secures the files or not, and that this is determined by whether or not the template has guest view access.

However, I do not want to have to disable guest viewing inside the ProcessWire template, since I want my customer to be able to control this on a per-page basis. (In fact, I want the page to load even for guests so that I can display a login form on the page in place of the content). 

I have created a password_protect checkbox on each page, and want this to determine whether the content gets displayed and whether or not the files get secured.

I created the following hook in /site/ready.php:

$this->addHookAfter('Page::isPublic', function($e) {
   $that = $e->object;
   $public = $e->return;

   if($that->password_protect) $public = false;

   $e->return = $public;
});

If I check $page->isPublic() inside of my template, it correctly shows as false. However, the files are not being secured. For some reason it is not effecting pagefileSecure. Any idea what I'm doing wrong?

Link to comment
Share on other sites

I tried it out and it works for me. I set $config->pagefileSecure = true; and put the following hook in ready.php

wire()->addHookBefore('ProcessPageView::sendFile', function($e) {
    throw new Wire404Exception('File not found');
});

 

Edited by kixe
messing up hooks while testing
Link to comment
Share on other sites

I ended up to do both:

wire()->addHookAfter('Page::isPublic', function($e) {
    $e->return = false;
});

wire()->addHookBefore('ProcessPageView::sendFile', function($e) {
    throw new Wire404Exception('File not found');
});

This is all a bit hacky, maybe you should follow the solution @LostKobrakai mentioned.

Link to comment
Share on other sites


@kixe In your ProcessPageView::sendFile hook, are you checking Page::isPublic first? The way you have written it, it would block all files.

Should I submit a report on GitHub? It seems like the isPublic hook should be enough to make this work. I don't understand why it isn't honoring it.

Edit: Does sendFile() actually get called when a file in the assets folder is accessed directly? 

Link to comment
Share on other sites

9 hours ago, tpr said:

Maybe $e->replace = true? But I would use Tracy Debugger to find out what's going on.

I don't actually want to replace the method entirely, just change the result under some circumstances. But the hook clearly is working, it just doesn't look like it's running early enough, perhaps. Thanks for the debugger suggestion... I am working on getting a proper debug setup going.

Link to comment
Share on other sites

20 hours ago, thetuningspoon said:


@kixe In your ProcessPageView::sendFile hook, are you checking Page::isPublic first? The way you have written it, it would block all files.

Should I submit a report on GitHub? It seems like the isPublic hook should be enough to make this work. I don't understand why it isn't honoring it.

Edit: Does sendFile() actually get called when a file in the assets folder is accessed directly? 

1) Yes my example block all files, I made this just for testing. You have to modify it for your needs.
2) Yes. It looks like it is accessed directly. A http header will be sent by WireHttp::sendFile() and output the content of the given file if the page is unpublished.

If you don't set $config->pagefileSecure = true; all files will stay accessible even if the page is unpublished.
If you hook in Page::isPublic() and return false the name of the files directory changes and your .htaccess will prevent accessing files under the secured path. If you make changings in your page and save it the files directory will be renamed again. Thats why this solution is a bit hacky. I wouldn't go forward in this direction. You should look for another solution.

Edited by kixe
  • Like 1
Link to comment
Share on other sites

33 minutes ago, kixe said:

If you hook in Page::isPublic() and return false the name of the files directory changes and your .htaccess will prevent accessing files under the secured path.

Right, except that it doesn't - This is the part that isn't working right.

Quote

If you make changings in your page and save it the files directory will be renamed again. Thats why this solution is a bit hacky.

Are you saying that the prefix would be removed after saving the page? I thought that it would be added after saving the page. Why would the directory be renamed as long as isPublic() continues to return the same value?

  • Like 1
Link to comment
Share on other sites

17 hours ago, thetuningspoon said:

Right, except that it doesn't - This is the part that isn't working right.

Are you saying that the prefix would be removed after saving the page? I thought that it would be added after saving the page. Why would the directory be renamed as long as isPublic() continues to return the same value?

1) The default behaviour in PW is, it would be added if you save with status unpublished. If you save and publish it will be removed.
2) You are right and I have to correct myself there will be no changes later if you use both:
force isPublic() to return false and setting $config->pagefileSecure = true;

The problem is ProcessPageView::sendFile() as I mentioned here.
A new header will be created and passthru the .htaccess settings. So you need to hook in ProcessPageView::sendFile() too;

    /**
     * Passthru a file for a non-public page
     *
     * If the page is public, then it just does a 301 redirect to the file.
     *
     */
    protected function ___sendFile($page, $basename) { }

 

  • Like 1
Link to comment
Share on other sites

19 hours ago, thetuningspoon said:

Why does sendFile() send the file even if isPublic() is false?

To make it visible in the backend?

All in all this one should work

/**
 * 1) add $config->pagefileSecure = true; in your config.php
 * 2) add this to your ready.php
 * 3) enable $page->password_protect in the backend and save the page
 *
 * if you do step 3 before 1 and 2 you need to save/view the page once to prevent access to the file
 * because Page::isPublic() is not triggered if you try to access the file directly
 *
 */

/**
 * if Page::isPublic() returns false a prefix (-) will be added to the name of the assets directory
 * the directory is not accessible directly anymore
 *
 */
wire()->addHookAfter('Page::isPublic', function($e) {
    $page = $e->object;
    if ($page->password_protect) {
        $e->return = false;
    }
});

/**
 * ProcessPageView::sendFile() is called only if the file is not directly accessible
 * if this function is called AND the page is not public it passthru the protected file path (.htaccess) by default
 * therefore we need this hook too
 *
 */
wire()->addHookBefore('ProcessPageView::sendFile', function($e) {
    $page = $e->arguments[0];
    if ($page->password_protect) throw new Wire404Exception('File not found');
});

 

  • Like 1
Link to comment
Share on other sites

Thanks very much, Kixe. Good call on the before hook.

Maybe you can help me better my own understanding of the situation... I guess I am still not understanding a piece of the puzzle here. Consider the following:

  1. $config->pagefileSecure is set to true
  2. Page::isPublic() returns false
  3. User is not logged in

If all 3 of the above are true, the file should not be directly accessible... correct?

This is how PW behaves normally, with no need to hook ProcessPageView::sendFile(). Yet when all of the above is true except that Page::isPublic() was modified by a hook, suddenly it doesn't work as expected.

There must be some other requirement that isn't being met, since this functionality doesn't normally require hooking sendFile(). See what I'm getting at?

  • Like 1
Link to comment
Share on other sites

7 hours ago, thetuningspoon said:
  1. $config->pagefileSecure is set to true
  2. Page::isPublic() returns false
  3. User is not logged in

If all 3 of the above are true, the file should not be directly accessible... correct?

No. The default behaviour is slightly different. Under the following conditions the file is not accessible:

  1. $config->pagefileSecure is set to true
  2. Page::status >= 2048 (unpubished)
  3. User is not logged in OR has not page-view permission for this page  (template)

Currently, Processwire does not meet your needs and provides Pagefiles if the status is appropriate for the current user and the user has page-view permission.

The function Page::isPublic() does not change the status of the page it returns false if the the status is >= 2048. This function is used by PagefilesManager and ProcessPageView. It is a kind of bridge between the status and these classes.
ProcessPageView::sendFile() is meant to go around the .htaccess protection anyway to provide the files in the backend. The function is not called if you can access the file via Apache. It will be called only if a secured file path is detected.

Perhaps it would be useful if ProcessPageView::sendFile() checks whether the file is requested by the admin template or by the frontend. As far as I can see there is no reason against the following changings. You maybe want to post an issue on github.

    /**
     * Passthru a file for a non-public page to make it visible in the backend
     *
     * If the page is public, then it just does a 301 redirect to the file.
     *
     */
    protected function ___sendFile($page, $basename) {

        $err = 'File not found';

        // use the static hasPath first to make sure this page actually has a files directory
        // this ensures one isn't automatically created when we call $page->filesManager->path below
        if(!PagefilesManager::hasPath($page)) throw new Wire404Exception($err);

        $filename = $page->filesManager->path() . $basename;
        if(!is_file($filename)) throw new Wire404Exception($err);

        if($page->isPublic()) {
            // deprecated, only necessary for method 2 in checkRequestFile
            $this->wire('session')->redirect($page->filesManager->url() . $basename);

        } else if ($this->wire('page')->template == 'admin') {
            $options = array('exit' => false);
            wireSendFile($filename, $options);
        }
        else throw new Wire404Exception($err);
    }

 

  • Like 1
Link to comment
Share on other sites

On 2/25/2017 at 0:35 AM, kixe said:

No. The default behaviour is slightly different. Under the following conditions the file is not accessible:

  1. $config->pagefileSecure is set to true
  2. Page::status >= 2048 (unpubished)
  3. User is not logged in OR has not page-view permission for this page  (template)

Hmm... But #2 isn't required, it's only one of the conditions under which the file access will be blocked, so I still don't get it. I think I will post on GitHub.

  • Like 1
Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share

  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...