From 623384e48d75d4a96d835c4ad380a01478684c23 Mon Sep 17 00:00:00 2001 From: Rias Date: Thu, 2 Jul 2026 12:42:14 +0200 Subject: [PATCH 01/16] Add oauth provider management --- docs/oauth.md | 2 + resources/js/pages/auth/Login.vue | 15 +- resources/js/pages/users/SignInProviders.vue | 116 ++++++ resources/templates/_special/login.twig | 5 +- routes/cp.php | 4 + src/Auth/OAuth/Actions/ButtonRenderer.php | 5 +- src/Auth/OAuth/Data/ProviderDefinition.php | 1 + src/Auth/OAuth/OAuth.php | 76 +++- src/Config/GeneralConfig.php | 1 + src/Http/Controllers/Auth/LoginController.php | 24 +- src/Http/Controllers/Auth/OAuthController.php | 94 +++++ src/Http/Controllers/Users/EditUserTrait.php | 17 +- .../Users/SignInProvidersController.php | 84 +++++ .../UserSignInProvidersViewModel.php | 86 +++++ tests/Feature/Auth/OAuth/OAuthManagerTest.php | 48 +-- .../Controllers/Auth/LoginControllerTest.php | 28 +- .../User/SignInProvidersControllerTest.php | 338 ++++++++++++++++++ .../TypeScriptTransformerServiceProvider.php | 2 + 18 files changed, 871 insertions(+), 75 deletions(-) create mode 100644 resources/js/pages/users/SignInProviders.vue create mode 100644 src/Http/Controllers/Users/SignInProvidersController.php create mode 100644 src/Http/ViewModels/UserSignInProvidersViewModel.php create mode 100644 tests/Feature/Http/Controllers/User/SignInProvidersControllerTest.php diff --git a/docs/oauth.md b/docs/oauth.md index 5c326c8b1ff..e3c510e3400 100644 --- a/docs/oauth.md +++ b/docs/oauth.md @@ -54,6 +54,7 @@ Each provider supports the following keys: - `clientSecret` optional for named drivers when already defined in Laravel's `services` config. Required for provider classes. - `name` optional. Human-friendly provider name. - `label` optional. Button label. Defaults to `Sign in with {name}`. +- `icon` optional. Control panel brand icon name for the sign-in providers screen. - `scopes` optional. Array of scopes passed to Socialite. - `with` optional. Array of extra request parameters passed to the provider. - `stateless` optional. Set to `true` to bypass Socialite state validation. @@ -83,6 +84,7 @@ return GeneralConfig::create() 'clientId' => env('GITHUB_CLIENT_ID'), 'clientSecret' => env('GITHUB_CLIENT_SECRET'), 'label' => 'Continue with GitHub', + 'icon' => 'github', 'scopes' => ['read:user', 'user:email'], 'groups' => ['members', 'editors'], 'createsUsers' => true, diff --git a/resources/js/pages/auth/Login.vue b/resources/js/pages/auth/Login.vue index 52a2204f2d2..45a5d635cfb 100644 --- a/resources/js/pages/auth/Login.vue +++ b/resources/js/pages/auth/Login.vue @@ -1,14 +1,21 @@ + + diff --git a/resources/templates/_special/login.twig b/resources/templates/_special/login.twig index 5e7c542a774..6d6608587d0 100644 --- a/resources/templates/_special/login.twig +++ b/resources/templates/_special/login.twig @@ -12,5 +12,8 @@ 'initial-error': initialError ?? null, 'use-email-as-username': generalConfig.useEmailAsUsername }) }}> -
{% hook 'cp.login.alternative-login-methods' %}
+
+ {{ (oauthLoginButtons ?? [])|join|raw }} + {% hook 'cp.login.alternative-login-methods' %} +
diff --git a/routes/cp.php b/routes/cp.php index ff19345e068..be123683142 100644 --- a/routes/cp.php +++ b/routes/cp.php @@ -43,6 +43,7 @@ use CraftCms\Cms\Http\Controllers\Users\PasswordController; use CraftCms\Cms\Http\Controllers\Users\PermissionsController; use CraftCms\Cms\Http\Controllers\Users\PreferencesController; +use CraftCms\Cms\Http\Controllers\Users\SignInProvidersController; use CraftCms\Cms\Http\Controllers\Users\UsersController; use CraftCms\Cms\Http\Controllers\Utilities\DeprecationErrorsController; use CraftCms\Cms\Http\Controllers\Utilities\SystemMessagesController; @@ -146,6 +147,9 @@ Route::get('myaccount/passkeys', [PasskeysController::class, 'index']); Route::get('myaccount/password', [PasswordController::class, 'index']); Route::get('myaccount/preferences', [PreferencesController::class, 'index']); + Route::get('myaccount/sign-in-providers', [SignInProvidersController::class, 'index']); + Route::get('myaccount/sign-in-providers/{provider}/connect', [SignInProvidersController::class, 'connect']); + Route::delete('myaccount/sign-in-providers/{provider}', [SignInProvidersController::class, 'destroy']); Route::middleware([ RequireEdition::class.':'.Edition::Team->value, diff --git a/src/Auth/OAuth/Actions/ButtonRenderer.php b/src/Auth/OAuth/Actions/ButtonRenderer.php index 492874228d7..88bda2000c3 100644 --- a/src/Auth/OAuth/Actions/ButtonRenderer.php +++ b/src/Auth/OAuth/Actions/ButtonRenderer.php @@ -13,8 +13,9 @@ class ButtonRenderer implements RendersOAuthButton { public function handle(ButtonData $button): HtmlString { - return new HtmlString(Html::a(Html::encode($button->label), $button->url, [ - 'class' => ['btn'], + return new HtmlString(Html::a(Html::tag('craft-button', Html::encode($button->label), [ + 'class' => 'w-full', + ]), $button->url, [ 'data' => [ 'provider' => $button->provider->handle, ], diff --git a/src/Auth/OAuth/Data/ProviderDefinition.php b/src/Auth/OAuth/Data/ProviderDefinition.php index e9a5f23f3fc..b3d67cfc542 100644 --- a/src/Auth/OAuth/Data/ProviderDefinition.php +++ b/src/Auth/OAuth/Data/ProviderDefinition.php @@ -47,5 +47,6 @@ public function __construct( public string $groupResolver = UserGroupResolver::class, /** @var class-string */ public string $buttonRenderer = ButtonRenderer::class, + public ?string $icon = null, ) {} } diff --git a/src/Auth/OAuth/OAuth.php b/src/Auth/OAuth/OAuth.php index 1e90bd86cf4..9340b5a27ba 100644 --- a/src/Auth/OAuth/OAuth.php +++ b/src/Auth/OAuth/OAuth.php @@ -51,16 +51,29 @@ #[Scoped] class OAuth { + public const string CONNECT_SESSION_KEY = 'craft.oauth.connect'; + private const string CP_CONTEXT_VALUE = 'cp'; - private const array DEFAULT_TRUSTED_EMAIL_PROVIDERS = [ - 'google', - 'github', - 'apple', - 'bitbucket', - 'slack', - 'slack-openid', - 'twitter-oauth-2', + public const array DRIVER_ICONS = [ + 'apple' => 'apple', + 'azure' => 'microsoft', + 'azure-ad' => 'microsoft', + 'bitbucket' => 'bitbucket', + 'discord' => 'discord', + 'facebook' => 'facebook', + 'github' => 'github', + 'gitlab' => 'gitlab', + 'google' => 'google', + 'linkedin' => 'linkedin', + 'linkedin-openid' => 'linkedin', + 'microsoft' => 'microsoft', + 'slack' => 'slack', + 'slack-openid' => 'slack', + 'twitter' => 'twitter', + 'twitter-oauth-2' => 'x-twitter', + 'x' => 'x-twitter', + 'x-twitter' => 'x-twitter', ]; /** @var Collection|null */ @@ -102,8 +115,11 @@ public function getProviderDefinitions(): Collection [$handle, $config] = $this->normalizeProviderConfigEntry($handle, $config); $definition = $this->normalizeProvider($handle, $config); } catch (ProviderConfigurationException $e) { + if (app()->hasDebugModeEnabled()) { + throw $e; + } + report($e); - Log::error($e->getMessage(), [__METHOD__]); return []; } @@ -247,6 +263,28 @@ public function findLinkedUser(ProviderDefinition $provider, string $identity): return $userId ? User::findOne($userId) : null; } + public function linkedUserId(ProviderDefinition $provider, string $identity): ?int + { + $userId = SsoIdentity::query() + ->where('provider', $provider->handle) + ->where('identityId', $identity) + ->orderByDesc('dateUpdated') + ->value('userId'); + + return is_numeric($userId) ? (int) $userId : null; + } + + public function identityFor(User $user, ProviderDefinition $provider): ?string + { + $identity = SsoIdentity::query() + ->where('userId', $user->id) + ->where('provider', $provider->handle) + ->orderByDesc('dateUpdated') + ->value('identityId'); + + return is_string($identity) ? $identity : null; + } + public function findUserByEmail(string $email): ?User { $email = trim($email); @@ -270,6 +308,14 @@ public function hasIdentity(int $userId): bool return SsoIdentity::query()->where('userId', $userId)->exists(); } + public function unlinkIdentity(User $user, ProviderDefinition $provider): void + { + SsoIdentity::query() + ->where('userId', $user->id) + ->where('provider', $provider->handle) + ->delete(); + } + /** * @return array{0: string, 1: mixed} */ @@ -361,14 +407,13 @@ private function normalizeProvider(string $handle, mixed $config): ?ProviderDefi label: $label, clientId: $clientId !== null ? (string) $clientId : null, clientSecret: $clientSecret !== null ? (string) $clientSecret : null, + icon: ($config['icon'] ?? null) !== null ? (string) $config['icon'] : null, stateless: (bool) ($config['stateless'] ?? false), createsUsers: array_key_exists('createsUsers', $config) ? ($config['createsUsers'] === null ? null : (bool) $config['createsUsers']) : null, activatesUsers: (bool) ($config['activatesUsers'] ?? false), - trustsEmail: array_key_exists('trustsEmail', $config) - ? (bool) $config['trustsEmail'] - : $this->trustsProviderEmail($handle, $driver), + trustsEmail: array_key_exists('trustsEmail', $config) && (bool) $config['trustsEmail'], scopes: array_values($config['scopes'] ?? []), with: $config['with'] ?? [], groupIds: $this->resolveConfiguredGroups($handle, $config['groups'] ?? []), @@ -388,12 +433,6 @@ private function normalizeProvider(string $handle, mixed $config): ?ProviderDefi return $definition; } - private function trustsProviderEmail(string $handle, string $driver): bool - { - return in_array($driver, self::DEFAULT_TRUSTED_EMAIL_PROVIDERS, true) || - in_array($handle, self::DEFAULT_TRUSTED_EMAIL_PROVIDERS, true); - } - private function validateProviderConfig(string $handle, array $config): array { try { @@ -404,6 +443,7 @@ private function validateProviderConfig(string $handle, array $config): array 'clientSecret' => ['sometimes', 'nullable', 'string', 'filled'], 'name' => ['sometimes', 'nullable', 'string', 'filled'], 'label' => ['sometimes', 'nullable', 'string', 'filled'], + 'icon' => ['sometimes', 'nullable', 'string', 'filled'], 'scopes' => ['sometimes', 'array'], 'scopes.*' => ['string', 'filled'], 'with' => ['sometimes', 'array'], diff --git a/src/Config/GeneralConfig.php b/src/Config/GeneralConfig.php index dd2dfeaf8cd..3cff4b3a18c 100644 --- a/src/Config/GeneralConfig.php +++ b/src/Config/GeneralConfig.php @@ -1617,6 +1617,7 @@ class GeneralConfig extends BaseConfig * - `enabled`: Whether the provider should be available. Defaults to `true`. * - `name`: A human-friendly provider name. * - `label`: The rendered button label. + * - `icon`: The control panel brand icon name. * - `scopes`: Additional Socialite scopes. * - `with`: Additional Socialite request parameters. * - `stateless`: Whether the provider should bypass Socialite state checks. diff --git a/src/Http/Controllers/Auth/LoginController.php b/src/Http/Controllers/Auth/LoginController.php index 8319ff2b448..ba2e6f6d0df 100644 --- a/src/Http/Controllers/Auth/LoginController.php +++ b/src/Http/Controllers/Auth/LoginController.php @@ -10,6 +10,7 @@ use CraftCms\Cms\Auth\Events\LoginUserRetrieved; use CraftCms\Cms\Auth\Events\LoginUserRetrieving; use CraftCms\Cms\Auth\Impersonation; +use CraftCms\Cms\Auth\OAuth\OAuth; use CraftCms\Cms\Config\GeneralConfig; use CraftCms\Cms\User\Contracts\CraftUser; use CraftCms\Cms\User\Models\User; @@ -22,6 +23,7 @@ use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Support\Facades\URL; +use Illuminate\Support\HtmlString; use Illuminate\Support\Timebox; use Illuminate\Validation\Rule; use Illuminate\Validation\Rules\Password; @@ -33,7 +35,7 @@ readonly class LoginController extends AuthenticationController { - public function showLogin(Request $request, GeneralConfig $generalConfig, AuthMethods $authMethods): Response|View|\Inertia\Response + public function showLogin(Request $request, GeneralConfig $generalConfig, AuthMethods $authMethods, OAuth $oauth): Response|View|\Inertia\Response { // see if they're already logged in if ($user = $request->craftUser()) { @@ -45,10 +47,22 @@ public function showLogin(Request $request, GeneralConfig $generalConfig, AuthMe return redirect()->action([TwoFactorAuthenticationController::class, 'showForm']); } - return $this->renderViewWithFallback(cpTemplate: 'login', inertiaComponent: 'auth/Login', inertiaProps: [ - 'action' => action([LoginController::class, 'attemptLogin']), - 'username' => $generalConfig->rememberUsernameDuration ? $authMethods->getRememberedUsername() : '', - ]); + $oauthLoginButtons = array_map( + static fn (HtmlString $button): string => (string) $button, + $oauth->getLoginButtons(), + ); + + return $this->renderViewWithFallback( + cpTemplate: 'login', + data: [ + 'oauthLoginButtons' => $oauthLoginButtons, + ], + inertiaComponent: 'auth/Login', + inertiaProps: [ + 'username' => $generalConfig->rememberUsernameDuration ? $authMethods->getRememberedUsername() : '', + 'oauthLoginButtons' => $oauthLoginButtons, + ], + ); } /** diff --git a/src/Http/Controllers/Auth/OAuthController.php b/src/Http/Controllers/Auth/OAuthController.php index 95c90ad1608..b962e695d50 100644 --- a/src/Http/Controllers/Auth/OAuthController.php +++ b/src/Http/Controllers/Auth/OAuthController.php @@ -6,9 +6,11 @@ use CraftCms\Cms\Auth\Enums\AuthError; use CraftCms\Cms\Auth\Enums\CpAuthPath; +use CraftCms\Cms\Auth\OAuth\Data\ProviderDefinition; use CraftCms\Cms\Auth\OAuth\OAuth; use CraftCms\Cms\Element\Elements; use CraftCms\Cms\Element\Exceptions\InvalidElementException; +use CraftCms\Cms\Http\Controllers\Users\SignInProvidersController; use CraftCms\Cms\Support\Flash; use CraftCms\Cms\User\Elements\User; use CraftCms\Cms\User\Users; @@ -48,6 +50,11 @@ public function callback(Request $request, string $provider, OAuth $oauthManager try { $socialiteUser = $oauthManager->buildProvider($definition, $isCpRequest)->user(); $identity = $oauthManager->resolveIdentity($definition, $socialiteUser); + + if ($connectRequest = $this->pullConnectRequest($request)) { + return $this->connectResponse($request, $connectRequest, $definition, $identity, $oauthManager); + } + $user = $oauthManager->resolveUser($definition, $socialiteUser, $identity); $email = $socialiteUser->getEmail(); @@ -120,10 +127,97 @@ public function callback(Request $request, string $provider, OAuth $oauthManager previous: $e, ); } catch (Throwable $e) { + if ($this->pullConnectRequest($request) !== null) { + return $this->connectFailedResponse(t('Authentication failed.'), $e); + } + return $this->failedResponse($isCpRequest, t('Authentication failed.'), previous: $e); } } + /** + * @param array{provider?: string, userId?: int} $connectRequest + */ + private function connectResponse( + Request $request, + array $connectRequest, + ProviderDefinition $definition, + string $identity, + OAuth $oauthManager, + ): Response { + if (($connectRequest['provider'] ?? null) !== $definition->handle) { + return $this->connectFailedResponse(t('Authentication failed.')); + } + + if ($definition->stateless) { + return $this->connectFailedResponse(t('This OAuth provider cannot be connected to an account.')); + } + + $user = $request->craftUser()?->asElement(); + + if (! $user || $user->id !== (int) ($connectRequest['userId'] ?? 0)) { + return $this->connectFailedResponse(t('Authentication failed.')); + } + + $currentIdentity = $oauthManager->identityFor($user, $definition); + + if ($currentIdentity === $identity) { + return $this->connectSuccessResponse(t('{provider} is already connected.', [ + 'provider' => $definition->name, + ])); + } + + if ($currentIdentity !== null) { + return $this->connectFailedResponse(t('Disconnect {provider} before connecting a different account.', [ + 'provider' => $definition->name, + ])); + } + + $linkedUserId = $oauthManager->linkedUserId($definition, $identity); + + if ($linkedUserId !== null && $linkedUserId !== $user->id) { + return $this->connectFailedResponse(t('{provider} is already connected to another user.', [ + 'provider' => $definition->name, + ])); + } + + $oauthManager->linkIdentity($user, $definition, $identity); + + return $this->connectSuccessResponse(t('{provider} connected.', [ + 'provider' => $definition->name, + ])); + } + + /** + * @return array{provider?: string, userId?: int}|null + */ + private function pullConnectRequest(Request $request): ?array + { + $connectRequest = $request->session()->pull(OAuth::CONNECT_SESSION_KEY); + + return is_array($connectRequest) ? $connectRequest : null; + } + + private function connectSuccessResponse(string $message): Response + { + Flash::success($message); + + return to_action([SignInProvidersController::class, 'index'])->with('success', $message); + } + + private function connectFailedResponse( + string $message, + ?Throwable $previous = null, + ): Response { + if ($previous) { + Log::warning($message, [__METHOD__, 'exception' => $previous]); + } + + Flash::error($message); + + return to_action([SignInProvidersController::class, 'index'])->with('error', $message); + } + private function failedResponse( bool $isCpRequest, string $message, diff --git a/src/Http/Controllers/Users/EditUserTrait.php b/src/Http/Controllers/Users/EditUserTrait.php index 760b874dda8..317ede6bc78 100644 --- a/src/Http/Controllers/Users/EditUserTrait.php +++ b/src/Http/Controllers/Users/EditUserTrait.php @@ -5,6 +5,7 @@ namespace CraftCms\Cms\Http\Controllers\Users; use CraftCms\Cms\Auth\Concerns\EnforcesPermissions; +use CraftCms\Cms\Auth\OAuth\OAuth; use CraftCms\Cms\Cp\Data\NavItem; use CraftCms\Cms\Cp\Html\ContentHtml; use CraftCms\Cms\Cp\Html\ElementHtml; @@ -35,6 +36,8 @@ trait EditUserTrait private const string SCREEN_PASSKEYS = 'passkeys'; + private const string SCREEN_SIGN_IN_PROVIDERS = 'sign-in-providers'; + /** * Returns the user being edited. * @@ -91,6 +94,10 @@ protected function asEditUserScreen(User $user, string $screen, ?CpScreenRespons $screens[self::SCREEN_PASSKEYS] = ['label' => t('Passkeys')]; } + if ($this->showSignInProvidersScreen($user)) { + $screens[self::SCREEN_SIGN_IN_PROVIDERS] = ['label' => t('Sign-in Providers')]; + } + abort_if(! isset($screens[$screen]), 403, 'User not authorized to perform this action.'); $pageName = $screens[$screen]['label']; @@ -115,7 +122,10 @@ function (CpScreenResponse $response) use ($user, $pageName) { $currentSubnavItems = &$subnavItems; foreach ($screens as $s => $screenInfo) { - if ($s === self::SCREEN_PASSWORD) { + if ( + $accountSecurityItem === null && + in_array($s, [self::SCREEN_PASSWORD, self::SCREEN_SIGN_IN_PROVIDERS], true) + ) { $sidebarItem = [ 'heading' => t('Account Security'), 'nested' => [], @@ -215,6 +225,11 @@ private function showPermissionsScreen(): bool return $currentUser->can('viewPermissionsScreen', User::class); } + private function showSignInProvidersScreen(User $user): bool + { + return $user->getIsCurrent() && app(OAuth::class)->getProviderDefinitions()->isNotEmpty(); + } + private function editUserScreenUrl(User $user, string $screen): string { $basePath = $user->getIsCurrent() ? 'myaccount' : "users/$user->id"; diff --git a/src/Http/Controllers/Users/SignInProvidersController.php b/src/Http/Controllers/Users/SignInProvidersController.php new file mode 100644 index 00000000000..9187681d6f0 --- /dev/null +++ b/src/Http/Controllers/Users/SignInProvidersController.php @@ -0,0 +1,84 @@ +oauth->getProviderDefinitions()->isEmpty(), 404); + + if (! $currentUser = $request->craftUser()) { + abort(401); + } + + $user = $currentUser->asElement(); + + return $this->asEditUserScreen($user, self::SCREEN_SIGN_IN_PROVIDERS) + ->inertiaPage('users/SignInProviders', new UserSignInProvidersViewModel($user, $this->oauth)); + } + + public function connect(Request $request, string $provider): Response + { + $this->requireConfirmedPassword(t('An elevated session is required to connect a sign-in provider.')); + + if (! $currentUser = $request->craftUser()) { + abort(401); + } + + abort_if(! $definition = $this->oauth->getProviderDefinition($provider), 404); + + if ($definition->stateless) { + $message = t('This OAuth provider cannot be connected to an account.'); + + Flash::error($message); + + return to_action([self::class, 'index'])->with('error', $message); + } + + $request->session()->put(OAuth::CONNECT_SESSION_KEY, [ + 'provider' => $definition->handle, + 'userId' => $currentUser->getCraftUserId(), + ]); + + return $this->oauth->buildProvider($definition, true)->redirect(); + } + + public function destroy(Request $request, string $provider): Response + { + $this->requireConfirmedPassword(t('An elevated session is required to disconnect a sign-in provider.')); + + if (! $currentUser = $request->craftUser()) { + abort(401); + } + + abort_if(! $definition = $this->oauth->getProviderDefinition($provider), 404); + + $this->oauth->unlinkIdentity($currentUser->asElement(), $definition); + + return $this->asSuccess(t('{provider} disconnected.', [ + 'provider' => $definition->name, + ]), redirect: action([self::class, 'index'])); + } +} diff --git a/src/Http/ViewModels/UserSignInProvidersViewModel.php b/src/Http/ViewModels/UserSignInProvidersViewModel.php new file mode 100644 index 00000000000..b6306684192 --- /dev/null +++ b/src/Http/ViewModels/UserSignInProvidersViewModel.php @@ -0,0 +1,86 @@ + + */ + public array $providers; + + public function __construct(User $user, OAuth $oauth) + { + $providers = $oauth->getProviderDefinitions(); + + $connectedProviderCount = $providers + ->filter(fn (ProviderDefinition $provider): bool => $oauth->identityFor($user, $provider) !== null) + ->count(); + + $this->providers = $providers + ->map(function (ProviderDefinition $provider) use ($user, $oauth, $connectedProviderCount): array { + $connected = $oauth->identityFor($user, $provider) !== null; + $lastPrimaryCredential = $connected && ! $user->getHasPassword() && $connectedProviderCount === 1; + + return [ + 'handle' => $provider->handle, + 'name' => $provider->name, + 'icon' => $provider->icon ?? OAuth::DRIVER_ICONS[$provider->driver] ?? null, + 'connected' => $connected, + 'canConnect' => ! $connected && ! $provider->stateless, + 'disabledReason' => $this->disabledReason($provider, $connected), + 'disconnectWarning' => $connected ? $this->disconnectWarning($provider, $lastPrimaryCredential) : null, + ]; + }) + ->values() + ->all(); + } + + public function toArray(): array + { + return [ + 'readOnly' => $this->readOnly, + 'providers' => $this->providers, + ]; + } + + private function disabledReason(ProviderDefinition $provider, bool $connected): ?string + { + if ($connected || ! $provider->stateless) { + return null; + } + + return t('This provider can’t be connected because it is configured to use stateless OAuth.'); + } + + private function disconnectWarning(ProviderDefinition $provider, bool $lastPrimaryCredential): string + { + if ($lastPrimaryCredential) { + return t('Disconnecting {provider} will leave your account without a password or connected sign-in provider. You may be unable to sign in again.', [ + 'provider' => $provider->name, + ]); + } + + return t('Are you sure you want to disconnect {provider}?', [ + 'provider' => $provider->name, + ]); + } +} diff --git a/tests/Feature/Auth/OAuth/OAuthManagerTest.php b/tests/Feature/Auth/OAuth/OAuthManagerTest.php index 0b03d14b4df..8ef7b30dd7f 100644 --- a/tests/Feature/Auth/OAuth/OAuthManagerTest.php +++ b/tests/Feature/Auth/OAuth/OAuthManagerTest.php @@ -89,6 +89,8 @@ function configureOAuthManagerProvider(array $config = []): void }); test('invalid configured groups make the provider unavailable', function () { + config()->set('app.debug', false); + configureOAuthManagerProvider([ 'groups' => ['missing-group'], ]); @@ -103,52 +105,6 @@ function configureOAuthManagerProvider(array $config = []): void expect(app(OAuth::class)->getProviderDefinition('test')->trustsEmail)->toBeFalse(); }); - test('known provider handles trust email fallback by default', function (string $provider) { - app(GeneralConfig::class)->oauthProviders([ - $provider => [ - 'driver' => FakeOAuthProvider::class, - 'clientId' => 'client-id', - 'clientSecret' => 'client-secret', - ], - ]); - - expect(app(OAuth::class)->getProviderDefinition($provider)->trustsEmail)->toBeTrue(); - })->with([ - 'google', - 'github', - 'apple', - 'bitbucket', - 'slack', - 'slack-openid', - 'twitter-oauth-2', - ]); - - test('known provider drivers trust email fallback by default', function () { - Config::set('services.github', [ - 'client_id' => 'services-github-client', - 'client_secret' => 'services-github-secret', - ]); - - app(GeneralConfig::class)->oauthProviders([ - 'custom-github' => 'github', - ]); - - expect(app(OAuth::class)->getProviderDefinition('custom-github')->trustsEmail)->toBeTrue(); - }); - - test('known provider email trust can be disabled', function () { - app(GeneralConfig::class)->oauthProviders([ - 'github' => [ - 'driver' => FakeOAuthProvider::class, - 'clientId' => 'client-id', - 'clientSecret' => 'client-secret', - 'trustsEmail' => false, - ], - ]); - - expect(app(OAuth::class)->getProviderDefinition('github')->trustsEmail)->toBeFalse(); - }); - test('providers can trust email fallback when configured', function () { configureOAuthManagerProvider([ 'trustsEmail' => true, diff --git a/tests/Feature/Http/Controllers/Auth/LoginControllerTest.php b/tests/Feature/Http/Controllers/Auth/LoginControllerTest.php index 74bf06557f9..f786fde5793 100644 --- a/tests/Feature/Http/Controllers/Auth/LoginControllerTest.php +++ b/tests/Feature/Http/Controllers/Auth/LoginControllerTest.php @@ -2,18 +2,24 @@ declare(strict_types=1); +use CraftCms\Cms\Auth\Enums\CpAuthPath; use CraftCms\Cms\Auth\Events\LoginUserRetrieved; use CraftCms\Cms\Auth\Events\LoginUserRetrieving; use CraftCms\Cms\Cms; +use CraftCms\Cms\Config\GeneralConfig; use CraftCms\Cms\Database\Table; +use CraftCms\Cms\Edition; use CraftCms\Cms\Http\Controllers\Auth\LoginController; +use CraftCms\Cms\Tests\TestClasses\OAuth\FakeOAuthProvider; use CraftCms\Cms\User\Elements\User; use CraftCms\Cms\User\Models\User as UserModel; use Illuminate\Auth\Events\Failed; use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Event; +use Inertia\Testing\AssertableInertia; +use function CraftCms\Cms\cp_url; use function Pest\Laravel\actingAs; use function Pest\Laravel\get; use function Pest\Laravel\postJson; @@ -30,6 +36,26 @@ ->assertOk(); }); +test('showLogin includes configured OAuth buttons', function () { + Edition::set(Edition::Pro); + + app(GeneralConfig::class)->oauthProviders([ + 'test' => [ + 'driver' => FakeOAuthProvider::class, + 'clientId' => 'client-id', + 'clientSecret' => 'client-secret', + 'label' => 'Continue with Test OAuth', + ], + ]); + + get(cp_url(CpAuthPath::Login->value)) + ->assertOk() + ->assertInertia(fn (AssertableInertia $page) => $page + ->component('auth/Login') + ->where('oauthLoginButtons.0', fn (string $button) => str_contains($button, 'Continue with Test OAuth') && + str_contains($button, 'oauth/test/redirect'))); +}); + test('showLogin redirects to 2fa form when verify parameter is present', function () { get(action([LoginController::class, 'showLogin'], ['verify' => 1])) ->assertRedirect(); @@ -174,7 +200,7 @@ 'password' => 'wrongpassword', ]); - Event::assertDispatched(fn (Failed $event) => $event->user->id === $user->id + Event::assertDispatched(fn (Failed $event) => $event->user?->getAuthIdentifier() === $user->id && $event->credentials['loginName'] === $user->email); }); diff --git a/tests/Feature/Http/Controllers/User/SignInProvidersControllerTest.php b/tests/Feature/Http/Controllers/User/SignInProvidersControllerTest.php new file mode 100644 index 00000000000..3b9bbc67c0a --- /dev/null +++ b/tests/Feature/Http/Controllers/User/SignInProvidersControllerTest.php @@ -0,0 +1,338 @@ +oauthProviders([]); + + actingAs(UserModel::first()); + Session::passwordConfirmed(); +}); + +function configureSignInProvider(array $config = []): void +{ + app(GeneralConfig::class)->oauthProviders([ + 'test' => array_merge([ + 'driver' => FakeOAuthProvider::class, + 'clientId' => 'client-id', + 'clientSecret' => 'client-secret', + 'name' => 'Test Provider', + 'label' => 'Continue with Test OAuth', + ], $config), + ]); +} + +function signInProviderUser(): User +{ + $user = User::find()->id(Auth::id())->status(null)->addSelect('users.password')->one(); + + if (! $user) { + throw new RuntimeException('Unable to resolve the current test user.'); + } + + return $user; +} + +function signInProviderDefinition(): ProviderDefinition +{ + $provider = app(OAuth::class)->getProviderDefinition('test'); + + if (! $provider) { + throw new RuntimeException('Unable to resolve the test OAuth provider.'); + } + + return $provider; +} + +function signInProviderCallback(array $attributes = []) +{ + FakeOAuthProvider::$fakeUser = FakeOAuthProvider::fakeUser($attributes); + + return get('oauth/test/callback?context=cp'); +} + +function signInProviderHasIdentity(string $identity, ?int $userId = null): bool +{ + return DB::table(Table::SSO_IDENTITIES) + ->where('provider', 'test') + ->where('identityId', $identity) + ->when($userId, fn ($query) => $query->where('userId', $userId)) + ->exists(); +} + +function signInProviderIdentityCount(string $identity): int +{ + return DB::table(Table::SSO_IDENTITIES) + ->where('provider', 'test') + ->where('identityId', $identity) + ->count(); +} + +function signInProviderRemoveCurrentPassword(): User +{ + $userId = Auth::id(); + + if (! $userId) { + throw new RuntimeException('Unable to resolve the current test user ID.'); + } + + DB::table(Table::USERS) + ->where('id', $userId) + ->update(['password' => null]); + + $user = UserModel::query()->find($userId); + + if (! $user) { + throw new RuntimeException('Unable to reload the current test user.'); + } + + actingAs($user); + + return signInProviderUser(); +} + +it('requires login', function () { + configureSignInProvider(); + + Auth::logout(); + + get(cp_url('myaccount/sign-in-providers'))->assertRedirect(); + get(cp_url('myaccount/sign-in-providers/test/connect'))->assertRedirect(); + delete(cp_url('myaccount/sign-in-providers/test'))->assertRedirect(); +}); + +it('returns 404 when no providers are configured', function () { + get(cp_url('myaccount/sign-in-providers'))->assertNotFound(); +}); + +it('shows configured providers', function () { + configureSignInProvider(); + + get(cp_url('myaccount/sign-in-providers')) + ->assertOk() + ->assertInertia(fn (AssertableInertia $page) => $page + ->component('users/SignInProviders') + ->where('providers.0.handle', 'test') + ->where('providers.0.name', 'Test Provider') + ->where('providers.0.icon', null) + ->where('providers.0.connected', false) + ->where('providers.0.canConnect', true) + ->where('providers.0.disabledReason', null) + ->has('subnav')); +}); + +it('shows brand icons for common socialite providers', function () { + app(GeneralConfig::class)->oauthProviders([ + 'company-github' => [ + 'driver' => 'github', + 'clientId' => 'client-id', + 'clientSecret' => 'client-secret', + 'name' => 'GitHub', + ], + ]); + + get(cp_url('myaccount/sign-in-providers')) + ->assertOk() + ->assertInertia(fn (AssertableInertia $page) => $page + ->where('providers.0.handle', 'company-github') + ->where('providers.0.icon', 'github')); +}); + +it('uses configured provider icons before driver icons', function () { + configureSignInProvider([ + 'driver' => 'github', + 'icon' => 'custom-provider-icon', + ]); + + get(cp_url('myaccount/sign-in-providers')) + ->assertOk() + ->assertInertia(fn (AssertableInertia $page) => $page + ->where('providers.0.icon', 'custom-provider-icon')); +}); + +it('shows the account security nav for sso only users', function () { + configureSignInProvider(); + + signInProviderRemoveCurrentPassword(); + + get(cp_url('myaccount/sign-in-providers')) + ->assertOk() + ->assertInertia(fn (AssertableInertia $page) => $page + ->where('subnav', fn ($subnav) => collect($subnav)->contains( + fn (array $item) => ($item['label'] ?? null) === 'Account Security' && + collect($item['subnav'] ?? [])->contains(fn (array $subitem) => ($subitem['label'] ?? null) === 'Sign-in Providers') && + collect($item['subnav'] ?? [])->doesntContain(fn (array $subitem) => ($subitem['label'] ?? null) === 'Password & Verification') + ))); +}); + +it('marks stateless providers as unavailable for connecting', function () { + configureSignInProvider([ + 'stateless' => true, + ]); + + get(cp_url('myaccount/sign-in-providers')) + ->assertOk() + ->assertInertia(fn (AssertableInertia $page) => $page + ->where('providers.0.canConnect', false) + ->where('providers.0.disabledReason', fn (?string $reason) => str_contains($reason ?? '', 'stateless OAuth'))); +}); + +it('requires an elevated session to connect a provider', function () { + configureSignInProvider(); + Session::forget('auth.password_confirmed_at'); + + get(cp_url('myaccount/sign-in-providers/test/connect'))->assertForbidden(); + + expect(session()->has(OAuth::CONNECT_SESSION_KEY))->toBeFalse(); +}); + +it('does not connect stateless providers', function () { + configureSignInProvider([ + 'stateless' => true, + ]); + + get(cp_url('myaccount/sign-in-providers/test/connect')) + ->assertRedirect(cp_url('myaccount/sign-in-providers')) + ->assertSessionHas('error'); + + expect(session()->has(OAuth::CONNECT_SESSION_KEY))->toBeFalse(); +}); + +it('connects a provider to the current user', function () { + configureSignInProvider(); + $user = signInProviderUser(); + + get(cp_url('myaccount/sign-in-providers/test/connect')) + ->assertRedirect('https://provider.test/oauth/authorize'); + + signInProviderCallback([ + 'id' => 'provider-user-1', + 'email' => 'provider-user-1@example.com', + ]) + ->assertRedirect(cp_url('myaccount/sign-in-providers')) + ->assertSessionHas('success'); + + expect(Auth::id())->toBe($user->id) + ->and(signInProviderHasIdentity('provider-user-1', $user->id))->toBeTrue(); +}); + +it('does not duplicate an already connected provider identity', function () { + configureSignInProvider(); + $user = signInProviderUser(); + app(OAuth::class)->linkIdentity($user, signInProviderDefinition(), 'provider-user-1'); + + get(cp_url('myaccount/sign-in-providers/test/connect')); + + signInProviderCallback([ + 'id' => 'provider-user-1', + 'email' => 'provider-user-1@example.com', + ]) + ->assertRedirect(cp_url('myaccount/sign-in-providers')) + ->assertSessionHas('success'); + + expect(signInProviderIdentityCount('provider-user-1'))->toBe(1); +}); + +it('does not connect an identity that belongs to another user', function () { + configureSignInProvider(); + $user = signInProviderUser(); + $otherUser = UserModel::factory()->active()->createElement([ + 'email' => 'other-provider-user@example.com', + 'username' => 'other-provider-user', + ]); + + app(OAuth::class)->linkIdentity($otherUser, signInProviderDefinition(), 'provider-user-1'); + + get(cp_url('myaccount/sign-in-providers/test/connect')); + + signInProviderCallback([ + 'id' => 'provider-user-1', + 'email' => 'provider-user-1@example.com', + ]) + ->assertRedirect(cp_url('myaccount/sign-in-providers')) + ->assertSessionHas('error'); + + expect(signInProviderHasIdentity('provider-user-1', $otherUser->id))->toBeTrue() + ->and(signInProviderHasIdentity('provider-user-1', $user->id))->toBeFalse(); +}); + +it('does not replace a different identity for the same provider', function () { + configureSignInProvider(); + $user = signInProviderUser(); + app(OAuth::class)->linkIdentity($user, signInProviderDefinition(), 'provider-user-1'); + + get(cp_url('myaccount/sign-in-providers/test/connect')); + + signInProviderCallback([ + 'id' => 'provider-user-2', + 'email' => 'provider-user-2@example.com', + ]) + ->assertRedirect(cp_url('myaccount/sign-in-providers')) + ->assertSessionHas('error'); + + expect(signInProviderHasIdentity('provider-user-1', $user->id))->toBeTrue() + ->and(signInProviderHasIdentity('provider-user-2', $user->id))->toBeFalse(); +}); + +it('requires an elevated session to disconnect a provider', function () { + configureSignInProvider(); + $user = signInProviderUser(); + app(OAuth::class)->linkIdentity($user, signInProviderDefinition(), 'provider-user-1'); + Session::forget('auth.password_confirmed_at'); + + delete(cp_url('myaccount/sign-in-providers/test'))->assertForbidden(); + + expect(signInProviderHasIdentity('provider-user-1', $user->id))->toBeTrue(); +}); + +it('disconnects only the current user provider identity', function () { + configureSignInProvider(); + $user = signInProviderRemoveCurrentPassword(); + $otherUser = UserModel::factory()->active()->createElement([ + 'email' => 'other-disconnect-user@example.com', + 'username' => 'other-disconnect-user', + ]); + + app(OAuth::class)->linkIdentity($user, signInProviderDefinition(), 'provider-user-1'); + app(OAuth::class)->linkIdentity($otherUser, signInProviderDefinition(), 'provider-user-2'); + + delete(cp_url('myaccount/sign-in-providers/test')) + ->assertRedirect(cp_url('myaccount/sign-in-providers')) + ->assertSessionHas('success'); + + expect(signInProviderHasIdentity('provider-user-1', $user->id))->toBeFalse() + ->and(signInProviderHasIdentity('provider-user-2', $otherUser->id))->toBeTrue(); +}); + +it('warns when disconnecting the last primary sign in method', function () { + configureSignInProvider(); + $user = signInProviderRemoveCurrentPassword(); + + app(OAuth::class)->linkIdentity($user, signInProviderDefinition(), 'provider-user-1'); + + get(cp_url('myaccount/sign-in-providers')) + ->assertOk() + ->assertInertia(fn (AssertableInertia $page) => $page + ->where('providers.0.connected', true) + ->where('providers.0.disconnectWarning', fn (?string $warning) => str_contains($warning ?? '', 'without a password or connected sign-in provider'))); +}); diff --git a/workbench/app/Providers/TypeScriptTransformerServiceProvider.php b/workbench/app/Providers/TypeScriptTransformerServiceProvider.php index d03fef8f1f5..99e793f91b9 100644 --- a/workbench/app/Providers/TypeScriptTransformerServiceProvider.php +++ b/workbench/app/Providers/TypeScriptTransformerServiceProvider.php @@ -10,6 +10,7 @@ use CraftCms\Cms\Gql\Data\GqlSchema; use CraftCms\Cms\Gql\Data\GqlToken; use CraftCms\Cms\Http\ViewModels\UserPermissionsViewModel; +use CraftCms\Cms\Http\ViewModels\UserSignInProvidersViewModel; use CraftCms\Cms\Image\Data\ImageTransform; use CraftCms\Cms\Route\Data\Route; use CraftCms\Cms\Update\Data\Updates; @@ -45,6 +46,7 @@ protected function configure(TypeScriptTransformerConfigFactory $config): void Route::class, Updates::class, UserPermissionsViewModel::class, + UserSignInProvidersViewModel::class, UserSettings::class, ], [ From 2647f0054acc3feeb73394c00e77897a1639878c Mon Sep 17 00:00:00 2001 From: Rias Date: Thu, 2 Jul 2026 13:02:44 +0200 Subject: [PATCH 02/16] Remove brittle assertion --- tests/Feature/Auth/OAuth/DefaultButtonRendererTest.php | 1 - .../Http/Controllers/Auth/AuthenticationControllerTest.php | 1 - 2 files changed, 2 deletions(-) diff --git a/tests/Feature/Auth/OAuth/DefaultButtonRendererTest.php b/tests/Feature/Auth/OAuth/DefaultButtonRendererTest.php index efe4f8a5a7f..1ca5b3f55c2 100644 --- a/tests/Feature/Auth/OAuth/DefaultButtonRendererTest.php +++ b/tests/Feature/Auth/OAuth/DefaultButtonRendererTest.php @@ -26,7 +26,6 @@ expect($button->toHtml()) ->toContain('<Test OAuth>') - ->toContain('class="btn"') ->toContain('data-provider="test"') ->toContain('/admin/oauth/test/redirect'); }); diff --git a/tests/Feature/Http/Controllers/Auth/AuthenticationControllerTest.php b/tests/Feature/Http/Controllers/Auth/AuthenticationControllerTest.php index a1efe3bc16d..80711f121a5 100644 --- a/tests/Feature/Http/Controllers/Auth/AuthenticationControllerTest.php +++ b/tests/Feature/Http/Controllers/Auth/AuthenticationControllerTest.php @@ -19,7 +19,6 @@ get(action([LoginController::class, 'showLogin'])) ->assertOk() ->assertInertia(fn (AssertableInertia $page) => $page - ->has('action') ->has('username') ); }); From 6540d8f853e9bbf6cab08571150399a1cdb78d04 Mon Sep 17 00:00:00 2001 From: Rias Date: Fri, 3 Jul 2026 21:11:00 +0200 Subject: [PATCH 03/16] Remove extra newline in cp.php routes --- routes/cp.php | 1 - 1 file changed, 1 deletion(-) diff --git a/routes/cp.php b/routes/cp.php index e66f07e4eb6..21e223dbf24 100644 --- a/routes/cp.php +++ b/routes/cp.php @@ -152,7 +152,6 @@ Route::get('myaccount/sign-in-providers/{provider}/connect', [SignInProvidersController::class, 'connect']); Route::delete('myaccount/sign-in-providers/{provider}', [SignInProvidersController::class, 'destroy']); - Route::middleware([ RequireEdition::class.':'.Edition::Team->value, ])->group(function () { From 830d004a65348c840f237a5f6123eb60a7601e8a Mon Sep 17 00:00:00 2001 From: Rias Date: Fri, 3 Jul 2026 21:18:34 +0200 Subject: [PATCH 04/16] Catch potential connection error Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/Http/Controllers/Auth/OAuthController.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Http/Controllers/Auth/OAuthController.php b/src/Http/Controllers/Auth/OAuthController.php index b962e695d50..cbf5807fc43 100644 --- a/src/Http/Controllers/Auth/OAuthController.php +++ b/src/Http/Controllers/Auth/OAuthController.php @@ -52,7 +52,11 @@ public function callback(Request $request, string $provider, OAuth $oauthManager $identity = $oauthManager->resolveIdentity($definition, $socialiteUser); if ($connectRequest = $this->pullConnectRequest($request)) { - return $this->connectResponse($request, $connectRequest, $definition, $identity, $oauthManager); + try { + return $this->connectResponse($request, $connectRequest, $definition, $identity, $oauthManager); + } catch (Throwable $e) { + return $this->connectFailedResponse(t('Authentication failed.'), $e); + } } $user = $oauthManager->resolveUser($definition, $socialiteUser, $identity); From 65ce04aff6336e00b0d2398e4e22f930255739f5 Mon Sep 17 00:00:00 2001 From: Rias Date: Fri, 3 Jul 2026 21:19:06 +0200 Subject: [PATCH 05/16] Add an accessible name that includes the provider name (e.g. "Connect GitHub"). Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- resources/js/pages/users/SignInProviders.vue | 1 + 1 file changed, 1 insertion(+) diff --git a/resources/js/pages/users/SignInProviders.vue b/resources/js/pages/users/SignInProviders.vue index bab76bf8312..58d464b0187 100644 --- a/resources/js/pages/users/SignInProviders.vue +++ b/resources/js/pages/users/SignInProviders.vue @@ -92,6 +92,7 @@ v-if="provider.connected" type="button" size="small" + :aria-label="t('Disconnect {provider}', {provider: provider.name})" :loading="processingProvider === provider.handle" @click="disconnectProvider(provider)" > From 84f7c8c4a943a8323e4e85e2411860bd75ff9722 Mon Sep 17 00:00:00 2001 From: Rias Date: Fri, 3 Jul 2026 21:19:36 +0200 Subject: [PATCH 06/16] Add accessible connect label Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- resources/js/pages/users/SignInProviders.vue | 1 + 1 file changed, 1 insertion(+) diff --git a/resources/js/pages/users/SignInProviders.vue b/resources/js/pages/users/SignInProviders.vue index 58d464b0187..5ddff94f0d3 100644 --- a/resources/js/pages/users/SignInProviders.vue +++ b/resources/js/pages/users/SignInProviders.vue @@ -102,6 +102,7 @@ v-else type="button" size="small" + :aria-label="t('Connect {provider}', {provider: provider.name})" :disabled="!provider.canConnect" @click="connectProvider(provider)" > From bfb359c1f9cac648a24823b4007ccdd33eda7692 Mon Sep 17 00:00:00 2001 From: Rias Date: Fri, 3 Jul 2026 21:20:01 +0200 Subject: [PATCH 07/16] Add action for twig fallback Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/Http/Controllers/Auth/LoginController.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Http/Controllers/Auth/LoginController.php b/src/Http/Controllers/Auth/LoginController.php index ba2e6f6d0df..14a00b028f5 100644 --- a/src/Http/Controllers/Auth/LoginController.php +++ b/src/Http/Controllers/Auth/LoginController.php @@ -55,6 +55,7 @@ public function showLogin(Request $request, GeneralConfig $generalConfig, AuthMe return $this->renderViewWithFallback( cpTemplate: 'login', data: [ + 'action' => action([self::class, 'attemptLogin']), 'oauthLoginButtons' => $oauthLoginButtons, ], inertiaComponent: 'auth/Login', From 1a47abb74cd1a5ab5e51bbdd08c88c3aac05812c Mon Sep 17 00:00:00 2001 From: Rias Date: Fri, 3 Jul 2026 22:01:33 +0200 Subject: [PATCH 08/16] Update docblocks --- docs/oauth.md | 2 +- src/Config/GeneralConfig.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/oauth.md b/docs/oauth.md index e3c510e3400..92783491463 100644 --- a/docs/oauth.md +++ b/docs/oauth.md @@ -61,7 +61,7 @@ Each provider supports the following keys: - `groups` optional. Array of user group IDs, UIDs, or handles to assign to newly-created users. - `createsUsers` optional. Defaults to `null`, which inherits Craft's public registration setting. - `activatesUsers` optional. Defaults to `false`. -- `trustsEmail` optional. Defaults to `true` for known trusted providers (`google`, `github`, `apple`, `bitbucket`, `slack`, `slack-openid`, and `twitter-oauth-2`), and `false` otherwise. Set to `true` only when the provider is trusted to verify ownership of the returned email address; this allows first-time matching to existing Craft users by email. +- `trustsEmail` optional. Defaults to `false`. Set to `true` only when the provider is trusted to verify ownership of the returned email address; this allows first-time matching to existing Craft users by email. - `identityResolver` optional. Class implementing `\CraftCms\Cms\Auth\OAuth\Contracts\ResolvesOAuthIdentity`. - `userResolver` optional. Class implementing `\CraftCms\Cms\Auth\OAuth\Contracts\ResolvesOAuthUser`. - `userPopulator` optional. Class implementing `\CraftCms\Cms\Auth\OAuth\Contracts\PopulatesOAuthUser`. diff --git a/src/Config/GeneralConfig.php b/src/Config/GeneralConfig.php index 3cff4b3a18c..51100200c91 100644 --- a/src/Config/GeneralConfig.php +++ b/src/Config/GeneralConfig.php @@ -1624,7 +1624,7 @@ class GeneralConfig extends BaseConfig * - `groups`: User group IDs, UIDs, or handles to assign to new users. * - `createsUsers`: Whether the provider may create new users when no existing account can be matched. Defaults to the public registration setting when `null` or omitted. * - `activatesUsers`: Whether matched or newly-created users should be activated automatically. - * - `trustsEmail`: Whether the provider is trusted to verify email ownership, allowing first-time matches to existing users by email. Defaults to `true` for known trusted providers and `false` otherwise. + * - `trustsEmail`: Whether the provider is trusted to verify email ownership, allowing first-time matches to existing users by email. Defaults to `false`. * - `identityResolver`: A custom identity resolver class implementing `\CraftCms\Cms\Auth\OAuth\Contracts\ResolvesOAuthIdentity`. * - `userResolver`: A custom linked-user resolver class implementing `\CraftCms\Cms\Auth\OAuth\Contracts\ResolvesOAuthUser`. * - `userPopulator`: A custom user populator class implementing `\CraftCms\Cms\Auth\OAuth\Contracts\PopulatesOAuthUser`. From 03007a118bbd0886815b49a323a522ff63648c37 Mon Sep 17 00:00:00 2001 From: Brian Hanson Date: Mon, 6 Jul 2026 16:09:24 -0500 Subject: [PATCH 09/16] Update to use AdminTable Maybe a little worse for now, but at least this way improvements to the table will "automatically" be applied here --- resources/js/pages/users/SignInProviders.vue | 138 +++++++++++-------- 1 file changed, 77 insertions(+), 61 deletions(-) diff --git a/resources/js/pages/users/SignInProviders.vue b/resources/js/pages/users/SignInProviders.vue index 5ddff94f0d3..e16bbe841fd 100644 --- a/resources/js/pages/users/SignInProviders.vue +++ b/resources/js/pages/users/SignInProviders.vue @@ -1,9 +1,13 @@ From cb5debd791e58458f25de13c012184f5badd744c Mon Sep 17 00:00:00 2001 From: Brian Hanson Date: Mon, 6 Jul 2026 16:09:24 -0500 Subject: [PATCH 10/16] Show login errors from OAuth process --- resources/js/pages/auth/Login.vue | 5 + resources/js/pages/users/SignInProviders.vue | 138 +++++++++++-------- 2 files changed, 82 insertions(+), 61 deletions(-) diff --git a/resources/js/pages/auth/Login.vue b/resources/js/pages/auth/Login.vue index 45a5d635cfb..31d01af73f3 100644 --- a/resources/js/pages/auth/Login.vue +++ b/resources/js/pages/auth/Login.vue @@ -18,6 +18,10 @@ const page = usePage<{ username?: string; + flash?: { + success: string | null; + error: string | null; + }; }>(); const {general} = useCraftData(); @@ -29,6 +33,7 @@ show-reset-password show-remember-me :username="page.props.username" + :initial-error="page.props.flash?.error ?? ''" :use-email-as-username="general.useEmailAsUsername ? '' : null" >
- import {ref} from 'vue'; + import {h, ref} from 'vue'; import {router, usePage} from '@inertiajs/vue3'; import {t} from '@craftcms/cp'; import IndexLayout from '@/common/layouts/IndexLayout.vue'; import {connect, destroy} from '@actions/Users/SignInProvidersController'; + import {getCoreRowModel, useVueTable} from '@tanstack/vue-table'; + import {createCraftColumnHelper} from '@/modules/admin-table/helpers/createCraftColumnHelper'; + import AdminTable from '@/modules/admin-table/components/AdminTable.vue'; + import Badge from '@/common/components/Badge.vue'; defineOptions({ inheritAttrs: false, @@ -50,69 +54,81 @@ }); }); } + + const columnHelper = createCraftColumnHelper(); + const table = useVueTable({ + get data() { + return page.props.providers; + }, + get columns() { + return [ + columnHelper.display({ + id: 'name', + header: t('Provider'), + cell: ({row}) => + h('div', {class: 'flex items-center gap-1'}, [ + row.original.icon && + h('craft-icon', { + name: row.original.icon, + family: 'brands', + }), + row.original.name, + ]), + }), + columnHelper.display({ + id: 'status', + header: t('Status'), + + cell: ({row}) => + h( + Badge, + { + variant: row.original.connected ? 'success' : 'default', + }, + () => + row.original.connected ? t('Connected') : t('Not connected') + ), + }), + columnHelper.actions(({row}) => [ + !row.original.connected && + h( + 'craft-button', + { + type: 'button', + size: 'small', + 'aria-label': t('Connect {provider}', { + provider: row.original.name, + }), + disabled: !row.original.canConnect, + onclick: () => connectProvider(row.original), + }, + t('Connect') + ), + row.original.connected && + h( + 'craft-button', + { + type: 'button', + size: 'small', + 'aria-label': t('Disconnect {provider}', { + provider: row.original.name, + }), + + loading: processingProvider.value === row.original.handle, + onclick: () => disconnectProvider(row.original), + }, + t('Disconnect') + ), + ]), + ]; + }, + getCoreRowModel: getCoreRowModel(), + enableSorting: false, + }); From ad6a4d615c459d3b4fea0149d7ca73617b40f57e Mon Sep 17 00:00:00 2001 From: Brian Hanson Date: Mon, 6 Jul 2026 16:56:07 -0500 Subject: [PATCH 11/16] Move icon resolution into normalizing setp --- src/Auth/OAuth/OAuth.php | 3 ++- src/Http/ViewModels/UserSignInProvidersViewModel.php | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Auth/OAuth/OAuth.php b/src/Auth/OAuth/OAuth.php index 9340b5a27ba..c421511d95a 100644 --- a/src/Auth/OAuth/OAuth.php +++ b/src/Auth/OAuth/OAuth.php @@ -398,6 +398,7 @@ private function normalizeProvider(string $handle, mixed $config): ?ProviderDefi $providerClass = $this->resolveProviderClass($handle, $driver); $name = ($config['name'] ?? null) ?: Str::headline($handle); $label = ($config['label'] ?? null) ?: t('Sign in with {name}', ['name' => $name]); + $icon = $config['icon'] ?? self::DRIVER_ICONS[$handle] ?? null; $definition = new ProviderDefinition( handle: $handle, @@ -407,7 +408,7 @@ private function normalizeProvider(string $handle, mixed $config): ?ProviderDefi label: $label, clientId: $clientId !== null ? (string) $clientId : null, clientSecret: $clientSecret !== null ? (string) $clientSecret : null, - icon: ($config['icon'] ?? null) !== null ? (string) $config['icon'] : null, + icon: $icon, stateless: (bool) ($config['stateless'] ?? false), createsUsers: array_key_exists('createsUsers', $config) ? ($config['createsUsers'] === null ? null : (bool) $config['createsUsers']) diff --git a/src/Http/ViewModels/UserSignInProvidersViewModel.php b/src/Http/ViewModels/UserSignInProvidersViewModel.php index b6306684192..6abd27982a6 100644 --- a/src/Http/ViewModels/UserSignInProvidersViewModel.php +++ b/src/Http/ViewModels/UserSignInProvidersViewModel.php @@ -43,7 +43,7 @@ public function __construct(User $user, OAuth $oauth) return [ 'handle' => $provider->handle, 'name' => $provider->name, - 'icon' => $provider->icon ?? OAuth::DRIVER_ICONS[$provider->driver] ?? null, + 'icon' => $provider->icon, 'connected' => $connected, 'canConnect' => ! $connected && ! $provider->stateless, 'disabledReason' => $this->disabledReason($provider, $connected), From d9be9d00ad57e3f93dd29bbcf5e735f3f098f657 Mon Sep 17 00:00:00 2001 From: Brian Hanson Date: Mon, 6 Jul 2026 21:16:08 -0500 Subject: [PATCH 12/16] feat(craft-button): render as a link when given an href Adds href/target/rel/download support so craft-button can render a real inner styled identically to the button, with link-appropriate host semantics (role=presentation, single tab stop, no form submit) and a full-bleed anchor so the whole control is clickable. Disabled+href renders an inert button. Ported/adapted for this branch's button baseline. Co-Authored-By: Claude Opus 4.8 --- .../src/components/button/button.stories.ts | 32 ++++ .../src/components/button/button.styles.ts | 40 +++++ .../src/components/button/button.test.ts | 160 ++++++++++++++++++ .../src/components/button/button.ts | 90 +++++++++- 4 files changed, 318 insertions(+), 4 deletions(-) create mode 100644 packages/craftcms-cp/src/components/button/button.test.ts diff --git a/packages/craftcms-cp/src/components/button/button.stories.ts b/packages/craftcms-cp/src/components/button/button.stories.ts index 045c298eabe..e102011313c 100644 --- a/packages/craftcms-cp/src/components/button/button.stories.ts +++ b/packages/craftcms-cp/src/components/button/button.stories.ts @@ -124,3 +124,35 @@ export const Loading: Story = { Submit `, }; + +export const Links: Story = { + args: {}, + render: () => html` +
+
+ ${appearance.map( + (a) => html` + ${a} link + ` + )} +
+
+ ${['zero', 'small', 'medium', 'large'].map( + (size) => + html`${size}` + )} +
+
+ New tab + Download + Disabled link +
+
+ `, +}; diff --git a/packages/craftcms-cp/src/components/button/button.styles.ts b/packages/craftcms-cp/src/components/button/button.styles.ts index aa7428a5189..4889d6b3c86 100644 --- a/packages/craftcms-cp/src/components/button/button.styles.ts +++ b/packages/craftcms-cp/src/components/button/button.styles.ts @@ -332,4 +332,44 @@ export default css` transform: translateX(-100%); } } + + /* + Link mode: the inner is the full interactive surface. + Move inline padding from :host to the anchor so the whole button is clickable. + */ + :host([href]:not([disabled])) { + padding-inline: 0; + } + + .link { + display: flex; + align-items: center; + justify-content: inherit; + gap: inherit; + inline-size: 100%; + /* Stretch to the host's full cross size so the whole button (including + block padding) is the clickable link. The host's height is indefinite + (min-height), so a percentage min-block-size would not resolve. */ + align-self: stretch; + color: inherit; + font: inherit; + text-decoration: none; + padding-inline: var( + --c-button-spacing-inline, + var(--c-form-control-spacing-inline) + ); + } + + :host([href][size~='small']:not([disabled])) .link { + padding-inline: var(--c-spacing-sm); + } + + :host([href][size~='large']:not([disabled])) .link { + padding-inline: var(--c-spacing-lg); + } + + :host([href][size~='zero']:not([disabled])) .link, + :host([href][icon]:not([disabled])) .link { + padding-inline: 0; + } `; diff --git a/packages/craftcms-cp/src/components/button/button.test.ts b/packages/craftcms-cp/src/components/button/button.test.ts new file mode 100644 index 00000000000..83c66ed4e2f --- /dev/null +++ b/packages/craftcms-cp/src/components/button/button.test.ts @@ -0,0 +1,160 @@ +import {beforeEach, describe, expect, it} from 'vitest'; +import type CraftButton from './button.js'; +import './button.js'; + +async function createButton( + attrs: Record = {}, + text = 'Label' +): Promise { + const element = document.createElement('craft-button') as CraftButton; + for (const [name, value] of Object.entries(attrs)) { + element.setAttribute(name, value); + } + element.textContent = text; + document.body.append(element); + await element.updateComplete; + return element; +} + +function anchor(element: CraftButton): HTMLAnchorElement | null { + return element.shadowRoot?.querySelector('a.link') ?? null; +} + +beforeEach(() => { + document.body.innerHTML = ''; +}); + +describe('craft-button link mode', () => { + it('renders no anchor when href is absent', async () => { + const element = await createButton(); + expect(anchor(element)).toBeNull(); + expect(element.shadowRoot?.querySelector('.button-content')).not.toBeNull(); + }); + + it('renders a real anchor wrapping the content when href is set', async () => { + const element = await createButton({href: '/settings'}); + const a = anchor(element); + expect(a).not.toBeNull(); + expect(a!.getAttribute('href')).toBe('/settings'); + expect(a!.querySelector('.button-content')).not.toBeNull(); + }); + + it('forwards target and download to the anchor', async () => { + const element = await createButton({ + href: '/file.zip', + target: '_self', + download: 'file.zip', + }); + const a = anchor(element)!; + expect(a.getAttribute('target')).toBe('_self'); + expect(a.getAttribute('download')).toBe('file.zip'); + }); + + it('adds noopener to rel when target is _blank', async () => { + const element = await createButton({href: '/x', target: '_blank'}); + expect(anchor(element)!.getAttribute('rel')).toContain('noopener'); + }); + + it('preserves an explicit rel and still adds noopener for _blank', async () => { + const element = await createButton({ + href: '/x', + target: '_blank', + rel: 'nofollow', + }); + const rel = anchor(element)!.getAttribute('rel')!; + expect(rel).toContain('nofollow'); + expect(rel).toContain('noopener'); + }); + + it('does not set rel when target is not _blank and no rel given', async () => { + const element = await createButton({href: '/x'}); + expect(anchor(element)!.hasAttribute('rel')).toBe(false); + }); + + it('does not flag an accessible-name error for a labeled link', async () => { + const element = await createButton({href: '/x'}, 'Settings'); + // Wait for firstUpdated's async accessible-name computation. + await element.updateComplete; + expect(element.accessibleName).toBe('Settings'); + }); +}); + +describe('craft-button link semantics', () => { + it('is a presentation host and not a tab stop in link mode', async () => { + const element = await createButton({href: '/x'}); + expect(element.getAttribute('role')).toBe('presentation'); + expect(element.tabIndex).toBe(-1); + expect(element.type).toBe('button'); + }); + + it('keeps Lion button semantics when there is no href', async () => { + const element = await createButton(); + expect(element.getAttribute('role')).toBe('button'); + expect(element.tabIndex).toBe(0); + expect(element.type).toBe('submit'); + }); + + it('treats disabled+href as an inert button, not a link', async () => { + const element = await createButton({href: '/x', disabled: ''}); + expect(anchor(element)).toBeNull(); + expect(element.getAttribute('aria-disabled')).toBe('true'); + expect(element.getAttribute('role')).toBe('button'); + }); + + it('does not submit a form when a link-mode button is clicked', async () => { + const form = document.createElement('form'); + const element = document.createElement('craft-button') as CraftButton; + element.setAttribute('href', '/x'); + element.textContent = 'Go'; + form.append(element); + document.body.append(form); + await element.updateComplete; + + let submitted = false; + form.addEventListener('submit', (e) => { + submitted = true; + e.preventDefault(); + }); + element.click(); + + expect(submitted).toBe(false); + }); + + it('re-syncs host state when href is added after connect', async () => { + const element = await createButton(); + expect(element.getAttribute('role')).toBe('button'); + + element.href = '/later'; + await element.updateComplete; + + expect(element.getAttribute('role')).toBe('presentation'); + expect(element.tabIndex).toBe(-1); + }); + + it('leaves a disabled non-link button non-focusable (tabIndex -1)', async () => { + const element = await createButton({disabled: ''}); + expect(element.tabIndex).toBe(-1); + }); + + it('keeps disabled+href non-focusable (tabIndex -1)', async () => { + const element = await createButton({href: '/x', disabled: ''}); + expect(element.tabIndex).toBe(-1); + expect(anchor(element)).toBeNull(); + }); + + it('does not override an explicit type on a plain button', async () => { + const element = await createButton({type: 'button'}); + expect(element.type).toBe('button'); + }); + + it('restores button semantics when href is removed at runtime', async () => { + const element = await createButton({href: '/x'}); + expect(element.getAttribute('role')).toBe('presentation'); + + element.href = null; + await element.updateComplete; + + expect(element.getAttribute('role')).toBe('button'); + expect(element.tabIndex).toBe(0); + }); +}); diff --git a/packages/craftcms-cp/src/components/button/button.ts b/packages/craftcms-cp/src/components/button/button.ts index 3f62f4f93fb..b0a1c3f93bb 100644 --- a/packages/craftcms-cp/src/components/button/button.ts +++ b/packages/craftcms-cp/src/components/button/button.ts @@ -38,12 +38,47 @@ export type ButtonAppearance = * @csspart label - The button's label slot. * @csspart suffix - The button's suffix slot. * @csspart spinner - Spinner that shows when the button is in a loading state. + * @csspart link - The anchor element rendered when the button has an href. */ export default class CraftButton extends LionButtonSubmit { static override get styles() { return [...super.styles, styles]; } + override connectedCallback() { + // Set link-appropriate host state *before* Lion runs, so it skips its + // role="button" assignment and (via type) its submit-helper wiring. + if (this.href && !this.disabled) { + this.type = 'button'; + this.setAttribute('role', 'presentation'); + } + super.connectedCallback(); + this.syncLinkHostState(); + } + + override updated(changedProperties: Map) { + super.updated(changedProperties); + if (changedProperties.has('href') || changedProperties.has('disabled')) { + this.syncLinkHostState(); + } + } + + private syncLinkHostState() { + if (this.isLink) { + this.setAttribute('role', 'presentation'); + this.tabIndex = -1; + this.type = 'button'; + this.linkHostStateApplied = true; + } else if (this.linkHostStateApplied) { + this.setAttribute('role', 'button'); + this.type = 'submit'; + if (!this.disabled) { + this.tabIndex = 0; + } + this.linkHostStateApplied = false; + } + } + override async firstUpdated(changedProperties: Map) { super.firstUpdated(changedProperties); @@ -55,7 +90,13 @@ export default class CraftButton extends LionButtonSubmit { ); if (!this.accessibleName) { - this.accessibleName = computeAccessibleName(this); + // In link mode the host is role="presentation" (name not computable on + // it); the real accessible element is the inner anchor. + const nameTarget = this.isLink + ? ((this.shadowRoot?.querySelector('a.link') as HTMLElement | null) ?? + this) + : this; + this.accessibleName = computeAccessibleName(nameTarget); } this._hasAccessibilityError = @@ -90,13 +131,38 @@ export default class CraftButton extends LionButtonSubmit { @property() icon: string | null = null; + /** When set, the button renders as a link to this URL. */ + @property({reflect: true}) href: string | null = null; + + /** Anchor target (e.g. "_blank"). Forwarded to the rendered . */ + @property() target: string | null = null; + + /** Anchor rel. Forwarded to the ; "noopener" is added for target="_blank". */ + @property() rel: string | null = null; + + /** Anchor download attribute. Forwarded to the . */ + @property() download: string | null = null; + @state() private _hasAccessibilityError: boolean = false; + private linkHostStateApplied = false; + + private get isLink(): boolean { + return !!this.href && !this.disabled; + } + + private get computedRel(): string | null { + if (this.target === '_blank') { + const tokens = new Set((this.rel ?? '').split(/\s+/).filter(Boolean)); + tokens.add('noopener'); + return Array.from(tokens).join(' '); + } + return this.rel; + } + override render() { - return html` - - + const content = html`