Fix issue processwire/processwire-issues#629 where $config->pagefileSecure combined with non-superuser editing user profile didn't show images in image fields

wire/core/User.php

@@ -27,7 +27,15 @@
  *
  */
 
-class User extends Page { 
+class User extends Page {
+
+	/**
+	 * Cached value for $user->isSuperuser() checks
+	 * 
+	 * @var null|bool
+	 * 
+	 */
+	protected $isSuperuser = null;
 
 	/**
 	 * Create a new User page in memory. 
@@ -155,7 +163,7 @@ class User extends Page {
 	 * @param Page|Template|bool|string $context Page or Template... 
 	 *  - or specify boolean true to return if user has permission OR if it was added at any template
 	 *  - or specify string "templates" to return array of Template objects where user has permission
-	 * @return bool
+	 * @return bool|array
 	 *
 	 */
 	public function hasPermission($name, $context = null) {
@@ -233,7 +241,7 @@ class User extends Page {
 
 		if(!$permission || !$permission->id) return false;
 
-		$roles = $this->get('roles'); 
+		$roles = $this->getUnformatted('roles'); 
 		if(empty($roles) || !$roles instanceof PageArray) return false; 
 		$has = false; 
 		$accessTemplate = is_null($page) ? false : $page->getAccessTemplate($permission->name);
@@ -390,17 +398,23 @@ class User extends Page {
 	 *
 	 */
 	public function isSuperuser() {
+		if(is_bool($this->isSuperuser)) return $this->isSuperuser;
 		$config = $this->wire('config');
-		if($this->id === $config->superUserPageID) return true; 
+		if($this->id === $config->superUserPageID) {
-		if($this->id === $config->guestUserPageID) return false;
-		$superuserRoleID = (int) $config->superUserRolePageID; 
-		$roles = $this->get('roles');
-		if(empty($roles)) return false;
-		$is = false;
-		foreach($roles as $role) if(((int) $role->id) === $superuserRoleID) {
 			$is = true;
-			break;
+		} else if($this->id === $config->guestUserPageID) {
+			$is = false;
+		} else {
+			$superuserRoleID = (int) $config->superUserRolePageID;
+			$roles = $this->getUnformatted('roles');
+			if(empty($roles)) return false; // no cache intentional
+			$is = false;
+			foreach($roles as $role) if(((int) $role->id) === $superuserRoleID) {
+				$is = true;
+				break;
+			}
 		}
+		$this->isSuperuser = $is;
 		return $is;
 	}
 
@@ -485,4 +499,17 @@ class User extends Page {
 		return $this->wire('users');
 	}
 
+	/**
+	 * Hook called when field has changed
+	 * 
+	 * @param string $what
+	 * @param mixed $old
+	 * @param mixed $new
+	 * 
+	 */
+	public function ___changed($what, $old = null, $new = null) {
+		if($what == 'roles' && is_bool($this->isSuperuser)) $this->isSuperuser = null;
+		parent::___changed($what, $old, $new); 
+	}
+
 }

wire/modules/PagePermissions.module

@@ -239,27 +239,41 @@ class PagePermissions extends WireData implements Module {
 	 */
 	public function userEditable(Page $page, array $options = array()) {
 		
+		/** @var User $user */
+		$user = $this->wire('user');
+		
+		/** @var Process|ProcessProfile|ProcessPageView|ProcessUser|ProcessPageList|ProcessPageLister $process */
+		$process = $this->wire('process');
+		
+		/** @var Config $config */
+		$config = $this->wire('config');
+		
 		$defaults = array(
 			'viewable' => false, // specify true if method is being used to determine viewable state
 		);
 		
 		$options = count($options) ? array_merge($defaults, $options) : $defaults;
-		
+	
+		if(!$page->id) return false;
 		if($page->className() != 'User') $page = $this->wire('users')->get($page->id);
 		if(!$page || $page instanceof NullPage) return false;
-		
+	
-		$user = $this->wire('user');
+		if($user->id === $page->id && !$user->isGuest() && $user->hasPermission('profile-edit')) {
-
+			// user is the same as the page being edited or viewed
-		// if user is editing themselves in ProcessProfile, and they have permission to do so
+			if($process == 'ProcessProfile') {
-		if($user->id === $page->id) {
+				// user editing themself in ProcssProfile
-			if($this->wire('page')->process == 'ProcessProfile' && $user->hasPermission('profile-edit')) {
 				return true;
+			} else if($this->wire('page') && $this->wire('page')->process == 'ProcessProfile') {
+				// user editing themself in ProcessProfile, when process not yet established
+				return true;
+			} else if($process == 'ProcessPageView' && $config->pagefileSecure && $options['viewable']) {
+				// user is viewing a file that is part of their User page when pagefileSecure mode active
+				return $process->getResponseType() == ProcessPageView::responseTypeFile;
 			}
 		}
 
 		// if the current process is something other than ProcessUser, they don't have permission
 		if(!$options['viewable']) {
-			$process = $this->wire('process');
 			if($process != 'ProcessUser' && (!$process instanceof ProcessPageList) && (!$process instanceof ProcessPageLister)) {
 				return false;
 			}
@@ -270,7 +284,7 @@ class PagePermissions extends WireData implements Module {
 
 		// if the user page being edited has a superuser role, and the current user doesn't,
 		// never let them edit regardless of any other permissions
-		$superuserRole = $this->wire('roles')->get($this->wire('config')->superUserRolePageID);
+		$superuserRole = $this->wire('roles')->get($config->superUserRolePageID);
 		if($page->roles->has($superuserRole) && !$user->roles->has($superuserRole)) return false;
 
 		// if we reach this point then check if there are more granular user-admin permissions available
@@ -285,7 +299,7 @@ class PagePermissions extends WireData implements Module {
 
 		// there are role-specific permissions in the system, and user must have appropriate one to edit
 		$userEditable = false;
-		$guestRoleID = $this->wire('config')->guestUserRolePageID;
+		$guestRoleID = $config->guestUserRolePageID;
 		$n = 0;
 		foreach($page->roles as $role) {
 			$n++;
@@ -302,6 +316,11 @@ class PagePermissions extends WireData implements Module {
 		
 		return false;
 	}
+	
+	public function userViewable(Page $page, array $options = array()) {
+		$options['viewable'] = true; 
+		return $this->userEditable($page, $options); 
+	}
 
 	/**
 	 * Can the current user add/remove the given role from other users?
@@ -530,13 +549,28 @@ class PagePermissions extends WireData implements Module {
 		if($status & Page::statusCorrupted) $status = $status & ~Page::statusCorrupted;
 
 		// perform several viewable checks, in order
-		if($status >= Page::statusUnpublished) $viewable = false;
+		if($status >= Page::statusUnpublished) {
-			else if(!$page->template || ($checkFile && !$page->template->filenameExists())) $viewable = false; 
+			// unpublished pages are not viewable, but see override below this if/else statement
-			else if($user->isSuperuser()) $viewable = true; 
+			$viewable = false;
-			else if($page->process) $viewable = $this->processViewable($page);
+		} else if(!$page->template || ($checkFile && !$page->template->filenameExists())) {
-			else if($page instanceof User && $user->hasPermission('user-admin')) $viewable = $this->userEditable($page, array('viewable' => true)); 
+			// template file does not exist
-			else if(!$user->hasPermission("page-view", $page)) $viewable = false;
+			$viewable = false;
-			else if($page->isTrash()) $viewable = false; 
+		} else if($user->isSuperuser()) {
+			// superuser always allowed
+			$viewable = true;
+		} else if($page->process) {
+			// delegate access to permissions defined with Process module
+			$viewable = $this->processViewable($page);
+		} else if($page instanceof User && !$user->isGuest() && ($user->hasPermission('user-admin') || $page->id === $user->id)) {
+			// user administrator or user viewing themself
+			$viewable = $this->userViewable($page); 
+		} else if(!$user->hasPermission("page-view", $page)) {
+			// user lacks basic view permission to page
+			$viewable = false;
+		} else if($page->isTrash()) {
+			// pages in trash are not viewable, except to superuser
+			$viewable = false;
+		}
 
 		// if the page is editable by the current user, force it to be viewable (if not viewable due to being unpublished)
 		if(!$viewable && !$user->isGuest() && ($status & Page::statusUnpublished)) {

wire/modules/Process/ProcessPageView.module

@@ -344,7 +344,10 @@ class ProcessPageView extends Process {
 
 		if($config->pagefileSecure) {
 			$page = $this->checkRequestFile($it);
-			if(is_object($page)) return $page; // Page or NullPage
+			if(is_object($page)) {
+				$this->responseType = self::responseTypeFile;
+				return $page; // Page or NullPage
+			}
 		}
 
 		// optimization to filter out page numbers first
@@ -602,14 +605,14 @@ class ProcessPageView extends Process {
 	 */
 	protected function checkAccess($page) {
 
-		if($page->viewable()) return $page; 
-	
 		if($this->requestFile) {
 			// if a file was requested, we still allow view even if page doesn't have template file
-			// this is something that viewable() does not echeck
+			if($page->viewable(false)) return $page; 
-			if($page->editable()) return $page;
+			// if($page->editable()) return $page;
 			if($this->checkAccessDelegated($page)) return $page; 
 			if($page->status < Page::statusUnpublished && $this->wire('user')->hasPermission('page-view', $page)) return $page;
+		} else if($page->viewable()) {
+			return $page;
 		}
 
 		$accessTemplate = $page->getAccessTemplate();