mirror of
https://github.com/invoiceninja/invoiceninja.git
synced 2025-05-24 02:14:21 -04:00
New Validation rule for USER POST route (#3138)
This commit is contained in:
parent
90eeb59754
commit
ec5cbe66a0
@ -204,8 +204,8 @@ class UserController extends BaseController
|
|||||||
{
|
{
|
||||||
|
|
||||||
$company = auth()->user()->company();
|
$company = auth()->user()->company();
|
||||||
|
|
||||||
$user = $this->user_repo->save($request->all(), UserFactory::create());
|
$user = $this->user_repo->save($request->all(), $request->fetchUser());
|
||||||
|
|
||||||
$user_agent = request()->input('token_name') ?: request()->server('HTTP_USER_AGENT');
|
$user_agent = request()->input('token_name') ?: request()->server('HTTP_USER_AGENT');
|
||||||
|
|
||||||
|
@ -51,7 +51,7 @@ class StorePaymentRequest extends Request
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
if(is_array($input['invoices']) === false)
|
if(isset($input['invoices']) && is_array($input['invoices']) === false)
|
||||||
$input['invoices'] = null;
|
$input['invoices'] = null;
|
||||||
|
|
||||||
$this->replace($input);
|
$this->replace($input);
|
||||||
|
@ -12,8 +12,11 @@
|
|||||||
namespace App\Http\Requests\User;
|
namespace App\Http\Requests\User;
|
||||||
|
|
||||||
use App\DataMapper\DefaultSettings;
|
use App\DataMapper\DefaultSettings;
|
||||||
|
use App\Factory\UserFactory;
|
||||||
use App\Http\Requests\Request;
|
use App\Http\Requests\Request;
|
||||||
use App\Http\ValidationRules\NewUniqueUserRule;
|
use App\Http\ValidationRules\NewUniqueUserRule;
|
||||||
|
use App\Http\ValidationRules\ValidUserForCompany;
|
||||||
|
use App\Libraries\MultiDB;
|
||||||
use App\Models\User;
|
use App\Models\User;
|
||||||
|
|
||||||
class StoreUserRequest extends Request
|
class StoreUserRequest extends Request
|
||||||
@ -36,11 +39,17 @@ class StoreUserRequest extends Request
|
|||||||
|
|
||||||
$this->sanitize();
|
$this->sanitize();
|
||||||
|
|
||||||
return [
|
$rules = [];
|
||||||
'first_name' => 'required|string|max:100',
|
|
||||||
'last_name' => 'required|string:max:100',
|
$rules['first_name'] = 'required|string|max:100';
|
||||||
'email' => new NewUniqueUserRule(),
|
$rules['last_name'] = 'required|string|max:100';
|
||||||
];
|
|
||||||
|
if (config('ninja.db.multi_db_enabled'))
|
||||||
|
{
|
||||||
|
$rules['email'] = new ValidUserForCompany();
|
||||||
|
}
|
||||||
|
|
||||||
|
return $rules;
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -74,5 +83,14 @@ class StoreUserRequest extends Request
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
public function fetchUser() :User
|
||||||
|
{
|
||||||
|
$user = MultiDB::hasUser(['email' => $this->input('email')]);
|
||||||
|
|
||||||
|
if(!$user)
|
||||||
|
$user = UserFactory::create();
|
||||||
|
|
||||||
|
return $user;
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
45
app/Http/ValidationRules/ValidUserForCompany.php
Normal file
45
app/Http/ValidationRules/ValidUserForCompany.php
Normal file
@ -0,0 +1,45 @@
|
|||||||
|
<?php
|
||||||
|
/**
|
||||||
|
* Invoice Ninja (https://invoiceninja.com)
|
||||||
|
*
|
||||||
|
* @link https://github.com/invoiceninja/invoiceninja source repository
|
||||||
|
*
|
||||||
|
* @copyright Copyright (c) 2019. Invoice Ninja LLC (https://invoiceninja.com)
|
||||||
|
*
|
||||||
|
* @license https://opensource.org/licenses/AAL
|
||||||
|
*/
|
||||||
|
|
||||||
|
namespace App\Http\ValidationRules;
|
||||||
|
|
||||||
|
use App\Libraries\MultiDB;
|
||||||
|
use App\Models\User;
|
||||||
|
use Illuminate\Contracts\Validation\Rule;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Class ValidUserForCompany
|
||||||
|
* @package App\Http\ValidUserForCompany
|
||||||
|
*/
|
||||||
|
class ValidUserForCompany implements Rule
|
||||||
|
{
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @param string $attribute
|
||||||
|
* @param mixed $value
|
||||||
|
* @return bool
|
||||||
|
*/
|
||||||
|
public function passes($attribute, $value)
|
||||||
|
{
|
||||||
|
return MultiDB::checkUserAndCompanyCoExist($value, auth()->user()->company()->company_key);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @return string
|
||||||
|
*/
|
||||||
|
public function message()
|
||||||
|
{
|
||||||
|
return 'This user is unable to be attached to this company. Perhaps they have already registered a user on another account?';
|
||||||
|
//return ctrans('texts.email_already_register');
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
}
|
@ -18,6 +18,21 @@ use App\Models\User;
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Class MultiDB
|
* Class MultiDB
|
||||||
|
*
|
||||||
|
* Caution!
|
||||||
|
*
|
||||||
|
* When we perform scans across databases,
|
||||||
|
* we need to remember that if we don't
|
||||||
|
* return a DB 'HIT' the DB connection will
|
||||||
|
* be set to the last DB in the chain,
|
||||||
|
*
|
||||||
|
* So for these cases, we need to reset the
|
||||||
|
* DB connection to the default connection.
|
||||||
|
*
|
||||||
|
* Even that may be problematic, and we
|
||||||
|
* may need to know the current DB connection
|
||||||
|
* so that we can fall back gracefully.
|
||||||
|
*
|
||||||
* @package App\Libraries
|
* @package App\Libraries
|
||||||
*/
|
*/
|
||||||
class MultiDB
|
class MultiDB
|
||||||
@ -53,6 +68,7 @@ class MultiDB
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
self::setDefaultDatabase();
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -71,10 +87,44 @@ class MultiDB
|
|||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
self::setDefaultDatabase();
|
||||||
return false;
|
return false;
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A user and company must co exists on the same database.
|
||||||
|
*
|
||||||
|
* This function will check that if a user exists on the system,
|
||||||
|
* the company is also located on the same database.
|
||||||
|
*
|
||||||
|
* If no user is found, then we also return true as this must be
|
||||||
|
* a new user request.
|
||||||
|
*
|
||||||
|
* @param string $email The user email
|
||||||
|
* @param stirng $company_key The company key
|
||||||
|
* @return bool True|False
|
||||||
|
*/
|
||||||
|
public static function checkUserAndCompanyCoExist($email, $company_key) :bool
|
||||||
|
{
|
||||||
|
|
||||||
|
foreach (self::$dbs as $db)
|
||||||
|
{
|
||||||
|
if(User::on($db)->where(['email' => $email])->get()->count() >=1) // if user already exists, validation will fail
|
||||||
|
{
|
||||||
|
if(Company::on($db)->where(['company_key' => $company_key])->get()->count() >=1)
|
||||||
|
return true;
|
||||||
|
else{
|
||||||
|
self::setDefaultDatabase();
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
self::setDefaultDatabase();
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @param array $data
|
* @param array $data
|
||||||
* @return App\Models\User | bool
|
* @return App\Models\User | bool
|
||||||
@ -101,6 +151,7 @@ class MultiDB
|
|||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
self::setDefaultDatabase();
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -119,6 +170,8 @@ class MultiDB
|
|||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
self::setDefaultDatabase();
|
||||||
return false;
|
return false;
|
||||||
|
|
||||||
}
|
}
|
||||||
@ -172,6 +225,8 @@ class MultiDB
|
|||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
self::setDefaultDatabase();
|
||||||
return false;
|
return false;
|
||||||
|
|
||||||
}
|
}
|
||||||
@ -189,6 +244,8 @@ class MultiDB
|
|||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
self::setDefaultDatabase();
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -36,7 +36,7 @@ class ClientContactRepository extends BaseRepository
|
|||||||
|
|
||||||
$this->is_primary = true;
|
$this->is_primary = true;
|
||||||
/* Set first record to primary - always */
|
/* Set first record to primary - always */
|
||||||
$contacts = $contacts->sortBy('is_primary')->map(function ($contact){
|
$contacts = $contacts->sortByDesc('is_primary')->map(function ($contact){
|
||||||
$contact['is_primary'] = $this->is_primary;
|
$contact['is_primary'] = $this->is_primary;
|
||||||
$this->is_primary = false;
|
$this->is_primary = false;
|
||||||
return $contact;
|
return $contact;
|
||||||
|
@ -48,7 +48,7 @@ class UserRepository extends BaseRepository
|
|||||||
*
|
*
|
||||||
* @return user|\App\Models\user|null user Object
|
* @return user|\App\Models\user|null user Object
|
||||||
*/
|
*/
|
||||||
public function save(array $data, User $user) : ?user
|
public function save(array $data, User $user) : ?User
|
||||||
{
|
{
|
||||||
|
|
||||||
$user->fill($data);
|
$user->fill($data);
|
||||||
|
@ -2,9 +2,11 @@
|
|||||||
|
|
||||||
namespace Tests\Integration;
|
namespace Tests\Integration;
|
||||||
|
|
||||||
|
use App\Factory\CompanyUserFactory;
|
||||||
use App\Libraries\MultiDB;
|
use App\Libraries\MultiDB;
|
||||||
use App\Models\Account;
|
use App\Models\Account;
|
||||||
use App\Models\Company;
|
use App\Models\Company;
|
||||||
|
use App\Models\CompanyToken;
|
||||||
use App\Models\User;
|
use App\Models\User;
|
||||||
use Illuminate\Foundation\Testing\Concerns\InteractsWithDatabase;
|
use Illuminate\Foundation\Testing\Concerns\InteractsWithDatabase;
|
||||||
use Illuminate\Foundation\Testing\DatabaseTransactions;
|
use Illuminate\Foundation\Testing\DatabaseTransactions;
|
||||||
@ -53,7 +55,8 @@ class MultiDBUserTest extends TestCase
|
|||||||
$company->setHidden(['settings', 'settings_object', 'hashed_id']);
|
$company->setHidden(['settings', 'settings_object', 'hashed_id']);
|
||||||
$company2->setHidden(['settings', 'settings_object', 'hashed_id']);
|
$company2->setHidden(['settings', 'settings_object', 'hashed_id']);
|
||||||
|
|
||||||
Company::on('db-ninja-01')->create($company->toArray());
|
$coco = Company::on('db-ninja-01')->create($company->toArray());
|
||||||
|
|
||||||
Company::on('db-ninja-02')->create($company2->toArray());
|
Company::on('db-ninja-02')->create($company2->toArray());
|
||||||
|
|
||||||
$user = [
|
$user = [
|
||||||
@ -82,8 +85,27 @@ class MultiDBUserTest extends TestCase
|
|||||||
|
|
||||||
];
|
];
|
||||||
|
|
||||||
User::on('db-ninja-01')->create($user);
|
$user = User::on('db-ninja-01')->create($user);
|
||||||
|
|
||||||
|
$cu = CompanyUserFactory::create($user->id, $coco->id, $account->id);
|
||||||
|
$cu->is_owner = true;
|
||||||
|
$cu->is_admin = true;
|
||||||
|
$cu->save();
|
||||||
|
|
||||||
User::on('db-ninja-02')->create($user2);
|
User::on('db-ninja-02')->create($user2);
|
||||||
|
|
||||||
|
$this->token = \Illuminate\Support\Str::random(40);
|
||||||
|
|
||||||
|
$this->company_token = CompanyToken::on('db-ninja-01')->create([
|
||||||
|
'user_id' => $user->id,
|
||||||
|
'company_id' => $coco->id,
|
||||||
|
'account_id' => $account->id,
|
||||||
|
'name' => 'test token',
|
||||||
|
'token' => $this->token,
|
||||||
|
]);
|
||||||
|
|
||||||
|
User::unguard(false);
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
public function test_oauth_user_db2_exists()
|
public function test_oauth_user_db2_exists()
|
||||||
@ -135,9 +157,85 @@ class MultiDBUserTest extends TestCase
|
|||||||
$this->expectNotToPerformAssertions(MultiDB::setDB('db-ninja-01'));
|
$this->expectNotToPerformAssertions(MultiDB::setDB('db-ninja-01'));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function test_cross_db_user_linking_fails_appropriately()
|
||||||
|
{
|
||||||
|
|
||||||
|
//$this->withoutExceptionHandling();
|
||||||
|
|
||||||
|
$data = [
|
||||||
|
'first_name' => 'hey',
|
||||||
|
'last_name' => 'you',
|
||||||
|
'email' => 'db2@example.com',
|
||||||
|
'company_user' => [
|
||||||
|
'is_admin' => false,
|
||||||
|
'is_owner' => false,
|
||||||
|
'permissions' => 'create_client,create_invoice'
|
||||||
|
],
|
||||||
|
];
|
||||||
|
|
||||||
|
try{
|
||||||
|
$response = $this->withHeaders([
|
||||||
|
'X-API-SECRET' => config('ninja.api_secret'),
|
||||||
|
'X-API-TOKEN' => $this->token,
|
||||||
|
])->post('/api/v1/users?include=company_user', $data);
|
||||||
|
|
||||||
|
|
||||||
|
}
|
||||||
|
catch(ValidationException $e) {
|
||||||
|
\Log::error('in the validator');
|
||||||
|
$message = json_decode($e->validator->getMessageBag(),1);
|
||||||
|
\Log::error($message);
|
||||||
|
$this->assertNotNull($message);
|
||||||
|
|
||||||
|
}
|
||||||
|
|
||||||
|
if($response)
|
||||||
|
$response->assertStatus(302);
|
||||||
|
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
public function test_cross_db_user_linking_succeeds_appropriately()
|
||||||
|
{
|
||||||
|
|
||||||
|
//$this->withoutExceptionHandling();
|
||||||
|
|
||||||
|
$data = [
|
||||||
|
'first_name' => 'hey',
|
||||||
|
'last_name' => 'you',
|
||||||
|
'email' => 'db1@example.com',
|
||||||
|
'company_user' => [
|
||||||
|
'is_admin' => false,
|
||||||
|
'is_owner' => false,
|
||||||
|
'permissions' => 'create_client,create_invoice'
|
||||||
|
],
|
||||||
|
];
|
||||||
|
|
||||||
|
try{
|
||||||
|
$response = $this->withHeaders([
|
||||||
|
'X-API-SECRET' => config('ninja.api_secret'),
|
||||||
|
'X-API-TOKEN' => $this->token,
|
||||||
|
])->post('/api/v1/users?include=company_user', $data);
|
||||||
|
|
||||||
|
|
||||||
|
}
|
||||||
|
catch(ValidationException $e) {
|
||||||
|
\Log::error('in the validator');
|
||||||
|
$message = json_decode($e->validator->getMessageBag(),1);
|
||||||
|
\Log::error($message);
|
||||||
|
$this->assertNotNull($message);
|
||||||
|
|
||||||
|
}
|
||||||
|
|
||||||
|
if($response)
|
||||||
|
$response->assertStatus(200);
|
||||||
|
|
||||||
|
}
|
||||||
|
|
||||||
public function tearDown() :void
|
public function tearDown() :void
|
||||||
{
|
{
|
||||||
DB::connection('db-ninja-01')->table('users')->delete();
|
DB::connection('db-ninja-01')->table('users')->delete();
|
||||||
DB::connection('db-ninja-02')->table('users')->delete();
|
DB::connection('db-ninja-02')->table('users')->delete();
|
||||||
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -24,36 +24,40 @@ class UniqueEmailTest extends TestCase
|
|||||||
{
|
{
|
||||||
parent::setUp();
|
parent::setUp();
|
||||||
|
|
||||||
|
User::unguard();
|
||||||
|
|
||||||
if (! config('ninja.db.multi_db_enabled'))
|
if (! config('ninja.db.multi_db_enabled'))
|
||||||
$this->markTestSkipped('Multi DB not enabled - skipping');
|
$this->markTestSkipped('Multi DB not enabled - skipping');
|
||||||
|
|
||||||
DB::connection('db-ninja-01')->table('users')->delete();
|
|
||||||
DB::connection('db-ninja-02')->table('users')->delete();
|
|
||||||
|
|
||||||
$this->rule = new NewUniqueUserRule();
|
$this->rule = new NewUniqueUserRule();
|
||||||
|
|
||||||
$ac = factory(\App\Models\Account::class)->make();
|
$ac = factory(\App\Models\Account::class)->make();
|
||||||
|
|
||||||
$ac->setHidden(['hashed_id']);
|
$ac->setHidden(['hashed_id']);
|
||||||
|
|
||||||
$account = Account::on('db-ninja-01')->create($ac->toArray());
|
$account = Account::on('db-ninja-01')->create($ac->toArray());
|
||||||
$account2 = Account::on('db-ninja-02')->create($ac->toArray());
|
|
||||||
|
|
||||||
$company = factory(\App\Models\Company::class)->make([
|
$company = factory(\App\Models\Company::class)->make([
|
||||||
'account_id' => $account->id,
|
'account_id' => $account->id,
|
||||||
'domain' => 'ninja.test',
|
'domain' => 'ninja.test',
|
||||||
]);
|
]);
|
||||||
|
|
||||||
|
$company->setHidden(['settings', 'settings_object', 'hashed_id']);
|
||||||
|
|
||||||
|
Company::on('db-ninja-01')->create($company->toArray());
|
||||||
|
|
||||||
|
|
||||||
|
$ac2 = factory(\App\Models\Account::class)->make();
|
||||||
|
$ac2->setHidden(['hashed_id']);
|
||||||
|
$account2 = Account::on('db-ninja-02')->create($ac2->toArray());
|
||||||
|
|
||||||
$company2 = factory(\App\Models\Company::class)->make([
|
$company2 = factory(\App\Models\Company::class)->make([
|
||||||
'account_id' => $account2->id,
|
'account_id' => $account2->id,
|
||||||
'domain' => 'ninja.test',
|
'domain' => 'ninja.test',
|
||||||
|
|
||||||
]);
|
]);
|
||||||
|
|
||||||
$company->setHidden(['settings', 'settings_object', 'hashed_id']);
|
|
||||||
$company2->setHidden(['settings', 'settings_object', 'hashed_id']);
|
$company2->setHidden(['settings', 'settings_object', 'hashed_id']);
|
||||||
|
|
||||||
Company::on('db-ninja-01')->create($company->toArray());
|
|
||||||
Company::on('db-ninja-02')->create($company2->toArray());
|
Company::on('db-ninja-02')->create($company2->toArray());
|
||||||
|
|
||||||
|
|
||||||
@ -91,9 +95,8 @@ class UniqueEmailTest extends TestCase
|
|||||||
}
|
}
|
||||||
|
|
||||||
public function tearDown() :void
|
public function tearDown() :void
|
||||||
{
|
{
|
||||||
DB::connection('db-ninja-01')->table('users')->delete();
|
DB::connection('db-ninja-01')->table('users')->delete();
|
||||||
DB::connection('db-ninja-02')->table('users')->delete();
|
DB::connection('db-ninja-02')->table('users')->delete();
|
||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
Loading…
x
Reference in New Issue
Block a user