From 495f3ae471d033f854375bcd19e24fd154635787 Mon Sep 17 00:00:00 2001 From: ignacionelson Date: Sun, 13 Sep 2026 15:05:40 -0300 Subject: [PATCH] Let each role, and each person, choose where they land after signing in A role now has a start page: the dashboard, files, upload, groups, clients or the activity log (the last two for staff only). Anyone can override their role's choice in their profile. The administrator role takes a start page too, while everything else about it stays locked. A choice is only used if the account can open that page now. Otherwise the next one down is tried, ending at the dashboard, so a permission removed later never lands somebody on a 403. A role cannot be saved with a start page its own permissions block. A link followed before signing in still wins, and a waiting getting-started or what's-new page still goes first. Applies to password, two-factor and provider sign-ins, and to the site root for someone already signed in. StartPageTest opens every page for real, with and without its permission, so the enum cannot drift from the routes. Requested by @Zodiac1978 in #1777. --- .../Auth/AuthenticatedSessionController.php | 8 +- .../Settings/ProfileController.php | 9 + .../Settings/ProfileUpdateRequest.php | 8 + app/Models/User.php | 4 + .../Http/Controllers/RolesController.php | 72 +++- .../Controllers/SocialLoginController.php | 4 +- .../TwoFactorChallengeController.php | 4 +- app/Modules/Identity/Models/Role.php | 1 + app/Modules/Identity/StartPage.php | 110 ++++++ app/Modules/Identity/StartPages.php | 119 +++++++ ...0000_add_start_page_to_roles_and_users.php | 38 +++ resources/js/components/start-page-select.tsx | 70 ++++ resources/js/pages/roles/create.tsx | 17 +- resources/js/pages/roles/edit.tsx | 57 +++- resources/js/pages/settings/profile.tsx | 18 + routes/web.php | 10 +- tests/Feature/Auth/SocialLoginTest.php | 13 + tests/Feature/Identity/StartPageTest.php | 314 ++++++++++++++++++ 18 files changed, 859 insertions(+), 17 deletions(-) create mode 100644 app/Modules/Identity/StartPage.php create mode 100644 app/Modules/Identity/StartPages.php create mode 100644 database/migrations/2026_09_13_100000_add_start_page_to_roles_and_users.php create mode 100644 resources/js/components/start-page-select.tsx create mode 100644 tests/Feature/Identity/StartPageTest.php diff --git a/app/Http/Controllers/Auth/AuthenticatedSessionController.php b/app/Http/Controllers/Auth/AuthenticatedSessionController.php index f86ce468..2824e7b8 100644 --- a/app/Http/Controllers/Auth/AuthenticatedSessionController.php +++ b/app/Http/Controllers/Auth/AuthenticatedSessionController.php @@ -4,6 +4,7 @@ namespace App\Http\Controllers\Auth; use App\Http\Controllers\Controller; use App\Http\Requests\Auth\LoginRequest; +use App\Modules\Identity\StartPages; use App\Modules\Platform\Settings\Setting; use App\Modules\Platform\Settings\Settings; use Illuminate\Http\RedirectResponse; @@ -30,7 +31,7 @@ class AuthenticatedSessionController extends Controller /** * Handle an incoming authentication request. */ - public function store(LoginRequest $request): RedirectResponse + public function store(LoginRequest $request, StartPages $startPages): RedirectResponse { if ($request->authenticate()) { return redirect()->route('two-factor.challenge'); @@ -38,7 +39,10 @@ class AuthenticatedSessionController extends Controller $request->session()->regenerate(); - return redirect()->intended(route('dashboard', absolute: false)); + $user = $request->user(); + assert($user !== null); + + return redirect()->intended($startPages->pathFor($user)); } /** diff --git a/app/Http/Controllers/Settings/ProfileController.php b/app/Http/Controllers/Settings/ProfileController.php index 01a64b3a..8d9bc4e8 100644 --- a/app/Http/Controllers/Settings/ProfileController.php +++ b/app/Http/Controllers/Settings/ProfileController.php @@ -10,6 +10,8 @@ use App\Modules\Clients\ClientFieldContext; use App\Modules\Clients\ClientPortalCustomFields; use App\Modules\Identity\Erasure\ErasureSchedule; use App\Modules\Identity\StaffAccounts; +use App\Modules\Identity\StartPage; +use App\Modules\Identity\StartPages; use App\Modules\Platform\Localization\TimezoneRegistry; use App\Modules\Platform\Settings\Setting; use App\Modules\Platform\Settings\Settings; @@ -26,6 +28,7 @@ class ProfileController extends Controller private readonly ClientPortalCustomFields $customFields, private readonly TimezoneRegistry $timezones, private readonly StaffAccounts $accounts, + private readonly StartPages $startPages, ) {} /** @@ -44,6 +47,12 @@ class ProfileController extends Controller // browser was detected as, not something they ever chose. 'timezone' => $this->timezones->resolve($user), 'timezones' => $this->timezones->options(), + // Stored, not resolved: an empty choice means "follow my role", + // and the form has to be able to say that rather than show the + // role's page as if the person had picked it. + 'start_page' => $user->start_page, + 'start_page_options' => $this->startPages->personalOptions($user), + 'role_start_page' => (string) __(($this->startPages->roleDefault($user) ?? StartPage::Dashboard)->label($user->type)), 'custom_fields' => $user->isClient() ? $this->customFields->rows(ClientFieldContext::AccountEdit, $user) : [], 'custom_field_values' => $user->isClient() ? $this->customFields->values(ClientFieldContext::AccountEdit, $user) : [], ]); diff --git a/app/Http/Requests/Settings/ProfileUpdateRequest.php b/app/Http/Requests/Settings/ProfileUpdateRequest.php index 31e13806..df074c97 100644 --- a/app/Http/Requests/Settings/ProfileUpdateRequest.php +++ b/app/Http/Requests/Settings/ProfileUpdateRequest.php @@ -6,6 +6,7 @@ use App\Models\User; use App\Modules\Clients\ClientFieldContext; use App\Modules\Clients\ClientPortalCustomFields; use App\Modules\Identity\AuthSource; +use App\Modules\Identity\StartPages; use App\Support\Rules; use Illuminate\Contracts\Validation\ValidationRule; use Illuminate\Foundation\Http\FormRequest; @@ -66,6 +67,13 @@ class ProfileUpdateRequest extends FormRequest $user = $this->user(); + // Only the pages this person can open right now. `sometimes` for + // the same reason as timezone; empty clears the choice and follows + // the role again. + $rules['start_page'] = ['sometimes', 'nullable', 'string', Rule::in( + $user === null ? [] : array_column(app(StartPages::class)->personalOptions($user), 'value'), + )]; + // An account whose credentials live in a directory or at an // identity provider holds a local password nobody knows — see // LdapProvisioner, which stores Str::password(64) exactly so it diff --git a/app/Models/User.php b/app/Models/User.php index 5b475699..ebaac6a5 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -29,6 +29,7 @@ use Laravel\Sanctum\HasApiTokens; * @property bool $account_requested * @property string|null $locale * @property string|null $timezone + * @property string|null $start_page a StartPage value; see StartPages * @property int|null $dashboard_columns * @property int $storage_quota_mb * @property Carbon|null $erase_after @@ -55,6 +56,9 @@ class User extends Authenticatable implements HasLocalePreference 'password', 'locale', 'timezone', + // A personal preference, like timezone: the profile form fills it + // from its own validated request. See StartPages. + 'start_page', 'dashboard_columns', 'storage_quota_mb', ]; diff --git a/app/Modules/Identity/Http/Controllers/RolesController.php b/app/Modules/Identity/Http/Controllers/RolesController.php index 395784fb..598a3c34 100644 --- a/app/Modules/Identity/Http/Controllers/RolesController.php +++ b/app/Modules/Identity/Http/Controllers/RolesController.php @@ -13,6 +13,9 @@ use App\Modules\Identity\Permissions\Permission; use App\Modules\Identity\Permissions\PermissionCategory; use App\Modules\Identity\Permissions\PermissionChecker; use App\Modules\Identity\Permissions\SystemRole; +use App\Modules\Identity\StartPage; +use App\Modules\Identity\StartPages; +use App\Modules\Identity\UserType; use Illuminate\Database\Eloquent\Builder; use Illuminate\Http\RedirectResponse; use Illuminate\Http\Request; @@ -31,6 +34,7 @@ class RolesController extends Controller public function __construct( private readonly ActivityLogger $activity, private readonly PermissionChecker $permissions, + private readonly StartPages $startPages, ) {} public function index(Request $request): Response @@ -75,6 +79,9 @@ class RolesController extends Controller { return Inertia::render('roles/create', [ 'catalog' => $this->catalog(), + // A role made here is always a staff role: the Client role is + // built in, and there is no second one. + 'start_page_options' => $this->startPages->roleOptions(UserType::Staff), ]); } @@ -85,9 +92,11 @@ class RolesController extends Controller 'client_scoped' => ['boolean'], 'permissions' => ['array'], 'permissions.*' => [Rule::enum(Permission::class)], + 'start_page' => $this->startPageRules(UserType::Staff), ]); $this->guardGrantablePermissions($request, $validated['permissions'] ?? []); + $this->guardStartPage($validated['start_page'] ?? null, UserType::Staff, $validated['permissions'] ?? []); $clientScoped = $request->boolean('client_scoped'); $this->guardScopeRemoval($request, removesScope: ! $clientScoped); @@ -95,6 +104,7 @@ class RolesController extends Controller $role = Role::query()->create([ 'name' => $validated['name'], 'client_scoped' => $clientScoped, + 'start_page' => $validated['start_page'] ?? null, ]); $this->syncPermissions($role, $validated['permissions'] ?? []); @@ -115,17 +125,35 @@ class RolesController extends Controller 'client_scoped' => $role->client_scoped, 'users_count' => $role->users()->count(), 'permissions' => $role->permissions()->pluck('permission')->all(), + 'start_page' => $role->start_page, ], 'catalog' => $this->catalog(), + 'start_page_options' => $this->startPages->roleOptions(StartPages::typeOf($role)), ]); } public function update(Request $request, Role $role): RedirectResponse { + $type = StartPages::typeOf($role); + + // The one thing about the administrator role that is not + // authority: where its members land. Everything else stays locked, + // and a request carrying anything more is refused rather than + // quietly half-applied. if ($role->is_administrator) { - throw ValidationException::withMessages([ - 'permissions' => __('The administrator role always has every permission and cannot be edited.'), - ]); + if ($request->hasAny(['name', 'client_scoped', 'permissions'])) { + throw ValidationException::withMessages([ + 'permissions' => __('The administrator role always has every permission and cannot be edited.'), + ]); + } + + $validated = $request->validate(['start_page' => $this->startPageRules($type)]); + + $role->update(['start_page' => $validated['start_page'] ?? null]); + + $this->activity->log(Action::RoleUpdated, subject: $role); + + return back()->with('success', __('Role updated.')); } $validated = $request->validate([ @@ -133,8 +161,12 @@ class RolesController extends Controller 'client_scoped' => ['boolean'], 'permissions' => ['array'], 'permissions.*' => [Rule::enum(Permission::class)], + 'start_page' => $this->startPageRules($type), ]); + $this->guardStartPage($validated['start_page'] ?? null, $type, $validated['permissions'] ?? []); + $role->start_page = $validated['start_page'] ?? null; + // Built-in roles have fixed names and a fixed scope flag; only their // permission set is editable. Custom roles can change name + scope. if (! $role->is_system) { @@ -158,6 +190,10 @@ class RolesController extends Controller $this->syncPermissions($role, $newPermissions); + // Built-in roles skip the update() above, so the start page is + // saved here for every role alike. + $role->save(); + $this->activity->log(Action::RoleUpdated, subject: $role, context: [ 'permissions_added' => array_values(array_diff($newPermissions, $oldPermissions)), 'permissions_removed' => array_values(array_diff($oldPermissions, $newPermissions)), @@ -258,6 +294,36 @@ class RolesController extends Controller ]); } + /** + * @return list + */ + private function startPageRules(UserType $type): array + { + return ['nullable', 'string', Rule::in(array_map(fn (StartPage $page): string => $page->value, StartPage::optionsFor($type)))]; + } + + /** + * A role cannot send its members to a page its own permissions keep + * them out of. Checked against the permissions saved in the same + * request, so granting "Manage clients" and choosing Clients as the + * start page is one save, not two. StartPages would fall back to the + * dashboard anyway; this says so at the moment it can be fixed. + * + * @param list $permissions + */ + private function guardStartPage(?string $value, UserType $type, array $permissions): void + { + $required = $value === null ? null : StartPage::tryFrom($value)?->requiredPermission($type); + + if ($required !== null && ! in_array($required->value, $permissions, true)) { + throw ValidationException::withMessages([ + 'start_page' => __('This role cannot open that page. Give it the ":permission" permission, or choose another start page.', [ + 'permission' => __($required->label()), + ]), + ]); + } + } + /** * @param list $permissions */ diff --git a/app/Modules/Identity/Http/Controllers/SocialLoginController.php b/app/Modules/Identity/Http/Controllers/SocialLoginController.php index a30fd227..055083cf 100644 --- a/app/Modules/Identity/Http/Controllers/SocialLoginController.php +++ b/app/Modules/Identity/Http/Controllers/SocialLoginController.php @@ -8,6 +8,7 @@ use App\Http\Controllers\Controller; use App\Modules\Audit\Action; use App\Modules\Audit\ActivityLogger; use App\Modules\Identity\SignIn; +use App\Modules\Identity\StartPages; use App\Modules\Identity\Social\SocialAuthenticator; use App\Modules\Identity\Social\SocialGateway; use App\Modules\Identity\Social\SocialIdentity; @@ -40,6 +41,7 @@ class SocialLoginController extends Controller private readonly SocialAuthenticator $authenticator, private readonly SignIn $signIn, private readonly ActivityLogger $activity, + private readonly StartPages $startPages, ) {} /** @@ -128,7 +130,7 @@ class SocialLoginController extends Controller $request->session()->regenerate(); - return redirect()->intended(route('dashboard', absolute: false)); + return redirect()->intended($this->startPages->pathFor($resolution->user)); } private function begin(Request $request, string $provider, string $intent): Response diff --git a/app/Modules/Identity/Http/Controllers/TwoFactorChallengeController.php b/app/Modules/Identity/Http/Controllers/TwoFactorChallengeController.php index d7c32e0d..6a4047e4 100644 --- a/app/Modules/Identity/Http/Controllers/TwoFactorChallengeController.php +++ b/app/Modules/Identity/Http/Controllers/TwoFactorChallengeController.php @@ -7,6 +7,7 @@ namespace App\Modules\Identity\Http\Controllers; use App\Http\Controllers\Controller; use App\Models\User; use App\Modules\Identity\SignIn; +use App\Modules\Identity\StartPages; use App\Modules\Identity\TwoFactor\TwoFactorService; use Illuminate\Http\RedirectResponse; use Illuminate\Http\Request; @@ -25,6 +26,7 @@ class TwoFactorChallengeController extends Controller { public function __construct( private readonly TwoFactorService $twoFactor, + private readonly StartPages $startPages, ) {} public function create(Request $request): Response|RedirectResponse @@ -77,7 +79,7 @@ class TwoFactorChallengeController extends Controller $request->session()->forget(SignIn::TWO_FACTOR_ID); $request->session()->regenerate(); - return redirect()->intended(route('dashboard', absolute: false)); + return redirect()->intended($this->startPages->pathFor($user)); } private function pendingUser(Request $request): ?User diff --git a/app/Modules/Identity/Models/Role.php b/app/Modules/Identity/Models/Role.php index 776b249b..9f87acb5 100644 --- a/app/Modules/Identity/Models/Role.php +++ b/app/Modules/Identity/Models/Role.php @@ -15,6 +15,7 @@ use RuntimeException; * @property bool $is_system * @property bool $is_administrator * @property bool $client_scoped + * @property string|null $start_page a StartPage value; see StartPages * @property-read int $users_count * @property-read int $permissions_count */ diff --git a/app/Modules/Identity/StartPage.php b/app/Modules/Identity/StartPage.php new file mode 100644 index 00000000..110b1369 --- /dev/null +++ b/app/Modules/Identity/StartPage.php @@ -0,0 +1,110 @@ + + */ + public static function optionsFor(UserType $type): array + { + return array_values(array_filter(self::cases(), fn (self $page): bool => $page->appliesTo($type))); + } + + public function appliesTo(UserType $type): bool + { + return match ($this) { + self::Clients, self::Activity => $type === UserType::Staff, + default => true, + }; + } + + /** + * What an account of this type needs to open the page, or null when + * every account of that type can. + */ + public function requiredPermission(UserType $type): ?Permission + { + $staff = $type === UserType::Staff; + + return match ($this) { + self::Dashboard => null, + self::Files => $staff ? Permission::Upload : null, + self::Upload => Permission::Upload, + self::Groups => $staff ? Permission::ManageGroups : null, + self::Clients => Permission::ManageClients, + self::Activity => Permission::ViewActionsLog, + }; + } + + public function routeName(UserType $type): string + { + $staff = $type === UserType::Staff; + + return match ($this) { + self::Dashboard => 'dashboard', + self::Files => $staff ? 'files.index' : 'my-files.index', + self::Upload => $staff ? 'files.create' : 'my-files.upload.create', + self::Groups => $staff ? 'groups.index' : 'my-groups.index', + self::Clients => 'clients.index', + self::Activity => 'activity.index', + }; + } + + /** + * English, and the translation key: the same words the navigation + * already uses for each page, so they are already translated. + */ + public function label(UserType $type): string + { + $staff = $type === UserType::Staff; + + return match ($this) { + self::Dashboard => 'Dashboard', + self::Files => $staff ? 'Files' : 'My files', + self::Upload => 'Upload files', + self::Groups => $staff ? 'Groups' : 'My groups', + self::Clients => 'Clients', + self::Activity => 'Activity log', + }; + } + + public function isReachableBy(User $user): bool + { + if (! $this->appliesTo($user->type)) { + return false; + } + + $permission = $this->requiredPermission($user->type); + + return $permission === null || $user->can($permission->value); + } +} diff --git a/app/Modules/Identity/StartPages.php b/app/Modules/Identity/StartPages.php new file mode 100644 index 00000000..c4a0d1e8 --- /dev/null +++ b/app/Modules/Identity/StartPages.php @@ -0,0 +1,119 @@ +intended(). + */ + public function pathFor(User $user): string + { + $dashboard = route('dashboard', absolute: false); + + if ($this->installation->isWaitingFor($user) || $this->update->isWaitingFor($user)) { + return $dashboard; + } + + $page = $this->resolve($user); + + return $page === null ? $dashboard : route($page->routeName($user->type), absolute: false); + } + + /** + * The start page in force for this account, or null for the dashboard. + */ + public function resolve(User $user): ?StartPage + { + foreach ([$user->start_page, $user->role?->start_page] as $value) { + $page = is_string($value) ? StartPage::tryFrom($value) : null; + + if ($page !== null && $page->isReachableBy($user)) { + return $page; + } + } + + return null; + } + + /** + * The role's default as it applies to this account: null when the role + * names none, or names one this account cannot open. + */ + public function roleDefault(User $user): ?StartPage + { + $value = $user->role?->start_page; + $page = is_string($value) ? StartPage::tryFrom($value) : null; + + return $page !== null && $page->isReachableBy($user) ? $page : null; + } + + /** + * What a person may pick for themselves: the pages they can open. + * + * @return list + */ + public function personalOptions(User $user): array + { + return array_values(array_map( + fn (StartPage $page): array => ['value' => $page->value, 'label' => (string) __($page->label($user->type))], + array_filter(StartPage::optionsFor($user->type), fn (StartPage $page): bool => $page->isReachableBy($user)), + )); + } + + /** + * What a role may name as its default. Every page its kind of account + * can have; RolesController checks the choice against the permissions + * saved with it. + * + * @return list + */ + public function roleOptions(UserType $type): array + { + return array_map( + fn (StartPage $page): array => [ + 'value' => $page->value, + 'label' => (string) __($page->label($type)), + 'permission' => $page->requiredPermission($type)?->value, + ], + StartPage::optionsFor($type), + ); + } + + /** + * The kind of account a role is for. The Client system role holds + * clients; every other role, built-in or custom, holds staff. + */ + public static function typeOf(Role $role): UserType + { + return $role->name === Permissions\SystemRole::Client->value && $role->is_system + ? UserType::Client + : UserType::Staff; + } +} diff --git a/database/migrations/2026_09_13_100000_add_start_page_to_roles_and_users.php b/database/migrations/2026_09_13_100000_add_start_page_to_roles_and_users.php new file mode 100644 index 00000000..f6475579 --- /dev/null +++ b/database/migrations/2026_09_13_100000_add_start_page_to_roles_and_users.php @@ -0,0 +1,38 @@ +string('start_page', 32)->nullable()->after('client_scoped'); + }); + + Schema::table('users', function (Blueprint $table) { + $table->string('start_page', 32)->nullable()->after('timezone'); + }); + } + + public function down(): void + { + Schema::table('roles', function (Blueprint $table) { + $table->dropColumn('start_page'); + }); + + Schema::table('users', function (Blueprint $table) { + $table->dropColumn('start_page'); + }); + } +}; diff --git a/resources/js/components/start-page-select.tsx b/resources/js/components/start-page-select.tsx new file mode 100644 index 00000000..cd41e56c --- /dev/null +++ b/resources/js/components/start-page-select.tsx @@ -0,0 +1,70 @@ +import InputError from '@/components/input-error'; +import { Label } from '@/components/ui/label'; +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select'; +import { useTranslation } from '@/hooks/use-translation'; + +export interface StartPageOption { + value: string; + /** Already translated by the server. */ + label: string; + /** The permission a role needs for this page — role screens only. */ + permission?: string | null; +} + +// Radix Select cannot hold an empty value, so "no choice of my own" needs +// a stand-in. Converted back to null on the way out. +const INHERIT = '__inherit'; + +/** + * Where somebody lands after signing in. Used on the role screens (the + * default for everyone in the role) and on the profile (a person's own + * choice). See StartPages on the server for how the two combine. + */ +export function StartPageSelect({ + value, + onChange, + options, + error, + description, + inheritLabel, + grantedPermissions, +}: { + value: string | null; + onChange: (value: string | null) => void; + options: StartPageOption[]; + error?: string; + description: string; + /** When set, offers "no choice of my own" under this label; otherwise null shows as the dashboard. */ + inheritLabel?: string; + /** On a role screen: the permissions being saved, so pages the role could not open are disabled. */ + grantedPermissions?: string[]; +}) { + const { t } = useTranslation(); + + const selected = value ?? (inheritLabel ? INHERIT : 'dashboard'); + + return ( +
+ + +

{description}

+ +
+ ); +} diff --git a/resources/js/pages/roles/create.tsx b/resources/js/pages/roles/create.tsx index d40577bd..47b9736f 100644 --- a/resources/js/pages/roles/create.tsx +++ b/resources/js/pages/roles/create.tsx @@ -4,22 +4,25 @@ import { FormEventHandler } from 'react'; import Heading from '@/components/heading'; import { PermissionCatalogCategory, RoleForm } from '@/components/role-form'; +import { StartPageSelect, type StartPageOption } from '@/components/start-page-select'; import { Button } from '@/components/ui/button'; import { useTranslation } from '@/hooks/use-translation'; import AppLayout from '@/layouts/app-layout'; interface RolesCreateProps { catalog: PermissionCatalogCategory[]; + start_page_options: StartPageOption[]; } interface RoleFormData { - [key: string]: string | string[] | boolean; + [key: string]: string | string[] | boolean | null; name: string; client_scoped: boolean; permissions: string[]; + start_page: string | null; } -export default function RolesCreate({ catalog }: RolesCreateProps) { +export default function RolesCreate({ catalog, start_page_options }: RolesCreateProps) { const { t } = useTranslation(); const breadcrumbs: BreadcrumbItem[] = [ @@ -31,6 +34,7 @@ export default function RolesCreate({ catalog }: RolesCreateProps) { name: '', client_scoped: false, permissions: [], + start_page: null, }); const submit: FormEventHandler = (e) => { @@ -59,6 +63,15 @@ export default function RolesCreate({ catalog }: RolesCreateProps) { errors={errors} /> + setData('start_page', value)} + options={start_page_options} + grantedPermissions={data.permissions} + error={errors.start_page} + description={t('Where people with this role land after signing in. Each person can still choose their own in their profile.')} + /> + diff --git a/resources/js/pages/roles/edit.tsx b/resources/js/pages/roles/edit.tsx index fcb2cea5..e6230305 100644 --- a/resources/js/pages/roles/edit.tsx +++ b/resources/js/pages/roles/edit.tsx @@ -8,6 +8,7 @@ import Heading from '@/components/heading'; import InputError from '@/components/input-error'; import { PermissionCatalogCategory, RoleForm } from '@/components/role-form'; import { SavedIndicator } from '@/components/save-button'; +import { StartPageSelect, type StartPageOption } from '@/components/start-page-select'; import { Alert, AlertDescription } from '@/components/ui/alert'; import { Button } from '@/components/ui/button'; import { useTranslation } from '@/hooks/use-translation'; @@ -22,18 +23,21 @@ interface RolesEditProps { client_scoped: boolean; users_count: number; permissions: string[]; + start_page: string | null; }; catalog: PermissionCatalogCategory[]; + start_page_options: StartPageOption[]; } interface RoleFormData { - [key: string]: string | string[] | boolean; + [key: string]: string | string[] | boolean | null; name: string; client_scoped: boolean; permissions: string[]; + start_page: string | null; } -export default function RolesEdit({ role, catalog }: RolesEditProps) { +export default function RolesEdit({ role, catalog, start_page_options }: RolesEditProps) { const { t } = useTranslation(); const displayName = role.is_system ? t(role.name) : role.name; @@ -47,10 +51,17 @@ export default function RolesEdit({ role, catalog }: RolesEditProps) { name: role.name, client_scoped: role.client_scoped, permissions: role.permissions, + start_page: role.start_page, }); + // The administrator role's permissions are fixed, so its form sends the + // start page alone — anything more is refused by the server. + const adminForm = useForm<{ start_page: string | null }>({ start_page: role.start_page }); + const deleteForm = useForm({}); + const startPageDescription = t('Where people with this role land after signing in. Each person can still choose their own in their profile.'); + const submit: FormEventHandler = (e) => { e.preventDefault(); patch(route('roles.update', role.id)); @@ -64,10 +75,35 @@ export default function RolesEdit({ role, catalog }: RolesEditProps) { {role.is_administrator ? ( - - - {t('The administrator role always has every permission and cannot be edited.')} - +
+ + + {t('The administrator role always has every permission and cannot be edited.')} + + +
{ + e.preventDefault(); + adminForm.patch(route('roles.update', role.id)); + }} + className="space-y-6" + > + adminForm.setData('start_page', value)} + options={start_page_options} + error={adminForm.errors.start_page} + description={startPageDescription} + /> + +
+ + +
+ +
) : (
+ setData('start_page', value)} + options={start_page_options} + grantedPermissions={data.permissions} + error={errors.start_page} + description={startPageDescription} + /> +
+ setData('start_page', value)} + options={start_page_options} + inheritLabel={t('Default (:page)', { page: role_start_page })} + error={errors.start_page} + description={t('The page you land on after signing in.')} + /> + route(auth()->check() ? 'dashboard' : 'login'); +// A signed-in visitor goes where signing in would have sent them — their +// own start page, their role's, or the dashboard (see StartPages). +Route::get('/', function (Request $request, StartPages $startPages) { + $user = $request->user(); + + return $user === null ? redirect()->route('login') : redirect($startPages->pathFor($user)); })->name('home'); Route::put('locale', [LocaleController::class, 'update']) diff --git a/tests/Feature/Auth/SocialLoginTest.php b/tests/Feature/Auth/SocialLoginTest.php index e01d2f27..2e29b177 100644 --- a/tests/Feature/Auth/SocialLoginTest.php +++ b/tests/Feature/Auth/SocialLoginTest.php @@ -7,6 +7,8 @@ use App\Modules\Audit\Action; use App\Modules\Audit\ActivityLog; use App\Modules\Groups\Models\Group; use App\Modules\Identity\AuthSource; +use App\Modules\Identity\Models\Role; +use App\Modules\Identity\Permissions\SystemRole; use App\Modules\Identity\Social\SocialAccount; use App\Modules\Identity\Social\SocialGateway; use App\Modules\Identity\Social\SocialIdentity; @@ -486,3 +488,14 @@ test('the takeover refusal explains what to do instead', function () { ->component('auth/login') ->where('flash.error', 'An account already uses this email address, and Google did not confirm that you own it. Sign in with your password and connect Google from your settings instead.')); }); + +test('a provider sign-in lands on the account\'s start page, like a password sign-in', function () { + socialSettings(); + Role::query() + ->where('name', SystemRole::Client->value) + ->update(['start_page' => 'files']); + User::factory()->client()->create(['email' => 'client@example.test']); + fakeProvider(identity(email: 'client@example.test', verified: true)); + + signInWith()->assertRedirect('/my-files'); +}); diff --git a/tests/Feature/Identity/StartPageTest.php b/tests/Feature/Identity/StartPageTest.php new file mode 100644 index 00000000..a07daf64 --- /dev/null +++ b/tests/Feature/Identity/StartPageTest.php @@ -0,0 +1,314 @@ +admin = User::factory()->create(); + + // A waiting greeting sends everyone to the dashboard, and Settings + // survive RefreshDatabase's rollback in the cache — so state both. + app(Settings::class)->set(Setting::GettingStartedPending, false); + app(Settings::class)->set(Setting::UpdateWelcomeTo, ''); +}); + +/** + * @param list $permissions + */ +function staffStartingOn(array $permissions, ?string $startPage = null): User +{ + $role = Role::query()->create(['name' => 'Role '.uniqid(), 'start_page' => $startPage]); + + if ($permissions !== []) { + RolePermission::query()->insert(array_map( + fn (Permission $p): array => ['role_id' => $role->id, 'permission' => $p->value], + $permissions, + )); + } + + return User::factory()->create(['role_id' => $role->id]); +} + +function startPageClientRole(): Role +{ + return Role::query()->where('name', SystemRole::Client->value)->sole(); +} + +/* +|-------------------------------------------------------------------------- +| The vocabulary agrees with the routes +|-------------------------------------------------------------------------- +| +| requiredPermission() is a second statement of what each route's +| middleware asks. If the two drift, somebody is sent to a 403 after +| signing in — so this opens every page for real, with and without the +| permission, instead of trusting the enum. +| +*/ + +test('every staff start page opens for staff holding its permission, and not for staff without it', function () { + foreach (StartPage::optionsFor(UserType::Staff) as $page) { + $required = $page->requiredPermission(UserType::Staff); + $path = route($page->routeName(UserType::Staff), absolute: false); + + $holder = staffStartingOn($required === null ? [] : [$required]); + expect($page->isReachableBy($holder))->toBeTrue(); + $this->actingAs($holder)->get($path)->assertOk(); + + if ($required !== null) { + $without = staffStartingOn([]); + expect($page->isReachableBy($without))->toBeFalse(); + expect($this->actingAs($without)->get($path)->status())->not->toBe(200, "{$page->value} opened without {$required->value}"); + } + } +}); + +test('every client start page opens for a client holding its permission, and not for one without it', function () { + foreach (StartPage::optionsFor(UserType::Client) as $page) { + $required = $page->requiredPermission(UserType::Client); + $path = route($page->routeName(UserType::Client), absolute: false); + + RolePermission::query()->where('role_id', startPageClientRole()->id)->delete(); + if ($required !== null) { + RolePermission::query()->insert(['role_id' => startPageClientRole()->id, 'permission' => $required->value]); + } + forgetRequestState(); + + $holder = User::factory()->client()->create(); + expect($page->isReachableBy($holder))->toBeTrue(); + $this->actingAs($holder)->get($path)->assertOk(); + + if ($required !== null) { + RolePermission::query()->where('role_id', startPageClientRole()->id)->delete(); + forgetRequestState(); + $without = User::factory()->client()->create(); + expect($page->isReachableBy($without))->toBeFalse(); + expect($this->actingAs($without)->get($path)->status())->not->toBe(200, "{$page->value} opened without {$required->value}"); + } + } +}); + +test('a client is never offered a staff-only page', function () { + $values = array_map(fn (StartPage $p) => $p->value, StartPage::optionsFor(UserType::Client)); + + expect($values)->not->toContain('clients')->not->toContain('activity'); +}); + +/* +|-------------------------------------------------------------------------- +| Where a sign-in lands +|-------------------------------------------------------------------------- +*/ + +test('signing in lands on the role\'s start page', function () { + $user = staffStartingOn([Permission::ManageClients], startPage: 'clients'); + + $this->post('/login', ['email' => $user->email, 'password' => 'password']) + ->assertRedirect('/clients'); +}); + +test('a personal choice beats the role\'s', function () { + $user = staffStartingOn([Permission::ManageClients, Permission::ViewActionsLog], startPage: 'clients'); + $user->update(['start_page' => 'activity']); + + $this->post('/login', ['email' => $user->email, 'password' => 'password']) + ->assertRedirect('/activity'); +}); + +test('an explicit personal Dashboard beats a role default', function () { + $user = staffStartingOn([Permission::ManageClients], startPage: 'clients'); + $user->update(['start_page' => 'dashboard']); + + $this->post('/login', ['email' => $user->email, 'password' => 'password']) + ->assertRedirect('/dashboard'); +}); + +test('a choice the account can no longer open falls back to the role, then to the dashboard', function () { + // Saved while they could open it; the permission went away later. + $user = staffStartingOn([Permission::ManageClients], startPage: 'clients'); + $user->update(['start_page' => 'activity']); + + $this->post('/login', ['email' => $user->email, 'password' => 'password']) + ->assertRedirect('/clients'); + + Auth::logout(); + RolePermission::query()->where('role_id', $user->role_id)->delete(); + forgetRequestState(); + + $this->post('/login', ['email' => $user->email, 'password' => 'password']) + ->assertRedirect('/dashboard'); +}); + +test('a value no version offers any more falls back to the dashboard instead of failing', function () { + $user = staffStartingOn([], startPage: 'something-removed'); + + $this->post('/login', ['email' => $user->email, 'password' => 'password']) + ->assertRedirect('/dashboard'); +}); + +test('a page somebody was trying to reach still wins over the start page', function () { + $user = staffStartingOn([Permission::ManageClients, Permission::ViewActionsLog], startPage: 'clients'); + + $this->get('/activity')->assertRedirect(route('login')); + + $this->post('/login', ['email' => $user->email, 'password' => 'password']) + ->assertRedirect('/activity'); +}); + +test('a waiting greeting sends the administrator to the dashboard first', function () { + Role::query()->whereKey($this->admin->role_id)->update(['start_page' => 'clients']); + app(Settings::class)->set(Setting::GettingStartedPending, true); + + $this->post('/login', ['email' => $this->admin->email, 'password' => 'password']) + ->assertRedirect('/dashboard'); +}); + +test('a client lands on their role\'s start page', function () { + startPageClientRole()->update(['start_page' => 'files']); + $client = User::factory()->client()->create(); + + $this->post('/login', ['email' => $client->email, 'password' => 'password']) + ->assertRedirect('/my-files'); +}); + +test('the site root sends a signed-in account to its start page', function () { + $user = staffStartingOn([Permission::ManageClients], startPage: 'clients'); + + $this->actingAs($user)->get('/')->assertRedirect('/clients'); +}); + +test('finishing a two-factor challenge lands on the start page', function () { + $user = staffStartingOn([Permission::ManageClients], startPage: 'clients'); + enableTwoFactor($user); + + Auth::logout(); + $this->flushSession(); + + $this->post('/login', ['email' => $user->email, 'password' => 'password']) + ->assertRedirect(route('two-factor.challenge')); + + $code = app(Google2FA::class)->getCurrentOtp((string) $user->refresh()->two_factor_secret); + + $this->post('/two-factor-challenge', ['code' => $code])->assertRedirect('/clients'); +}); + +/* +|-------------------------------------------------------------------------- +| Role screens +|-------------------------------------------------------------------------- +*/ + +test('a role cannot start on a page its own permissions keep it out of', function () { + $this->actingAs($this->admin)->post('/roles', [ + 'name' => 'No clients', + 'permissions' => [Permission::Upload->value], + 'start_page' => 'clients', + ])->assertSessionHasErrors('start_page'); + + expect(Role::query()->where('name', 'No clients')->exists())->toBeFalse(); +}); + +test('granting the permission and choosing the page is one save', function () { + $this->actingAs($this->admin)->post('/roles', [ + 'name' => 'Client desk', + 'permissions' => [Permission::ManageClients->value], + 'start_page' => 'clients', + ])->assertSessionHasNoErrors(); + + expect(Role::query()->where('name', 'Client desk')->value('start_page'))->toBe('clients'); +}); + +test('a built-in role keeps its name but takes a start page', function () { + $manager = Role::query()->where('name', SystemRole::AccountManager->value)->sole(); + + $this->actingAs($this->admin)->patch("/roles/{$manager->id}", [ + 'name' => $manager->name, + 'permissions' => $manager->permissions()->pluck('permission')->all(), + 'start_page' => 'activity', + ])->assertSessionHasNoErrors(); + + expect($manager->refresh()->start_page)->toBe('activity'); +}); + +test('the Client role cannot be given a staff-only page', function () { + $role = startPageClientRole(); + + $this->actingAs($this->admin)->patch("/roles/{$role->id}", [ + 'name' => $role->name, + 'permissions' => $role->permissions()->pluck('permission')->all(), + 'start_page' => 'activity', + ])->assertSessionHasErrors('start_page'); +}); + +test('the administrator role takes a start page and nothing else', function () { + $adminRole = Role::query()->where('is_administrator', true)->sole(); + + $this->actingAs($this->admin)->patch("/roles/{$adminRole->id}", ['start_page' => 'files']) + ->assertSessionHasNoErrors(); + + expect($adminRole->refresh()->start_page)->toBe('files'); + + $this->actingAs($this->admin)->patch("/roles/{$adminRole->id}", [ + 'start_page' => 'clients', + 'permissions' => [], + ])->assertSessionHasErrors('permissions'); + + expect($adminRole->refresh()->start_page)->toBe('files') + ->and(RolePermission::query()->where('role_id', $adminRole->id)->count())->toBe(0); +}); + +/* +|-------------------------------------------------------------------------- +| Profile +|-------------------------------------------------------------------------- +*/ + +test('a person can choose their own start page, and clear it to follow their role', function () { + $user = staffStartingOn([Permission::ManageClients]); + + $this->actingAs($user)->patch('/settings/profile', [ + 'name' => $user->name, + 'email' => $user->email, + 'start_page' => 'clients', + ])->assertSessionHasNoErrors(); + + expect($user->refresh()->start_page)->toBe('clients'); + + $this->actingAs($user)->patch('/settings/profile', [ + 'name' => $user->name, + 'email' => $user->email, + 'start_page' => '', + ])->assertSessionHasNoErrors(); + + expect($user->refresh()->start_page)->toBeNull(); +}); + +test('a person cannot choose a page they cannot open', function () { + $user = staffStartingOn([]); + + $this->actingAs($user)->patch('/settings/profile', [ + 'name' => $user->name, + 'email' => $user->email, + 'start_page' => 'clients', + ])->assertSessionHasErrors('start_page'); + + $client = User::factory()->client()->create(); + + $this->actingAs($client)->patch('/settings/profile', [ + 'name' => $client->name, + 'email' => $client->email, + 'start_page' => 'activity', + ])->assertSessionHasErrors('start_page'); +});