diff --git a/app/Auth/Access/SocialAuthService.php b/app/Auth/Access/SocialAuthService.php index b0383a938..df8513a84 100644 --- a/app/Auth/Access/SocialAuthService.php +++ b/app/Auth/Access/SocialAuthService.php @@ -221,7 +221,7 @@ class SocialAuthService * Detach a social account from a user. * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector */ - public function detachSocialAccount(string $socialDriver) + public function detachSocialAccount(string $socialDriver): void { user()->socialAccounts()->where('driver', '=', $socialDriver)->delete(); } diff --git a/app/Auth/User.php b/app/Auth/User.php index 9d2210101..9855ab4e7 100644 --- a/app/Auth/User.php +++ b/app/Auth/User.php @@ -1,7 +1,9 @@ first(); + static::$defaultUser = static::query()->where('system_name', '=', 'public')->first(); return static::$defaultUser; } /** * Check if the user is the default public user. - * @return bool */ - public function isDefault() + public function isDefault(): bool { return $this->system_name === 'public'; } @@ -116,12 +117,10 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon /** * Check if the user has a role. - * @param $role - * @return mixed */ - public function hasSystemRole($role) + public function hasSystemRole(string $roleSystemName): bool { - return $this->roles->pluck('system_name')->contains($role); + return $this->roles->pluck('system_name')->contains($roleSystemName); } /** @@ -185,9 +184,8 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon /** * Get the social account associated with this user. - * @return HasMany */ - public function socialAccounts() + public function socialAccounts(): HasMany { return $this->hasMany(SocialAccount::class); } @@ -208,11 +206,9 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon } /** - * Returns the user's avatar, - * @param int $size - * @return string + * Returns a URL to the user's avatar */ - public function getAvatar($size = 50) + public function getAvatar(int $size = 50): string { $default = url('/user_avatar.png'); $imageId = $this->image_id; @@ -230,9 +226,8 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon /** * Get the avatar for the user. - * @return BelongsTo */ - public function avatar() + public function avatar(): BelongsTo { return $this->belongsTo(Image::class, 'image_id'); } @@ -272,15 +267,13 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon */ public function getProfileUrl(): string { - return url('/user/' . $this->id); + return url('/user/' . $this->slug); } /** * Get a shortened version of the user's name. - * @param int $chars - * @return string */ - public function getShortName($chars = 8) + public function getShortName(int $chars = 8): string { if (mb_strlen($this->name) <= $chars) { return $this->name; @@ -311,4 +304,13 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon { return "({$this->id}) {$this->name}"; } + + /** + * @inheritDoc + */ + public function refreshSlug(): string + { + $this->slug = app(SlugGenerator::class)->generate($this); + return $this->slug; + } } diff --git a/app/Auth/UserRepo.php b/app/Auth/UserRepo.php index 29a0ebc14..e437ff1e3 100644 --- a/app/Auth/UserRepo.php +++ b/app/Auth/UserRepo.php @@ -45,6 +45,14 @@ class UserRepo return User::query()->findOrFail($id); } + /** + * Get a user by their slug. + */ + public function getBySlug(string $slug): User + { + return User::query()->where('slug', '=', $slug)->firstOrFail(); + } + /** * Get all the users with their permissions. */ @@ -159,7 +167,13 @@ class UserRepo 'email_confirmed' => $emailConfirmed, 'external_auth_id' => $data['external_auth_id'] ?? '', ]; - return User::query()->forceCreate($details); + + $user = new User(); + $user->forceFill($details); + $user->refreshSlug(); + $user->save(); + + return $user; } /** diff --git a/app/Entities/Models/Entity.php b/app/Entities/Models/Entity.php index c6b2468b0..d4b477304 100644 --- a/app/Entities/Models/Entity.php +++ b/app/Entities/Models/Entity.php @@ -9,6 +9,7 @@ use BookStack\Auth\Permissions\JointPermission; use BookStack\Entities\Tools\SearchIndex; use BookStack\Entities\Tools\SlugGenerator; use BookStack\Facades\Permissions; +use BookStack\Interfaces\Sluggable; use BookStack\Model; use BookStack\Traits\HasCreatorAndUpdater; use BookStack\Traits\HasOwner; @@ -37,7 +38,7 @@ use Illuminate\Database\Eloquent\SoftDeletes; * @method static Builder withLastView() * @method static Builder withViewCount() */ -abstract class Entity extends Model +abstract class Entity extends Model implements Sluggable { use SoftDeletes; use HasCreatorAndUpdater; @@ -289,11 +290,11 @@ abstract class Entity extends Model } /** - * Generate and set a new URL slug for this model. + * @inheritdoc */ public function refreshSlug(): string { - $this->slug = (new SlugGenerator)->generate($this); + $this->slug = app(SlugGenerator::class)->generate($this); return $this->slug; } } diff --git a/app/Entities/Tools/SearchRunner.php b/app/Entities/Tools/SearchRunner.php index acfe8d956..dc2c04e3e 100644 --- a/app/Entities/Tools/SearchRunner.php +++ b/app/Entities/Tools/SearchRunner.php @@ -1,6 +1,7 @@ slug : trim($input); + $user = User::query()->where('slug', '=', $userSlug)->first(['id']); + if ($user) { + $query->where('created_by', '=', $user->id); } - if ($input === 'me') { - $input = user()->id; - } - $query->where('created_by', '=', $input); } protected function filterUpdatedBy(EloquentBuilder $query, Entity $model, $input) { - if (!is_numeric($input) && $input !== 'me') { - return; + $userSlug = $input === 'me' ? user()->slug : trim($input); + $user = User::query()->where('slug', '=', $userSlug)->first(['id']); + if ($user) { + $query->where('updated_by', '=', $user->id); } - if ($input === 'me') { - $input = user()->id; - } - $query->where('updated_by', '=', $input); } protected function filterInName(EloquentBuilder $query, Entity $model, $input) diff --git a/app/Entities/Tools/SlugGenerator.php b/app/Entities/Tools/SlugGenerator.php index 7075bc72c..4501279f2 100644 --- a/app/Entities/Tools/SlugGenerator.php +++ b/app/Entities/Tools/SlugGenerator.php @@ -1,6 +1,7 @@ formatNameAsSlug($entity->name); - while ($this->slugInUse($slug, $entity)) { - $slug .= '-' . substr(md5(rand(1, 500)), 0, 3); + $slug = $this->formatNameAsSlug($model->name); + while ($this->slugInUse($slug, $model)) { + $slug .= '-' . Str::random(3); } return $slug; } @@ -35,16 +36,16 @@ class SlugGenerator * Check if a slug is already in-use for this * type of model within the same parent. */ - protected function slugInUse(string $slug, Entity $entity): bool + protected function slugInUse(string $slug, Sluggable $model): bool { - $query = $entity->newQuery()->where('slug', '=', $slug); + $query = $model->newQuery()->where('slug', '=', $slug); - if ($entity instanceof BookChild) { - $query->where('book_id', '=', $entity->book_id); + if ($model instanceof BookChild) { + $query->where('book_id', '=', $model->book_id); } - if ($entity->id) { - $query->where('id', '!=', $entity->id); + if ($model->id) { + $query->where('id', '!=', $model->id); } return $query->count() > 0; diff --git a/app/Http/Controllers/Auth/SocialController.php b/app/Http/Controllers/Auth/SocialController.php index 0c53c9233..d4cfe4fe3 100644 --- a/app/Http/Controllers/Auth/SocialController.php +++ b/app/Http/Controllers/Auth/SocialController.php @@ -9,9 +9,7 @@ use BookStack\Exceptions\SocialSignInAccountNotUsed; use BookStack\Exceptions\SocialSignInException; use BookStack\Exceptions\UserRegistrationException; use BookStack\Http\Controllers\Controller; -use Illuminate\Http\RedirectResponse; use Illuminate\Http\Request; -use Illuminate\Routing\Redirector; use Illuminate\Support\Str; use Laravel\Socialite\Contracts\User as SocialUser; @@ -31,12 +29,11 @@ class SocialController extends Controller $this->registrationService = $registrationService; } - /** * Redirect to the relevant social site. - * @throws \BookStack\Exceptions\SocialDriverNotConfigured + * @throws SocialDriverNotConfigured */ - public function getSocialLogin(string $socialDriver) + public function login(string $socialDriver) { session()->put('social-callback', 'login'); return $this->socialAuthService->startLogIn($socialDriver); @@ -47,7 +44,7 @@ class SocialController extends Controller * @throws SocialDriverNotConfigured * @throws UserRegistrationException */ - public function socialRegister(string $socialDriver) + public function register(string $socialDriver) { $this->registrationService->ensureRegistrationAllowed(); session()->put('social-callback', 'register'); @@ -60,7 +57,7 @@ class SocialController extends Controller * @throws SocialDriverNotConfigured * @throws UserRegistrationException */ - public function socialCallback(Request $request, string $socialDriver) + public function callback(Request $request, string $socialDriver) { if (!session()->has('social-callback')) { throw new SocialSignInException(trans('errors.social_no_action_defined'), '/login'); @@ -99,7 +96,7 @@ class SocialController extends Controller /** * Detach a social account from a user. */ - public function detachSocialAccount(string $socialDriver) + public function detach(string $socialDriver) { $this->socialAuthService->detachSocialAccount($socialDriver); session()->flash('success', trans('settings.users_social_disconnected', ['socialAccount' => Str::title($socialDriver)])); diff --git a/app/Http/Controllers/SearchController.php b/app/Http/Controllers/SearchController.php index 21ebea378..bb824fd9b 100644 --- a/app/Http/Controllers/SearchController.php +++ b/app/Http/Controllers/SearchController.php @@ -1,9 +1,6 @@ external_auth_id = $request->get('external_auth_id'); } + $user->refreshSlug(); $user->save(); if ($sendInvite) { @@ -132,8 +136,8 @@ class UserController extends Controller /** * Update the specified user in storage. * @throws UserUpdateException - * @throws \BookStack\Exceptions\ImageUploadException - * @throws \Illuminate\Validation\ValidationException + * @throws ImageUploadException + * @throws ValidationException */ public function update(Request $request, int $id) { @@ -157,6 +161,11 @@ class UserController extends Controller $user->email = $request->get('email'); } + // Refresh the slug if the user's name has changed + if ($user->isDirty('name')) { + $user->refreshSlug(); + } + // Role updates if (userCan('users-manage') && $request->filled('roles')) { $roles = $request->get('roles'); @@ -216,7 +225,7 @@ class UserController extends Controller /** * Remove the specified user from storage. - * @throws \Exception + * @throws Exception */ public function destroy(Request $request, int $id) { @@ -243,25 +252,6 @@ class UserController extends Controller return redirect('/settings/users'); } - /** - * Show the user profile page - */ - public function showProfilePage($id) - { - $user = $this->userRepo->getById($id); - - $userActivity = $this->userRepo->getActivity($user); - $recentlyCreated = $this->userRepo->getRecentlyCreated($user, 5); - $assetCounts = $this->userRepo->getAssetCounts($user); - - return view('users.profile', [ - 'user' => $user, - 'activity' => $userActivity, - 'recentlyCreated' => $recentlyCreated, - 'assetCounts' => $assetCounts - ]); - } - /** * Update the user's preferred book-list display setting. */ diff --git a/app/Http/Controllers/UserProfileController.php b/app/Http/Controllers/UserProfileController.php new file mode 100644 index 000000000..95e68cb07 --- /dev/null +++ b/app/Http/Controllers/UserProfileController.php @@ -0,0 +1,25 @@ +getBySlug($slug); + + $userActivity = $repo->getActivity($user); + $recentlyCreated = $repo->getRecentlyCreated($user, 5); + $assetCounts = $repo->getAssetCounts($user); + + return view('users.profile', [ + 'user' => $user, + 'activity' => $userActivity, + 'recentlyCreated' => $recentlyCreated, + 'assetCounts' => $assetCounts + ]); + } +} diff --git a/app/Interfaces/Sluggable.php b/app/Interfaces/Sluggable.php new file mode 100644 index 000000000..6aa94e69c --- /dev/null +++ b/app/Interfaces/Sluggable.php @@ -0,0 +1,23 @@ +define(\BookStack\Auth\User::class, function ($faker) { + $name = $faker->name; return [ - 'name' => $faker->name, + 'name' => $name, 'email' => $faker->email, + 'slug' => \Illuminate\Support\Str::slug($name . '-' . \Illuminate\Support\Str::random(5)), 'password' => Str::random(10), 'remember_token' => Str::random(10), 'email_confirmed' => 1 diff --git a/database/migrations/2021_03_08_215138_add_user_slug.php b/database/migrations/2021_03_08_215138_add_user_slug.php new file mode 100644 index 000000000..906e06b95 --- /dev/null +++ b/database/migrations/2021_03_08_215138_add_user_slug.php @@ -0,0 +1,50 @@ +string('slug', 250); + }); + + $slugMap = []; + DB::table('users')->cursor()->each(function ($user) use (&$slugMap) { + $userSlug = Str::slug($user->name); + while (isset($slugMap[$userSlug])) { + $userSlug = Str::slug($user->name . Str::random(4)); + } + $slugMap[$userSlug] = true; + + DB::table('users') + ->where('id', $user->id) + ->update(['slug' => $userSlug]); + }); + + Schema::table('users', function (Blueprint $table) { + $table->unique('slug'); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::table('users', function (Blueprint $table) { + $table->dropColumn('slug'); + }); + } +} diff --git a/resources/views/common/header.blade.php b/resources/views/common/header.blade.php index 43ac273ef..52f6b8cbb 100644 --- a/resources/views/common/header.blade.php +++ b/resources/views/common/header.blade.php @@ -58,10 +58,10 @@