feat: gate oauth2 authorization on per-client roles

config/packages/league_oauth2_server.yaml

@@ -19,6 +19,9 @@ league_oauth2_server:
         doctrine: null
     client:
         allow_plaintext_secrets: false
+        # App\Entity\OAuth2Client adds the required_roles column that
+        # App\EventListener\AuthorizationCodeListener gates authorization on.
+        classname: App\Entity\OAuth2Client
 
 when@test:
     league_oauth2_server:

migrations/Version20260923120000.php

@@ -0,0 +1,60 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DoctrineMigrations;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+/**
+ * Adds the per-client role requirement that App\EventListener\AuthorizationCodeListener
+ * gates the authorization code grant on.
+ *
+ * The column defaults to an empty list and an empty list denies, so every client is
+ * refused until its roles are set. The two clients in actual use are therefore set here
+ * rather than by hand after the deploy: it closes the window in which both staff tools
+ * would be live but unable to authorize anyone, and it keeps the role strings somewhere
+ * review can see them.
+ *
+ * Matched on name, not identifier: client identifiers are generated per environment and
+ * differ between local, staging and production, while the names do not.
+ *
+ * Deliberately left empty, and so denied: myepapp (the mobile app) and myep (the Keycloak
+ * broker), both R&D only and not in use. The client_credentials clients never reach the
+ * authorization endpoint and are unaffected either way.
+ */
+final class Version20260923120000 extends AbstractMigration
+{
+    public function getDescription(): string
+    {
+        return 'Adds oauth2_client.required_roles and sets it for the two staff clients.';
+    }
+
+    public function up(Schema $schema): void
+    {
+        // Added nullable and backfilled before being tightened: a NOT NULL column with no
+        // default leaves existing rows holding an empty string, which is not valid JSON and
+        // fails to hydrate -- the row cannot even reach the listener that would deny it.
+        $this->addSql("ALTER TABLE oauth2_client ADD required_roles JSON DEFAULT NULL COMMENT '(DC2Type:json)'");
+        $this->addSql("UPDATE oauth2_client SET required_roles = '[]'");
+        $this->addSql("ALTER TABLE oauth2_client MODIFY required_roles JSON NOT NULL COMMENT '(DC2Type:json)'");
+
+        // mirrors ephub's App\Security\Role::ELIGIBLE
+        $this->addSql('UPDATE oauth2_client SET required_roles = :roles WHERE name = :name', [
+            'roles' => json_encode(['ROLE_EMPLOYEE', 'ROLE_ADMIN'], JSON_THROW_ON_ERROR),
+            'name' => 'EPHub',
+        ]);
+
+        // mirrors myep-team's App\Security\MyEpAuthenticator::ELIGIBLE_ROLES
+        $this->addSql('UPDATE oauth2_client SET required_roles = :roles WHERE name = :name', [
+            'roles' => json_encode(['ROLE_TEAM_ADMIN', 'ROLE_TEAMER', 'ROLE_MANAGER', 'ROLE_HOUSE_MANAGER'], JSON_THROW_ON_ERROR),
+            'name' => 'MyEPTeam',
+        ]);
+    }
+
+    public function down(Schema $schema): void
+    {
+        $this->addSql('ALTER TABLE oauth2_client DROP required_roles');
+    }
+}

src/Entity/OAuth2Client.php

@@ -0,0 +1,70 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Entity;
+
+use Doctrine\ORM\Mapping as ORM;
+use League\Bundle\OAuth2ServerBundle\Model\AbstractClient;
+
+/**
+ * An OAuth2 client, extended with the roles an account must hold to authorize it.
+ *
+ * The bundle ships its own Client model and maps it for us. Replacing it (via
+ * league_oauth2_server.client.classname) is the supported way to add a column: the bundle
+ * maps AbstractClient as a mapped superclass and carries name, secret, redirectUris,
+ * grants, scopes, active and allowPlainTextPkce, then stops mapping its own Client, so
+ * this class only has to declare the table, the primary key and what it adds.
+ *
+ * Why the requirement lives here rather than in configuration: the alternative was a map
+ * in services.yaml keyed on the client identifier, and identifiers differ per environment,
+ * so that map would have needed an env var per client and would have drifted from the
+ * client table it describes. A column cannot drift from its own row.
+ *
+ * Nothing validates the contents. A misspelled role is not an error, it is a client that
+ * authorizes nobody, so AuthorizationCodeListener logs the requirement whenever it
+ * refuses — that is what makes the mistake visible without a database query.
+ */
+#[ORM\Entity]
+#[ORM\Table(name: 'oauth2_client')]
+class OAuth2Client extends AbstractClient
+{
+    /**
+     * Declared protected on AbstractClient and mapped by the bundle only for its own
+     * Client, so the custom class has to map it itself. The type restates the parent's
+     * contract, which redeclaring the property would otherwise widen.
+     *
+     * @var non-empty-string
+     */
+    #[ORM\Id]
+    #[ORM\Column(type: 'string', length: 32)]
+    protected string $identifier;
+
+    /**
+     * The roles that entitle an account to authorize this client. An empty list denies
+     * everyone: a row nobody has configured and a client that needs no roles are the same
+     * stored value, and of the two readings only "deny" is safe.
+     *
+     * @var list<string>
+     */
+    #[ORM\Column(type: 'json')]
+    private array $requiredRoles = [];
+
+    /**
+     * @return list<string>
+     */
+    public function getRequiredRoles(): array
+    {
+        return $this->requiredRoles;
+    }
+
+    /**
+     * @param list<string> $requiredRoles
+     */
+    public function setRequiredRoles(array $requiredRoles): self
+    {
+        $this->requiredRoles = $requiredRoles;
+
+        return $this;
+    }
+}

src/EventListener/AuthorizationCodeListener.php

@@ -4,7 +4,9 @@ declare(strict_types=1);
 
 namespace App\EventListener;
 
+use App\Entity\OAuth2Client;
 use App\Entity\User;
+use App\Security\Role;
 use League\Bundle\OAuth2ServerBundle\Event\AuthorizationRequestResolveEvent;
 use League\Bundle\OAuth2ServerBundle\OAuth2Events;
 use Psr\Log\LoggerInterface;
@@ -14,14 +16,21 @@ use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
+use Twig\Environment;
 
 /**
  * Handles OAuth2 authorization code grant flow.
  *
- * Automatically approves authorization requests for authenticated users without
- * consent prompts. Redirects anonymous users to login. For external OAuth2
- * requests (flagged via session), logs out the user after authorization to
- * prevent session hijacking.
+ * Redirects anonymous users to login, then decides whether the account may authorize the
+ * client that asked. Consent is deliberately not part of that decision: every client is
+ * one of our own applications, so there is nothing for the user to weigh up. What the user
+ * cannot be asked, though, is whether they are entitled to the application at all, and
+ * that is what this listener answers — a client declares the roles it requires in
+ * OAuth2Client::$requiredRoles and an account holding none of them is refused here rather
+ * than handed a token the client will reject for reasons it cannot explain.
+ *
+ * For external OAuth2 requests (flagged via session), logs out the user after a successful
+ * authorization to prevent session hijacking.
  */
 #[AsEventListener(event: OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE, method: 'onAuthorizationRequestResolve')]
 class AuthorizationCodeListener
@@ -31,6 +40,7 @@ class AuthorizationCodeListener
         private readonly RequestStack $requestStack,
         private readonly Security $security,
         private readonly LoggerInterface $authenticationLogger,
+        private readonly Environment $twig,
     ) {
     }
 
@@ -53,13 +63,81 @@ class AuthorizationCodeListener
             );
             $event->setResponse($response);
             $this->authenticationLogger->info('Authorization request without session');
-        } else {
-            $event->resolveAuthorization(AuthorizationRequestResolveEvent::AUTHORIZATION_APPROVED);
-            // in case this authorization request has been flagged as external in login controller,
-            // immediately logout the current user (see App\Controller\Core\Security\LoginController).
-            if (true === $request->getSession()->get('_oauth2', false)) {
-                $this->security->logout(false);
-            }
+
+            return;
+        }
+
+        $client = $event->getClient();
+
+        // the event is typed to the bundle's ClientInterface, which knows nothing about the
+        // column below: anything that is not our entity is a client we cannot have configured
+        if (false === $client instanceof OAuth2Client) {
+            $this->deny($event, $user, $client->getIdentifier(), [], 'unexpected client class');
+
+            return;
+        }
+
+        $required = $client->getRequiredRoles();
+
+        // An empty requirement is not "no requirement": it is the state a client row starts
+        // in, so it means nobody has said who may use this client yet.
+        if ([] === $required) {
+            $this->deny($event, $user, $client->getName(), $required, 'client has no roles configured');
+
+            return;
+        }
+
+        // effectiveOnly(), never getRoles(): a *_PENDING marker is a CRM nomination that
+        // grants nothing until an administrator approves it, and matching the raw role set
+        // would let a nomination open a staff application on its own.
+        if ([] === array_intersect($required, Role::effectiveOnly($user->getRoles()))) {
+            $this->deny($event, $user, $client->getName(), $required, 'insufficient roles');
+
+            return;
+        }
+
+        $event->resolveAuthorization(AuthorizationRequestResolveEvent::AUTHORIZATION_APPROVED);
+
+        // in case this authorization request has been flagged as external in login controller,
+        // immediately logout the current user (see App\Controller\SecurityController).
+        if (true === $request->getSession()->get('_oauth2', false)) {
+            $this->security->logout(false);
         }
     }
+
+    /**
+     * Refuses the request and explains it on our own page.
+     *
+     * The resolution is left at its AUTHORIZATION_DENIED default; setting a response is what
+     * keeps the user here instead of bouncing them back to a client that would only be able
+     * to report that no authorization code arrived. The session is deliberately left intact
+     * — the user has to still be signed in to read the page and navigate away from it.
+     *
+     * Logged with the client's requirement but never with the user's roles: the requirement
+     * is configuration, and it is what makes a misspelled entry diagnosable from the log,
+     * while the account's roles are personal data and the auth channel is database-backed.
+     *
+     * @param list<string> $required
+     */
+    private function deny(
+        AuthorizationRequestResolveEvent $event,
+        User $user,
+        string $client,
+        array $required,
+        string $reason,
+    ): void {
+        $this->authenticationLogger->info('Authorization request denied', [
+            'client' => $client,
+            'user_id' => $user->getId(),
+            'required_roles' => $required,
+            'reason' => $reason,
+        ]);
+
+        $event->setResponse(new Response(
+            // oauth2: true drops hx-boost in base.html.twig -- this page is rendered
+            // mid-redirect-chain, where boosting breaks the navigation
+            $this->twig->render('security/oauth2_denied.html.twig', ['client' => $client, 'oauth2' => true]),
+            Response::HTTP_FORBIDDEN
+        ));
+    }
 }

templates/security/oauth2_denied.html.twig

@@ -0,0 +1,23 @@
+{% extends 'layout.html.twig' %}
+
+{% block title %}Kein Zugriff{% endblock %}
+
+{% block content %}
+    <div class="px-4 lg:px-8 py-8 lg:py-16 max-w-2xl">
+        <h1 class="text-white uppercase pb-4 mb-8 border-b border-primary-bg/40">
+            Kein Zugriff
+        </h1>
+        <p class="mb-4 text-white">
+            Du bist bei MyE&amp;P angemeldet, aber dein Konto ist nicht für
+            <strong>{{ client }}</strong> freigeschaltet.
+        </p>
+        <p class="mb-8 text-white">
+            Das liegt an den Berechtigungen deines Kontos, nicht an deinem Passwort &ndash;
+            ein erneuter Login ändert daran nichts. Wenn du hier Zugriff brauchst, wende
+            dich bitte an dein E&amp;P-Team.
+        </p>
+        <a href="{{ path('app_account') }}" class="button button--primary">
+            Zurück zu MyE&amp;P
+        </a>
+    </div>
+{% endblock %}

tests/EventListener/AuthorizationCodeListenerTest.php

@@ -0,0 +1,191 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\EventListener;
+
+use App\Entity\OAuth2Client;
+use App\Entity\User;
+use App\EventListener\AuthorizationCodeListener;
+use App\Security\Role;
+use League\Bundle\OAuth2ServerBundle\Event\AuthorizationRequestResolveEvent;
+use League\Bundle\OAuth2ServerBundle\Model\ClientInterface;
+use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface;
+use PHPUnit\Framework\TestCase;
+use Psr\Log\LoggerInterface;
+use Symfony\Bundle\SecurityBundle\Security;
+use Symfony\Component\HttpFoundation\RedirectResponse;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpFoundation\Session\Session;
+use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage;
+use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
+use Symfony\Component\Security\Core\User\UserInterface;
+use Twig\Environment;
+
+class AuthorizationCodeListenerTest extends TestCase
+{
+    public function testAnonymousRequestIsRedirectedToLoginWithoutResolving(): void
+    {
+        $event = $this->dispatch(null, $this->client(['ROLE_EMPLOYEE']));
+
+        $response = $event->getResponse();
+        self::assertInstanceOf(RedirectResponse::class, $response);
+        self::assertSame(Response::HTTP_TEMPORARY_REDIRECT, $response->getStatusCode());
+        self::assertFalse($event->getAuthorizationResolution());
+    }
+
+    public function testAccountHoldingOneOfTheRequiredRolesIsApproved(): void
+    {
+        $event = $this->dispatch(
+            $this->user([Role::EMPLOYEE]),
+            $this->client([Role::EMPLOYEE, Role::ADMIN])
+        );
+
+        self::assertTrue($event->getAuthorizationResolution());
+        self::assertNull($event->getResponse());
+    }
+
+    public function testAccountHoldingNoneOfTheRequiredRolesIsDenied(): void
+    {
+        $event = $this->dispatch(
+            $this->user([Role::CUSTOMER]),
+            $this->client([Role::EMPLOYEE, Role::ADMIN])
+        );
+
+        self::assertFalse($event->getAuthorizationResolution());
+        self::assertSame(Response::HTTP_FORBIDDEN, $event->getResponse()?->getStatusCode());
+    }
+
+    /**
+     * A CRM nomination grants nothing until an administrator approves it, so it must not
+     * open a client either.
+     */
+    public function testPendingMarkerDoesNotSatisfyTheRequirement(): void
+    {
+        $event = $this->dispatch(
+            $this->user([Role::ADMIN.Role::PENDING_SUFFIX]),
+            $this->client([Role::ADMIN])
+        );
+
+        self::assertFalse($event->getAuthorizationResolution());
+    }
+
+    /**
+     * The column starts empty, so "nobody configured this client" and "this client needs
+     * no roles" are the same value. Only one of the two readings is safe.
+     */
+    public function testClientWithoutConfiguredRolesIsDenied(): void
+    {
+        $event = $this->dispatch($this->user([Role::ADMIN]), $this->client([]));
+
+        self::assertFalse($event->getAuthorizationResolution());
+        self::assertSame(Response::HTTP_FORBIDDEN, $event->getResponse()?->getStatusCode());
+    }
+
+    public function testClientOfAnotherImplementationIsDenied(): void
+    {
+        $foreign = $this->createStub(ClientInterface::class);
+        $foreign->method('getIdentifier')->willReturn('f3f10f8807e191c127d74cc9a247bf45');
+
+        $event = $this->dispatch($this->user([Role::ADMIN]), $foreign);
+
+        self::assertFalse($event->getAuthorizationResolution());
+        self::assertSame(Response::HTTP_FORBIDDEN, $event->getResponse()?->getStatusCode());
+    }
+
+    public function testExternalRequestLogsTheUserOutAfterApproval(): void
+    {
+        $security = $this->createMock(Security::class);
+        $security->method('getUser')->willReturn($this->user([Role::EMPLOYEE]));
+        $security->expects(self::once())->method('logout')->with(false);
+
+        $this->dispatch(
+            $this->user([Role::EMPLOYEE]),
+            $this->client([Role::EMPLOYEE]),
+            external: true,
+            security: $security
+        );
+    }
+
+    /**
+     * A denied user has to stay signed in: they need a session to read the explanation and
+     * navigate away from it.
+     */
+    public function testExternalRequestDoesNotLogTheUserOutAfterDenial(): void
+    {
+        $security = $this->createMock(Security::class);
+        $security->method('getUser')->willReturn($this->user([Role::CUSTOMER]));
+        $security->expects(self::never())->method('logout');
+
+        $this->dispatch(
+            $this->user([Role::CUSTOMER]),
+            $this->client([Role::EMPLOYEE]),
+            external: true,
+            security: $security
+        );
+    }
+
+    /**
+     * @param list<string> $requiredRoles
+     */
+    private function client(array $requiredRoles): OAuth2Client
+    {
+        return (new OAuth2Client('EPHub', 'f3f10f8807e191c127d74cc9a247bf45', 'secret'))
+            ->setRequiredRoles($requiredRoles);
+    }
+
+    /**
+     * @param list<string> $roles
+     */
+    private function user(array $roles): User
+    {
+        return (new User('[email protected]'))->setRoles($roles);
+    }
+
+    private function dispatch(
+        ?UserInterface $user,
+        ClientInterface $client,
+        bool $external = false,
+        ?Security $security = null,
+    ): AuthorizationRequestResolveEvent {
+        $request = new Request();
+        $session = new Session(new MockArraySessionStorage());
+        $session->set('_oauth2', $external);
+        $request->setSession($session);
+
+        $requestStack = new RequestStack();
+        $requestStack->push($request);
+
+        if (null === $security) {
+            $security = $this->createStub(Security::class);
+            $security->method('getUser')->willReturn($user);
+        }
+
+        $urlGenerator = $this->createStub(UrlGeneratorInterface::class);
+        $urlGenerator->method('generate')->willReturn('/');
+
+        $twig = $this->createStub(Environment::class);
+        $twig->method('render')->willReturn('<html lang="de"></html>');
+
+        $listener = new AuthorizationCodeListener(
+            $urlGenerator,
+            $requestStack,
+            $security,
+            $this->createStub(LoggerInterface::class),
+            $twig,
+        );
+
+        $event = new AuthorizationRequestResolveEvent(
+            $this->createStub(AuthorizationRequestInterface::class),
+            [],
+            $client,
+            $user ?? new User('[email protected]'),
+        );
+
+        $listener->onAuthorizationRequestResolve($event);
+
+        return $event;
+    }
+}