Fix foreign keys and deletion service

This commit is contained in:
Constantin Graf
2024-11-05 11:35:55 +01:00
committed by Constantin Graf
parent 4224fdd57e
commit 3b3f593080
8 changed files with 344 additions and 5 deletions

View File

@@ -25,7 +25,9 @@ use Illuminate\Support\Facades\Storage;
use Laravel\Fortify\TwoFactorAuthenticatable; use Laravel\Fortify\TwoFactorAuthenticatable;
use Laravel\Jetstream\HasProfilePhoto; use Laravel\Jetstream\HasProfilePhoto;
use Laravel\Jetstream\HasTeams; use Laravel\Jetstream\HasTeams;
use Laravel\Passport\AuthCode;
use Laravel\Passport\HasApiTokens; use Laravel\Passport\HasApiTokens;
use Laravel\Passport\Token;
use OwenIt\Auditing\Contracts\Auditable as AuditableContract; use OwenIt\Auditing\Contracts\Auditable as AuditableContract;
/** /**
@@ -178,6 +180,22 @@ class User extends Authenticatable implements AuditableContract, FilamentUser, M
return $this->hasMany(ProjectMember::class, 'user_id'); return $this->hasMany(ProjectMember::class, 'user_id');
} }
/**
* @return HasMany<Token>
*/
public function accessTokens(): HasMany
{
return $this->hasMany(Token::class);
}
/**
* @return HasMany<AuthCode>
*/
public function authCodes(): HasMany
{
return $this->hasMany(AuthCode::class);
}
/** /**
* @param Builder<User> $builder * @param Builder<User> $builder
*/ */

View File

@@ -144,6 +144,7 @@ class DeletionService
->get(); ->get();
foreach ($members as $member) { foreach ($members as $member) {
/** @var Member $member */
if ($member->role === Role::Owner->value && $member->organization->users()->count() > 1) { if ($member->role === Role::Owner->value && $member->organization->users()->count() > 1) {
throw new CanNotDeleteUserWhoIsOwnerOfOrganizationWithMultipleMembers; throw new CanNotDeleteUserWhoIsOwnerOfOrganizationWithMultipleMembers;
} }
@@ -154,10 +155,13 @@ class DeletionService
if ($member->role === Role::Owner->value) { if ($member->role === Role::Owner->value) {
$this->deleteOrganization($member->organization, false, $user); $this->deleteOrganization($member->organization, false, $user);
} else { } else {
$this->memberService->makeMemberToPlaceholder($member); $this->memberService->makeMemberToPlaceholder($member, false);
} }
} }
$user->accessTokens()->delete();
$user->authCodes()->delete();
// Note: Since the deletion of the profile photo is not reversible via a database rollback this needs to be done last // Note: Since the deletion of the profile photo is not reversible via a database rollback this needs to be done last
$user->deleteProfilePhoto(); $user->deleteProfilePhoto();

View File

@@ -44,7 +44,7 @@ class MemberService
} }
} }
public function makeMemberToPlaceholder(Member $member): void public function makeMemberToPlaceholder(Member $member, bool $makeSureUserHasAtLeastOneOrganization = true): void
{ {
$user = $member->user; $user = $member->user;
$placeholderUser = $user->replicate(); $placeholderUser = $user->replicate();
@@ -56,6 +56,8 @@ class MemberService
$member->save(); $member->save();
$this->userService->assignOrganizationEntitiesToDifferentMember($member->organization, $user, $placeholderUser, $member); $this->userService->assignOrganizationEntitiesToDifferentMember($member->organization, $user, $placeholderUser, $member);
$this->userService->makeSureUserHasAtLeastOneOrganization($user); if ($makeSureUserHasAtLeastOneOrganization) {
$this->userService->makeSureUserHasAtLeastOneOrganization($user);
}
} }
} }

View File

@@ -0,0 +1,93 @@
<?php
declare(strict_types=1);
use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Query\Builder;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;
return new class extends Migration
{
/**
* Run the migrations.
*/
public function up(): void
{
$foreignKeyProblems = DB::table('organizations')
->select(['organizations.id', 'organizations.user_id'])
->whereNotExists(function (Builder $query): void {
$query->select('id')
->from('users')
->whereColumn('organizations.user_id', 'users.id');
})
->get();
foreach ($foreignKeyProblems as $foreignKeyProblem) {
Log::error('Organization with ID '.$foreignKeyProblem->id.' has non-existing owner with ID '.$foreignKeyProblem->user_id);
}
if ($foreignKeyProblems->count() > 0) {
throw new Exception('There are organizations with non-existing owners, check the logs for more information');
}
Schema::table('organizations', function (Blueprint $table): void {
$table->foreign('user_id')
->references('id')
->on('users')
->onDelete('restrict')
->onUpdate('cascade');
});
$foreignKeyProblems = DB::table('members')
->select(['members.id', 'members.organization_id'])
->whereNotExists(function (Builder $query): void {
$query->select('id')
->from('organizations')
->whereColumn('members.organization_id', 'organizations.id');
})
->get();
foreach ($foreignKeyProblems as $foreignKeyProblem) {
Log::error('Member with ID '.$foreignKeyProblem->id.' has non-existing organization with ID '.$foreignKeyProblem->organization_id);
}
if ($foreignKeyProblems->count() > 0) {
throw new Exception('There are members with non-existing organizations, check the logs for more information');
}
$foreignKeyProblems = DB::table('members')
->select(['members.id', 'members.user_id'])
->whereNotExists(function (Builder $query): void {
$query->select('id')
->from('users')
->whereColumn('members.user_id', 'users.id');
})
->get();
foreach ($foreignKeyProblems as $foreignKeyProblem) {
Log::error('Member with ID '.$foreignKeyProblem->id.' has non-existing user with ID '.$foreignKeyProblem->user_id);
}
if ($foreignKeyProblems->count() > 0) {
throw new Exception('There are members with non-existing users, check the logs for more information');
}
Schema::table('members', function (Blueprint $table): void {
$table->foreign('organization_id')
->references('id')
->on('organizations')
->onDelete('restrict')
->onUpdate('cascade');
$table->foreign('user_id')
->references('id')
->on('users')
->onDelete('restrict')
->onUpdate('cascade');
});
}
/**
* Reverse the migrations.
*/
public function down(): void
{
Schema::table('organizations', function (Blueprint $table): void {
$table->dropForeign(['user_id']);
});
Schema::table('members', function (Blueprint $table): void {
$table->dropForeign(['organization_id']);
$table->dropForeign(['user_id']);
});
}
};

View File

@@ -0,0 +1,114 @@
<?php
declare(strict_types=1);
use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Query\Builder;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;
return new class extends Migration
{
/**
* Run the migrations.
*/
public function up(): void
{
DB::table('oauth_access_tokens')
->whereNotNull('user_id')
->whereNotExists(function (Builder $query): void {
$query->select('id')
->from('users')
->whereColumn('oauth_access_tokens.user_id', 'users.id');
})
->delete();
DB::table('oauth_access_tokens')
->whereNotExists(function (Builder $query): void {
$query->select('id')
->from('oauth_clients')
->whereColumn('oauth_access_tokens.client_id', 'oauth_clients.id');
})
->delete();
Schema::table('oauth_access_tokens', function (Blueprint $table): void {
$table->foreign('user_id')
->references('id')
->on('users')
->onDelete('restrict')
->onUpdate('cascade');
$table->foreign('client_id')
->references('id')
->on('oauth_clients')
->onDelete('restrict')
->onUpdate('cascade');
});
DB::table('oauth_auth_codes')
->whereNotExists(function (Builder $query): void {
$query->select('id')
->from('users')
->whereColumn('oauth_auth_codes.user_id', 'users.id');
})
->delete();
DB::table('oauth_auth_codes')
->whereNotExists(function (Builder $query): void {
$query->select('id')
->from('oauth_clients')
->whereColumn('oauth_auth_codes.client_id', 'oauth_clients.id');
})
->delete();
Schema::table('oauth_auth_codes', function (Blueprint $table): void {
$table->foreign('user_id')
->references('id')
->on('users')
->onDelete('restrict')
->onUpdate('cascade');
$table->foreign('client_id')
->references('id')
->on('oauth_clients')
->onDelete('restrict')
->onUpdate('cascade');
});
DB::table('oauth_clients')
->whereNotNull('user_id')
->whereNotExists(function (Builder $query): void {
$query->select('id')
->from('users')
->whereColumn('oauth_clients.user_id', 'users.id');
})
->delete();
Schema::table('oauth_clients', function (Blueprint $table): void {
$table->foreign('user_id')
->references('id')
->on('users')
->onDelete('restrict')
->onUpdate('cascade');
});
Schema::table('oauth_personal_access_clients', function (Blueprint $table): void {
$table->foreign('client_id')
->references('id')
->on('oauth_clients')
->onDelete('restrict')
->onUpdate('cascade');
});
}
/**
* Reverse the migrations.
*/
public function down(): void
{
Schema::table('oauth_access_tokens', function (Blueprint $table): void {
$table->dropForeign(['user_id']);
$table->dropForeign(['client_id']);
});
Schema::table('oauth_auth_codes', function (Blueprint $table): void {
$table->dropForeign(['user_id']);
$table->dropForeign(['client_id']);
});
Schema::table('oauth_clients', function (Blueprint $table): void {
$table->dropForeign(['user_id']);
});
Schema::table('oauth_personal_access_clients', function (Blueprint $table): void {
$table->dropForeign(['client_id']);
});
}
};

View File

@@ -18,6 +18,12 @@ use App\Models\TimeEntry;
use App\Models\User; use App\Models\User;
use Illuminate\Database\Seeder; use Illuminate\Database\Seeder;
use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\DB;
use Laravel\Passport\AuthCode;
use Laravel\Passport\Client as PassportClient;
use Laravel\Passport\ClientRepository;
use Laravel\Passport\PersonalAccessClient;
use Laravel\Passport\RefreshToken;
use Laravel\Passport\Token;
class DatabaseSeeder extends Seeder class DatabaseSeeder extends Seeder
{ {
@@ -150,10 +156,35 @@ class DatabaseSeeder extends Seeder
User::factory()->withPersonalOrganization()->create([ User::factory()->withPersonalOrganization()->create([
'email' => 'admin@example.com', 'email' => 'admin@example.com',
]); ]);
app(ClientRepository::class)->create(
null,
'desktop',
'solidtime://oauth/callback',
null,
false,
false,
false
);
} }
private function deleteAll(): void private function deleteAll(): void
{ {
// Laravel Passport tables
DB::table((new RefreshToken)->getTable())->delete();
DB::table((new Token)->getTable())->delete();
DB::table((new AuthCode)->getTable())->delete();
DB::table((new PersonalAccessClient)->getTable())->delete();
DB::table((new PassportClient)->getTable())->delete();
// Internal tables
DB::table('cache')->delete();
DB::table('cache_locks')->delete();
DB::table('jobs')->delete();
DB::table('failed_jobs')->delete();
DB::table('sessions')->delete();
// Application tables
DB::table((new Audit)->getTable())->delete(); DB::table((new Audit)->getTable())->delete();
DB::table((new TimeEntry)->getTable())->delete(); DB::table((new TimeEntry)->getTable())->delete();
DB::table((new Task)->getTable())->delete(); DB::table((new Task)->getTable())->delete();
@@ -161,8 +192,9 @@ class DatabaseSeeder extends Seeder
DB::table((new ProjectMember)->getTable())->delete(); DB::table((new ProjectMember)->getTable())->delete();
DB::table((new Project)->getTable())->delete(); DB::table((new Project)->getTable())->delete();
DB::table((new Client)->getTable())->delete(); DB::table((new Client)->getTable())->delete();
DB::table((new User)->getTable())->delete(); DB::table((new Member)->getTable())->delete();
DB::table((new OrganizationInvitation)->getTable())->delete(); DB::table((new OrganizationInvitation)->getTable())->delete();
DB::table((new Organization)->getTable())->delete(); DB::table((new Organization)->getTable())->delete();
DB::table((new User)->getTable())->delete();
} }
} }

View File

@@ -13,6 +13,9 @@ use App\Models\User;
use App\Providers\Filament\AdminPanelProvider; use App\Providers\Filament\AdminPanelProvider;
use Filament\Panel; use Filament\Panel;
use Illuminate\Support\Facades\Config; use Illuminate\Support\Facades\Config;
use Laravel\Passport\AuthCode;
use Laravel\Passport\Client;
use Laravel\Passport\Token;
use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\Attributes\UsesClass;
@@ -138,4 +141,66 @@ class UserModelTest extends ModelTestAbstract
$this->assertCount(1, $activeUsers); $this->assertCount(1, $activeUsers);
$this->assertTrue($activeUsers->first()->is($user)); $this->assertTrue($activeUsers->first()->is($user));
} }
public function test_it_has_many_access_tokens(): void
{
// Arrange
$user = User::factory()->create();
$client = new Client;
$client->name = 'desktop';
$client->redirect = 'solidtime://oauth/callback';
$client->personal_access_client = false;
$client->password_client = false;
$client->revoked = false;
$client->save();
$token = new Token;
$token->id = 'some-id';
$token->user_id = $user->getKey();
$token->client_id = $client->getKey();
$token->revoked = false;
$token->save();
// Act
$user->refresh();
$tokensRel = $user->accessTokens;
// Assert
$this->assertNotNull($tokensRel);
$this->assertCount(1, $tokensRel);
$this->assertEqualsCanonicalizing(
[$token->getKey()],
$tokensRel->pluck('id')->toArray()
);
}
public function test_it_has_many_auth_codes(): void
{
// Arrange
$user = User::factory()->create();
$client = new Client;
$client->name = 'desktop';
$client->redirect = 'solidtime://oauth/callback';
$client->personal_access_client = false;
$client->password_client = false;
$client->revoked = false;
$client->save();
$authCode = new AuthCode;
$authCode->id = 'some-id';
$authCode->user_id = $user->getKey();
$authCode->client_id = $client->getKey();
$authCode->revoked = false;
$authCode->save();
// Act
$user->refresh();
$authCodesRel = $user->authCodes;
// Assert
$this->assertNotNull($authCodesRel);
$this->assertCount(1, $authCodesRel);
$this->assertEqualsCanonicalizing(
[$authCode->getKey()],
$authCodesRel->pluck('id')->toArray()
);
}
} }

View File

@@ -307,20 +307,31 @@ class DeletionServiceTest extends TestCaseWithDatabase
{ {
// Arrange // Arrange
$user = User::factory()->create(); $user = User::factory()->create();
$otherUser = User::factory()->create();
$organizationOwned = Organization::factory()->withOwner($user)->create(); $organizationOwned = Organization::factory()->withOwner($user)->create();
$organizationNotOwned = Organization::factory()->create(); $organizationNotOwned = Organization::factory()->withOwner($otherUser)->create();
$memberOwned = Member::factory()->forUser($user)->forOrganization($organizationOwned)->role(Role::Owner)->create(); $memberOwned = Member::factory()->forUser($user)->forOrganization($organizationOwned)->role(Role::Owner)->create();
$memberNotOwned = Member::factory()->forUser($user)->forOrganization($organizationNotOwned)->role(Role::Employee)->create(); $memberNotOwned = Member::factory()->forUser($user)->forOrganization($organizationNotOwned)->role(Role::Employee)->create();
TimeEntry::factory()->forOrganization($organizationOwned)->forMember($memberOwned)->createMany(2); TimeEntry::factory()->forOrganization($organizationOwned)->forMember($memberOwned)->createMany(2);
TimeEntry::factory()->forOrganization($organizationNotOwned)->forMember($memberNotOwned)->createMany(2); TimeEntry::factory()->forOrganization($organizationNotOwned)->forMember($memberNotOwned)->createMany(2);
$this->assertDatabaseCount(User::class, 2);
// Act // Act
$this->deletionService->deleteUser($user); $this->deletionService->deleteUser($user);
// Assert // Assert
$this->assertDatabaseCount(Organization::class, 1);
$this->assertDatabaseCount(User::class, 2);
$this->assertDatabaseMissing(User::class, [ $this->assertDatabaseMissing(User::class, [
'id' => $user->getKey(), 'id' => $user->getKey(),
]); ]);
$this->assertDatabaseHas(User::class, [
'id' => $otherUser->getKey(),
'is_placeholder' => false,
]);
$this->assertDatabaseHas(User::class, [
'is_placeholder' => true,
]);
$this->assertDatabaseMissing(Organization::class, [ $this->assertDatabaseMissing(Organization::class, [
'id' => $organizationOwned->getKey(), 'id' => $organizationOwned->getKey(),
]); ]);