Merge pull request #2626 from BookStackApp/2525_add_user_slugs

User slugs
This commit is contained in:
Dan Brown 2021-03-10 23:12:25 +00:00 committed by GitHub
commit 615038ac6d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 210 additions and 102 deletions

View File

@ -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();
}

View File

@ -1,7 +1,9 @@
<?php namespace BookStack\Auth;
use BookStack\Api\ApiToken;
use BookStack\Entities\Tools\SlugGenerator;
use BookStack\Interfaces\Loggable;
use BookStack\Interfaces\Sluggable;
use BookStack\Model;
use BookStack\Notifications\ResetPassword;
use BookStack\Uploads\Image;
@ -22,6 +24,7 @@ use Illuminate\Support\Collection;
* Class User
* @property string $id
* @property string $name
* @property string $slug
* @property string $email
* @property string $password
* @property Carbon $created_at
@ -32,7 +35,7 @@ use Illuminate\Support\Collection;
* @property string $system_name
* @property Collection $roles
*/
class User extends Model implements AuthenticatableContract, CanResetPasswordContract, Loggable
class User extends Model implements AuthenticatableContract, CanResetPasswordContract, Loggable, Sluggable
{
use Authenticatable, CanResetPassword, Notifiable;
@ -73,23 +76,21 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon
/**
* Returns the default public user.
* @return User
*/
public static function getDefault()
public static function getDefault(): User
{
if (!is_null(static::$defaultUser)) {
return static::$defaultUser;
}
static::$defaultUser = static::where('system_name', '=', 'public')->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;
}
}

View File

@ -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;
}
/**

View File

@ -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;
}
}

View File

@ -1,6 +1,7 @@
<?php namespace BookStack\Entities\Tools;
use BookStack\Auth\Permissions\PermissionService;
use BookStack\Auth\User;
use BookStack\Entities\EntityProvider;
use BookStack\Entities\Models\Entity;
use Illuminate\Database\Connection;
@ -270,24 +271,20 @@ class SearchRunner
protected function filterCreatedBy(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('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)

View File

@ -1,6 +1,7 @@
<?php namespace BookStack\Entities\Tools;
use BookStack\Entities\Models\Entity;
use BookStack\Entities\Models\BookChild;
use BookStack\Interfaces\Sluggable;
use Illuminate\Support\Str;
class SlugGenerator
@ -10,11 +11,11 @@ class SlugGenerator
* Generate a fresh slug for the given entity.
* The slug will generated so it does not conflict within the same parent item.
*/
public function generate(Entity $entity): string
public function generate(Sluggable $model): string
{
$slug = $this->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;

View File

@ -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)]));

View File

@ -1,9 +1,6 @@
<?php namespace BookStack\Http\Controllers;
use BookStack\Actions\ViewService;
use BookStack\Entities\Models\Book;
use BookStack\Entities\Models\Bookshelf;
use BookStack\Entities\Models\Entity;
use BookStack\Entities\Tools\SearchRunner;
use BookStack\Entities\Tools\ShelfContext;
use BookStack\Entities\Tools\SearchOptions;

View File

@ -5,10 +5,13 @@ use BookStack\Auth\Access\SocialAuthService;
use BookStack\Auth\Access\UserInviteService;
use BookStack\Auth\User;
use BookStack\Auth\UserRepo;
use BookStack\Exceptions\ImageUploadException;
use BookStack\Exceptions\UserUpdateException;
use BookStack\Uploads\ImageRepo;
use Exception;
use Illuminate\Http\Request;
use Illuminate\Support\Str;
use Illuminate\Validation\ValidationException;
class UserController extends Controller
{
@ -61,7 +64,7 @@ class UserController extends Controller
/**
* Store a newly created user in storage.
* @throws UserUpdateException
* @throws \Illuminate\Validation\ValidationException
* @throws ValidationException
*/
public function store(Request $request)
{
@ -90,6 +93,7 @@ class UserController extends Controller
$user->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.
*/

View File

@ -0,0 +1,25 @@
<?php namespace BookStack\Http\Controllers;
use BookStack\Auth\UserRepo;
class UserProfileController extends Controller
{
/**
* Show the user profile page
*/
public function show(UserRepo $repo, string $slug)
{
$user = $repo->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
]);
}
}

View File

@ -0,0 +1,23 @@
<?php namespace BookStack\Interfaces;
use Illuminate\Database\Eloquent\Builder;
/**
* Interface Sluggable
*
* Assigned to models that can have slugs.
* Must have the below properties.
*
* @property int $id
* @property string $name
* @method Builder newQuery
*/
interface Sluggable
{
/**
* Regenerate the slug for this model.
*/
public function refreshSlug(): string;
}

View File

@ -12,9 +12,11 @@
*/
$factory->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

View File

@ -0,0 +1,50 @@
<?php
use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;
use Illuminate\Support\Str;
class AddUserSlug extends Migration
{
/**
* Run the migrations.
*
* @return void
*/
public function up()
{
Schema::table('users', function (Blueprint $table) {
$table->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');
});
}
}

View File

@ -58,10 +58,10 @@
</span>
<ul refs="dropdown@menu" class="dropdown-menu" role="menu">
<li>
<a href="{{ url("/user/{$currentUser->id}") }}">@icon('user'){{ trans('common.view_profile') }}</a>
<a href="{{ $currentUser->getProfileUrl() }}">@icon('user'){{ trans('common.view_profile') }}</a>
</li>
<li>
<a href="{{ url("/settings/users/{$currentUser->id}") }}">@icon('edit'){{ trans('common.edit_profile') }}</a>
<a href="{{ $currentUser->getEditUrl() }}">@icon('edit'){{ trans('common.edit_profile') }}</a>
</li>
<li>
@if(config('auth.method') === 'saml2')

View File

@ -99,7 +99,7 @@ Route::group(['middleware' => 'auth'], function () {
});
// User Profile routes
Route::get('/user/{userId}', 'UserController@showProfilePage');
Route::get('/user/{slug}', 'UserProfileController@show');
// Image routes
Route::get('/images/gallery', 'Images\GalleryImageController@list');
@ -217,12 +217,12 @@ Route::group(['middleware' => 'auth'], function () {
});
// Social auth routes
Route::get('/login/service/{socialDriver}', 'Auth\SocialController@getSocialLogin');
Route::get('/login/service/{socialDriver}/callback', 'Auth\SocialController@socialCallback');
Route::get('/login/service/{socialDriver}', 'Auth\SocialController@login');
Route::get('/login/service/{socialDriver}/callback', 'Auth\SocialController@callback');
Route::group(['middleware' => 'auth'], function () {
Route::get('/login/service/{socialDriver}/detach', 'Auth\SocialController@detachSocialAccount');
Route::get('/login/service/{socialDriver}/detach', 'Auth\SocialController@detach');
});
Route::get('/register/service/{socialDriver}', 'Auth\SocialController@socialRegister');
Route::get('/register/service/{socialDriver}', 'Auth\SocialController@register');
// Login/Logout routes
Route::get('/login', 'Auth\LoginController@getLogin');

View File

@ -9,6 +9,7 @@ use BookStack\Settings\SettingService;
use DB;
use Hash;
use Illuminate\Support\Facades\Notification;
use Illuminate\Support\Str;
use Tests\BrowserKitTest;
class AuthTest extends BrowserKitTest
@ -221,6 +222,7 @@ class AuthTest extends BrowserKitTest
public function test_user_creation()
{
/** @var User $user */
$user = factory(User::class)->make();
$adminRole = Role::getRole('admin');
@ -234,8 +236,11 @@ class AuthTest extends BrowserKitTest
->type($user->password, '#password-confirm')
->press('Save')
->seePageIs('/settings/users')
->seeInDatabase('users', $user->toArray())
->seeInDatabase('users', $user->only(['name', 'email']))
->see($user->name);
$user->refresh();
$this->assertStringStartsWith(Str::slug($user->name), $user->slug);
}
public function test_user_updating()
@ -252,6 +257,9 @@ class AuthTest extends BrowserKitTest
->seePageIs('/settings/users')
->seeInDatabase('users', ['id' => $user->id, 'name' => 'Barry Scott', 'password' => $password])
->notSeeInDatabase('users', ['name' => $user->name]);
$user->refresh();
$this->assertStringStartsWith(Str::slug($user->name), $user->slug);
}
public function test_user_password_update()

View File

@ -122,6 +122,7 @@ class EntitySearchTest extends TestCase
$page = $this->newPage(['name' => 'My new test quaffleachits', 'html' => 'this is about an orange donkey danzorbhsing']);
$this->asEditor();
$editorId = $this->getEditor()->id;
$editorSlug = $this->getEditor()->slug;
// Viewed filter searches
$this->get('/search?term=' . urlencode('danzorbhsing {not_viewed_by_me}'))->assertSee($page->name);
@ -133,16 +134,16 @@ class EntitySearchTest extends TestCase
// User filters
$this->get('/search?term=' . urlencode('danzorbhsing {created_by:me}'))->assertDontSee($page->name);
$this->get('/search?term=' . urlencode('danzorbhsing {updated_by:me}'))->assertDontSee($page->name);
$this->get('/search?term=' . urlencode('danzorbhsing {updated_by:'.$editorId.'}'))->assertDontSee($page->name);
$this->get('/search?term=' . urlencode('danzorbhsing {updated_by: '.$editorSlug.'}'))->assertDontSee($page->name);
$page->created_by = $editorId;
$page->save();
$this->get('/search?term=' . urlencode('danzorbhsing {created_by:me}'))->assertSee($page->name);
$this->get('/search?term=' . urlencode('danzorbhsing {created_by:'.$editorId.'}'))->assertSee($page->name);
$this->get('/search?term=' . urlencode('danzorbhsing {created_by: '.$editorSlug.'}'))->assertSee($page->name);
$this->get('/search?term=' . urlencode('danzorbhsing {updated_by:me}'))->assertDontSee($page->name);
$page->updated_by = $editorId;
$page->save();
$this->get('/search?term=' . urlencode('danzorbhsing {updated_by:me}'))->assertSee($page->name);
$this->get('/search?term=' . urlencode('danzorbhsing {updated_by:'.$editorId.'}'))->assertSee($page->name);
$this->get('/search?term=' . urlencode('danzorbhsing {updated_by: '.$editorSlug.'}'))->assertSee($page->name);
// Content filters
$this->get('/search?term=' . urlencode('{in_name:danzorbhsing}'))->assertDontSee($page->name);

View File

@ -19,7 +19,7 @@ class UserProfileTest extends BrowserKitTest
public function test_profile_page_shows_name()
{
$this->asAdmin()
->visit('/user/' . $this->user->id)
->visit('/user/' . $this->user->slug)
->see($this->user->name);
}
@ -28,7 +28,7 @@ class UserProfileTest extends BrowserKitTest
$content = $this->createEntityChainBelongingToUser($this->user, $this->user);
$this->asAdmin()
->visit('/user/' . $this->user->id)
->visit('/user/' . $this->user->slug)
// Check the recently created page is shown
->see($content['page']->name)
// Check the recently created chapter is shown
@ -41,7 +41,7 @@ class UserProfileTest extends BrowserKitTest
{
$newUser = $this->getNewBlankUser();
$this->asAdmin()->visit('/user/' . $newUser->id)
$this->asAdmin()->visit('/user/' . $newUser->slug)
->see($newUser->name)
->seeInElement('#content-counts', '0 Books')
->seeInElement('#content-counts', '0 Chapters')
@ -49,7 +49,7 @@ class UserProfileTest extends BrowserKitTest
$this->createEntityChainBelongingToUser($newUser, $newUser);
$this->asAdmin()->visit('/user/' . $newUser->id)
$this->asAdmin()->visit('/user/' . $newUser->slug)
->see($newUser->name)
->seeInElement('#content-counts', '1 Book')
->seeInElement('#content-counts', '1 Chapter')
@ -64,7 +64,7 @@ class UserProfileTest extends BrowserKitTest
Activity::addForEntity($entities['book'], ActivityType::BOOK_UPDATE);
Activity::addForEntity($entities['page'], ActivityType::PAGE_CREATE);
$this->asAdmin()->visit('/user/' . $newUser->id)
$this->asAdmin()->visit('/user/' . $newUser->slug)
->seeInElement('#recent-user-activity', 'updated book')
->seeInElement('#recent-user-activity', 'created page')
->seeInElement('#recent-user-activity', $entities['page']->name);
@ -79,7 +79,7 @@ class UserProfileTest extends BrowserKitTest
Activity::addForEntity($entities['page'], ActivityType::PAGE_CREATE);
$this->asAdmin()->visit('/')->clickInElement('#recent-activity', $newUser->name)
->seePageIs('/user/' . $newUser->id)
->seePageIs('/user/' . $newUser->slug)
->see($newUser->name);
}