Remove authz from User driver; moved to main class

lib/User.php

@@ -36,6 +36,44 @@ class User {
         return (string) $this->id;
     }
 
+    // checks whether the logged in user is authorized to act for the affected user (used especially when granting rights)
+    function authorize(string $affectedUser, string $action, int $newRightsLevel = 0): bool {
+        // if authorization checks are disabled (either because we're running the installer or the background updater) just return true
+        if(!$this->authz) return true;
+        // if we don't have a logged-in user, fetch credentials
+        if($this->id===null) $this->credentials();
+        // if the affected user is the actor and the actor is not trying to grant themselves rights, accept the request
+        if($affectedUser==$this->data->user->id && $action != "userRightsSet") return true;
+        // get properties of actor if not already available
+        if(!sizeof($this->actor)) $this->actor = $this->propertiesGet($this->data->user->id);
+        $rights =& $this->actor["rights"];
+        // if actor is a global admin, accept the request
+        if($rights==User\Driver::RIGHTS_GLOBAL_ADMIN) return true;
+        // if actor is a common user, deny the request
+        if($rights==User\Driver::RIGHTS_NONE) return false;
+        // if actor is not some other sort of admin, deny the request
+        if(!in_array($rights,[User\Driver::RIGHTS_GLOBAL_MANAGER,User\Driver::RIGHTS_DOMAIN_MANAGER,User\Driver::RIGHTS_DOMAIN_ADMIN],true)) return false;
+        // if actor is a domain admin/manager and domains don't match, deny the request
+        if($this->data->conf->userComposeNames && $this->actor["domain"] && $rights != User\Driver::RIGHTS_GLOBAL_MANAGER) {
+            $test = "@".$this->actor["domain"];
+            if(substr($affectedUser,-1*strlen($test)) != $test) return false;
+        }
+        // certain actions shouldn't check affected user's rights
+        if(in_array($action, ["userRightsGet","userExists","userList"], true)) return true;
+        if($action=="userRightsSet") {
+            // managers can only set their own rights, and only lower
+            if(($rights==User\Driver::RIGHTS_DOMAIN_MANAGER || $rights==User\Driver::RIGHTS_GLOBAL_MANAGER)) {
+                if($affectedUser != $this->data->user->id || $newRightsLevel != User\Driver::RIGHTS_NONE) return false;
+            }
+            // setting rights above your own is not allowed
+            if($newRightsLevel > $rights) return false;
+        }
+        $affectedRights = $this->rightsGet($affectedUser);
+        // acting for users with rights greater than your own (or equal, for managers) is not allowed
+        if($affectedRights > $rights || ($rights != User\Driver::RIGHTS_DOMAIN_ADMIN && $affectedRights==$rights)) return false;
+        return true;
+    }
+    
     public function credentials(): array {
         if($this->data->conf->userAuthPreferHTTP) {
             return $this->credentialsHTTP();
@@ -119,18 +157,6 @@ class User {
         }
     }
 
-    public function authorize(string $affectedUser, string $action, int $promoteLevel = 0): bool {
-        // if authorization checks are disabled (either because we're running the installer or the background updater) just return true
-        if(!$this->authz) return true;
-        // if we don't have a logged-in user, fetch credentials
-        if($this->id===null) $this->credentials();
-        // if the driver implements authorization, return the result
-        if($this->authzSupported) return $this->u->authorize($affectedUser, $action, $promoteLevel);
-        // if the driver does not implement authorization, only allow operation for the logged-in user (this means no new users can be added)
-        if($affectedUser==$this->id && $action != "userRightsSet") return true;
-        return false;        
-    }
-
     public function authorizationEnabled(bool $setting = null): bool {
         if($setting===null) return $this->authz;
         $this->authz = $setting;

lib/User/Driver.php

@@ -10,7 +10,7 @@ Interface Driver {
     const RIGHTS_NONE           = 0;    // normal user
     const RIGHTS_DOMAIN_MANAGER = 25;   // able to act for any normal users on same domain; cannot elevate other users
     const RIGHTS_DOMAIN_ADMIN   = 50;   // able to act for any users on same domain not above themselves; may elevate users on same domain to domain manager or domain admin
-    const RIGHTS_GLOBAL_MANAGER = 75;   // able to act for any user below themselves; can elevate users to domain manager or domain admin
+    const RIGHTS_GLOBAL_MANAGER = 75;   // able to act for any normal users on any domain; cannot elevate other users
     const RIGHTS_GLOBAL_ADMIN   = 100;  // is completely unrestricted
 
     // returns an instance of a class implementing this interface. Implemented as a static method for consistency with database classes
@@ -21,8 +21,6 @@ Interface Driver {
     function driverFunctions(string $function = null);
     // authenticates a user against their name and password
     function auth(string $user, string $password): bool;
-    // checks whether the logged in user is authorized to act for the affected user (used especially when granting rights)
-    function authorize(string $affectedUser, string $action): bool;
     // checks whether a user exists
     function userExists(string $user): bool;
     // adds a user

lib/User/DriverInternal.php

@@ -10,7 +10,6 @@ final class DriverInternal implements Driver {
     protected $db;
     protected $functions = [
         "auth"                    => Driver::FUNC_INTERNAL,
-        "authorize"               => Driver::FUNC_INTERNAL,
         "userList"                => Driver::FUNC_INTERNAL,
         "userExists"              => Driver::FUNC_INTERNAL,
         "userAdd"                 => Driver::FUNC_INTERNAL,

lib/User/InternalFunctions.php

@@ -17,35 +17,6 @@ trait InternalFunctions {
         return password_verify($password, $hash);
     }
 
-    function authorize(string $affectedUser, string $action, int $newRightsLevel = 0): bool {
-        // if the affected user is the actor and the actor is not trying to grant themselves rights, accept the request
-        if($affectedUser==$this->data->user->id && $action != "userRightsSet") return true;
-        // get properties of actor if not already available
-        if(!sizeof($this->actor)) $this->actor = $this->data->user->propertiesGet($this->data->user->id);
-        $rights =& $this->actor["rights"];
-        // if actor is a global admin, accept the request
-        if($rights==self::RIGHTS_GLOBAL_ADMIN) return true;
-        // if actor is a common user, deny the request
-        if($rights==self::RIGHTS_NONE) return false;
-        // if actor is not some other sort of admin, deny the request
-        if(!in_array($rights,[self::RIGHTS_GLOBAL_MANAGER,self::RIGHTS_DOMAIN_MANAGER,self::RIGHTS_DOMAIN_ADMIN],true)) return false;
-        // if actor is a domain admin/manager and domains don't match, deny the request
-        if($this->data->conf->userComposeNames && $this->actor["domain"] && $rights != self::RIGHTS_GLOBAL_MANAGER) {
-            $test = "@".$this->actor["domain"];
-            if(substr($affectedUser,-1*strlen($test)) != $test) return false;
-        }
-        // certain actions shouldn't check affected user's rights
-        if(in_array($action, ["userRightsGet","userExists","userList"], true)) return true;
-        if($action=="userRightsSet") {
-            // setting rights above your own (or equal to your own, for managers) is not allowed
-            if($newRightsLevel > $rights || ($rights != self::RIGHTS_DOMAIN_ADMIN && $newRightsLevel==$rights)) return false;
-        }
-        $affectedRights = $this->data->user->rightsGet($affectedUser);
-        // acting for users with rights greater than your own (or equal, for managers) is not allowed
-        if($affectedRights > $rights || ($rights != self::RIGHTS_DOMAIN_ADMIN && $affectedRights==$rights)) return false;
-        return true;
-    }
-
     function userExists(string $user): bool {
         return $this->db->userExists($user);
     }

locale/en.php

@@ -1,9 +1,10 @@
 <?php
 return [
     'Driver.User.Internal.Name'                                        => 'Internal',
-
+    
+    // this should only be encountered in testing (because tests should cover all exceptions!)
     'Exception.JKingWeb/NewsSync/Exception.uncoded'                     => 'The specified exception symbol {0} has no code specified in Exception.php',
-    //this should not usually be encountered
+    // this should not usually be encountered
     'Exception.JKingWeb/NewsSync/Exception.unknown'                     => 'An unknown error has occurred',
 
     'Exception.JKingWeb/NewsSync/Lang/Exception.defaultFileMissing'     => 'Default language file "{0}" missing',

tests/lib/User/DriverInternalMock.php

@@ -13,7 +13,6 @@ class DriverInternalMock implements Driver {
     protected $data;
     protected $functions = [
         "auth"                    => Driver::FUNC_INTERNAL,
-        "authorize"               => Driver::FUNC_INTERNAL,
         "userList"                => Driver::FUNC_INTERNAL,
         "userExists"              => Driver::FUNC_INTERNAL,
         "userAdd"                 => Driver::FUNC_INTERNAL,
@@ -52,10 +51,6 @@ class DriverInternalMock implements Driver {
         return false;
     }
 
-    function authorize(string $affectedUser, string $action, int $newRightsLevel = 0): bool {
-        return false;
-    }
-
     function userExists(string $user): bool {
         if(!$this->data->user->authorize($user, __FUNCTION__)) throw new ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]);
         return array_key_exists($user, $this->db);