diff --git a/app/Api/V1/Controllers/UserController.php b/app/Api/V1/Controllers/UserController.php index f582513ca0..22bc71200f 100644 --- a/app/Api/V1/Controllers/UserController.php +++ b/app/Api/V1/Controllers/UserController.php @@ -78,7 +78,7 @@ class UserController extends Controller { /** @var User $admin */ $admin = auth()->user(); - if ($this->repository->hasRole($admin, 'owner')) { + if ($admin->id !== $user->id && $this->repository->hasRole($admin, 'owner')) { $this->repository->destroy($user); return response()->json([], 204); diff --git a/app/Api/V1/Requests/AccountRequest.php b/app/Api/V1/Requests/AccountRequest.php index 52d1db7726..adebf62dbf 100644 --- a/app/Api/V1/Requests/AccountRequest.php +++ b/app/Api/V1/Requests/AccountRequest.php @@ -47,6 +47,7 @@ class AccountRequest extends Request */ public function getAll(): array { + $data = [ 'name' => $this->string('name'), 'active' => $this->boolean('active'), @@ -95,28 +96,34 @@ class AccountRequest extends Request $types = implode(',', array_keys(config('firefly.subTitlesByIdentifier'))); $ccPaymentTypes = implode(',', array_keys(config('firefly.ccTypes'))); $rules = [ - 'name' => 'required|min:1|uniqueAccountForUser', - 'opening_balance' => 'numeric|required_with:opening_balance_date|nullable', - 'opening_balance_date' => 'date|required_with:opening_balance|nullable', - 'iban' => 'iban|nullable', - 'bic' => 'bic|nullable', - 'virtual_balance' => 'numeric|nullable', - 'currency_id' => 'numeric|exists:transaction_currencies,id|required_without:currency_code', - 'currency_code' => 'min:3|max:3|exists:transaction_currencies,code|required_without:currency_id', - 'account_number' => 'between:1,255|nullable|uniqueAccountNumberForUser', - 'account_role' => 'in:' . $accountRoles . '|required_if:type,asset', - 'active' => 'required|boolean', - 'include_net_worth' => 'required|boolean', - 'cc_type' => 'in:' . $ccPaymentTypes . '|required_if:account_role,ccAsset', - 'cc_monthly_payment_date' => 'date' . '|required_if:account_role,ccAsset|required_if:cc_type,monthlyFull', - 'type' => 'required|in:' . $types, - 'notes' => 'min:0|max:65536', + 'name' => 'required|min:1|uniqueAccountForUser', + 'type' => 'required|in:' . $types, + 'active' => 'required|boolean', + 'account_role' => 'in:' . $accountRoles . '|required_if:type,asset', + 'currency_id' => 'numeric|exists:transaction_currencies,id|required_without:currency_code', + 'currency_code' => 'min:3|max:3|exists:transaction_currencies,code|required_without:currency_id', + 'notes' => 'min:0|max:65536', + 'monthly_payment_date' => 'date' . '|required_if:account_role,ccAsset|required_if:cc_type,monthlyFull', + 'credit_card_type' => 'in:' . $ccPaymentTypes . '|required_if:account_role,ccAsset', + 'account_number' => 'between:1,255|nullable|uniqueAccountNumberForUser', + 'iban' => 'iban|nullable', + 'bic' => 'bic|nullable', + 'virtual_balance' => 'numeric|nullable', + 'opening_balance' => 'numeric|required_with:opening_balance_date|nullable', + 'opening_balance_date' => 'date|required_with:opening_balance|nullable', + + + + + 'include_net_worth' => 'required|boolean', + + // required fields for liabilities: - 'liability_type' => 'required_if:type,liability|in:loan,debt,mortgage,credit card', - 'liability_amount' => 'required_if:type,liability|min:0|numeric', - 'liability_start_date' => 'required_if:type,liability|date', - 'interest' => 'required_if:type,liability|between:0,100|numeric', - 'interest_period' => 'required_if:type,liability|in:daily,monthly,yearly', + 'liability_type' => 'required_if:type,liability|in:loan,debt,mortgage,credit card', + 'liability_amount' => 'required_if:type,liability|min:0|numeric', + 'liability_start_date' => 'required_if:type,liability|date', + 'interest' => 'required_if:type,liability|between:0,100|numeric', + 'interest_period' => 'required_if:type,liability|in:daily,monthly,yearly', ]; switch ($this->method()) { diff --git a/app/Api/V1/Requests/UserRequest.php b/app/Api/V1/Requests/UserRequest.php index 31cd81f1e2..bd0afa207f 100644 --- a/app/Api/V1/Requests/UserRequest.php +++ b/app/Api/V1/Requests/UserRequest.php @@ -25,6 +25,7 @@ declare(strict_types=1); namespace FireflyIII\Api\V1\Requests; use FireflyIII\Repositories\User\UserRepositoryInterface; +use FireflyIII\Rules\IsBoolean; use FireflyIII\User; @@ -68,6 +69,7 @@ class UserRequest extends Request 'email' => $this->string('email'), 'blocked' => $this->boolean('blocked'), 'blocked_code' => $this->string('blocked_code'), + 'role' => $this->string('role'), ]; return $data; @@ -82,8 +84,9 @@ class UserRequest extends Request { $rules = [ 'email' => 'required|email|unique:users,email,', - 'blocked' => 'required|boolean', + 'blocked' => [new IsBoolean], 'blocked_code' => 'in:email_changed', + 'role' => 'in:owner,demo', ]; switch ($this->method()) { default: diff --git a/app/Repositories/User/UserRepository.php b/app/Repositories/User/UserRepository.php index 987ec400e9..646bd248c9 100644 --- a/app/Repositories/User/UserRepository.php +++ b/app/Repositories/User/UserRepository.php @@ -288,7 +288,7 @@ class UserRepository implements UserRepositoryInterface */ public function store(array $data): User { - return User::create( + $user = User::create( [ 'blocked' => $data['blocked'] ?? false, 'blocked_code' => $data['blocked_code'] ?? null, @@ -296,6 +296,12 @@ class UserRepository implements UserRepositoryInterface 'password' => str_random(24), ] ); + $role = $data['role'] ?? ''; + if ('' !== $role) { + $this->attachRole($user, $role); + } + + return $user; } /** diff --git a/app/Rules/IsBoolean.php b/app/Rules/IsBoolean.php new file mode 100644 index 0000000000..2f385db2f4 --- /dev/null +++ b/app/Rules/IsBoolean.php @@ -0,0 +1,56 @@ + $account->name, 'active' => 1 === (int)$account->active, 'type' => $type, + 'account_role' => $role, 'currency_id' => $currencyId, 'currency_code' => $currencyCode, 'currency_symbol' => $currencySymbol, @@ -211,13 +212,12 @@ class AccountTransformer extends TransformerAbstract 'virtual_balance' => round($account->virtual_balance, $decimalPlaces), 'opening_balance' => $openingBalance, 'opening_balance_date' => $openingBalanceDate, - 'role' => $role, 'liability_type' => $type, 'liability_amount' => $openingBalance, 'liability_start_date' => $openingBalanceDate, 'interest' => $interest, 'interest_period' => $interestPeriod, - 'include_in_net_worth' => $includeNetworth, + 'include_net_worth' => $includeNetworth, 'links' => [ [ 'rel' => 'self', diff --git a/app/Transformers/UserTransformer.php b/app/Transformers/UserTransformer.php index cca96ecedd..851e87e13b 100644 --- a/app/Transformers/UserTransformer.php +++ b/app/Transformers/UserTransformer.php @@ -194,7 +194,7 @@ class UserTransformer extends TransformerAbstract 'created_at' => $user->created_at->toAtomString(), 'email' => $user->email, 'blocked' => 1 === (int)$user->blocked, - 'blocked_code' => $user->blocked_code, + 'blocked_code' => '' === $user->blocked_code ? null : $user->blocked_code, 'role' => $role, 'links' => [ [ diff --git a/app/Validation/FireflyValidator.php b/app/Validation/FireflyValidator.php index cc4766ed26..7318dd34f5 100644 --- a/app/Validation/FireflyValidator.php +++ b/app/Validation/FireflyValidator.php @@ -628,6 +628,11 @@ class FireflyValidator extends Validator { /** @var array $search */ $search = Config::get('firefly.accountTypeByIdentifier.' . $type); + + if (null === $search) { + return false; + } + /** @var Collection $accountTypes */ $accountTypes = AccountType::whereIn('type', $search)->get(); $ignore = (int)($parameters[0] ?? 0.0); diff --git a/resources/lang/en_US/import.php b/resources/lang/en_US/import.php index 2e00ce3ce2..f5a142517c 100644 --- a/resources/lang/en_US/import.php +++ b/resources/lang/en_US/import.php @@ -314,6 +314,7 @@ return [ 'column_sepa-ci' => 'SEPA Creditor Identifier', 'column_sepa-ep' => 'SEPA External Purpose', 'column_sepa-country' => 'SEPA Country Code', + 'column_sepa-batch-id' => 'SEPA Batch ID', 'column_tags-comma' => 'Tags (comma separated)', 'column_tags-space' => 'Tags (space separated)', 'column_account-number' => 'Asset account (account number)', diff --git a/tests/Api/V1/Controllers/UserControllerTest.php b/tests/Api/V1/Controllers/UserControllerTest.php index 823e489c96..5d207592e3 100644 --- a/tests/Api/V1/Controllers/UserControllerTest.php +++ b/tests/Api/V1/Controllers/UserControllerTest.php @@ -59,9 +59,8 @@ class UserControllerTest extends TestCase $userRepository = $this->mock(UserRepositoryInterface::class); $userRepository->shouldReceive('hasRole')->withArgs([Mockery::any(), 'owner'])->atLeast()->once()->andReturn(true); $userRepository->shouldReceive('destroy')->once(); - // create a user first: // call API - $response = $this->delete('/api/v1/users/' . $this->user()->id); + $response = $this->delete('/api/v1/users/2'); $response->assertStatus(204); } @@ -81,11 +80,28 @@ class UserControllerTest extends TestCase $user = User::create(['email' => 'some@newu' . random_int(1, 10000) . 'ser.nl', 'password' => 'hello', 'blocked' => 0]); // call API - $response = $this->delete('/api/v1/users/' . $user->id); + $response = $this->delete('/api/v1/users/' . $user->id, [], ['Accept' => 'application/json']); $response->assertStatus(302); $this->assertDatabaseHas('users', ['id' => $user->id]); } + /** + * Cannot delete yourself. + * + * @covers \FireflyIII\Api\V1\Controllers\UserController + * @covers \FireflyIII\Api\V1\Requests\UserRequest + */ + public function testDeleteYourself(): void + { + $userRepository = $this->mock(UserRepositoryInterface::class); + $userRepository->shouldReceive('hasRole')->withArgs([Mockery::any(), 'owner'])->atLeast()->once()->andReturn(true); + // create a user first: + // call API + $response = $this->delete('/api/v1/users/' . $this->user()->id, [], ['Accept' => 'application/json']); + $response->assertStatus(500); + $response->assertSee('No access to method.'); + } + /** * Show list of users. * @@ -103,13 +119,11 @@ class UserControllerTest extends TestCase $repository->shouldReceive('all')->withAnyArgs()->andReturn($users)->once(); // test API - $response = $this->get('/api/v1/users'); + $response = $this->get('/api/v1/users', ['Accept' => 'application/json']); $response->assertStatus(200); $response->assertJson(['data' => [],]); $response->assertJson(['meta' => ['pagination' => ['total' => 10, 'count' => 10, 'current_page' => 1, 'total_pages' => 1]],]); - $response->assertJson( - ['links' => ['self' => true, 'first' => true, 'last' => true,],] - ); + $response->assertJson(['links' => ['self' => true, 'first' => true, 'last' => true,],]); $response->assertHeader('Content-Type', 'application/vnd.api+json'); } @@ -125,7 +139,7 @@ class UserControllerTest extends TestCase $repository->shouldReceive('hasRole')->withArgs([Mockery::any(), 'owner'])->once()->andReturn(true); // test API - $response = $this->get('/api/v1/users/' . $user->id); + $response = $this->get('/api/v1/users/' . $user->id, ['Accept' => 'application/json']); $response->assertStatus(200); $response->assertSee($user->email); } @@ -139,8 +153,7 @@ class UserControllerTest extends TestCase public function testStoreBasic(): void { $data = [ - 'email' => 'some_new@user' . random_int(1, 10000) . '.com', - 'blocked' => 0, + 'email' => 'some_new@user' . random_int(1, 10000) . '.com', ]; // mock @@ -149,7 +162,32 @@ class UserControllerTest extends TestCase $userRepos->shouldReceive('store')->once()->andReturn($this->user()); // test API - $response = $this->post('/api/v1/users', $data); + $response = $this->post('/api/v1/users', $data, ['Content-Type' => 'application/x-www-form-urlencoded']); + $response->assertStatus(200); + $response->assertSee($this->user()->email); + } + + /** + * Store new user using JSON. + * + * @covers \FireflyIII\Api\V1\Controllers\UserController + * @covers \FireflyIII\Api\V1\Requests\UserRequest + */ + public function testStoreBasicJson(): void + { + $data = [ + 'email' => 'some_new@user' . random_int(1, 10000) . '.com', + 'blocked' => true, + 'blocked_code' => 'email_changed', + ]; + + // mock + $userRepos = $this->mock(UserRepositoryInterface::class); + $userRepos->shouldReceive('hasRole')->withArgs([Mockery::any(), 'owner'])->twice()->andReturn(true); + $userRepos->shouldReceive('store')->once()->andReturn($this->user()); + + // test API + $response = $this->postJson('/api/v1/users', $data, ['Accept' => 'application/json']); $response->assertStatus(200); $response->assertSee($this->user()->email); } @@ -185,6 +223,37 @@ class UserControllerTest extends TestCase ); } + /** + * Store user with info already used. + * + * @covers \FireflyIII\Api\V1\Controllers\UserController + * @covers \FireflyIII\Api\V1\Requests\UserRequest + */ + public function testStoreNotUniqueJson(): void + { + $data = [ + 'email' => $this->user()->email, + 'blocked' => 0, + ]; + + // mock + $userRepos = $this->mock(UserRepositoryInterface::class); + $userRepos->shouldReceive('hasRole')->withArgs([Mockery::any(), 'owner'])->twice()->andReturn(true); + // test API + $response = $this->postJson('/api/v1/users', $data, ['Accept' => 'application/json']); + $response->assertStatus(422); + $response->assertExactJson( + [ + 'message' => 'The given data was invalid.', + 'errors' => [ + 'email' => [ + 'The email address has already been taken.', + ], + ], + ] + ); + } + /** * Update user. * @@ -208,9 +277,35 @@ class UserControllerTest extends TestCase $userRepos->shouldReceive('hasRole')->withArgs([Mockery::any(), 'owner'])->twice()->andReturn(true); // call API - $response = $this->put('/api/v1/users/' . $user->id, $data); + $response = $this->put('/api/v1/users/' . $user->id, $data, ['Accept' => 'application/json']); $response->assertStatus(200); + } + /** + * Update user. + * + * @covers \FireflyIII\Api\V1\Controllers\UserController + * @covers \FireflyIII\Api\V1\Requests\UserRequest + */ + public function testUpdateJson(): void + { + // create a user first: + $user = User::create(['email' => 'some@newu' . random_int(1, 10000) . 'ser.nl', 'password' => 'hello', 'blocked' => 0]); + + // data: + $data = [ + 'email' => 'some-new@email' . random_int(1, 10000) . '.com', + 'blocked' => 0, + ]; + + // mock + $userRepos = $this->mock(UserRepositoryInterface::class); + $userRepos->shouldReceive('update')->once()->andReturn($user); + $userRepos->shouldReceive('hasRole')->withArgs([Mockery::any(), 'owner'])->twice()->andReturn(true); + + // call API + $response = $this->putJson('/api/v1/users/' . $user->id, $data, ['Accept' => 'application/json']); + $response->assertStatus(200); } }