1212class PermissionsRepo
1313{
1414 protected JointPermissionBuilder $ permissionBuilder ;
15- protected $ systemRoles = ['admin ' , 'public ' ];
15+ protected array $ systemRoles = ['admin ' , 'public ' ];
1616
17- /**
18- * PermissionsRepo constructor.
19- */
2017 public function __construct (JointPermissionBuilder $ permissionBuilder )
2118 {
2219 $ this ->permissionBuilder = $ permissionBuilder ;
@@ -41,7 +38,7 @@ public function getAllRolesExcept(Role $role): Collection
4138 /**
4239 * Get a role via its ID.
4340 */
44- public function getRoleById ($ id ): Role
41+ public function getRoleById (int $ id ): Role
4542 {
4643 return Role::query ()->findOrFail ($ id );
4744 }
@@ -52,10 +49,10 @@ public function getRoleById($id): Role
5249 public function saveNewRole (array $ roleData ): Role
5350 {
5451 $ role = new Role ($ roleData );
55- $ role ->mfa_enforced = ($ roleData ['mfa_enforced ' ] ?? ' false ' ) === ' true ' ;
52+ $ role ->mfa_enforced = boolval ($ roleData ['mfa_enforced ' ] ?? false ) ;
5653 $ role ->save ();
5754
58- $ permissions = isset ( $ roleData ['permissions ' ]) ? array_keys ( $ roleData [ ' permissions ' ]) : [];
55+ $ permissions = $ roleData ['permissions ' ] ?? [];
5956 $ this ->assignRolePermissions ($ role , $ permissions );
6057 $ this ->permissionBuilder ->rebuildForRole ($ role );
6158
@@ -66,42 +63,45 @@ public function saveNewRole(array $roleData): Role
6663
6764 /**
6865 * Updates an existing role.
69- * Ensure Admin role always have core permissions.
66+ * Ensures Admin system role always have core permissions.
7067 */
71- public function updateRole ($ roleId , array $ roleData )
68+ public function updateRole ($ roleId , array $ roleData ): Role
7269 {
7370 $ role = $ this ->getRoleById ($ roleId );
7471
75- $ permissions = isset ($ roleData ['permissions ' ]) ? array_keys ($ roleData ['permissions ' ]) : [];
76- if ($ role ->system_name === 'admin ' ) {
77- $ permissions = array_merge ($ permissions , [
78- 'users-manage ' ,
79- 'user-roles-manage ' ,
80- 'restrictions-manage-all ' ,
81- 'restrictions-manage-own ' ,
82- 'settings-manage ' ,
83- ]);
72+ if (isset ($ roleData ['permissions ' ])) {
73+ $ this ->assignRolePermissions ($ role , $ roleData ['permissions ' ]);
8474 }
8575
86- $ this ->assignRolePermissions ($ role , $ permissions );
87-
8876 $ role ->fill ($ roleData );
89- $ role ->mfa_enforced = ($ roleData ['mfa_enforced ' ] ?? 'false ' ) === 'true ' ;
9077 $ role ->save ();
9178 $ this ->permissionBuilder ->rebuildForRole ($ role );
9279
9380 Activity::add (ActivityType::ROLE_UPDATE , $ role );
81+
82+ return $ role ;
9483 }
9584
9685 /**
97- * Assign a list of permission names to a role.
86+ * Assign a list of permission names to the given role.
9887 */
99- protected function assignRolePermissions (Role $ role , array $ permissionNameArray = [])
88+ protected function assignRolePermissions (Role $ role , array $ permissionNameArray = []): void
10089 {
10190 $ permissions = [];
10291 $ permissionNameArray = array_values ($ permissionNameArray );
10392
104- if ($ permissionNameArray ) {
93+ // Ensure the admin system role retains vital system permissions
94+ if ($ role ->system_name === 'admin ' ) {
95+ $ permissionNameArray = array_unique (array_merge ($ permissionNameArray , [
96+ 'users-manage ' ,
97+ 'user-roles-manage ' ,
98+ 'restrictions-manage-all ' ,
99+ 'restrictions-manage-own ' ,
100+ 'settings-manage ' ,
101+ ]));
102+ }
103+
104+ if (!empty ($ permissionNameArray )) {
105105 $ permissions = RolePermission::query ()
106106 ->whereIn ('name ' , $ permissionNameArray )
107107 ->pluck ('id ' )
@@ -114,13 +114,13 @@ protected function assignRolePermissions(Role $role, array $permissionNameArray
114114 /**
115115 * Delete a role from the system.
116116 * Check it's not an admin role or set as default before deleting.
117- * If an migration Role ID is specified the users assign to the current role
117+ * If a migration Role ID is specified the users assign to the current role
118118 * will be added to the role of the specified id.
119119 *
120120 * @throws PermissionsException
121121 * @throws Exception
122122 */
123- public function deleteRole ($ roleId , $ migrateRoleId)
123+ public function deleteRole (int $ roleId , int $ migrateRoleId = 0 ): void
124124 {
125125 $ role = $ this ->getRoleById ($ roleId );
126126
@@ -131,7 +131,7 @@ public function deleteRole($roleId, $migrateRoleId)
131131 throw new PermissionsException (trans ('errors.role_registration_default_cannot_delete ' ));
132132 }
133133
134- if ($ migrateRoleId ) {
134+ if ($ migrateRoleId !== 0 ) {
135135 $ newRole = Role::query ()->find ($ migrateRoleId );
136136 if ($ newRole ) {
137137 $ users = $ role ->users ()->pluck ('id ' )->toArray ();
0 commit comments