1
0
mirror of https://github.com/BookStackApp/BookStack.git synced 2025-10-23 18:48:37 +03:00

Merge pull request #5844 from BookStackApp/user_ids

Updated handling of deleted user ID handling in DB
This commit is contained in:
Dan Brown
2025-10-21 13:45:00 +01:00
committed by GitHub
24 changed files with 525 additions and 56 deletions

View File

@@ -4,6 +4,7 @@ namespace BookStack\Access\Mfa;
use BookStack\Users\Models\User; use BookStack\Users\Models\User;
use Carbon\Carbon; use Carbon\Carbon;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Model;
/** /**
@@ -16,6 +17,8 @@ use Illuminate\Database\Eloquent\Model;
*/ */
class MfaValue extends Model class MfaValue extends Model
{ {
use HasFactory;
protected static $unguarded = true; protected static $unguarded = true;
const METHOD_TOTP = 'totp'; const METHOD_TOTP = 'totp';

View File

@@ -5,18 +5,23 @@ namespace BookStack\Access;
use BookStack\Activity\Models\Loggable; use BookStack\Activity\Models\Loggable;
use BookStack\App\Model; use BookStack\App\Model;
use BookStack\Users\Models\User; use BookStack\Users\Models\User;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Relations\BelongsTo;
/** /**
* Class SocialAccount.
*
* @property string $driver * @property string $driver
* @property User $user * @property User $user
*/ */
class SocialAccount extends Model implements Loggable class SocialAccount extends Model implements Loggable
{ {
protected $fillable = ['user_id', 'driver', 'driver_id', 'timestamps']; use HasFactory;
public function user() protected $fillable = ['user_id', 'driver', 'driver_id'];
/**
* @return BelongsTo<User, $this>
*/
public function user(): BelongsTo
{ {
return $this->belongsTo(User::class); return $this->belongsTo(User::class);
} }

View File

@@ -6,6 +6,7 @@ use BookStack\App\Model;
use BookStack\Entities\Models\Entity; use BookStack\Entities\Models\Entity;
use BookStack\Permissions\Models\JointPermission; use BookStack\Permissions\Models\JointPermission;
use BookStack\Users\Models\User; use BookStack\Users\Models\User;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\BelongsTo;
use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Database\Eloquent\Relations\HasMany;
use Illuminate\Database\Eloquent\Relations\MorphTo; use Illuminate\Database\Eloquent\Relations\MorphTo;
@@ -24,6 +25,8 @@ use Illuminate\Support\Str;
*/ */
class Activity extends Model class Activity extends Model
{ {
use HasFactory;
/** /**
* Get the loggable model related to this activity. * Get the loggable model related to this activity.
* Currently only used for entities (previously entity_[id/type] columns). * Currently only used for entities (previously entity_[id/type] columns).

View File

@@ -4,11 +4,14 @@ namespace BookStack\Activity\Models;
use BookStack\App\Model; use BookStack\App\Model;
use BookStack\Permissions\Models\JointPermission; use BookStack\Permissions\Models\JointPermission;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Database\Eloquent\Relations\HasMany;
use Illuminate\Database\Eloquent\Relations\MorphTo; use Illuminate\Database\Eloquent\Relations\MorphTo;
class Favourite extends Model class Favourite extends Model
{ {
use HasFactory;
protected $fillable = ['user_id']; protected $fillable = ['user_id'];
/** /**

View File

@@ -5,6 +5,7 @@ namespace BookStack\Activity\Models;
use BookStack\Activity\WatchLevels; use BookStack\Activity\WatchLevels;
use BookStack\Permissions\Models\JointPermission; use BookStack\Permissions\Models\JointPermission;
use Carbon\Carbon; use Carbon\Carbon;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Database\Eloquent\Relations\HasMany;
use Illuminate\Database\Eloquent\Relations\MorphTo; use Illuminate\Database\Eloquent\Relations\MorphTo;
@@ -20,6 +21,8 @@ use Illuminate\Database\Eloquent\Relations\MorphTo;
*/ */
class Watch extends Model class Watch extends Model
{ {
use HasFactory;
protected $guarded = []; protected $guarded = [];
public function watchable(): MorphTo public function watchable(): MorphTo

View File

@@ -4,6 +4,7 @@ namespace BookStack\Entities\Models;
use BookStack\Activity\Models\Loggable; use BookStack\Activity\Models\Loggable;
use BookStack\Users\Models\User; use BookStack\Users\Models\User;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\BelongsTo;
use Illuminate\Database\Eloquent\Relations\MorphTo; use Illuminate\Database\Eloquent\Relations\MorphTo;
@@ -17,6 +18,8 @@ use Illuminate\Database\Eloquent\Relations\MorphTo;
*/ */
class Deletion extends Model implements Loggable class Deletion extends Model implements Loggable
{ {
use HasFactory;
protected $hidden = []; protected $hidden = [];
/** /**

View File

@@ -6,6 +6,7 @@ use BookStack\Activity\Models\Loggable;
use BookStack\App\Model; use BookStack\App\Model;
use BookStack\Users\Models\User; use BookStack\Users\Models\User;
use Carbon\Carbon; use Carbon\Carbon;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\BelongsTo;
/** /**
@@ -30,6 +31,8 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo;
*/ */
class PageRevision extends Model implements Loggable class PageRevision extends Model implements Loggable
{ {
use HasFactory;
protected $fillable = ['name', 'text', 'summary']; protected $fillable = ['name', 'text', 'summary'];
protected $hidden = ['html', 'markdown', 'text']; protected $hidden = ['html', 'markdown', 'text'];

View File

@@ -31,8 +31,6 @@ use Illuminate\Notifications\Notifiable;
use Illuminate\Support\Collection; use Illuminate\Support\Collection;
/** /**
* Class User.
*
* @property int $id * @property int $id
* @property string $name * @property string $name
* @property string $slug * @property string $slug

View File

@@ -5,8 +5,6 @@ namespace BookStack\Users;
use BookStack\Access\UserInviteException; use BookStack\Access\UserInviteException;
use BookStack\Access\UserInviteService; use BookStack\Access\UserInviteService;
use BookStack\Activity\ActivityType; use BookStack\Activity\ActivityType;
use BookStack\Entities\EntityProvider;
use BookStack\Entities\Models\Entity;
use BookStack\Exceptions\NotifyException; use BookStack\Exceptions\NotifyException;
use BookStack\Exceptions\UserUpdateException; use BookStack\Exceptions\UserUpdateException;
use BookStack\Facades\Activity; use BookStack\Facades\Activity;
@@ -27,7 +25,6 @@ class UserRepo
) { ) {
} }
/** /**
* Get a user by their email address. * Get a user by their email address.
*/ */
@@ -161,15 +158,12 @@ class UserRepo
* *
* @throws Exception * @throws Exception
*/ */
public function destroy(User $user, ?int $newOwnerId = null) public function destroy(User $user, ?int $newOwnerId = null): void
{ {
$this->ensureDeletable($user); $this->ensureDeletable($user);
$user->socialAccounts()->delete(); $this->removeUserDependantRelations($user);
$user->apiTokens()->delete(); $this->nullifyUserNonDependantRelations($user);
$user->favourites()->delete();
$user->mfaValues()->delete();
$user->watches()->delete();
$user->delete(); $user->delete();
// Delete user profile images // Delete user profile images
@@ -178,17 +172,52 @@ class UserRepo
// Delete related activities // Delete related activities
setting()->deleteUserSettings($user->id); setting()->deleteUserSettings($user->id);
// Migrate or nullify ownership
$newOwner = null;
if (!empty($newOwnerId)) { if (!empty($newOwnerId)) {
$newOwner = User::query()->find($newOwnerId); $newOwner = User::query()->find($newOwnerId);
if (!is_null($newOwner)) {
$this->migrateOwnership($user, $newOwner);
}
// TODO - Should be be nullifying ownership instead?
} }
$this->migrateOwnership($user, $newOwner);
Activity::add(ActivityType::USER_DELETE, $user); Activity::add(ActivityType::USER_DELETE, $user);
} }
protected function removeUserDependantRelations(User $user): void
{
$user->apiTokens()->delete();
$user->socialAccounts()->delete();
$user->favourites()->delete();
$user->mfaValues()->delete();
$user->watches()->delete();
$tables = ['email_confirmations', 'user_invites', 'views'];
foreach ($tables as $table) {
DB::table($table)->where('user_id', '=', $user->id)->delete();
}
}
protected function nullifyUserNonDependantRelations(User $user): void
{
$toNullify = [
'attachments' => ['created_by', 'updated_by'],
'comments' => ['created_by', 'updated_by'],
'deletions' => ['deleted_by'],
'entities' => ['created_by', 'updated_by'],
'images' => ['created_by', 'updated_by'],
'imports' => ['created_by'],
'joint_permissions' => ['owner_id'],
'page_revisions' => ['created_by'],
'sessions' => ['user_id'],
];
foreach ($toNullify as $table => $columns) {
foreach ($columns as $column) {
DB::table($table)
->where($column, '=', $user->id)
->update([$column => null]);
}
}
}
/** /**
* @throws NotifyException * @throws NotifyException
*/ */
@@ -206,11 +235,12 @@ class UserRepo
/** /**
* Migrate ownership of items in the system from one user to another. * Migrate ownership of items in the system from one user to another.
*/ */
protected function migrateOwnership(User $fromUser, User $toUser): void protected function migrateOwnership(User $fromUser, User|null $toUser): void
{ {
$newOwnerValue = $toUser ? $toUser->id : null;
DB::table('entities') DB::table('entities')
->where('owned_by', '=', $fromUser->id) ->where('owned_by', '=', $fromUser->id)
->update(['owned_by' => $toUser->id]); ->update(['owned_by' => $newOwnerValue]);
} }
/** /**
@@ -248,7 +278,7 @@ class UserRepo
* *
* @throws UserUpdateException * @throws UserUpdateException
*/ */
protected function setUserRoles(User $user, array $roles) protected function setUserRoles(User $user, array $roles): void
{ {
$roles = array_filter(array_values($roles)); $roles = array_filter(array_values($roles));
@@ -261,7 +291,7 @@ class UserRepo
/** /**
* Check if the given user is the last admin and their new roles no longer * Check if the given user is the last admin and their new roles no longer
* contains the admin role. * contain the admin role.
*/ */
protected function demotingLastAdmin(User $user, array $newRoles): bool protected function demotingLastAdmin(User $user, array $newRoles): bool
{ {

View File

@@ -0,0 +1,28 @@
<?php
namespace Database\Factories\Access\Mfa;
use BookStack\Users\Models\User;
use Illuminate\Database\Eloquent\Factories\Factory;
/**
* @extends \Illuminate\Database\Eloquent\Factories\Factory<\BookStack\Access\Mfa\MfaValue>
*/
class MfaValueFactory extends Factory
{
protected $model = \BookStack\Access\Mfa\MfaValue::class;
/**
* Define the model's default state.
*
* @return array<string, mixed>
*/
public function definition(): array
{
return [
'user_id' => User::factory(),
'method' => 'totp',
'value' => '123456',
];
}
}

View File

@@ -0,0 +1,29 @@
<?php
namespace Database\Factories\Access;
use BookStack\Users\Models\User;
use Illuminate\Database\Eloquent\Factories\Factory;
/**
* @extends \Illuminate\Database\Eloquent\Factories\Factory<\BookStack\Access\SocialAccount>
*/
class SocialAccountFactory extends Factory
{
protected $model = \BookStack\Access\SocialAccount::class;
/**
* Define the model's default state.
*
* @return array<string, mixed>
*/
public function definition(): array
{
return [
'user_id' => User::factory(),
'driver' => 'github',
'driver_id' => '123456',
'avatar' => '',
];
}
}

View File

@@ -0,0 +1,34 @@
<?php
namespace Database\Factories\Activity\Models;
use BookStack\Activity\ActivityType;
use BookStack\Users\Models\User;
use Illuminate\Database\Eloquent\Factories\Factory;
/**
* @extends \Illuminate\Database\Eloquent\Factories\Factory<\BookStack\Activity\Models\Activity>
*/
class ActivityFactory extends Factory
{
protected $model = \BookStack\Activity\Models\Activity::class;
/**
* Define the model's default state.
*
* @return array<string, mixed>
*/
public function definition(): array
{
$activities = ActivityType::all();
$activity = $activities[array_rand($activities)];
return [
'type' => $activity,
'detail' => 'Activity detail for ' . $activity,
'user_id' => User::factory(),
'ip' => $this->faker->ipv4(),
'loggable_id' => null,
'loggable_type' => null,
];
}
}

View File

@@ -0,0 +1,31 @@
<?php
namespace Database\Factories\Activity\Models;
use BookStack\Entities\Models\Book;
use BookStack\Users\Models\User;
use Illuminate\Database\Eloquent\Factories\Factory;
/**
* @extends \Illuminate\Database\Eloquent\Factories\Factory<\BookStack\Activity\Models\Favourite>
*/
class FavouriteFactory extends Factory
{
protected $model = \BookStack\Activity\Models\Favourite::class;
/**
* Define the model's default state.
*
* @return array<string, mixed>
*/
public function definition(): array
{
$book = Book::query()->first();
return [
'user_id' => User::factory(),
'favouritable_id' => $book->id,
'favouritable_type' => 'book',
];
}
}

View File

@@ -0,0 +1,33 @@
<?php
namespace Database\Factories\Activity\Models;
use BookStack\Activity\WatchLevels;
use BookStack\Entities\Models\Book;
use BookStack\Users\Models\User;
use Illuminate\Database\Eloquent\Factories\Factory;
/**
* @extends \Illuminate\Database\Eloquent\Factories\Factory<\BookStack\Activity\Models\Watch>
*/
class WatchFactory extends Factory
{
protected $model = \BookStack\Activity\Models\Watch::class;
/**
* Define the model's default state.
*
* @return array<string, mixed>
*/
public function definition(): array
{
$book = Book::factory()->create();
return [
'user_id' => User::factory(),
'watchable_id' => $book->id,
'watchable_type' => 'book',
'level' => WatchLevels::NEW,
];
}
}

View File

@@ -0,0 +1,29 @@
<?php
namespace Database\Factories\Entities\Models;
use BookStack\Entities\Models\Page;
use BookStack\Users\Models\User;
use Illuminate\Database\Eloquent\Factories\Factory;
/**
* @extends \Illuminate\Database\Eloquent\Factories\Factory<\BookStack\Entities\Models\Deletion>
*/
class DeletionFactory extends Factory
{
protected $model = \BookStack\Entities\Models\Deletion::class;
/**
* Define the model's default state.
*
* @return array<string, mixed>
*/
public function definition(): array
{
return [
'deleted_by' => User::factory(),
'deletable_id' => Page::factory(),
'deletable_type' => 'page',
];
}
}

View File

@@ -17,10 +17,8 @@ class PageFactory extends Factory
/** /**
* Define the model's default state. * Define the model's default state.
*
* @return array
*/ */
public function definition() public function definition(): array
{ {
$html = '<p>' . implode('</p>', $this->faker->paragraphs(5)) . '</p>'; $html = '<p>' . implode('</p>', $this->faker->paragraphs(5)) . '</p>';

View File

@@ -0,0 +1,40 @@
<?php
namespace Database\Factories\Entities\Models;
use BookStack\Entities\Models\Page;
use BookStack\Users\Models\User;
use Illuminate\Database\Eloquent\Factories\Factory;
class PageRevisionFactory extends Factory
{
/**
* The name of the factory's corresponding model.
*
* @var string
*/
protected $model = \BookStack\Entities\Models\PageRevision::class;
/**
* Define the model's default state.
*/
public function definition(): array
{
$html = '<p>' . implode('</p>', $this->faker->paragraphs(5)) . '</p>';
$page = Page::query()->first();
return [
'page_id' => $page->id,
'name' => $this->faker->sentence(),
'html' => $html,
'text' => strip_tags($html),
'created_by' => User::factory(),
'slug' => $page->slug,
'book_slug' => $page->book->slug,
'type' => 'version',
'markdown' => strip_tags($html),
'summary' => $this->faker->sentence(),
'revision_number' => rand(1, 4000),
];
}
}

View File

@@ -62,8 +62,9 @@ return new class extends Migration
}); });
} }
// Convert image zero values to null // Convert image and activity zero values to null
DB::table('images')->where('uploaded_to', '=', 0)->update(['uploaded_to' => null]); DB::table('images')->where('uploaded_to', '=', 0)->update(['uploaded_to' => null]);
DB::table('activities')->where('loggable_id', '=', 0)->update(['loggable_id' => null]);
// Rebuild joint permissions if needed // Rebuild joint permissions if needed
// This was moved here from 2023_01_24_104625_refactor_joint_permissions_storage since the changes // This was moved here from 2023_01_24_104625_refactor_joint_permissions_storage since the changes

View File

@@ -0,0 +1,76 @@
<?php
use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;
return new class extends Migration {
protected static array $toNullify = [
'attachments' => ['created_by', 'updated_by'],
'comments' => ['created_by', 'updated_by'],
'deletions' => ['deleted_by'],
'entities' => ['created_by', 'updated_by', 'owned_by'],
'images' => ['created_by', 'updated_by'],
'imports' => ['created_by'],
'joint_permissions' => ['owner_id'],
'page_revisions' => ['created_by'],
];
protected static array $toClean = [
'api_tokens' => ['user_id'],
'email_confirmations' => ['user_id'],
'favourites' => ['user_id'],
'mfa_values' => ['user_id'],
'role_user' => ['user_id'],
'sessions' => ['user_id'],
'social_accounts' => ['user_id'],
'user_invites' => ['user_id'],
'views' => ['user_id'],
'watches' => ['user_id'],
];
/**
* Run the migrations.
*/
public function up(): void
{
$idSelectQuery = DB::table('users')->select('id');
foreach (self::$toNullify as $tableName => $columns) {
Schema::table($tableName, function (Blueprint $table) use ($columns) {
foreach ($columns as $columnName) {
$table->unsignedInteger($columnName)->nullable()->change();
}
});
foreach ($columns as $columnName) {
DB::table($tableName)->where($columnName, '=', 0)->update([$columnName => null]);
DB::table($tableName)->whereNotIn($columnName, $idSelectQuery)->update([$columnName => null]);
}
}
foreach (self::$toClean as $tableName => $columns) {
foreach ($columns as $columnName) {
DB::table($tableName)->whereNotIn($columnName, $idSelectQuery)->delete();
}
}
}
/**
* Reverse the migrations.
*/
public function down(): void
{
foreach (self::$toNullify as $tableName => $columns) {
foreach ($columns as $columnName) {
DB::table($tableName)->whereNull($columnName)->update([$columnName => 0]);
}
Schema::table($tableName, function (Blueprint $table) use ($columns) {
foreach ($columns as $columnName) {
$table->unsignedInteger($columnName)->nullable(false)->change();
}
});
}
}
};

View File

@@ -88,7 +88,11 @@
@foreach($activities as $activity) @foreach($activities as $activity)
<div class="item-list-row flex-container-row items-center wrap py-xxs"> <div class="item-list-row flex-container-row items-center wrap py-xxs">
<div class="flex-2 px-m py-xxs flex-container-row items-center min-width-m"> <div class="flex-2 px-m py-xxs flex-container-row items-center min-width-m">
@include('settings.parts.table-user', ['user' => $activity->user, 'user_id' => $activity->user_id]) @if($activity->user && $activity->user->created_at <= $activity->created_at)
@include('settings.parts.table-user', ['user' => $activity->user])
@else
[ID: {{ $activity->user_id }}] {{ trans('common.deleted_user') }}
@endif
</div> </div>
<div class="flex-2 px-m py-xxs min-width-m"><strong <div class="flex-2 px-m py-xxs min-width-m"><strong
class="mr-xs hide-over-m">{{ trans('settings.audit_table_event') }} class="mr-xs hide-over-m">{{ trans('settings.audit_table_event') }}

View File

@@ -1,12 +1,7 @@
{{-- {{--
$user - User mode to display, Can be null. $user - User to display.
$user_id - Id of user to show. Must be provided.
--}} --}}
@if($user) <a href="{{ $user->getEditUrl() }}" class="flex-container-row inline gap-s items-center">
<a href="{{ $user->getEditUrl() }}" class="flex-container-row inline gap-s items-center"> <div class="flex-none"><img width="40" height="40" class="avatar block" src="{{ $user->getAvatar(40)}}" alt="{{ $user->name }}"></div>
<div class="flex-none"><img width="40" height="40" class="avatar block" src="{{ $user->getAvatar(40)}}" alt="{{ $user->name }}"></div> <div class="flex">{{ $user->name }}</div>
<div class="flex">{{ $user->name }}</div> </a>
</a>
@else
[ID: {{ $user_id }}] {{ trans('common.deleted_user') }}
@endif

View File

@@ -33,7 +33,14 @@
@endif @endif
</div> </div>
<div class="flex-2 px-m py-xs flex-container-row items-center min-width-m"> <div class="flex-2 px-m py-xs flex-container-row items-center min-width-m">
<div><strong class="hide-over-l">{{ trans('settings.recycle_bin_deleted_by') }}:<br></strong>@include('settings.parts.table-user', ['user' => $deletion->deleter, 'user_id' => $deletion->deleted_by])</div> <div>
<strong class="hide-over-l">{{ trans('settings.recycle_bin_deleted_by') }}:<br></strong>
@if($deletion->deleter)
@include('settings.parts.table-user', ['user' => $deletion->deleter, 'user_id' => $deletion->deleted_by])
@else
{{ trans('common.deleted_user') }}
@endif
</div>
</div> </div>
<div class="flex px-m py-xs min-width-s"><strong class="hide-over-l">{{ trans('settings.recycle_bin_deleted_at') }}:<br></strong>{{ $deletion->created_at }}</div> <div class="flex px-m py-xs min-width-s"><strong class="hide-over-l">{{ trans('settings.recycle_bin_deleted_at') }}:<br></strong>{{ $deletion->created_at }}</div>
<div class="flex px-m py-xs text-m-right min-width-s"> <div class="flex px-m py-xs text-m-right min-width-s">

View File

@@ -83,6 +83,22 @@ class AuditLogTest extends TestCase
$resp->assertSeeText("[ID: {$viewer->id}] Deleted User"); $resp->assertSeeText("[ID: {$viewer->id}] Deleted User");
} }
public function test_deleted_user_shows_if_user_created_date_is_later_than_activity()
{
$viewer = $this->users->viewer();
$this->actingAs($viewer);
$page = $this->entities->page();
$this->activityService->add(ActivityType::PAGE_CREATE, $page);
$viewer->created_at = Carbon::now()->addDay();
$viewer->save();
$this->actingAs($this->users->admin());
$resp = $this->get('settings/audit');
$resp->assertSeeText("[ID: {$viewer->id}] Deleted User");
$resp->assertDontSee($viewer->name);
}
public function test_filters_by_key() public function test_filters_by_key()
{ {
$this->actingAs($this->users->admin()); $this->actingAs($this->users->admin());

View File

@@ -2,9 +2,21 @@
namespace Tests\User; namespace Tests\User;
use BookStack\Access\Mfa\MfaValue;
use BookStack\Access\SocialAccount;
use BookStack\Access\UserInviteException; use BookStack\Access\UserInviteException;
use BookStack\Access\UserInviteService; use BookStack\Access\UserInviteService;
use BookStack\Activity\ActivityType; use BookStack\Activity\ActivityType;
use BookStack\Activity\Models\Activity;
use BookStack\Activity\Models\Comment;
use BookStack\Activity\Models\Favourite;
use BookStack\Activity\Models\View;
use BookStack\Activity\Models\Watch;
use BookStack\Api\ApiToken;
use BookStack\Entities\Models\Deletion;
use BookStack\Entities\Models\PageRevision;
use BookStack\Exports\Import;
use BookStack\Uploads\Attachment;
use BookStack\Uploads\Image; use BookStack\Uploads\Image;
use BookStack\Users\Models\Role; use BookStack\Users\Models\Role;
use BookStack\Users\Models\User; use BookStack\Users\Models\User;
@@ -28,10 +40,10 @@ class UserManagementTest extends TestCase
$this->withHtml($resp)->assertElementContains('form[action="' . url('/settings/users/create') . '"]', 'Save'); $this->withHtml($resp)->assertElementContains('form[action="' . url('/settings/users/create') . '"]', 'Save');
$resp = $this->post('/settings/users/create', [ $resp = $this->post('/settings/users/create', [
'name' => $user->name, 'name' => $user->name,
'email' => $user->email, 'email' => $user->email,
'password' => $user->password, 'password' => $user->password,
'password-confirm' => $user->password, 'password-confirm' => $user->password,
'roles[' . $adminRole->id . ']' => 'true', 'roles[' . $adminRole->id . ']' => 'true',
]); ]);
$resp->assertRedirect('/settings/users'); $resp->assertRedirect('/settings/users');
@@ -77,7 +89,7 @@ class UserManagementTest extends TestCase
$this->get($userProfilePage)->assertSee('Password confirmation required'); $this->get($userProfilePage)->assertSee('Password confirmation required');
$this->put($userProfilePage, [ $this->put($userProfilePage, [
'password' => 'newpassword', 'password' => 'newpassword',
'password-confirm' => 'newpassword', 'password-confirm' => 'newpassword',
])->assertRedirect('/settings/users'); ])->assertRedirect('/settings/users');
@@ -167,7 +179,7 @@ class UserManagementTest extends TestCase
$this->asAdmin()->delete("settings/users/{$owner->id}", ['new_owner_id' => $newOwner->id])->assertRedirect(); $this->asAdmin()->delete("settings/users/{$owner->id}", ['new_owner_id' => $newOwner->id])->assertRedirect();
$this->assertDatabaseHasEntityData('page', [ $this->assertDatabaseHasEntityData('page', [
'id' => $page->id, 'id' => $page->id,
'owned_by' => $newOwner->id, 'owned_by' => $newOwner->id,
]); ]);
} }
@@ -182,6 +194,91 @@ class UserManagementTest extends TestCase
$this->assertSessionHas('success'); $this->assertSessionHas('success');
} }
public function test_delete_with_empty_owner_migration_id_clears_relevant_id_uses()
{
$user = $this->users->editor();
$page = $this->entities->page();
$this->actingAs($user);
// Create relations
$activity = Activity::factory()->create(['user_id' => $user->id]);
$attachment = Attachment::factory()->create(['created_by' => $user->id, 'updated_by' => $user->id]);
$comment = Comment::factory()->create(['created_by' => $user->id, 'updated_by' => $user->id]);
$deletion = Deletion::factory()->create(['deleted_by' => $user->id]);
$page->forceFill(['owned_by' => $user->id, 'created_by' => $user->id, 'updated_by' => $user->id])->save();
$page->rebuildPermissions();
$image = Image::factory()->create(['created_by' => $user->id, 'updated_by' => $user->id]);
$import = Import::factory()->create(['created_by' => $user->id]);
$revision = PageRevision::factory()->create(['created_by' => $user->id]);
$apiToken = ApiToken::factory()->create(['user_id' => $user->id]);
\DB::table('email_confirmations')->insert(['user_id' => $user->id, 'token' => 'abc123']);
$favourite = Favourite::factory()->create(['user_id' => $user->id]);
$mfaValue = MfaValue::factory()->create(['user_id' => $user->id]);
$socialAccount = SocialAccount::factory()->create(['user_id' => $user->id]);
\DB::table('user_invites')->insert(['user_id' => $user->id, 'token' => 'abc123']);
View::incrementFor($page);
$watch = Watch::factory()->create(['user_id' => $user->id]);
$userColumnsByTable = [
'api_tokens' => ['user_id'],
'attachments' => ['created_by', 'updated_by'],
'comments' => ['created_by', 'updated_by'],
'deletions' => ['deleted_by'],
'email_confirmations' => ['user_id'],
'entities' => ['created_by', 'updated_by', 'owned_by'],
'favourites' => ['user_id'],
'images' => ['created_by', 'updated_by'],
'imports' => ['created_by'],
'joint_permissions' => ['owner_id'],
'mfa_values' => ['user_id'],
'page_revisions' => ['created_by'],
'role_user' => ['user_id'],
'social_accounts' => ['user_id'],
'user_invites' => ['user_id'],
'views' => ['user_id'],
'watches' => ['user_id'],
];
// Ensure columns have user id before deletion
foreach ($userColumnsByTable as $table => $columns) {
foreach ($columns as $column) {
$this->assertDatabaseHas($table, [$column => $user->id]);
}
}
$resp = $this->asAdmin()->delete("settings/users/{$user->id}", ['new_owner_id' => '']);
$resp->assertRedirect('/settings/users');
// Ensure columns missing user id after deletion
foreach ($userColumnsByTable as $table => $columns) {
foreach ($columns as $column) {
$this->assertDatabaseMissing($table, [$column => $user->id]);
}
}
// Check models exist where should be retained
$this->assertDatabaseHas('attachments', ['id' => $attachment->id, 'created_by' => null, 'updated_by' => null]);
$this->assertDatabaseHas('comments', ['id' => $comment->id, 'created_by' => null, 'updated_by' => null]);
$this->assertDatabaseHas('deletions', ['id' => $deletion->id, 'deleted_by' => null]);
$this->assertDatabaseHas('entities', ['id' => $page->id, 'created_by' => null, 'updated_by' => null, 'owned_by' => null]);
$this->assertDatabaseHas('images', ['id' => $image->id, 'created_by' => null, 'updated_by' => null]);
$this->assertDatabaseHas('imports', ['id' => $import->id, 'created_by' => null]);
$this->assertDatabaseHas('page_revisions', ['id' => $revision->id, 'created_by' => null]);
// Check models no longer exist where should have been deleted with the user
$this->assertDatabaseMissing('api_tokens', ['id' => $apiToken->id]);
$this->assertDatabaseMissing('email_confirmations', ['token' => 'abc123']);
$this->assertDatabaseMissing('favourites', ['id' => $favourite->id]);
$this->assertDatabaseMissing('mfa_values', ['id' => $mfaValue->id]);
$this->assertDatabaseMissing('social_accounts', ['id' => $socialAccount->id]);
$this->assertDatabaseMissing('user_invites', ['token' => 'abc123']);
$this->assertDatabaseMissing('watches', ['id' => $watch->id]);
// Ensure activity remains using the old ID (Special case for auditing changes)
$this->assertDatabaseHas('activities', ['id' => $activity->id, 'user_id' => $user->id]);
}
public function test_delete_removes_user_preferences() public function test_delete_removes_user_preferences()
{ {
$editor = $this->users->editor(); $editor = $this->users->editor();
@@ -247,9 +344,9 @@ class UserManagementTest extends TestCase
}); });
$this->asAdmin()->post('/settings/users/create', [ $this->asAdmin()->post('/settings/users/create', [
'name' => $user->name, 'name' => $user->name,
'email' => $user->email, 'email' => $user->email,
'send_invite' => 'true', 'send_invite' => 'true',
'roles[' . $adminRole->id . ']' => 'true', 'roles[' . $adminRole->id . ']' => 'true',
]); ]);
@@ -267,9 +364,9 @@ class UserManagementTest extends TestCase
}); });
$this->asAdmin()->post('/settings/users/create', [ $this->asAdmin()->post('/settings/users/create', [
'name' => $user->name, 'name' => $user->name,
'email' => $user->email, 'email' => $user->email,
'send_invite' => 'true', 'send_invite' => 'true',
]); ]);
$this->assertDatabaseMissing('activities', ['type' => 'USER_CREATE']); $this->assertDatabaseMissing('activities', ['type' => 'USER_CREATE']);
@@ -286,9 +383,9 @@ class UserManagementTest extends TestCase
}); });
$resp = $this->asAdmin()->post('/settings/users/create', [ $resp = $this->asAdmin()->post('/settings/users/create', [
'name' => $user->name, 'name' => $user->name,
'email' => $user->email, 'email' => $user->email,
'send_invite' => 'true', 'send_invite' => 'true',
]); ]);
$resp->assertRedirect('/settings/users/create'); $resp->assertRedirect('/settings/users/create');
@@ -314,8 +411,8 @@ class UserManagementTest extends TestCase
// Both on create // Both on create
$resp = $this->post('/settings/users/create', [ $resp = $this->post('/settings/users/create', [
'language' => 'en<GB_and_this_is_longer', 'language' => 'en<GB_and_this_is_longer',
'name' => 'My name', 'name' => 'My name',
'email' => 'jimmy@example.com', 'email' => 'jimmy@example.com',
]); ]);
$resp->assertSessionHasErrors(['language' => 'The language may not be greater than 15 characters.']); $resp->assertSessionHasErrors(['language' => 'The language may not be greater than 15 characters.']);
$resp->assertSessionHasErrors(['language' => 'The language may only contain letters, numbers, dashes and underscores.']); $resp->assertSessionHasErrors(['language' => 'The language may only contain letters, numbers, dashes and underscores.']);