Fix for #2439

app/Api/V1/Controllers/UserController.php

@@ -24,7 +24,8 @@ declare(strict_types=1);
 
 namespace FireflyIII\Api\V1\Controllers;
 
-use FireflyIII\Api\V1\Requests\UserRequest;
+use FireflyIII\Api\V1\Requests\UserStoreRequest;
+use FireflyIII\Api\V1\Requests\UserUpdateRequest;
 use FireflyIII\Exceptions\FireflyException;
 use FireflyIII\Repositories\User\UserRepositoryInterface;
 use FireflyIII\Transformers\UserTransformer;
@@ -155,11 +156,11 @@ class UserController extends Controller
     /**
      * Store a new user.
      *
-     * @param UserRequest $request
+     * @param UserStoreRequest $request
      *
      * @return JsonResponse
      */
-    public function store(UserRequest $request): JsonResponse
+    public function store(UserStoreRequest $request): JsonResponse
     {
         $data = $request->getAll();
         $user = $this->repository->store($data);
@@ -183,12 +184,12 @@ class UserController extends Controller
     /**
      * Update a user.
      *
-     * @param UserRequest $request
+     * @param UserUpdateRequest $request
      * @param User $user
      *
      * @return JsonResponse
      */
-    public function update(UserRequest $request, User $user): JsonResponse
+    public function update(UserUpdateRequest $request, User $user): JsonResponse
     {
         $data = $request->getAll();
         $user = $this->repository->update($user, $data);

app/Api/V1/Requests/UserStoreRequest.php

@@ -1,7 +1,7 @@
 <?php
 
 /**
- * UserRequest.php
+ * UserStoreRequest.php
  * Copyright (c) 2018 thegrumpydictator@gmail.com
  *
  * This file is part of Firefly III.
@@ -30,11 +30,9 @@ use FireflyIII\User;
 
 
 /**
- * Class UserRequest
- * @codeCoverageIgnore
- * TODO AFTER 4.8,0: split this into two request classes.
+ * Class UserStoreRequest
  */
-class UserRequest extends Request
+class UserStoreRequest extends Request
 {
     /**
      * Authorize logged in users.
@@ -68,7 +66,7 @@ class UserRequest extends Request
     public function getAll(): array
     {
         $blocked = false;
-        if (null === $this->get('blocked')) {
+        if (null !== $this->get('blocked')) {
             $blocked = $this->boolean('blocked');
         }
         $data = [
@@ -88,23 +86,12 @@ class UserRequest extends Request
      */
     public function rules(): array
     {
-        $rules = [
+        return [
             'email'        => 'required|email|unique:users,email,',
             'blocked'      => [new IsBoolean],
             'blocked_code' => 'in:email_changed',
             'role'         => 'in:owner,demo',
         ];
-        switch ($this->method()) {
-            default:
-                break;
-            case 'PUT':
-            case 'PATCH':
-                $user           = $this->route()->parameter('user');
-                $rules['email'] = 'required|email|unique:users,email,' . $user->id;
-                break;
-        }
-
-        return $rules;
     }
 
 }

app/Api/V1/Requests/UserUpdateRequest.php

@@ -0,0 +1,100 @@
+<?php
+
+/**
+ * UserUpdateRequest.php
+ * Copyright (c) 2018 thegrumpydictator@gmail.com
+ *
+ * This file is part of Firefly III.
+ *
+ * Firefly III is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Firefly III is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Firefly III. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+declare(strict_types=1);
+
+namespace FireflyIII\Api\V1\Requests;
+
+use FireflyIII\Repositories\User\UserRepositoryInterface;
+use FireflyIII\Rules\IsBoolean;
+use FireflyIII\User;
+
+
+/**
+ * Class UserUpdateRequest
+ */
+class UserUpdateRequest extends Request
+{
+    /**
+     * Authorize logged in users.
+     *
+     * @return bool
+     */
+    public function authorize(): bool
+    {
+        $result = false;
+        // Only allow authenticated users
+        if (auth()->check()) {
+            /** @var User $user */
+            $user = auth()->user();
+
+            /** @var UserRepositoryInterface $repository */
+            $repository = app(UserRepositoryInterface::class);
+
+            if ($repository->hasRole($user, 'owner')) {
+                $result = true; // @codeCoverageIgnore
+            }
+        }
+
+        return $result;
+    }
+
+    /**
+     * Get all data from the request.
+     *
+     * @return array
+     */
+    public function getAll(): array
+    {
+        $blocked = false;
+        if (null !== $this->get('blocked')) {
+            $blocked = $this->boolean('blocked');
+        }
+        $data = [
+            'email'        => $this->string('email'),
+            'blocked'      => $blocked,
+            'blocked_code' => $this->string('blocked_code'),
+            'role'         => $this->string('role'),
+        ];
+
+        return $data;
+    }
+
+    /**
+     * The rules that the incoming request must be matched against.
+     *
+     * @return array
+     */
+    public function rules(): array
+    {
+        $user  = $this->route()->parameter('user');
+        $rules = [
+            'email'        => sprintf('email|unique:users,email,%d', $user->id),
+            'blocked'      => [new IsBoolean],
+            'blocked_code' => 'in:email_changed',
+            'role'         => 'in:owner,demo,',
+        ];
+
+        return $rules;
+    }
+
+}

app/Repositories/User/UserRepository.php

@@ -86,10 +86,10 @@ class UserRepository implements UserRepositoryInterface
      * @param User   $user
      * @param string $newEmail
      *
-     * @see updateEmail
-     *
      * @return bool
      * @throws \Exception
+     * @see updateEmail
+     *
      */
     public function changeEmail(User $user, string $newEmail): bool
     {
@@ -245,14 +245,14 @@ class UserRepository implements UserRepositoryInterface
         $return = [];
 
         // two factor:
-        $return['has_2fa']      = $user->mfa_secret !== null;
-        $return['is_admin']     = $this->hasRole($user, 'owner');
-        $return['blocked']      = 1 === (int)$user->blocked;
-        $return['blocked_code'] = $user->blocked_code;
-        $return['accounts']     = $user->accounts()->count();
-        $return['journals']     = $user->transactionJournals()->count();
-        $return['transactions'] = $user->transactions()->count();
-        $return['attachments']  = $user->attachments()->count();
+        $return['has_2fa']             = $user->mfa_secret !== null;
+        $return['is_admin']            = $this->hasRole($user, 'owner');
+        $return['blocked']             = 1 === (int)$user->blocked;
+        $return['blocked_code']        = $user->blocked_code;
+        $return['accounts']            = $user->accounts()->count();
+        $return['journals']            = $user->transactionJournals()->count();
+        $return['transactions']        = $user->transactions()->count();
+        $return['attachments']         = $user->attachments()->count();
         $return['attachments_size']    = $user->attachments()->sum('size');
         $return['bills']               = $user->bills()->count();
         $return['categories']          = $user->categories()->count();
@@ -291,6 +291,28 @@ class UserRepository implements UserRepositoryInterface
         return false;
     }
 
+    /**
+     * Remove any role the user has.
+     *
+     * @param User $user
+     */
+    public function removeRole(User $user): void
+    {
+        $user->roles()->sync([]);
+    }
+
+    /**
+     * Set MFA code.
+     *
+     * @param User        $user
+     * @param string|null $code
+     */
+    public function setMFACode(User $user, ?string $code): void
+    {
+        $user->mfa_secret = $code;
+        $user->save();
+    }
+
     /**
      * @param array $data
      *
@@ -335,9 +357,17 @@ class UserRepository implements UserRepositoryInterface
      */
     public function update(User $user, array $data): User
     {
-        $this->updateEmail($user, $data['email']);
-        $user->blocked      = $data['blocked'] ?? false;
-        $user->blocked_code = $data['blocked_code'] ?? null;
+        $this->updateEmail($user, $data['email'] ?? '');
+        if (isset($data['blocked']) && is_bool($data['blocked'])) {
+            $user->blocked = $data['blocked'];
+        }
+        if (isset($data['blocked_code']) && '' !== $data['blocked_code'] && is_string($data['blocked_code'])) {
+            $user->blocked_code = $data['blocked_code'];
+        }
+        if (isset($data['role']) && '' === $data['role']) {
+            $this->removeRole($user);
+        }
+
         $user->save();
 
         return $user;
@@ -350,12 +380,15 @@ class UserRepository implements UserRepositoryInterface
      * @param User   $user
      * @param string $newEmail
      *
+     * @return bool
      * @see changeEmail
      *
-     * @return bool
      */
     public function updateEmail(User $user, string $newEmail): bool
     {
+        if ('' === $newEmail) {
+            return true;
+        }
         $oldEmail = $user->email;
 
         // save old email as pref
@@ -367,16 +400,4 @@ class UserRepository implements UserRepositoryInterface
 
         return true;
     }
-
-    /**
-     * Set MFA code.
-     *
-     * @param User   $user
-     * @param string|null $code
-     */
-    public function setMFACode(User $user, ?string $code): void
-    {
-        $user->mfa_secret = $code;
-        $user->save();
-    }
 }

app/Repositories/User/UserRepositoryInterface.php

@@ -39,14 +39,6 @@ interface UserRepositoryInterface
      */
     public function all(): Collection;
 
-    /**
-     * Set MFA code.
-     *
-     * @param User $user
-     * @param string|null $code
-     */
-    public function setMFACode(User $user, ?string $code): void;
-
     /**
      * Gives a user a role.
      *
@@ -64,9 +56,9 @@ interface UserRepositoryInterface
      * @param User   $user
      * @param string $newEmail
      *
+     * @return bool
      * @see updateEmail
      *
-     * @return bool
      */
     public function changeEmail(User $user, string $newEmail): bool;
 
@@ -162,6 +154,21 @@ interface UserRepositoryInterface
      */
     public function hasRole(User $user, string $role): bool;
 
+    /**
+     * Remove any role the user has.
+     *
+     * @param User $user
+     */
+    public function removeRole(User $user): void;
+
+    /**
+     * Set MFA code.
+     *
+     * @param User        $user
+     * @param string|null $code
+     */
+    public function setMFACode(User $user, ?string $code): void;
+
     /**
      * @param array $data
      *
@@ -191,9 +198,9 @@ interface UserRepositoryInterface
      * @param User   $user
      * @param string $newEmail
      *
+     * @return bool
      * @see changeEmail
      *
-     * @return bool
      */
     public function updateEmail(User $user, string $newEmail): bool;
 }