From fb2759a5b51cd5ac2fd769f49e6449bb4c2bfc57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20B=C3=B6hmer?= Date: Mon, 27 Jul 2026 16:09:39 +0200 Subject: [PATCH] Added missing features --- config/packages/rate_limiter.yaml | 10 + config/packages/security.yaml | 6 + config/packages/test/rate_limiter.yaml | 16 ++ config/permissions.yaml | 3 + config/routes/attributes.yaml | 9 + .../OAuth/ClientRegistrationController.php | 234 ++++++++++++++++++ src/Controller/OAuth/DiscoveryController.php | 89 +++++++ src/Controller/OAuthClientAdminController.php | 73 ++++++ src/Controller/UserSettingsController.php | 35 ++- src/Services/OAuth/ConnectedAppManager.php | 149 +++++++++++ .../OAuth/OAuthClientAdminManager.php | 133 ++++++++++ .../oauth_clients/oauth_clients.html.twig | 60 +++++ .../users/_oauth_connected_apps.html.twig | 52 ++++ templates/users/user_settings.html.twig | 1 + .../ClientRegistrationControllerTest.php | 207 ++++++++++++++++ .../OAuth/DiscoveryControllerTest.php | 75 ++++++ .../OAuthClientAdminControllerTest.php | 104 ++++++++ ...serSettingsControllerConnectedAppsTest.php | 114 +++++++++ .../OAuth/ConnectedAppManagerTest.php | 134 ++++++++++ .../OAuth/OAuthClientAdminManagerTest.php | 94 +++++++ translations/messages.en.xlf | 150 +++++++++++ 21 files changed, 1747 insertions(+), 1 deletion(-) create mode 100644 config/packages/rate_limiter.yaml create mode 100644 config/packages/test/rate_limiter.yaml create mode 100644 src/Controller/OAuth/ClientRegistrationController.php create mode 100644 src/Controller/OAuth/DiscoveryController.php create mode 100644 src/Controller/OAuthClientAdminController.php create mode 100644 src/Services/OAuth/ConnectedAppManager.php create mode 100644 src/Services/OAuth/OAuthClientAdminManager.php create mode 100644 templates/tools/oauth_clients/oauth_clients.html.twig create mode 100644 templates/users/_oauth_connected_apps.html.twig create mode 100644 tests/Controller/OAuth/ClientRegistrationControllerTest.php create mode 100644 tests/Controller/OAuth/DiscoveryControllerTest.php create mode 100644 tests/Controller/OAuthClientAdminControllerTest.php create mode 100644 tests/Controller/UserSettingsControllerConnectedAppsTest.php create mode 100644 tests/Services/OAuth/ConnectedAppManagerTest.php create mode 100644 tests/Services/OAuth/OAuthClientAdminManagerTest.php diff --git a/config/packages/rate_limiter.yaml b/config/packages/rate_limiter.yaml new file mode 100644 index 00000000..0a3f4b81 --- /dev/null +++ b/config/packages/rate_limiter.yaml @@ -0,0 +1,10 @@ +framework: + rate_limiter: + # Guards the open RFC 7591 Dynamic Client Registration endpoint (App\Controller\OAuth\ClientRegistrationController) + # against abuse (mass client creation) - registration itself requires no authentication by design + # (any client may self-register; the real gate is the per-user consent screen at /authorize), so + # this is the only throttle standing between it and the internet. + oauth_client_registration: + policy: 'sliding_window' + limit: 20 + interval: '1 hour' diff --git a/config/packages/security.yaml b/config/packages/security.yaml index f2da5ad5..d1a8c8b9 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -75,6 +75,12 @@ security: # authentication here (rather than relying on is_authenticated()) triggers Symfony's normal # redirect-to-login-then-back flow for anonymous requests, same as the user settings pages. - { path: ^/authorize, role: IS_AUTHENTICATED_FULLY } + # RFC 7591 Dynamic Client Registration (App\Controller\OAuth\ClientRegistrationController) is + # deliberately open/unauthenticated - see that class's docblock. Rate-limited, not access-controlled. + - { path: ^/oauth/register, role: PUBLIC_ACCESS } + # Note: RFC 8414/9728 discovery metadata under /.well-known/ needs no entry here - the "dev" + # firewall above already matches ^/\.well-known/ with security:false (fully public, no + # AccessListener at all), which is where those requests actually get handled. # Restrict access to API to users, which has the API access permission - { path: "^/api", allow_if: 'is_granted("@api.access_api") and is_authenticated()' } # Restrict access to KICAD to users, which has API access permission diff --git a/config/packages/test/rate_limiter.yaml b/config/packages/test/rate_limiter.yaml new file mode 100644 index 00000000..d52a239b --- /dev/null +++ b/config/packages/test/rate_limiter.yaml @@ -0,0 +1,16 @@ +framework: + # The default "oauth_client_registration" limiter (config/packages/rate_limiter.yaml) stores its state + # in the "cache.rate_limiter" pool, which is filesystem-backed and therefore persists across separate + # phpunit invocations (unlike the DB, which DAMADoctrineTestBundle wraps in a rolled-back transaction + # per test). Tests hitting POST /oauth/register for real (tests/Controller/OAuth/ClientRegistrationControllerTest.php) + # all share the same client IP (127.0.0.1) as the limiter key, so without this override, repeated test + # runs within the same hour would eventually start getting real 429s instead of the responses the + # tests actually assert on. An array-adapter pool is fresh for every kernel boot, so it never persists + # between test runs. + cache: + pools: + test.rate_limiter.cache: + adapter: cache.adapter.array + rate_limiter: + oauth_client_registration: + cache_pool: test.rate_limiter.cache diff --git a/config/permissions.yaml b/config/permissions.yaml index 704da880..a7a35c0a 100644 --- a/config/permissions.yaml +++ b/config/permissions.yaml @@ -297,6 +297,9 @@ perms: # Here comes a list with all Permission names (they have a perm_[name] co manage_oauth_tokens: label: "Manage OAuth tokens" apiTokenRole: ROLE_API_ADMIN + manage_oauth_clients: + label: "perm.system.manage_oauth_clients" + apiTokenRole: ROLE_API_ADMIN show_updates: label: "perm.system.show_available_updates" apiTokenRole: ROLE_API_ADMIN diff --git a/config/routes/attributes.yaml b/config/routes/attributes.yaml index 72d7c9bc..42eeed7f 100644 --- a/config/routes/attributes.yaml +++ b/config/routes/attributes.yaml @@ -4,6 +4,9 @@ controllers: namespace: App\Controller type: attribute prefix: '{_locale}' + # OAuth2 protocol endpoints (client registration, discovery metadata) must live at fixed, + # spec-mandated paths with no locale prefix - see config/routes/oauth_server_controllers.yaml. + exclude: '../../src/Controller/OAuth/*' defaults: _locale: '%kernel.default_locale%' @@ -11,6 +14,12 @@ controllers: # Match only locales like de_DE or de _locale: "^[a-z]{2}(_[A-Z]{2})?$" +oauth_server_controllers: + resource: + path: ../../src/Controller/OAuth/ + namespace: App\Controller\OAuth + type: attribute + kernel: resource: ../../src/Kernel.php type: attribute diff --git a/src/Controller/OAuth/ClientRegistrationController.php b/src/Controller/OAuth/ClientRegistrationController.php new file mode 100644 index 00000000..e7db5b66 --- /dev/null +++ b/src/Controller/OAuth/ClientRegistrationController.php @@ -0,0 +1,234 @@ +. + */ + +declare(strict_types=1); + +namespace App\Controller\OAuth; + +use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\ScopeManagerInterface; +use League\Bundle\OAuth2ServerBundle\Model\Client; +use League\Bundle\OAuth2ServerBundle\ValueObject\Grant; +use League\Bundle\OAuth2ServerBundle\ValueObject\RedirectUri; +use League\Bundle\OAuth2ServerBundle\ValueObject\Scope; +use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; +use Symfony\Component\DependencyInjection\Attribute\Autowire; +use Symfony\Component\HttpFoundation\JsonResponse; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\RateLimiter\RateLimiterFactory; +use Symfony\Component\Routing\Attribute\Route; + +/** + * RFC 7591 Dynamic Client Registration. Deliberately open/unauthenticated - any client may self-register + * a public (secret-less) OAuth2 client, since the actual access-control gate for the whole OAuth server + * is the per-user consent screen at /authorize (see App\EventListener\OAuth\AuthorizationConsentListener), + * not client registration. Only Authorization Code + PKCE / refresh_token are ever issued to registered + * clients (config/packages/league_oauth2_server.yaml), so registering a client grants no access by itself. + * + * Rate-limited per client IP (config/packages/rate_limiter.yaml, "oauth_client_registration") since this + * endpoint has no other gate protecting it from mass-registration abuse. + */ +#[Route('/oauth')] +class ClientRegistrationController extends AbstractController +{ + /** @var list */ + private const ALLOWED_GRANT_TYPES = ['authorization_code', 'refresh_token']; + + /** @var list */ + private const ALLOWED_RESPONSE_TYPES = ['code']; + + private const MAX_REDIRECT_URIS = 10; + private const MAX_REDIRECT_URI_LENGTH = 2000; + private const MAX_CLIENT_NAME_LENGTH = 128; + private const MAX_BODY_LENGTH = 65536; + + /** + * @param list $defaultScopes + */ + public function __construct( + private readonly ClientManagerInterface $clientManager, + private readonly ScopeManagerInterface $scopeManager, + #[Autowire(service: 'limiter.oauth_client_registration')] + private readonly RateLimiterFactory $registrationLimiter, + #[Autowire('%league.oauth2_server.scopes.default%')] + private readonly array $defaultScopes, + ) { + } + + #[Route('/register', name: 'oauth2_client_register', methods: ['POST'])] + public function register(Request $request): JsonResponse + { + $limit = $this->registrationLimiter->create($request->getClientIp() ?? 'unknown')->consume(); + if (!$limit->isAccepted()) { + $retryAfterSeconds = max(0, $limit->getRetryAfter()->getTimestamp() - time()); + + return new JsonResponse( + ['error' => 'temporarily_unavailable', 'error_description' => 'Too many client registration requests, please try again later.'], + Response::HTTP_TOO_MANY_REQUESTS, + ['Retry-After' => (string) $retryAfterSeconds] + ); + } + + $content = $request->getContent(); + if (\strlen($content) > self::MAX_BODY_LENGTH) { + return $this->error('invalid_client_metadata', 'Request body is too large.'); + } + + $data = json_decode($content, true); + if (!\is_array($data)) { + return $this->error('invalid_client_metadata', 'Request body must be a JSON object.'); + } + + $redirectUris = $this->validateRedirectUris($data['redirect_uris'] ?? null); + if ($redirectUris instanceof JsonResponse) { + return $redirectUris; + } + + $authMethod = $data['token_endpoint_auth_method'] ?? 'none'; + if ('none' !== $authMethod) { + return $this->error('invalid_client_metadata', 'Only public clients are supported; token_endpoint_auth_method must be "none".'); + } + + $grantTypes = $data['grant_types'] ?? self::ALLOWED_GRANT_TYPES; + if (!\is_array($grantTypes) || [] === $grantTypes || [] !== array_diff($grantTypes, self::ALLOWED_GRANT_TYPES)) { + return $this->error('invalid_client_metadata', 'grant_types may only contain "authorization_code" and "refresh_token".'); + } + + $responseTypes = $data['response_types'] ?? self::ALLOWED_RESPONSE_TYPES; + if (!\is_array($responseTypes) || [] === $responseTypes || [] !== array_diff($responseTypes, self::ALLOWED_RESPONSE_TYPES)) { + return $this->error('invalid_client_metadata', 'response_types may only contain "code".'); + } + + $clientName = $data['client_name'] ?? 'Unnamed OAuth client'; + if (!\is_string($clientName) || '' === $clientName) { + return $this->error('invalid_client_metadata', 'client_name must be a non-empty string.'); + } + $clientName = mb_substr($clientName, 0, self::MAX_CLIENT_NAME_LENGTH); + + $scopes = $this->validateScopes($data['scope'] ?? null); + if ($scopes instanceof JsonResponse) { + return $scopes; + } + + $identifier = bin2hex(random_bytes(16)); + + $client = new Client($clientName, $identifier, null); + $client->setRedirectUris(...array_map(static fn (string $uri): RedirectUri => new RedirectUri($uri), $redirectUris)); + $client->setGrants(...array_map(static fn (string $grant): Grant => new Grant($grant), $grantTypes)); + $client->setScopes(...array_map(static fn (string $scope): Scope => new Scope($scope), $scopes)); + $client->setActive(true); + + $this->clientManager->save($client); + + return new JsonResponse([ + 'client_id' => $identifier, + 'client_id_issued_at' => time(), + 'client_name' => $clientName, + 'redirect_uris' => $redirectUris, + 'grant_types' => array_values($grantTypes), + 'response_types' => array_values($responseTypes), + 'token_endpoint_auth_method' => 'none', + 'scope' => implode(' ', $scopes), + ], Response::HTTP_CREATED); + } + + /** + * @return list|JsonResponse + */ + private function validateRedirectUris(mixed $redirectUrisRaw): array|JsonResponse + { + if (!\is_array($redirectUrisRaw) || [] === $redirectUrisRaw || \count($redirectUrisRaw) > self::MAX_REDIRECT_URIS) { + return $this->error('invalid_redirect_uri', 'redirect_uris must be a non-empty array of at most '.self::MAX_REDIRECT_URIS.' URIs.'); + } + + $redirectUris = []; + foreach ($redirectUrisRaw as $uri) { + if (!\is_string($uri) || \strlen($uri) > self::MAX_REDIRECT_URI_LENGTH || !$this->isAllowedRedirectUri($uri)) { + return $this->error('invalid_redirect_uri', \sprintf('"%s" is not an allowed redirect URI.', \is_string($uri) ? $uri : get_debug_type($uri))); + } + $redirectUris[] = $uri; + } + + return $redirectUris; + } + + /** + * Accepts https:// URIs, loopback http:// URIs (127.0.0.1/localhost/[::1], any port), and private-use + * URI schemes containing a dot (e.g. "com.example.app:/callback") - the three redirect URI shapes + * RFC 8252 recommends for native/CLI apps like MCP clients. Plain http:// to a non-loopback host is + * rejected (token/code leakage over an insecure channel), as is any URI carrying a fragment. + */ + private function isAllowedRedirectUri(string $uri): bool + { + $parts = parse_url($uri); + if (false === $parts || !isset($parts['scheme']) || isset($parts['fragment'])) { + return false; + } + + $scheme = strtolower($parts['scheme']); + + if ('https' === $scheme) { + return isset($parts['host']) && '' !== $parts['host']; + } + + if ('http' === $scheme) { + $host = strtolower($parts['host'] ?? ''); + + return \in_array($host, ['127.0.0.1', 'localhost', '::1', '[::1]'], true); + } + + // Private-use URI scheme (RFC 8252 §7.1): require a dot to match the recommended reverse-DNS + // style (e.g. "com.example.app"), reducing collisions with generic/likely-preregistered schemes. + return 1 === preg_match('/^[a-z][a-z0-9+.-]*\.[a-z0-9+.-]*[a-z0-9]$/', $scheme); + } + + /** + * @return list|JsonResponse + */ + private function validateScopes(mixed $scopeString): array|JsonResponse + { + if (null === $scopeString) { + return $this->defaultScopes; + } + + if (!\is_string($scopeString)) { + return $this->error('invalid_client_metadata', 'scope must be a space-delimited string.'); + } + + $scopes = array_values(array_filter(explode(' ', $scopeString), static fn (string $s): bool => '' !== $s)); + if ([] === $scopes) { + return $this->defaultScopes; + } + + foreach ($scopes as $scope) { + if (null === $this->scopeManager->find($scope)) { + return $this->error('invalid_client_metadata', \sprintf('Unknown scope "%s".', $scope)); + } + } + + return $scopes; + } + + private function error(string $error, string $description): JsonResponse + { + return new JsonResponse(['error' => $error, 'error_description' => $description], Response::HTTP_BAD_REQUEST); + } +} diff --git a/src/Controller/OAuth/DiscoveryController.php b/src/Controller/OAuth/DiscoveryController.php new file mode 100644 index 00000000..e32b1518 --- /dev/null +++ b/src/Controller/OAuth/DiscoveryController.php @@ -0,0 +1,89 @@ +. + */ + +declare(strict_types=1); + +namespace App\Controller\OAuth; + +use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; +use Symfony\Component\HttpFoundation\JsonResponse; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Routing\Attribute\Route; +use Symfony\Component\Routing\Generator\UrlGeneratorInterface; + +/** + * RFC 8414 (OAuth 2.0 Authorization Server Metadata) and RFC 9728 (OAuth 2.0 Protected Resource Metadata) + * discovery endpoints, so OAuth/MCP clients can locate /authorize, /token and the RFC 7591 registration + * endpoint (App\Controller\OAuth\ClientRegistrationController) without any hardcoded configuration. + * + * Lives at the fixed /.well-known/... paths both RFCs mandate - unprefixed by locale (see + * config/routes/oauth_server_controllers.yaml) and, in practice, served by the "dev" firewall's + * ^/\.well-known/ pattern (config/packages/security.yaml), which is fully public (security: false). + * + * The issuer/resource identifier is derived from the current request's scheme+host rather than a fixed + * config value, so this keeps working unchanged behind any hostname/reverse proxy the instance is reached + * through. + */ +#[Route('/.well-known')] +class DiscoveryController extends AbstractController +{ + #[Route('/oauth-authorization-server', name: 'oauth2_discovery_authorization_server', methods: ['GET'])] + public function authorizationServerMetadata(Request $request): JsonResponse + { + $issuer = $request->getSchemeAndHttpHost(); + + return new JsonResponse([ + 'issuer' => $issuer, + 'authorization_endpoint' => $this->generateUrl('oauth2_authorize', [], UrlGeneratorInterface::ABSOLUTE_URL), + 'token_endpoint' => $this->generateUrl('oauth2_token', [], UrlGeneratorInterface::ABSOLUTE_URL), + 'registration_endpoint' => $this->generateUrl('oauth2_client_register', [], UrlGeneratorInterface::ABSOLUTE_URL), + 'scopes_supported' => self::availableScopes(), + 'response_types_supported' => ['code'], + 'response_modes_supported' => ['query'], + 'grant_types_supported' => ['authorization_code', 'refresh_token'], + 'token_endpoint_auth_methods_supported' => ['none'], + 'code_challenge_methods_supported' => ['S256'], + ]); + } + + #[Route('/oauth-protected-resource', name: 'oauth2_discovery_protected_resource', methods: ['GET'])] + public function protectedResourceMetadata(Request $request): JsonResponse + { + $issuer = $request->getSchemeAndHttpHost(); + + return new JsonResponse([ + 'resource' => $issuer, + 'authorization_servers' => [$issuer], + 'bearer_methods_supported' => ['header'], + 'scopes_supported' => self::availableScopes(), + ]); + } + + /** + * @return list + */ + private static function availableScopes(): array + { + // Kept in sync with config/packages/league_oauth2_server.yaml's scopes.available - the bundle + // does not expose the configured "available" scope list as a service/parameter (only "default" is, + // via league.oauth2_server.scopes.default), so this one list is hand-maintained. + return ['read_only', 'edit', 'admin', 'full']; + } +} diff --git a/src/Controller/OAuthClientAdminController.php b/src/Controller/OAuthClientAdminController.php new file mode 100644 index 00000000..9eb1816e --- /dev/null +++ b/src/Controller/OAuthClientAdminController.php @@ -0,0 +1,73 @@ +. + */ + +declare(strict_types=1); + +namespace App\Controller; + +use App\Services\OAuth\OAuthClientAdminManager; +use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Routing\Attribute\Route; + +/** + * Admin-only overview of every OAuth2 client (League\Bundle\OAuth2ServerBundle) registered against this + * instance - almost always RFC 7591 self-registered (App\Controller\OAuth\ClientRegistrationController), + * since that endpoint is open by design. This page is the moderation surface for that openness: an admin + * can see what has registered and permanently remove a client (and everything it was ever granted) if it + * looks abusive or abandoned. + */ +#[Route(path: '/tools/oauth_clients')] +class OAuthClientAdminController extends AbstractController +{ + public function __construct(private readonly OAuthClientAdminManager $manager) + { + } + + #[Route(path: '', name: 'oauth_clients_list', methods: ['GET'])] + public function list(): Response + { + $this->denyAccessUnlessGranted('@system.manage_oauth_clients'); + + return $this->render('tools/oauth_clients/oauth_clients.html.twig', [ + 'clients' => $this->manager->listClientsWithLiveTokenCounts(), + ]); + } + + #[Route(path: '/{identifier}/delete', name: 'oauth_clients_delete', methods: ['DELETE'])] + public function delete(string $identifier, Request $request): Response + { + $this->denyAccessUnlessGranted('@system.manage_oauth_clients'); + + if (!$this->isCsrfTokenValid('oauth_client_delete'.$identifier, $request->request->get('_token'))) { + $this->addFlash('error', 'csfr_invalid'); + return $this->redirectToRoute('oauth_clients_list'); + } + + if (!$this->manager->deleteClient($identifier)) { + $this->addFlash('error', 'tfa_u2f.u2f_delete.not_existing'); + return $this->redirectToRoute('oauth_clients_list'); + } + + $this->addFlash('success', 'oauth_clients.deleted'); + return $this->redirectToRoute('oauth_clients_list'); + } +} diff --git a/src/Controller/UserSettingsController.php b/src/Controller/UserSettingsController.php index 3f54e99c..2feb16ef 100644 --- a/src/Controller/UserSettingsController.php +++ b/src/Controller/UserSettingsController.php @@ -32,6 +32,7 @@ use App\Events\SecurityEvent; use App\Events\SecurityEvents; use App\Form\TFAGoogleSettingsType; use App\Form\UserSettingsType; +use App\Services\OAuth\ConnectedAppManager; use App\Services\UserSystem\TFA\BackupCodeManager; use App\Services\UserSystem\UserAvatarHelper; use Doctrine\ORM\EntityManagerInterface; @@ -58,7 +59,7 @@ use Symfony\Component\Validator\Constraints\Length; #[Route(path: '/user')] class UserSettingsController extends AbstractController { - public function __construct(protected bool $demo_mode, protected EventDispatcherInterface $eventDispatcher) + public function __construct(protected bool $demo_mode, protected EventDispatcherInterface $eventDispatcher, private readonly ConnectedAppManager $connectedAppManager) { } @@ -380,6 +381,7 @@ class UserSettingsController extends AbstractController 'settings_form' => $form, 'pw_form' => $pw_form, 'global_reload_needed' => $page_need_reload, + 'connected_apps' => $this->connectedAppManager->listConnectedClients($user->getUserIdentifier()), 'google_form' => $google_form, 'backup_form' => $backup_form, @@ -486,4 +488,35 @@ class UserSettingsController extends AbstractController $this->addFlash('success', 'api_tokens.deleted'); return $this->redirectToRoute('user_settings'); } + + #[Route(path: '/oauth_connected_apps/revoke', name: 'user_oauth_connected_apps_revoke', methods: ['DELETE'])] + public function revokeConnectedApp(Request $request): Response + { + $this->denyAccessUnlessGranted('@api.manage_tokens'); + //When user change its settings, he should be logged in fully. + $this->denyAccessUnlessGranted('IS_AUTHENTICATED_FULLY'); + + $user = $this->getUser(); + if (!$user instanceof User) { + throw new RuntimeException('This controller only works only for Part-DB User objects!'); + } + + if (!$this->isCsrfTokenValid('delete'.$user->getID(), $request->request->get('_token'))) { + $this->addFlash('error', 'csfr_invalid'); + return $this->redirectToRoute('user_settings'); + } + + $clientId = $request->request->getString('client_id'); + if ('' === $clientId) { + $this->addFlash('error', 'tfa_u2f.u2f_delete.not_existing'); + return $this->redirectToRoute('user_settings'); + } + + //Scoped to this user's own tokens by userIdentifier, so there is nothing to revoke (and no harm + //done) if $clientId does not actually belong to one of this user's connected apps. + $this->connectedAppManager->revokeForUserAndClient($user->getUserIdentifier(), $clientId); + + $this->addFlash('success', 'user.settings.oauth_connected_apps.revoked'); + return $this->redirectToRoute('user_settings'); + } } diff --git a/src/Services/OAuth/ConnectedAppManager.php b/src/Services/OAuth/ConnectedAppManager.php new file mode 100644 index 00000000..06f30c7d --- /dev/null +++ b/src/Services/OAuth/ConnectedAppManager.php @@ -0,0 +1,149 @@ +. + */ + +declare(strict_types=1); + +namespace App\Services\OAuth; + +use Doctrine\ORM\EntityManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; +use League\Bundle\OAuth2ServerBundle\Model\AccessToken; +use League\Bundle\OAuth2ServerBundle\Model\AuthorizationCode; +use League\Bundle\OAuth2ServerBundle\Model\ClientInterface; +use League\Bundle\OAuth2ServerBundle\Model\RefreshToken; + +/** + * Lists and revokes the OAuth2 clients (League\Bundle\OAuth2ServerBundle) a given user has authorized - + * the "connected apps" shown in user settings (templates/users/_oauth_connected_apps.html.twig). + * + * "Connected" means the client currently holds a live (non-revoked, non-expired) access token or refresh + * token for that user; a client the user approved but whose grant has since fully expired or been revoked + * no longer shows up here (there's nothing left for the user to revoke). + */ +class ConnectedAppManager +{ + public function __construct( + private readonly EntityManagerInterface $entityManager, + private readonly ClientManagerInterface $clientManager, + ) { + } + + /** + * @return list + */ + public function listConnectedClients(string $userIdentifier): array + { + $now = new \DateTimeImmutable(); + + $fromAccessTokens = $this->entityManager->createQueryBuilder() + ->select('IDENTITY(at.client) as clientId', 'MAX(at.expiry) as expiry') + ->from(AccessToken::class, 'at') + ->where('at.userIdentifier = :userIdentifier') + ->andWhere('at.revoked = false') + ->andWhere('at.expiry > :now') + ->groupBy('at.client') + ->setParameter('userIdentifier', $userIdentifier) + ->setParameter('now', $now) + ->getQuery() + ->getResult(); + + $fromRefreshTokens = $this->entityManager->createQueryBuilder() + ->select('IDENTITY(at.client) as clientId', 'MAX(rt.expiry) as expiry') + ->from(RefreshToken::class, 'rt') + ->join('rt.accessToken', 'at') + ->where('at.userIdentifier = :userIdentifier') + ->andWhere('rt.revoked = false') + ->andWhere('rt.expiry > :now') + ->groupBy('at.client') + ->setParameter('userIdentifier', $userIdentifier) + ->setParameter('now', $now) + ->getQuery() + ->getResult(); + + /** @var array $expiryByClientId */ + $expiryByClientId = []; + foreach ([...$fromAccessTokens, ...$fromRefreshTokens] as $row) { + $clientId = $row['clientId']; + $expiry = $row['expiry']; + if (!isset($expiryByClientId[$clientId]) || $expiry > $expiryByClientId[$clientId]) { + $expiryByClientId[$clientId] = $expiry; + } + } + + $result = []; + foreach ($expiryByClientId as $clientId => $expiry) { + $client = $this->clientManager->find($clientId); + if (null === $client) { + continue; + } + $result[] = ['client' => $client, 'expiry' => $expiry]; + } + + return $result; + } + + /** + * Revokes every access token, refresh token and authorization code the given client holds for the + * given user - bulk DQL UPDATEs (mirroring League\Bundle\OAuth2ServerBundle\Service\CredentialsRevoker\DoctrineCredentialsRevoker, + * which does the same thing but scoped to "all of a user" / "all of a client" rather than one pair), + * not entity-load-then-flush, since this can touch many rows at once. + */ + public function revokeForUserAndClient(string $userIdentifier, string $clientIdentifier): void + { + $this->entityManager->createQueryBuilder() + ->update(AccessToken::class, 'at') + ->set('at.revoked', ':revoked') + ->where('at.userIdentifier = :userIdentifier') + ->andWhere('at.client = :client') + ->setParameter('revoked', true) + ->setParameter('userIdentifier', $userIdentifier) + ->setParameter('client', $clientIdentifier) + ->getQuery() + ->execute(); + + $accessTokenIdsSubQuery = $this->entityManager->createQueryBuilder() + ->select('at2.identifier') + ->from(AccessToken::class, 'at2') + ->where('at2.userIdentifier = :userIdentifier') + ->andWhere('at2.client = :client') + ->getDQL(); + + $refreshTokenUpdate = $this->entityManager->createQueryBuilder(); + $refreshTokenUpdate->update(RefreshToken::class, 'rt') + ->set('rt.revoked', ':revoked') + ->where($refreshTokenUpdate->expr()->in('rt.accessToken', $accessTokenIdsSubQuery)) + ->setParameter('revoked', true) + ->setParameter('userIdentifier', $userIdentifier) + ->setParameter('client', $clientIdentifier) + ->getQuery() + ->execute(); + + $this->entityManager->createQueryBuilder() + ->update(AuthorizationCode::class, 'ac') + ->set('ac.revoked', ':revoked') + ->where('ac.userIdentifier = :userIdentifier') + ->andWhere('ac.client = :client') + ->setParameter('revoked', true) + ->setParameter('userIdentifier', $userIdentifier) + ->setParameter('client', $clientIdentifier) + ->getQuery() + ->execute(); + } +} diff --git a/src/Services/OAuth/OAuthClientAdminManager.php b/src/Services/OAuth/OAuthClientAdminManager.php new file mode 100644 index 00000000..5772439f --- /dev/null +++ b/src/Services/OAuth/OAuthClientAdminManager.php @@ -0,0 +1,133 @@ +. + */ + +declare(strict_types=1); + +namespace App\Services\OAuth; + +use Doctrine\ORM\EntityManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; +use League\Bundle\OAuth2ServerBundle\Model\AbstractClient; +use League\Bundle\OAuth2ServerBundle\Model\AccessToken; +use League\Bundle\OAuth2ServerBundle\Model\AuthorizationCode; +use League\Bundle\OAuth2ServerBundle\Model\ClientInterface; +use League\Bundle\OAuth2ServerBundle\Model\RefreshToken; + +/** + * Backs the admin OAuth2 client overview (App\Controller\OAuthClientAdminController) - listing every + * registered client (almost always RFC 7591 self-registered, see that controller's docblock) with how + * many currently-live access tokens each one holds, and permanently removing a client plus everything it + * was ever granted. + * + * Deliberately does NOT use League\Bundle\OAuth2ServerBundle\Service\CredentialsRevokerInterface's + * DoctrineCredentialsRevoker: it unconditionally also issues an UPDATE against the DeviceCode model/table, + * which does not exist in our schema (config/packages/league_oauth2_server.yaml disables the device code + * grant, so migrations/Version20260726180454.php never created oauth2_device_code) - that call would + * throw "no such table" here. The access/refresh tokens and authorization codes (the 3 credential types + * we actually persist) are bulk-deleted directly instead, before removing the client itself - the + * "client" FK columns are mapped ON DELETE CASCADE (see League's Persistence\Mapping\Driver), but SQLite + * only enforces that when "PRAGMA foreign_keys = ON" is set on the connection, which is not guaranteed + * here, so this does not rely on it. + */ +class OAuthClientAdminManager +{ + public function __construct( + private readonly EntityManagerInterface $entityManager, + private readonly ClientManagerInterface $clientManager, + ) { + } + + /** + * @return list + */ + public function listClientsWithLiveTokenCounts(): array + { + $clients = $this->clientManager->list(null); + + $rows = $this->entityManager->createQueryBuilder() + ->select('IDENTITY(at.client) as clientId', 'COUNT(at.identifier) as tokenCount') + ->from(AccessToken::class, 'at') + ->where('at.revoked = false') + ->andWhere('at.expiry > :now') + ->groupBy('at.client') + ->setParameter('now', new \DateTimeImmutable()) + ->getQuery() + ->getResult(); + + /** @var array $countByClientId */ + $countByClientId = []; + foreach ($rows as $row) { + $countByClientId[$row['clientId']] = (int) $row['tokenCount']; + } + + return array_map(static fn (ClientInterface $client): array => [ + 'client' => $client, + 'liveTokenCount' => $countByClientId[$client->getIdentifier()] ?? 0, + ], $clients); + } + + /** + * @return bool false if no client with that identifier exists + */ + public function deleteClient(string $identifier): bool + { + $client = $this->clientManager->find($identifier); + if (!$client instanceof AbstractClient) { + return false; + } + + $this->deleteAllCredentialsForClient($identifier); + $this->clientManager->remove($client); + + return true; + } + + private function deleteAllCredentialsForClient(string $clientIdentifier): void + { + // Refresh tokens first - they reference access tokens (not the client directly), so this has to + // run before the access tokens themselves are deleted. + $accessTokenIdsSubQuery = $this->entityManager->createQueryBuilder() + ->select('at2.identifier') + ->from(AccessToken::class, 'at2') + ->where('at2.client = :client') + ->getDQL(); + + $refreshTokenDelete = $this->entityManager->createQueryBuilder(); + $refreshTokenDelete->delete(RefreshToken::class, 'rt') + ->where($refreshTokenDelete->expr()->in('rt.accessToken', $accessTokenIdsSubQuery)) + ->setParameter('client', $clientIdentifier) + ->getQuery() + ->execute(); + + $this->entityManager->createQueryBuilder() + ->delete(AccessToken::class, 'at') + ->where('at.client = :client') + ->setParameter('client', $clientIdentifier) + ->getQuery() + ->execute(); + + $this->entityManager->createQueryBuilder() + ->delete(AuthorizationCode::class, 'ac') + ->where('ac.client = :client') + ->setParameter('client', $clientIdentifier) + ->getQuery() + ->execute(); + } +} diff --git a/templates/tools/oauth_clients/oauth_clients.html.twig b/templates/tools/oauth_clients/oauth_clients.html.twig new file mode 100644 index 00000000..02fd54e4 --- /dev/null +++ b/templates/tools/oauth_clients/oauth_clients.html.twig @@ -0,0 +1,60 @@ +{% extends "main_card.html.twig" %} + +{% block title %}{% trans %}oauth_clients.title{% endtrans %}{% endblock %} + +{% block card_title %} + {% trans %}oauth_clients.title{% endtrans %} +{% endblock %} + +{% block card_content %} +

{% trans %}oauth_clients.description{% endtrans %}

+ + {% if clients is empty %} + {% trans %}oauth_clients.none_yet{% endtrans %} + {% else %} + + + + + + + + + + + + + + {% for row in clients %} + {% set client = row.client %} + + + + + + + + + + {% endfor %} + +
{% trans %}oauth_clients.name{% endtrans %}{% trans %}oauth_clients.client_id{% endtrans %}{% trans %}oauth_clients.scopes{% endtrans %}{% trans %}oauth_clients.redirect_uris{% endtrans %}{% trans %}oauth_clients.status{% endtrans %}{% trans %}oauth_clients.live_tokens{% endtrans %}
{{ client.name }}{{ client.identifier }}{{ client.scopes|join(', ') }}{{ client.redirectUris|join(', ') }} + {% if client.active %} + {% trans %}oauth_clients.active{% endtrans %} + {% else %} + {% trans %}oauth_clients.inactive{% endtrans %} + {% endif %} + {{ row.liveTokenCount }} +
+ + + +
+
+ {% endif %} +{% endblock %} diff --git a/templates/users/_oauth_connected_apps.html.twig b/templates/users/_oauth_connected_apps.html.twig new file mode 100644 index 00000000..7645b0fb --- /dev/null +++ b/templates/users/_oauth_connected_apps.html.twig @@ -0,0 +1,52 @@ +{# @var user \App\Entity\UserSystem\User #} +{# @var connected_apps array #} + +{% import "helper.twig" as helper %} + +
+
+ + {% trans %}user.settings.oauth_connected_apps{% endtrans %} +
+
+ {% trans %}user.settings.oauth_connected_apps.description{% endtrans %} + + {% if connected_apps is empty %} +

+ {% trans %}user.settings.oauth_connected_apps.none_yet{% endtrans %} +

+ {% else %} +
+ + + + + + + + + + + + + {% for connected_app in connected_apps %} + + + + + + {% endfor %} + +
{% trans %}user.settings.oauth_connected_apps.app_name{% endtrans %}{% trans %}user.settings.oauth_connected_apps.access_until{% endtrans %}
{{ connected_app.client.name }}{{ helper.format_date_nullable(connected_app.expiry) }} + +
+
+ {% endif %} +
+
diff --git a/templates/users/user_settings.html.twig b/templates/users/user_settings.html.twig index 36cde643..12db959e 100644 --- a/templates/users/user_settings.html.twig +++ b/templates/users/user_settings.html.twig @@ -79,5 +79,6 @@ {% if is_granted("@api.access_api") %} {% include "users/_api_tokens.html.twig" %} + {% include "users/_oauth_connected_apps.html.twig" %} {% endif %} {% endblock %} diff --git a/tests/Controller/OAuth/ClientRegistrationControllerTest.php b/tests/Controller/OAuth/ClientRegistrationControllerTest.php new file mode 100644 index 00000000..0ba630ba --- /dev/null +++ b/tests/Controller/OAuth/ClientRegistrationControllerTest.php @@ -0,0 +1,207 @@ +. + */ + +declare(strict_types=1); + +namespace App\Tests\Controller\OAuth; + +use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; +use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; +use Symfony\Component\RateLimiter\RateLimiterFactory; + +/** + * Drives the real unauthenticated POST /oauth/register HTTP flow + * (App\Controller\OAuth\ClientRegistrationController). + */ +final class ClientRegistrationControllerTest extends WebTestCase +{ + public function testMinimalRegistrationUsesDefaults(): void + { + $httpClient = static::createClient(); + $httpClient->jsonRequest('POST', '/oauth/register', [ + 'redirect_uris' => ['https://client.example.invalid/callback'], + ]); + + self::assertResponseStatusCodeSame(201); + $data = json_decode((string) $httpClient->getResponse()->getContent(), true); + + self::assertMatchesRegularExpression('/^[0-9a-f]{32}$/', $data['client_id']); + self::assertIsInt($data['client_id_issued_at']); + self::assertSame(['https://client.example.invalid/callback'], $data['redirect_uris']); + self::assertSame(['authorization_code', 'refresh_token'], $data['grant_types']); + self::assertSame(['code'], $data['response_types']); + self::assertSame('none', $data['token_endpoint_auth_method']); + self::assertSame('read_only', $data['scope']); + + $client = $httpClient->getContainer()->get(ClientManagerInterface::class)->find($data['client_id']); + self::assertNotNull($client); + self::assertNull($client->getSecret()); + self::assertTrue($client->isActive()); + } + + public function testFullRegistrationHonoursRequestedFields(): void + { + $httpClient = static::createClient(); + $httpClient->jsonRequest('POST', '/oauth/register', [ + 'redirect_uris' => ['https://client.example.invalid/callback', 'https://client.example.invalid/callback2'], + 'client_name' => 'My MCP Client', + 'grant_types' => ['authorization_code'], + 'response_types' => ['code'], + 'token_endpoint_auth_method' => 'none', + 'scope' => 'edit full', + ]); + + self::assertResponseStatusCodeSame(201); + $data = json_decode((string) $httpClient->getResponse()->getContent(), true); + + self::assertSame('My MCP Client', $data['client_name']); + self::assertSame(['authorization_code'], $data['grant_types']); + self::assertSame('edit full', $data['scope']); + + $client = $httpClient->getContainer()->get(ClientManagerInterface::class)->find($data['client_id']); + self::assertNotNull($client); + self::assertSame('My MCP Client', $client->getName()); + self::assertCount(2, $client->getRedirectUris()); + self::assertCount(2, $client->getScopes()); + } + + public function testLoopbackHttpRedirectUriIsAllowed(): void + { + $httpClient = static::createClient(); + $httpClient->jsonRequest('POST', '/oauth/register', [ + 'redirect_uris' => ['http://127.0.0.1:51234/callback'], + ]); + + self::assertResponseStatusCodeSame(201); + } + + public function testPrivateUseSchemeRedirectUriIsAllowed(): void + { + $httpClient = static::createClient(); + $httpClient->jsonRequest('POST', '/oauth/register', [ + 'redirect_uris' => ['com.example.myapp:/callback'], + ]); + + self::assertResponseStatusCodeSame(201); + } + + public function testNonLoopbackHttpRedirectUriIsRejected(): void + { + $httpClient = static::createClient(); + $httpClient->jsonRequest('POST', '/oauth/register', [ + 'redirect_uris' => ['http://evil.example.invalid/callback'], + ]); + + self::assertResponseStatusCodeSame(400); + $data = json_decode((string) $httpClient->getResponse()->getContent(), true); + self::assertSame('invalid_redirect_uri', $data['error']); + } + + public function testRedirectUriWithFragmentIsRejected(): void + { + $httpClient = static::createClient(); + $httpClient->jsonRequest('POST', '/oauth/register', [ + 'redirect_uris' => ['https://client.example.invalid/callback#fragment'], + ]); + + self::assertResponseStatusCodeSame(400); + } + + public function testMissingRedirectUrisIsRejected(): void + { + $httpClient = static::createClient(); + $httpClient->jsonRequest('POST', '/oauth/register', []); + + self::assertResponseStatusCodeSame(400); + $data = json_decode((string) $httpClient->getResponse()->getContent(), true); + self::assertSame('invalid_redirect_uri', $data['error']); + } + + public function testConfidentialAuthMethodIsRejected(): void + { + $httpClient = static::createClient(); + $httpClient->jsonRequest('POST', '/oauth/register', [ + 'redirect_uris' => ['https://client.example.invalid/callback'], + 'token_endpoint_auth_method' => 'client_secret_basic', + ]); + + self::assertResponseStatusCodeSame(400); + $data = json_decode((string) $httpClient->getResponse()->getContent(), true); + self::assertSame('invalid_client_metadata', $data['error']); + } + + public function testUnsupportedGrantTypeIsRejected(): void + { + $httpClient = static::createClient(); + $httpClient->jsonRequest('POST', '/oauth/register', [ + 'redirect_uris' => ['https://client.example.invalid/callback'], + 'grant_types' => ['client_credentials'], + ]); + + self::assertResponseStatusCodeSame(400); + } + + public function testUnsupportedResponseTypeIsRejected(): void + { + $httpClient = static::createClient(); + $httpClient->jsonRequest('POST', '/oauth/register', [ + 'redirect_uris' => ['https://client.example.invalid/callback'], + 'response_types' => ['token'], + ]); + + self::assertResponseStatusCodeSame(400); + } + + public function testUnknownScopeIsRejected(): void + { + $httpClient = static::createClient(); + $httpClient->jsonRequest('POST', '/oauth/register', [ + 'redirect_uris' => ['https://client.example.invalid/callback'], + 'scope' => 'superadmin', + ]); + + self::assertResponseStatusCodeSame(400); + $data = json_decode((string) $httpClient->getResponse()->getContent(), true); + self::assertSame('invalid_client_metadata', $data['error']); + } + + public function testMalformedJsonBodyIsRejected(): void + { + $httpClient = static::createClient(); + $httpClient->request('POST', '/oauth/register', [], [], ['CONTENT_TYPE' => 'application/json'], '{not json'); + + self::assertResponseStatusCodeSame(400); + } + + public function testRateLimiterRejectsAfterConfiguredLimit(): void + { + // Exercises the actual "oauth_client_registration" limiter config (config/packages/rate_limiter.yaml) + // directly, with a random per-test key, rather than firing 21 real HTTP requests against a shared + // IP-keyed bucket - that would make this test (and any other test hitting the endpoint in the same + // run/hour) flaky, since the limiter's storage persists across requests and test runs. + $httpClient = static::createClient(); + $factory = $httpClient->getContainer()->get('limiter.oauth_client_registration'); + self::assertInstanceOf(RateLimiterFactory::class, $factory); + + $limiter = $factory->create('test-'.bin2hex(random_bytes(8))); + self::assertTrue($limiter->consume(20)->isAccepted()); + self::assertFalse($limiter->consume(1)->isAccepted()); + } +} diff --git a/tests/Controller/OAuth/DiscoveryControllerTest.php b/tests/Controller/OAuth/DiscoveryControllerTest.php new file mode 100644 index 00000000..987beb86 --- /dev/null +++ b/tests/Controller/OAuth/DiscoveryControllerTest.php @@ -0,0 +1,75 @@ +. + */ + +declare(strict_types=1); + +namespace App\Tests\Controller\OAuth; + +use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; + +/** + * Drives the real unauthenticated GET /.well-known/oauth-* HTTP flow + * (App\Controller\OAuth\DiscoveryController). + */ +final class DiscoveryControllerTest extends WebTestCase +{ + public function testAuthorizationServerMetadata(): void + { + $httpClient = static::createClient(); + $httpClient->request('GET', '/.well-known/oauth-authorization-server'); + + self::assertResponseIsSuccessful(); + self::assertResponseHeaderSame('Content-Type', 'application/json'); + $data = json_decode((string) $httpClient->getResponse()->getContent(), true); + + self::assertStringEndsWith('/authorize', $data['authorization_endpoint']); + self::assertStringEndsWith('/token', $data['token_endpoint']); + self::assertStringEndsWith('/oauth/register', $data['registration_endpoint']); + self::assertSame($data['issuer'], parse_url($data['authorization_endpoint'], PHP_URL_SCHEME).'://'.parse_url($data['authorization_endpoint'], PHP_URL_HOST)); + self::assertSame(['read_only', 'edit', 'admin', 'full'], $data['scopes_supported']); + self::assertSame(['code'], $data['response_types_supported']); + self::assertSame(['authorization_code', 'refresh_token'], $data['grant_types_supported']); + self::assertSame(['none'], $data['token_endpoint_auth_methods_supported']); + self::assertSame(['S256'], $data['code_challenge_methods_supported']); + } + + public function testProtectedResourceMetadata(): void + { + $httpClient = static::createClient(); + $httpClient->request('GET', '/.well-known/oauth-protected-resource'); + + self::assertResponseIsSuccessful(); + $data = json_decode((string) $httpClient->getResponse()->getContent(), true); + + self::assertSame([$data['resource']], $data['authorization_servers']); + self::assertSame(['header'], $data['bearer_methods_supported']); + self::assertSame(['read_only', 'edit', 'admin', 'full'], $data['scopes_supported']); + } + + public function testDiscoveryEndpointsRequireNoAuthentication(): void + { + $httpClient = static::createClient(); + $httpClient->request('GET', '/.well-known/oauth-authorization-server'); + self::assertResponseStatusCodeSame(200); + + $httpClient->request('GET', '/.well-known/oauth-protected-resource'); + self::assertResponseStatusCodeSame(200); + } +} diff --git a/tests/Controller/OAuthClientAdminControllerTest.php b/tests/Controller/OAuthClientAdminControllerTest.php new file mode 100644 index 00000000..13847ae0 --- /dev/null +++ b/tests/Controller/OAuthClientAdminControllerTest.php @@ -0,0 +1,104 @@ +. + */ + +declare(strict_types=1); + +namespace App\Tests\Controller; + +use App\Entity\UserSystem\User; +use Doctrine\ORM\EntityManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; +use League\Bundle\OAuth2ServerBundle\Model\Client; +use League\Bundle\OAuth2ServerBundle\ValueObject\Grant; +use League\Bundle\OAuth2ServerBundle\ValueObject\RedirectUri; +use League\Bundle\OAuth2ServerBundle\ValueObject\Scope; +use Symfony\Bundle\FrameworkBundle\KernelBrowser; +use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; + +/** + * Drives the real GET /tools/oauth_clients list + DELETE .../delete HTTP flow + * (App\Controller\OAuthClientAdminController, App\Services\OAuth\OAuthClientAdminManager). + */ +final class OAuthClientAdminControllerTest extends WebTestCase +{ + private function loginAsAdmin(): array + { + $httpClient = static::createClient(); + $entityManager = static::getContainer()->get(EntityManagerInterface::class); + $admin = $entityManager->getRepository(User::class)->findOneBy(['name' => 'admin']); + self::assertInstanceOf(User::class, $admin); + $httpClient->loginUser($admin); + + return [$httpClient, $admin]; + } + + private function registerTestClient(KernelBrowser $httpClient): Client + { + $clientManager = $httpClient->getContainer()->get(ClientManagerInterface::class); + + $client = new Client('Admin-visible Test App', 'test-admin-'.bin2hex(random_bytes(8)), null); + $client->setRedirectUris(new RedirectUri('https://client.example.invalid/callback')); + $client->setGrants(new Grant('authorization_code'), new Grant('refresh_token')); + $client->setScopes(new Scope('read_only')); + $client->setActive(true); + $clientManager->save($client); + + return $client; + } + + public function testRegisteredClientIsListed(): void + { + [$httpClient, ] = $this->loginAsAdmin(); + $client = $this->registerTestClient($httpClient); + + $crawler = $httpClient->request('GET', '/en/tools/oauth_clients'); + self::assertResponseIsSuccessful(); + self::assertStringContainsString('Admin-visible Test App', $crawler->filter('body')->text()); + self::assertStringContainsString($client->getIdentifier(), $crawler->filter('body')->text()); + } + + public function testDeleteRemovesClient(): void + { + [$httpClient, ] = $this->loginAsAdmin(); + $client = $this->registerTestClient($httpClient); + $identifier = $client->getIdentifier(); + + $crawler = $httpClient->request('GET', '/en/tools/oauth_clients'); + $form = $crawler->selectButton('Delete')->form(); + $httpClient->submit($form); + + self::assertResponseRedirects('/en/tools/oauth_clients'); + + $clientManager = $httpClient->getContainer()->get(ClientManagerInterface::class); + self::assertNull($clientManager->find($identifier)); + } + + public function testNonAdminUserIsDenied(): void + { + $httpClient = static::createClient(); + $entityManager = static::getContainer()->get(EntityManagerInterface::class); + $user = $entityManager->getRepository(User::class)->findOneBy(['name' => 'user']); + self::assertInstanceOf(User::class, $user); + $httpClient->loginUser($user); + + $httpClient->request('GET', '/en/tools/oauth_clients'); + self::assertResponseStatusCodeSame(403); + } +} diff --git a/tests/Controller/UserSettingsControllerConnectedAppsTest.php b/tests/Controller/UserSettingsControllerConnectedAppsTest.php new file mode 100644 index 00000000..f886439b --- /dev/null +++ b/tests/Controller/UserSettingsControllerConnectedAppsTest.php @@ -0,0 +1,114 @@ +. + */ + +declare(strict_types=1); + +namespace App\Tests\Controller; + +use App\Entity\UserSystem\User; +use App\Services\OAuth\ConnectedAppManager; +use Doctrine\ORM\EntityManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\AccessTokenManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; +use League\Bundle\OAuth2ServerBundle\Model\AccessToken; +use League\Bundle\OAuth2ServerBundle\Model\Client; +use League\Bundle\OAuth2ServerBundle\ValueObject\Grant; +use League\Bundle\OAuth2ServerBundle\ValueObject\RedirectUri; +use League\Bundle\OAuth2ServerBundle\ValueObject\Scope; +use Symfony\Bundle\FrameworkBundle\KernelBrowser; +use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; + +/** + * Drives the real user-settings "connected apps" list + revoke HTTP flow + * (App\Controller\UserSettingsController::revokeConnectedApp, App\Services\OAuth\ConnectedAppManager). + */ +final class UserSettingsControllerConnectedAppsTest extends WebTestCase +{ + private function loginAsAdmin(): array + { + $httpClient = static::createClient(); + $entityManager = static::getContainer()->get(EntityManagerInterface::class); + $admin = $entityManager->getRepository(User::class)->findOneBy(['name' => 'admin']); + self::assertInstanceOf(User::class, $admin); + $httpClient->loginUser($admin); + + return [$httpClient, $admin]; + } + + private function createConnectedApp(KernelBrowser $httpClient, User $admin): Client + { + $clientManager = $httpClient->getContainer()->get(ClientManagerInterface::class); + $accessTokenManager = $httpClient->getContainer()->get(AccessTokenManagerInterface::class); + + $client = new Client('My Connected Test App', 'test-conn-http-'.bin2hex(random_bytes(8)), null); + $client->setRedirectUris(new RedirectUri('https://client.example.invalid/callback')); + $client->setGrants(new Grant('authorization_code'), new Grant('refresh_token')); + $client->setScopes(new Scope('read_only')); + $client->setActive(true); + $clientManager->save($client); + + $accessTokenManager->save(new AccessToken( + 'at-'.bin2hex(random_bytes(16)), + new \DateTimeImmutable('+1 hour'), + $client, + $admin->getUserIdentifier(), + [new Scope('read_only')], + )); + + return $client; + } + + public function testConnectedAppIsListedAndCanBeRevoked(): void + { + [$httpClient, $admin] = $this->loginAsAdmin(); + $client = $this->createConnectedApp($httpClient, $admin); + + $crawler = $httpClient->request('GET', '/en/user/settings'); + self::assertResponseIsSuccessful(); + self::assertStringContainsString('My Connected Test App', $crawler->filter('body')->text()); + + $form = $crawler->filter('button[name="client_id"][value="'.$client->getIdentifier().'"]')->form([ + 'client_id' => $client->getIdentifier(), + ]); + $httpClient->submit($form); + + self::assertResponseRedirects('/en/user/settings'); + + $connectedAppManager = $httpClient->getContainer()->get(ConnectedAppManager::class); + self::assertSame([], $connectedAppManager->listConnectedClients($admin->getUserIdentifier())); + } + + public function testRevokeWithInvalidCsrfTokenDoesNothing(): void + { + [$httpClient, $admin] = $this->loginAsAdmin(); + $client = $this->createConnectedApp($httpClient, $admin); + + $httpClient->request('POST', '/en/user/oauth_connected_apps/revoke', [ + '_method' => 'DELETE', + '_token' => 'invalid', + 'client_id' => $client->getIdentifier(), + ]); + + self::assertResponseRedirects('/en/user/settings'); + + $connectedAppManager = $httpClient->getContainer()->get(ConnectedAppManager::class); + self::assertCount(1, $connectedAppManager->listConnectedClients($admin->getUserIdentifier())); + } +} diff --git a/tests/Services/OAuth/ConnectedAppManagerTest.php b/tests/Services/OAuth/ConnectedAppManagerTest.php new file mode 100644 index 00000000..1cd08586 --- /dev/null +++ b/tests/Services/OAuth/ConnectedAppManagerTest.php @@ -0,0 +1,134 @@ +. + */ + +declare(strict_types=1); + +namespace App\Tests\Services\OAuth; + +use App\Services\OAuth\ConnectedAppManager; +use Doctrine\ORM\EntityManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\AccessTokenManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\AuthorizationCodeManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\RefreshTokenManagerInterface; +use League\Bundle\OAuth2ServerBundle\Model\AccessToken; +use League\Bundle\OAuth2ServerBundle\Model\AuthorizationCode; +use League\Bundle\OAuth2ServerBundle\Model\Client; +use League\Bundle\OAuth2ServerBundle\Model\RefreshToken; +use League\Bundle\OAuth2ServerBundle\ValueObject\Grant; +use League\Bundle\OAuth2ServerBundle\ValueObject\RedirectUri; +use League\Bundle\OAuth2ServerBundle\ValueObject\Scope; +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; + +final class ConnectedAppManagerTest extends KernelTestCase +{ + private function createClient(ClientManagerInterface $clientManager): Client + { + $client = new Client('Test Connected App', 'test-conn-'.bin2hex(random_bytes(8)), null); + $client->setRedirectUris(new RedirectUri('https://client.example.invalid/callback')); + $client->setGrants(new Grant('authorization_code'), new Grant('refresh_token')); + $client->setScopes(new Scope('read_only')); + $client->setActive(true); + $clientManager->save($client); + + return $client; + } + + public function testListAndRevokeRoundTrip(): void + { + self::bootKernel(); + $container = static::getContainer(); + + $clientManager = $container->get(ClientManagerInterface::class); + $accessTokenManager = $container->get(AccessTokenManagerInterface::class); + $refreshTokenManager = $container->get(RefreshTokenManagerInterface::class); + $authCodeManager = $container->get(AuthorizationCodeManagerInterface::class); + $entityManager = $container->get(EntityManagerInterface::class); + $manager = $container->get(ConnectedAppManager::class); + + $userIdentifier = 'test-user-'.bin2hex(random_bytes(8)); + $client = $this->createClient($clientManager); + + $accessToken = new AccessToken( + 'at-'.bin2hex(random_bytes(16)), + new \DateTimeImmutable('+1 hour'), + $client, + $userIdentifier, + [new Scope('read_only')], + ); + $accessTokenManager->save($accessToken); + + $refreshToken = new RefreshToken( + 'rt-'.bin2hex(random_bytes(16)), + new \DateTimeImmutable('+30 days'), + $accessToken, + ); + $refreshTokenManager->save($refreshToken); + + $authCode = new AuthorizationCode( + 'ac-'.bin2hex(random_bytes(16)), + new \DateTimeImmutable('+10 minutes'), + $client, + $userIdentifier, + [new Scope('read_only')], + ); + $authCodeManager->save($authCode); + + // A second, unrelated user must not see this client as connected. + self::assertSame([], $manager->listConnectedClients('someone-else')); + + $connected = $manager->listConnectedClients($userIdentifier); + self::assertCount(1, $connected); + self::assertSame($client->getIdentifier(), $connected[0]['client']->getIdentifier()); + + $manager->revokeForUserAndClient($userIdentifier, $client->getIdentifier()); + + self::assertSame([], $manager->listConnectedClients($userIdentifier)); + + $entityManager->clear(); + self::assertTrue($accessTokenManager->find($accessToken->getIdentifier())->isRevoked()); + self::assertTrue($refreshTokenManager->find($refreshToken->getIdentifier())->isRevoked()); + self::assertTrue($authCodeManager->find($authCode->getIdentifier())->isRevoked()); + } + + public function testExpiredAccessTokenIsNotListedAsConnected(): void + { + self::bootKernel(); + $container = static::getContainer(); + + $clientManager = $container->get(ClientManagerInterface::class); + $accessTokenManager = $container->get(AccessTokenManagerInterface::class); + $manager = $container->get(ConnectedAppManager::class); + + $userIdentifier = 'test-user-'.bin2hex(random_bytes(8)); + $client = $this->createClient($clientManager); + + $accessToken = new AccessToken( + 'at-'.bin2hex(random_bytes(16)), + new \DateTimeImmutable('-1 hour'), + $client, + $userIdentifier, + [new Scope('read_only')], + ); + $accessTokenManager->save($accessToken); + + self::assertSame([], $manager->listConnectedClients($userIdentifier)); + } +} diff --git a/tests/Services/OAuth/OAuthClientAdminManagerTest.php b/tests/Services/OAuth/OAuthClientAdminManagerTest.php new file mode 100644 index 00000000..f7ab5fb3 --- /dev/null +++ b/tests/Services/OAuth/OAuthClientAdminManagerTest.php @@ -0,0 +1,94 @@ +. + */ + +declare(strict_types=1); + +namespace App\Tests\Services\OAuth; + +use App\Services\OAuth\OAuthClientAdminManager; +use Doctrine\ORM\EntityManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\AccessTokenManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; +use League\Bundle\OAuth2ServerBundle\Model\AccessToken; +use League\Bundle\OAuth2ServerBundle\Model\Client; +use League\Bundle\OAuth2ServerBundle\ValueObject\Grant; +use League\Bundle\OAuth2ServerBundle\ValueObject\RedirectUri; +use League\Bundle\OAuth2ServerBundle\ValueObject\Scope; +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; + +final class OAuthClientAdminManagerTest extends KernelTestCase +{ + public function testListIncludesLiveTokenCountAndDeleteRemovesClientAndTokens(): void + { + self::bootKernel(); + $container = static::getContainer(); + + $clientManager = $container->get(ClientManagerInterface::class); + $accessTokenManager = $container->get(AccessTokenManagerInterface::class); + $manager = $container->get(OAuthClientAdminManager::class); + + $client = new Client('Admin Manager Test App', 'test-admin-mgr-'.bin2hex(random_bytes(8)), null); + $client->setRedirectUris(new RedirectUri('https://client.example.invalid/callback')); + $client->setGrants(new Grant('authorization_code'), new Grant('refresh_token')); + $client->setScopes(new Scope('read_only')); + $client->setActive(true); + $clientManager->save($client); + + $accessToken = new AccessToken( + 'at-'.bin2hex(random_bytes(16)), + new \DateTimeImmutable('+1 hour'), + $client, + 'some-user', + [new Scope('read_only')], + ); + $accessTokenManager->save($accessToken); + + $rows = $manager->listClientsWithLiveTokenCounts(); + $row = self::findRowForClient($rows, $client->getIdentifier()); + self::assertNotNull($row); + self::assertSame(1, $row['liveTokenCount']); + + self::assertTrue($manager->deleteClient($client->getIdentifier())); + self::assertNull($clientManager->find($client->getIdentifier())); + + // AccessTokenManager::find() uses EntityManager::find(), which checks the identity map before + // hitting the DB - without clearing it here, it would just hand back the same in-memory object + // we already hold, masking whether the row was actually deleted. + $container->get(EntityManagerInterface::class)->clear(); + self::assertNull($accessTokenManager->find($accessToken->getIdentifier())); + + self::assertFalse($manager->deleteClient($client->getIdentifier())); + } + + /** + * @param list $rows + * @return array{client: \League\Bundle\OAuth2ServerBundle\Model\ClientInterface, liveTokenCount: int}|null + */ + private static function findRowForClient(array $rows, string $identifier): ?array + { + foreach ($rows as $row) { + if ($row['client']->getIdentifier() === $identifier) { + return $row; + } + } + + return null; + } +} diff --git a/translations/messages.en.xlf b/translations/messages.en.xlf index ef2a775f..927ad8b4 100644 --- a/translations/messages.en.xlf +++ b/translations/messages.en.xlf @@ -13859,5 +13859,155 @@ Buerklin-API Authentication server: Deny + + + user.settings.oauth_connected_apps + Connected apps + + + + + user.settings.oauth_connected_apps.description + These applications signed in through the Part-DB OAuth login and were granted access to your account. Revoking an application here immediately ends its access; it will need to go through the login/consent screen again to reconnect. + + + + + user.settings.oauth_connected_apps.none_yet + No connected apps yet. + + + + + user.settings.oauth_connected_apps.app_name + Application + + + + + user.settings.oauth_connected_apps.access_until + Access valid until + + + + + user.settings.oauth_connected_apps.revoke + Revoke + + + + + user.settings.oauth_connected_apps.revoke.title + Do you really want to revoke access for this application? + + + + + user.settings.oauth_connected_apps.revoke.message + The application will immediately lose access to Part-DB and will have to go through the login/consent screen again to reconnect. This action can not be undone! + + + + + user.settings.oauth_connected_apps.revoked + Access revoked successfully! + + + + + perm.system.manage_oauth_clients + Manage OAuth2 clients + + + + + oauth_clients.title + OAuth2 clients + + + + + oauth_clients.description + Applications registered with Part-DB's OAuth2 server (usually through open Dynamic Client Registration, RFC 7591 - any application can self-register, so the real access control gate is the per-user consent screen a user sees before granting one of these access to their account). Deleting a client here immediately revokes all access it was ever granted and prevents it from being used again. + + + + + oauth_clients.none_yet + No OAuth2 clients registered yet. + + + + + oauth_clients.name + Name + + + + + oauth_clients.client_id + Client ID + + + + + oauth_clients.scopes + Scopes + + + + + oauth_clients.redirect_uris + Redirect URIs + + + + + oauth_clients.status + Status + + + + + oauth_clients.active + Active + + + + + oauth_clients.inactive + Inactive + + + + + oauth_clients.live_tokens + Live access tokens + + + + + oauth_clients.delete + Delete + + + + + oauth_clients.delete.title + Do you really want to delete this OAuth2 client? + + + + + oauth_clients.delete.message + This immediately revokes all access this application was ever granted, for every user. This action can not be undone; the application would have to register again to reconnect. + + + + + oauth_clients.deleted + OAuth2 client deleted successfully! + +