diff --git a/.env b/.env index 67efedce..e9699672 100644 --- a/.env +++ b/.env @@ -11,6 +11,16 @@ APP_SECRET=a03498528f5a5fc089273ec9ae5b2849 # it once set, or all outstanding refresh tokens/in-flight authorization codes become invalid. OAUTH2_ENCRYPTION_KEY=def000000623b5c5664b11de7629fa777a91638b6cf0136ff4d7ef645bdc3d7665a1d605edefcf60a0fc3951b1e74529332005a3bd3cc402f0f46cf1e34d437b0b53e63b +# Enables the OAuth2 authorization server (client auto-provisioning for API/MCP apps: /authorize, /token, +# the admin client overview at /tools/oauth_clients, and OAuth2-issued bearer token authentication). +# Disabled by default - the login/consent screen and bearer token authentication are unreachable until +# this is turned on. +OAUTH_SERVER_ENABLED=0 +# Enables open/unauthenticated Dynamic Client Registration (RFC 7591, POST /oauth/register) on top of the +# OAuth2 server above. Has no effect if OAUTH_SERVER_ENABLED is not also set to 1. Clients can still be +# registered manually by an admin via /tools/oauth_clients regardless of this setting. +OAUTH_DCR_ENABLED=0 + # Change this and uncomment the following line to set the trusted hosts for your Part-DB installation, aka under which # domain names it is reachable. This is makes things more secure, because it can prevent certain header attacks. # You have to escape dots in the domain name with a backslash diff --git a/.env.test b/.env.test index 9117ff16..e6870ce2 100644 --- a/.env.test +++ b/.env.test @@ -12,4 +12,9 @@ DATABASE_URL="sqlite:///%kernel.project_dir%/var/app_test.db" # Disable update checks, as tests would fail, when github is not reachable CHECK_FOR_UPDATES=0 -INSTANCE_NAME="Part-DB" \ No newline at end of file +INSTANCE_NAME="Part-DB" + +# The OAuth2 authorization server (and its Dynamic Client Registration endpoint) are disabled by default +# in production (see .env) - enabled here so the full test suite keeps exercising them. +OAUTH_SERVER_ENABLED=1 +OAUTH_DCR_ENABLED=1 \ No newline at end of file diff --git a/config/parameters.yaml b/config/parameters.yaml index 64c791c1..52b99ec0 100644 --- a/config/parameters.yaml +++ b/config/parameters.yaml @@ -43,6 +43,12 @@ parameters: ###################################################################################################################### partdb.saml.enabled: '%env(bool:SAML_ENABLED)%' # If this is set to true, SAML authentication is enabled + ###################################################################################################################### + # OAuth2 authorization server (API/MCP client auto-provisioning) + ###################################################################################################################### + partdb.oauth_server.enabled: '%env(bool:OAUTH_SERVER_ENABLED)%' # If set to true, enables the OAuth2 authorization server (/authorize, /token, admin client overview, OAuth2 bearer token auth) + partdb.oauth_server.dcr_enabled: '%env(bool:OAUTH_DCR_ENABLED)%' # If set to true (and partdb.oauth_server.enabled is also true), enables open Dynamic Client Registration (RFC 7591, POST /oauth/register) + ###################################################################################################################### # Miscellaneous @@ -92,6 +98,9 @@ parameters: env(DEMO_MODE): 0 + env(OAUTH_SERVER_ENABLED): 0 + env(OAUTH_DCR_ENABLED): 0 + env(EMAIL_SENDER_EMAIL): 'noreply@partdb.changeme' env(EMAIL_SENDER_NAME): 'Part-DB Mailer' diff --git a/config/routes/league_oauth2_server.yaml b/config/routes/league_oauth2_server.yaml index 7e3daeef..5f8f2fb6 100644 --- a/config/routes/league_oauth2_server.yaml +++ b/config/routes/league_oauth2_server.yaml @@ -1,3 +1,6 @@ oauth2_server: resource: '@LeagueOAuth2ServerBundle/config/routes.php' type: php + # Only load the OAuth2 authorization server's own routes (/authorize, /token, /device-code) if the + # OAuth2 server is enabled - disabled by default, see .env's OAUTH_SERVER_ENABLED. + condition: "env('OAUTH_SERVER_ENABLED') == '1' or env('OAUTH_SERVER_ENABLED') == 'true'" diff --git a/src/Controller/OAuth/ClientRegistrationController.php b/src/Controller/OAuth/ClientRegistrationController.php index e7db5b66..e5bf14ea 100644 --- a/src/Controller/OAuth/ClientRegistrationController.php +++ b/src/Controller/OAuth/ClientRegistrationController.php @@ -22,6 +22,7 @@ declare(strict_types=1); namespace App\Controller\OAuth; +use App\Services\OAuth\OAuthRedirectUriValidator; use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\ScopeManagerInterface; use League\Bundle\OAuth2ServerBundle\Model\Client; @@ -45,6 +46,10 @@ use Symfony\Component\Routing\Attribute\Route; * * 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. + * + * Only reachable if both the OAuth2 server and Dynamic Client Registration specifically are enabled + * (OAUTH_SERVER_ENABLED and OAUTH_DCR_ENABLED, both disabled by default) - see the route condition below. + * Clients can still be registered manually by an admin via /tools/oauth_clients regardless of this flag. */ #[Route('/oauth')] class ClientRegistrationController extends AbstractController @@ -66,6 +71,7 @@ class ClientRegistrationController extends AbstractController public function __construct( private readonly ClientManagerInterface $clientManager, private readonly ScopeManagerInterface $scopeManager, + private readonly OAuthRedirectUriValidator $redirectUriValidator, #[Autowire(service: 'limiter.oauth_client_registration')] private readonly RateLimiterFactory $registrationLimiter, #[Autowire('%league.oauth2_server.scopes.default%')] @@ -73,7 +79,7 @@ class ClientRegistrationController extends AbstractController ) { } - #[Route('/register', name: 'oauth2_client_register', methods: ['POST'])] + #[Route('/register', name: 'oauth2_client_register', methods: ['POST'], condition: "(env('OAUTH_SERVER_ENABLED') == '1' or env('OAUTH_SERVER_ENABLED') == 'true') and (env('OAUTH_DCR_ENABLED') == '1' or env('OAUTH_DCR_ENABLED') == 'true')")] public function register(Request $request): JsonResponse { $limit = $this->registrationLimiter->create($request->getClientIp() ?? 'unknown')->consume(); @@ -161,7 +167,7 @@ class ClientRegistrationController extends AbstractController $redirectUris = []; foreach ($redirectUrisRaw as $uri) { - if (!\is_string($uri) || \strlen($uri) > self::MAX_REDIRECT_URI_LENGTH || !$this->isAllowedRedirectUri($uri)) { + if (!\is_string($uri) || \strlen($uri) > self::MAX_REDIRECT_URI_LENGTH || !$this->redirectUriValidator->isAllowed($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; @@ -170,36 +176,6 @@ class ClientRegistrationController extends AbstractController 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 */ diff --git a/src/Controller/OAuth/DiscoveryController.php b/src/Controller/OAuth/DiscoveryController.php index e32b1518..3b285267 100644 --- a/src/Controller/OAuth/DiscoveryController.php +++ b/src/Controller/OAuth/DiscoveryController.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace App\Controller\OAuth; 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\Routing\Attribute\Route; @@ -40,27 +41,43 @@ use Symfony\Component\Routing\Generator\UrlGeneratorInterface; * 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. + * + * Only reachable at all if the OAuth2 server itself is enabled (OAUTH_SERVER_ENABLED, disabled by + * default) - see the class-level route condition below. */ -#[Route('/.well-known')] +#[Route('/.well-known', condition: "env('OAUTH_SERVER_ENABLED') == '1' or env('OAUTH_SERVER_ENABLED') == 'true'")] class DiscoveryController extends AbstractController { + public function __construct( + #[Autowire('%partdb.oauth_server.dcr_enabled%')] + private readonly bool $oauth_dcr_enabled, + ) { + } + #[Route('/oauth-authorization-server', name: 'oauth2_discovery_authorization_server', methods: ['GET'])] public function authorizationServerMetadata(Request $request): JsonResponse { $issuer = $request->getSchemeAndHttpHost(); - return new JsonResponse([ + $metadata = [ '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'], - ]); + ]; + + // Dynamic Client Registration is a separate opt-in on top of the OAuth2 server (OAUTH_DCR_ENABLED) + // - clients can still be registered manually by an admin via /tools/oauth_clients regardless. + if ($this->oauth_dcr_enabled) { + $metadata['registration_endpoint'] = $this->generateUrl('oauth2_client_register', [], UrlGeneratorInterface::ABSOLUTE_URL); + } + + return new JsonResponse($metadata); } #[Route('/oauth-protected-resource', name: 'oauth2_discovery_protected_resource', methods: ['GET'])] diff --git a/src/Controller/OAuthClientAdminController.php b/src/Controller/OAuthClientAdminController.php index 9eb1816e..a087be42 100644 --- a/src/Controller/OAuthClientAdminController.php +++ b/src/Controller/OAuthClientAdminController.php @@ -22,20 +22,26 @@ declare(strict_types=1); namespace App\Controller; +use App\Entity\UserSystem\ApiTokenLevel; 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; +use function Symfony\Component\Translation\t; + /** * 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. + * instance - via RFC 7591 self-registration if enabled (App\Controller\OAuth\ClientRegistrationController) + * or manual registration right here. This page is the moderation surface for open self-registration: an + * admin can see what has registered and permanently remove a client (and everything it was ever granted) + * if it looks abusive or abandoned - and, regardless of whether self-registration is enabled, can + * register a client by hand for a known/trusted app. + * + * Only reachable if the OAuth2 server itself is enabled (OAUTH_SERVER_ENABLED, disabled by default). */ -#[Route(path: '/tools/oauth_clients')] +#[Route(path: '/tools/oauth_clients', condition: "env('OAUTH_SERVER_ENABLED') == '1' or env('OAUTH_SERVER_ENABLED') == 'true'")] class OAuthClientAdminController extends AbstractController { public function __construct(private readonly OAuthClientAdminManager $manager) @@ -49,9 +55,38 @@ class OAuthClientAdminController extends AbstractController return $this->render('tools/oauth_clients/oauth_clients.html.twig', [ 'clients' => $this->manager->listClientsWithLiveTokenCounts(), + 'scope_levels' => ApiTokenLevel::cases(), ]); } + #[Route(path: '/create', name: 'oauth_clients_create', methods: ['POST'])] + public function create(Request $request): Response + { + $this->denyAccessUnlessGranted('@system.manage_oauth_clients'); + + if (!$this->isCsrfTokenValid('oauth_client_create', $request->request->get('_token'))) { + $this->addFlash('error', 'csfr_invalid'); + return $this->redirectToRoute('oauth_clients_list'); + } + + $name = $request->request->getString('name'); + $redirectUris = array_values(array_filter(array_map( + trim(...), + explode("\n", $request->request->getString('redirect_uris')), + ), static fn (string $uri): bool => '' !== $uri)); + $scopes = array_values(array_filter($request->request->all('scopes'), \is_string(...))); + + try { + $client = $this->manager->createClient($name, $redirectUris, $scopes); + } catch (\InvalidArgumentException $e) { + $this->addFlash('error', $e->getMessage()); + return $this->redirectToRoute('oauth_clients_list'); + } + + $this->addFlash('success', t('oauth_clients.created', ['%client_id%' => $client->getIdentifier()])); + return $this->redirectToRoute('oauth_clients_list'); + } + #[Route(path: '/{identifier}/delete', name: 'oauth_clients_delete', methods: ['DELETE'])] public function delete(string $identifier, Request $request): Response { diff --git a/src/Controller/UserSettingsController.php b/src/Controller/UserSettingsController.php index 2feb16ef..2185454a 100644 --- a/src/Controller/UserSettingsController.php +++ b/src/Controller/UserSettingsController.php @@ -39,6 +39,7 @@ use Doctrine\ORM\EntityManagerInterface; use RuntimeException; use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\Google\GoogleAuthenticatorInterface; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; +use Symfony\Component\DependencyInjection\Attribute\Autowire; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\Form\Extension\Core\Type\DateTimeType; use Symfony\Component\Form\Extension\Core\Type\EnumType; @@ -59,8 +60,13 @@ use Symfony\Component\Validator\Constraints\Length; #[Route(path: '/user')] class UserSettingsController extends AbstractController { - public function __construct(protected bool $demo_mode, protected EventDispatcherInterface $eventDispatcher, private readonly ConnectedAppManager $connectedAppManager) - { + public function __construct( + protected bool $demo_mode, + protected EventDispatcherInterface $eventDispatcher, + private readonly ConnectedAppManager $connectedAppManager, + #[Autowire('%partdb.oauth_server.enabled%')] + private readonly bool $oauth_server_enabled, + ) { } #[Route(path: '/2fa_backup_codes', name: 'show_backup_codes')] @@ -381,7 +387,8 @@ class UserSettingsController extends AbstractController 'settings_form' => $form, 'pw_form' => $pw_form, 'global_reload_needed' => $page_need_reload, - 'connected_apps' => $this->connectedAppManager->listConnectedClients($user->getUserIdentifier()), + 'oauth_server_enabled' => $this->oauth_server_enabled, + 'connected_apps' => $this->oauth_server_enabled ? $this->connectedAppManager->listConnectedClients($user->getUserIdentifier()) : [], 'google_form' => $google_form, 'backup_form' => $backup_form, diff --git a/src/Security/OAuth/OAuthBearerAuthenticator.php b/src/Security/OAuth/OAuthBearerAuthenticator.php index 70971cbc..36198ce5 100644 --- a/src/Security/OAuth/OAuthBearerAuthenticator.php +++ b/src/Security/OAuth/OAuthBearerAuthenticator.php @@ -59,6 +59,11 @@ use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface * authentication with a 401. Restricting each authenticator to the token shapes it actually owns * (ApiTokenAuthenticator: our own "tcp_..." Personal Access Tokens; this class: everything else, i.e. * OAuth2-issued JWTs) avoids that collision entirely. See App\Entity\UserSystem\ApiTokenType::isRecognizedToken(). + * + * Also refuses to authenticate anything at all while the OAuth2 server is disabled (OAUTH_SERVER_ENABLED, + * disabled by default) - so any previously-issued OAuth2 token immediately stops working the moment the + * server is turned off, the same way its own /authorize, /token etc. routes stop being reachable (see + * config/routes/league_oauth2_server.yaml's route condition). */ class OAuthBearerAuthenticator implements AuthenticatorInterface, AuthenticationEntryPointInterface { @@ -74,11 +79,17 @@ class OAuthBearerAuthenticator implements AuthenticatorInterface, Authentication #[Autowire(service: 'security.user.provider.concrete.app_user_provider')] private readonly UserProviderInterface $userProvider, private readonly OAuthClientGrantPreferenceManager $grantPreferences, + #[Autowire('%partdb.oauth_server.enabled%')] + private readonly bool $oauth_server_enabled, ) { } public function supports(Request $request): bool { + if (!$this->oauth_server_enabled) { + return false; + } + $header = $request->headers->get('Authorization', ''); if (!str_starts_with((string) $header, 'Bearer ')) { return false; diff --git a/src/Services/OAuth/OAuthClientAdminManager.php b/src/Services/OAuth/OAuthClientAdminManager.php index 5772439f..3f90e9c0 100644 --- a/src/Services/OAuth/OAuthClientAdminManager.php +++ b/src/Services/OAuth/OAuthClientAdminManager.php @@ -24,11 +24,16 @@ namespace App\Services\OAuth; use Doctrine\ORM\EntityManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; +use League\Bundle\OAuth2ServerBundle\Manager\ScopeManagerInterface; use League\Bundle\OAuth2ServerBundle\Model\AbstractClient; use League\Bundle\OAuth2ServerBundle\Model\AccessToken; use League\Bundle\OAuth2ServerBundle\Model\AuthorizationCode; +use League\Bundle\OAuth2ServerBundle\Model\Client; use League\Bundle\OAuth2ServerBundle\Model\ClientInterface; use League\Bundle\OAuth2ServerBundle\Model\RefreshToken; +use League\Bundle\OAuth2ServerBundle\ValueObject\Grant; +use League\Bundle\OAuth2ServerBundle\ValueObject\RedirectUri; +use League\Bundle\OAuth2ServerBundle\ValueObject\Scope; /** * Backs the admin OAuth2 client overview (App\Controller\OAuthClientAdminController) - listing every @@ -48,12 +53,67 @@ use League\Bundle\OAuth2ServerBundle\Model\RefreshToken; */ class OAuthClientAdminManager { + private const MAX_CLIENT_NAME_LENGTH = 128; + public function __construct( private readonly EntityManagerInterface $entityManager, private readonly ClientManagerInterface $clientManager, + private readonly ScopeManagerInterface $scopeManager, + private readonly OAuthRedirectUriValidator $redirectUriValidator, ) { } + /** + * Manually registers a public (secret-less) client with a fixed authorization_code + refresh_token + * grant set - the same shape App\Controller\OAuth\ClientRegistrationController's RFC 7591 endpoint + * produces, just created directly by an admin instead of by the client itself. Useful when Dynamic + * Client Registration is disabled (OAUTH_DCR_ENABLED) or simply not desired for a particular app. + * + * @param list $redirectUris + * @param list $scopeNames + * + * @throws \InvalidArgumentException if $name, $redirectUris or $scopeNames are invalid - the message + * is safe to show directly to the (admin-only) user of this form + */ + public function createClient(string $name, array $redirectUris, array $scopeNames): ClientInterface + { + $name = trim($name); + if ('' === $name) { + throw new \InvalidArgumentException('Name must not be empty.'); + } + $name = mb_substr($name, 0, self::MAX_CLIENT_NAME_LENGTH); + + if ([] === $redirectUris) { + throw new \InvalidArgumentException('At least one redirect URI is required.'); + } + foreach ($redirectUris as $uri) { + if (!$this->redirectUriValidator->isAllowed($uri)) { + throw new \InvalidArgumentException(\sprintf('"%s" is not an allowed redirect URI.', $uri)); + } + } + + if ([] === $scopeNames) { + throw new \InvalidArgumentException('At least one scope is required.'); + } + foreach ($scopeNames as $scopeName) { + if (null === $this->scopeManager->find($scopeName)) { + throw new \InvalidArgumentException(\sprintf('Unknown scope "%s".', $scopeName)); + } + } + + $identifier = bin2hex(random_bytes(16)); + + $client = new Client($name, $identifier, null); + $client->setRedirectUris(...array_map(static fn (string $uri): RedirectUri => new RedirectUri($uri), $redirectUris)); + $client->setGrants(new Grant('authorization_code'), new Grant('refresh_token')); + $client->setScopes(...array_map(static fn (string $scope): Scope => new Scope($scope), $scopeNames)); + $client->setActive(true); + + $this->clientManager->save($client); + + return $client; + } + /** * @return list */ diff --git a/src/Services/OAuth/OAuthRedirectUriValidator.php b/src/Services/OAuth/OAuthRedirectUriValidator.php new file mode 100644 index 00000000..21bbc973 --- /dev/null +++ b/src/Services/OAuth/OAuthRedirectUriValidator.php @@ -0,0 +1,61 @@ +. + */ + +declare(strict_types=1); + +namespace App\Services\OAuth; + +/** + * Validates OAuth2 client redirect URIs - shared between RFC 7591 self-registration + * (App\Controller\OAuth\ClientRegistrationController) and manual admin registration + * (App\Controller\OAuthClientAdminController), so both paths accept exactly the same redirect URI shapes. + */ +class OAuthRedirectUriValidator +{ + /** + * 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. + */ + public function isAllowed(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); + } +} diff --git a/templates/tools/oauth_clients/oauth_clients.html.twig b/templates/tools/oauth_clients/oauth_clients.html.twig index 02fd54e4..3fdb2745 100644 --- a/templates/tools/oauth_clients/oauth_clients.html.twig +++ b/templates/tools/oauth_clients/oauth_clients.html.twig @@ -9,6 +9,44 @@ {% block card_content %}

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

+
+
{% trans %}oauth_clients.create.title{% endtrans %}
+
+
+ + +
+ + +
+ +
+ + +
{% trans %}oauth_clients.create.redirect_uris.help{% endtrans %}
+
+ +
+ + {% for level in scope_levels %} +
+ + +
+ {% endfor %} +
+ + +
+
+
+ {% if clients is empty %} {% trans %}oauth_clients.none_yet{% endtrans %} {% else %} diff --git a/templates/users/user_settings.html.twig b/templates/users/user_settings.html.twig index 12db959e..69189412 100644 --- a/templates/users/user_settings.html.twig +++ b/templates/users/user_settings.html.twig @@ -79,6 +79,8 @@ {% if is_granted("@api.access_api") %} {% include "users/_api_tokens.html.twig" %} - {% include "users/_oauth_connected_apps.html.twig" %} + {% if oauth_server_enabled %} + {% include "users/_oauth_connected_apps.html.twig" %} + {% endif %} {% endif %} {% endblock %} diff --git a/tests/Controller/OAuth/OAuthServerFeatureFlagTest.php b/tests/Controller/OAuth/OAuthServerFeatureFlagTest.php new file mode 100644 index 00000000..36f1564b --- /dev/null +++ b/tests/Controller/OAuth/OAuthServerFeatureFlagTest.php @@ -0,0 +1,223 @@ +. + */ + +declare(strict_types=1); + +namespace App\Tests\Controller\OAuth; + +use App\Entity\UserSystem\User; +use Doctrine\ORM\EntityManagerInterface; +use League\Bundle\OAuth2ServerBundle\Entity\User as LeagueUser; +use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; +use League\Bundle\OAuth2ServerBundle\Model\Client as OAuthClientModel; +use League\Bundle\OAuth2ServerBundle\ValueObject\Grant; +use League\Bundle\OAuth2ServerBundle\ValueObject\RedirectUri; +use League\Bundle\OAuth2ServerBundle\ValueObject\Scope; +use League\OAuth2\Server\AuthorizationServer; +use Nyholm\Psr7\Factory\Psr17Factory; +use Symfony\Bundle\FrameworkBundle\KernelBrowser; +use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; + +/** + * Covers OAUTH_SERVER_ENABLED / OAUTH_DCR_ENABLED (config/parameters.yaml, disabled by default - see + * .env). .env.test overrides both to enabled so the rest of the OAuth test suite keeps exercising the + * full feature; this file specifically flips them back off (via $_SERVER/$_ENV, which + * Symfony\Component\DependencyInjection\Container::getEnv() and routing's env() condition function both + * resolve dynamically per request - no cached/dumped container value takes precedence) to verify the + * *disabled* paths, restoring them in a finally block so no other test observes the override. + * + * Unauthenticated requests to /authorize always get a 401 from the firewall's own access_control + * (^/authorize requires IS_AUTHENTICATED_FULLY) *before* routing ever gets a chance to 404 the disabled + * route - so tests that need to see the true disabled-routing behavior for /authorize log in first and + * follow redirects (the route falls through to the app's locale-redirect catch-all, then 404s). + */ +final class OAuthServerFeatureFlagTest extends WebTestCase +{ + private function withEnv(array $vars, callable $callback): mixed + { + $previous = []; + foreach ($vars as $name => $value) { + $previous[$name] = $_SERVER[$name] ?? null; + $_SERVER[$name] = $value; + $_ENV[$name] = $value; + } + + try { + return $callback(); + } finally { + foreach ($previous as $name => $value) { + if (null === $value) { + unset($_SERVER[$name], $_ENV[$name]); + } else { + $_SERVER[$name] = $value; + $_ENV[$name] = $value; + } + } + } + } + + public function testProtocolEndpointsAreUnreachableWhenServerDisabled(): void + { + $this->withEnv(['OAUTH_SERVER_ENABLED' => '0', 'OAUTH_DCR_ENABLED' => '0'], function (): void { + $httpClient = static::createClient(); + // The unmatched routes fall through to the app's locale-redirect catch-all + // (App\Controller\RedirectController::addLocalePart()) before ultimately 404ing. + $httpClient->followRedirects(); + + // Public endpoints - no login needed to see the true routing behavior. + $httpClient->request('POST', '/token'); + self::assertResponseStatusCodeSame(404); + + $httpClient->request('POST', '/oauth/register', ['json' => []]); + self::assertResponseStatusCodeSame(404); + + $httpClient->request('GET', '/.well-known/oauth-authorization-server'); + self::assertResponseStatusCodeSame(404); + + $httpClient->request('GET', '/.well-known/oauth-protected-resource'); + self::assertResponseStatusCodeSame(404); + + // /authorize requires login before routing gets a chance to reject it (access_control matches + // on path, independent of whether the route beneath it actually exists). + $entityManager = static::getContainer()->get(EntityManagerInterface::class); + $admin = $entityManager->getRepository(User::class)->findOneBy(['name' => 'admin']); + self::assertInstanceOf(User::class, $admin); + $httpClient->loginUser($admin); + $httpClient->request('GET', '/authorize'); + self::assertResponseStatusCodeSame(404); + + $httpClient->request('GET', '/en/tools/oauth_clients'); + self::assertResponseStatusCodeSame(404); + }); + } + + public function testDcrEndpointRequiresBothFlagsIndependentlyOfServerAdminUi(): void + { + $this->withEnv(['OAUTH_SERVER_ENABLED' => '1', 'OAUTH_DCR_ENABLED' => '0'], function (): void { + $httpClient = static::createClient(); + $httpClient->followRedirects(); + + // DCR specifically off... + $httpClient->request('POST', '/oauth/register', ['json' => []]); + self::assertResponseStatusCodeSame(404); + + // ...but the rest of the server (and the admin UI, where clients can still be registered + // manually) keeps working. + $entityManager = static::getContainer()->get(EntityManagerInterface::class); + $admin = $entityManager->getRepository(User::class)->findOneBy(['name' => 'admin']); + self::assertInstanceOf(User::class, $admin); + $httpClient->loginUser($admin); + + $httpClient->request('GET', '/en/tools/oauth_clients'); + self::assertResponseIsSuccessful(); + + $httpClient->request('GET', '/.well-known/oauth-authorization-server'); + $discovery = json_decode( + (string) $httpClient->getResponse()->getContent(), + true, + flags: JSON_THROW_ON_ERROR, + ); + self::assertArrayNotHasKey('registration_endpoint', $discovery); + }); + } + + public function testDiscoveryAdvertisesRegistrationEndpointWhenDcrEnabled(): void + { + $httpClient = static::createClient(); + + $httpClient->request('GET', '/.well-known/oauth-authorization-server'); + $discovery = json_decode( + (string) $httpClient->getResponse()->getContent(), + true, + flags: JSON_THROW_ON_ERROR, + ); + self::assertArrayHasKey('registration_endpoint', $discovery); + } + + private function createTestClient(KernelBrowser $client, string $redirectUri): OAuthClientModel + { + $oauthClient = new OAuthClientModel('Test Client', 'test-client-'.bin2hex(random_bytes(8)), null); + $oauthClient->setRedirectUris(new RedirectUri($redirectUri)); + $oauthClient->setGrants(new Grant('authorization_code'), new Grant('refresh_token')); + $oauthClient->setScopes(new Scope('read_only')); + $oauthClient->setActive(true); + + $client->getContainer()->get(ClientManagerInterface::class)->save($oauthClient); + + return $oauthClient; + } + + public function testOAuthBearerTokenStopsAuthenticatingWhenServerDisabled(): void + { + // Issue a real token while the server is enabled (the test-env default). + $httpClient = static::createClient(); + $redirectUri = 'https://client.example.invalid/callback'; + $oauthClient = $this->createTestClient($httpClient, $redirectUri); + + $authServer = $httpClient->getContainer()->get(AuthorizationServer::class); + $psr17 = new Psr17Factory(); + $verifier = rtrim(strtr(base64_encode(random_bytes(40)), '+/', '-_'), '='); + $challenge = rtrim(strtr(base64_encode(hash('sha256', $verifier, true)), '+/', '-_'), '='); + + $authorizeRequest = $psr17->createServerRequest('GET', 'https://part-db.test/authorize?'.http_build_query([ + 'response_type' => 'code', + 'client_id' => $oauthClient->getIdentifier(), + 'redirect_uri' => $redirectUri, + 'code_challenge' => $challenge, + 'code_challenge_method' => 'S256', + 'scope' => 'read_only', + ])); + $authRequest = $authServer->validateAuthorizationRequest($authorizeRequest); + $leagueUser = new LeagueUser(); + $leagueUser->setIdentifier('admin'); + $authRequest->setUser($leagueUser); + $authRequest->setAuthorizationApproved(true); + $redirectResponse = $authServer->completeAuthorizationRequest($authRequest, $psr17->createResponse()); + parse_str((string) parse_url($redirectResponse->getHeaderLine('Location'), PHP_URL_QUERY), $query); + + $tokenRequest = $psr17->createServerRequest('POST', 'https://part-db.test/token')->withParsedBody([ + 'grant_type' => 'authorization_code', + 'client_id' => $oauthClient->getIdentifier(), + 'redirect_uri' => $redirectUri, + 'code' => $query['code'], + 'code_verifier' => $verifier, + ]); + $tokenResponse = $authServer->respondToAccessTokenRequest($tokenRequest, $psr17->createResponse()); + $data = json_decode((string) $tokenResponse->getBody(), true, flags: JSON_THROW_ON_ERROR); + + // Sanity check: the token works while the server is enabled. + $httpClient->request('GET', '/api/parts', [], [], ['HTTP_AUTHORIZATION' => 'Bearer '.$data['access_token']]); + self::assertResponseIsSuccessful(); + + // Now disable the server and boot a *fresh* kernel/container - OAuthBearerAuthenticator's + // "%partdb.oauth_server.enabled%"-bound constructor parameter is resolved once, when the + // (singleton) service is first instantiated within a container's lifetime, so an already-booted + // container/authenticator instance would keep the old value; only a fresh boot picks up the + // override (unlike route conditions, which call env() fresh on every single request regardless of + // container reuse). The access token itself lives in the shared test database, so it's still + // there for this fresh container to find. + $this->withEnv(['OAUTH_SERVER_ENABLED' => '0'], function () use ($data): void { + static::ensureKernelShutdown(); + $freshClient = static::createClient(); + $freshClient->request('GET', '/api/parts', [], [], ['HTTP_AUTHORIZATION' => 'Bearer '.$data['access_token']]); + self::assertResponseStatusCodeSame(401); + }); + } +} diff --git a/tests/Controller/OAuthClientAdminControllerTest.php b/tests/Controller/OAuthClientAdminControllerTest.php index 13847ae0..ea79a5b7 100644 --- a/tests/Controller/OAuthClientAdminControllerTest.php +++ b/tests/Controller/OAuthClientAdminControllerTest.php @@ -101,4 +101,118 @@ final class OAuthClientAdminControllerTest extends WebTestCase $httpClient->request('GET', '/en/tools/oauth_clients'); self::assertResponseStatusCodeSame(403); } + + public function testManualRegistrationCreatesPublicClient(): void + { + [$httpClient, ] = $this->loginAsAdmin(); + + $crawler = $httpClient->request('GET', '/en/tools/oauth_clients'); + $form = $crawler->selectButton('Register client')->form([ + 'name' => 'Manually Registered App', + 'redirect_uris' => "https://client.example.invalid/callback\nhttps://client.example.invalid/other", + 'scopes' => ['read_only', 'edit'], + ]); + $httpClient->submit($form); + + self::assertResponseRedirects('/en/tools/oauth_clients'); + $crawler = $httpClient->followRedirect(); + self::assertStringContainsString('Manually Registered App', $crawler->filter('body')->text()); + + $clientManager = $httpClient->getContainer()->get(ClientManagerInterface::class); + $clients = $clientManager->list(null); + $created = null; + foreach ($clients as $client) { + if ('Manually Registered App' === $client->getName()) { + $created = $client; + } + } + self::assertNotNull($created); + self::assertNull($created->getSecret()); + self::assertFalse($created->isConfidential()); + self::assertTrue($created->isActive()); + + $scopeNames = array_map(static fn (Scope $s) => (string) $s, $created->getScopes()); + sort($scopeNames); + self::assertSame(['edit', 'read_only'], $scopeNames); + + $grantNames = array_map(static fn (Grant $g) => (string) $g, $created->getGrants()); + sort($grantNames); + self::assertSame(['authorization_code', 'refresh_token'], $grantNames); + + $redirectUris = array_map(static fn (RedirectUri $r) => (string) $r, $created->getRedirectUris()); + self::assertSame( + ['https://client.example.invalid/callback', 'https://client.example.invalid/other'], + $redirectUris, + ); + } + + public function testManualRegistrationRejectsMissingName(): void + { + [$httpClient, ] = $this->loginAsAdmin(); + $before = \count($httpClient->getContainer()->get(ClientManagerInterface::class)->list(null)); + + $crawler = $httpClient->request('GET', '/en/tools/oauth_clients'); + $form = $crawler->selectButton('Register client')->form([ + 'name' => '', + 'redirect_uris' => 'https://client.example.invalid/callback', + 'scopes' => ['read_only'], + ]); + $httpClient->submit($form); + + self::assertResponseRedirects('/en/tools/oauth_clients'); + $after = \count($httpClient->getContainer()->get(ClientManagerInterface::class)->list(null)); + self::assertSame($before, $after); + } + + public function testManualRegistrationRejectsInvalidRedirectUri(): void + { + [$httpClient, ] = $this->loginAsAdmin(); + $before = \count($httpClient->getContainer()->get(ClientManagerInterface::class)->list(null)); + + $crawler = $httpClient->request('GET', '/en/tools/oauth_clients'); + $form = $crawler->selectButton('Register client')->form([ + 'name' => 'Bad Redirect App', + 'redirect_uris' => 'http://evil.example.invalid/callback', + 'scopes' => ['read_only'], + ]); + $httpClient->submit($form); + + self::assertResponseRedirects('/en/tools/oauth_clients'); + $after = \count($httpClient->getContainer()->get(ClientManagerInterface::class)->list(null)); + self::assertSame($before, $after); + } + + public function testManualRegistrationRejectsNoScopesSelected(): void + { + [$httpClient, ] = $this->loginAsAdmin(); + $before = \count($httpClient->getContainer()->get(ClientManagerInterface::class)->list(null)); + + $crawler = $httpClient->request('GET', '/en/tools/oauth_clients'); + $form = $crawler->selectButton('Register client')->form([ + 'name' => 'No Scopes App', + 'redirect_uris' => 'https://client.example.invalid/callback', + ]); + $httpClient->submit($form); + + self::assertResponseRedirects('/en/tools/oauth_clients'); + $after = \count($httpClient->getContainer()->get(ClientManagerInterface::class)->list(null)); + self::assertSame($before, $after); + } + + public function testManualRegistrationRejectsInvalidCsrfToken(): void + { + [$httpClient, ] = $this->loginAsAdmin(); + $before = \count($httpClient->getContainer()->get(ClientManagerInterface::class)->list(null)); + + $httpClient->request('POST', '/en/tools/oauth_clients/create', [ + '_token' => 'invalid', + 'name' => 'Csrf Bypass App', + 'redirect_uris' => 'https://client.example.invalid/callback', + 'scopes' => ['read_only'], + ]); + + self::assertResponseRedirects('/en/tools/oauth_clients'); + $after = \count($httpClient->getContainer()->get(ClientManagerInterface::class)->list(null)); + self::assertSame($before, $after); + } } diff --git a/translations/messages.en.xlf b/translations/messages.en.xlf index 149e90ea..820e1e4b 100644 --- a/translations/messages.en.xlf +++ b/translations/messages.en.xlf @@ -14069,5 +14069,47 @@ Buerklin-API Authentication server: OAuth2 client deleted successfully! + + + oauth_clients.create.title + Register a new client manually + + + + + oauth_clients.create.name + Name + + + + + oauth_clients.create.redirect_uris + Redirect URIs + + + + + oauth_clients.create.redirect_uris.help + One URI per line. Must be https://, a loopback http:// address (127.0.0.1/localhost), or a private-use URI scheme (e.g. com.example.app:/callback). + + + + + oauth_clients.create.scopes + Allowed access levels + + + + + oauth_clients.create.submit + Register client + + + + + oauth_clients.created + OAuth2 client registered successfully! Client ID: %client_id% + +