diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index 2e3236a5d7b2..7be0c917169d 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -204,8 +204,8 @@ class UserController extends BaseController { $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'); diff --git a/app/Http/Requests/Payment/StorePaymentRequest.php b/app/Http/Requests/Payment/StorePaymentRequest.php index 88bab4fb868e..4933149c3cdf 100644 --- a/app/Http/Requests/Payment/StorePaymentRequest.php +++ b/app/Http/Requests/Payment/StorePaymentRequest.php @@ -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; $this->replace($input); diff --git a/app/Http/Requests/User/StoreUserRequest.php b/app/Http/Requests/User/StoreUserRequest.php index 6cf6c3d751c9..073f9b0c92cd 100644 --- a/app/Http/Requests/User/StoreUserRequest.php +++ b/app/Http/Requests/User/StoreUserRequest.php @@ -12,8 +12,11 @@ namespace App\Http\Requests\User; use App\DataMapper\DefaultSettings; +use App\Factory\UserFactory; use App\Http\Requests\Request; use App\Http\ValidationRules\NewUniqueUserRule; +use App\Http\ValidationRules\ValidUserForCompany; +use App\Libraries\MultiDB; use App\Models\User; class StoreUserRequest extends Request @@ -36,11 +39,17 @@ class StoreUserRequest extends Request $this->sanitize(); - return [ - 'first_name' => 'required|string|max:100', - 'last_name' => 'required|string:max:100', - 'email' => new NewUniqueUserRule(), - ]; + $rules = []; + + $rules['first_name'] = 'required|string|max:100'; + $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; + } } \ No newline at end of file diff --git a/app/Http/ValidationRules/ValidUserForCompany.php b/app/Http/ValidationRules/ValidUserForCompany.php new file mode 100644 index 000000000000..c7c74294c99d --- /dev/null +++ b/app/Http/ValidationRules/ValidUserForCompany.php @@ -0,0 +1,45 @@ +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'); + } + + +} diff --git a/app/Libraries/MultiDB.php b/app/Libraries/MultiDB.php index b7d0915453f8..3b6aca633a17 100644 --- a/app/Libraries/MultiDB.php +++ b/app/Libraries/MultiDB.php @@ -18,6 +18,21 @@ use App\Models\User; /** * 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 */ class MultiDB @@ -53,6 +68,7 @@ class MultiDB return false; } + self::setDefaultDatabase(); return true; } @@ -71,10 +87,44 @@ class MultiDB return true; } + self::setDefaultDatabase(); 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 * @return App\Models\User | bool @@ -101,6 +151,7 @@ class MultiDB } + self::setDefaultDatabase(); return null; } @@ -119,6 +170,8 @@ class MultiDB } } + + self::setDefaultDatabase(); return false; } @@ -172,6 +225,8 @@ class MultiDB } } + + self::setDefaultDatabase(); return false; } @@ -189,6 +244,8 @@ class MultiDB return true; } } + + self::setDefaultDatabase(); return false; } diff --git a/app/Repositories/ClientContactRepository.php b/app/Repositories/ClientContactRepository.php index 3867097ce656..e4233522dd9f 100644 --- a/app/Repositories/ClientContactRepository.php +++ b/app/Repositories/ClientContactRepository.php @@ -36,7 +36,7 @@ class ClientContactRepository extends BaseRepository $this->is_primary = true; /* 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; $this->is_primary = false; return $contact; diff --git a/app/Repositories/UserRepository.php b/app/Repositories/UserRepository.php index c77e14ca4277..7ce2fe42b990 100644 --- a/app/Repositories/UserRepository.php +++ b/app/Repositories/UserRepository.php @@ -48,7 +48,7 @@ class UserRepository extends BaseRepository * * @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); diff --git a/tests/Integration/MultiDBUserTest.php b/tests/Integration/MultiDBUserTest.php index 938eb790e505..d17828e2eb91 100644 --- a/tests/Integration/MultiDBUserTest.php +++ b/tests/Integration/MultiDBUserTest.php @@ -2,9 +2,11 @@ namespace Tests\Integration; +use App\Factory\CompanyUserFactory; use App\Libraries\MultiDB; use App\Models\Account; use App\Models\Company; +use App\Models\CompanyToken; use App\Models\User; use Illuminate\Foundation\Testing\Concerns\InteractsWithDatabase; use Illuminate\Foundation\Testing\DatabaseTransactions; @@ -53,7 +55,8 @@ class MultiDBUserTest extends TestCase $company->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()); $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); + + $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() @@ -135,9 +157,85 @@ class MultiDBUserTest extends TestCase $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 { DB::connection('db-ninja-01')->table('users')->delete(); DB::connection('db-ninja-02')->table('users')->delete(); + } } diff --git a/tests/Integration/UniqueEmailTest.php b/tests/Integration/UniqueEmailTest.php index e7691032f5d5..bfa4749e5dad 100644 --- a/tests/Integration/UniqueEmailTest.php +++ b/tests/Integration/UniqueEmailTest.php @@ -24,36 +24,40 @@ class UniqueEmailTest extends TestCase { parent::setUp(); + User::unguard(); + if (! config('ninja.db.multi_db_enabled')) $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(); $ac = factory(\App\Models\Account::class)->make(); - $ac->setHidden(['hashed_id']); $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([ 'account_id' => $account->id, '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([ 'account_id' => $account2->id, - 'domain' => 'ninja.test', + 'domain' => 'ninja.test', ]); - $company->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()); @@ -91,9 +95,8 @@ class UniqueEmailTest extends TestCase } public function tearDown() :void - { - DB::connection('db-ninja-01')->table('users')->delete(); - DB::connection('db-ninja-02')->table('users')->delete(); + { + DB::connection('db-ninja-01')->table('users')->delete(); + DB::connection('db-ninja-02')->table('users')->delete(); } - } \ No newline at end of file