From 4ad1de26160d2e33bf3943d5b273382e167a9796 Mon Sep 17 00:00:00 2001 From: Diogo Peralta Cordeiro Date: Sun, 16 Jan 2022 17:44:21 +0000 Subject: [PATCH] some logic --- config/packages/security.yaml | 1 + config/routes/trikoder_oauth2.yaml | 10 ++-- src/Core/GNUsocial.php | 76 +++++++++++++++--------------- src/Security/Authenticator.php | 3 +- src/Util/Common.php | 13 +++-- 5 files changed, 55 insertions(+), 48 deletions(-) diff --git a/config/packages/security.yaml b/config/packages/security.yaml index 4b63830fbd..19d510ee78 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -22,6 +22,7 @@ security: security: false oauth_token: pattern: ^/oauth/(token|authorize)$ + provider: local_user security: false api_apps: pattern: ^/api/v1/apps$ diff --git a/config/routes/trikoder_oauth2.yaml b/config/routes/trikoder_oauth2.yaml index 5c4d421968..76f98e294a 100644 --- a/config/routes/trikoder_oauth2.yaml +++ b/config/routes/trikoder_oauth2.yaml @@ -1,3 +1,7 @@ -oauth2: - resource: '@TrikoderOAuth2Bundle/Resources/config/routes.xml' - prefix: '/oauth' +oauth2_authorization_code: + controller: Trikoder\Bundle\OAuth2Bundle\Controller\AuthorizationController::indexAction + path: '/oauth/authorize' + +oauth2_token: + controller: Trikoder\Bundle\OAuth2Bundle\Controller\TokenController::indexAction + path: '/oauth/token' diff --git a/src/Core/GNUsocial.php b/src/Core/GNUsocial.php index eef0965419..d2b09b0929 100644 --- a/src/Core/GNUsocial.php +++ b/src/Core/GNUsocial.php @@ -52,6 +52,7 @@ use App\Kernel; use App\Security\EmailVerifier; use App\Util\Common; use App\Util\Exception\ConfigurationException; +use App\Util\Exception\NoLoggedInUser; use App\Util\Formatting; use App\Util\HTML; use Doctrine\ORM\EntityManagerInterface; @@ -65,6 +66,7 @@ use Symfony\Component\DependencyInjection\ParameterBag\ContainerBagInterface; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\Form\FormFactoryInterface; +use Symfony\Component\HttpFoundation\Exception\BadRequestException; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpFoundation\Session\SessionInterface; @@ -233,7 +235,7 @@ class GNUsocial implements EventSubscriberInterface public static function configureContainer(ContainerBuilder $container, LoaderInterface $loader): void { // Overriding doesn't work as we want, overrides the top-most key, do it manually - $local_file = INSTALLDIR . '/social.local.yaml'; + $local_file = INSTALLDIR . \DIRECTORY_SEPARATOR . 'social.local.yaml'; if (!file_exists($local_file)) { file_put_contents($local_file, "parameters:\n locals:\n gnusocial:\n"); } @@ -289,53 +291,51 @@ class GNUsocial implements EventSubscriberInterface $event->setUser($user); } - public function authRequestResolve(AuthorizationRequestResolveEvent $event): void + public function authorizeRequestResolve(AuthorizationRequestResolveEvent $event): void { $request = $this->request; - // only handle post requests for logged-in users: - // get requests will be intercepted and shown the login form - // other verbs we will handle as an authorization denied - // and this implementation ensures a user is set at this point already - if ($request->getMethod() !== 'POST' && \is_null($event->getUser())) { - $event->resolveAuthorization(AuthorizationRequestResolveEvent::AUTHORIZATION_DENIED); - return; - } - - if (!$request->request->has('action')) { - // 1. successful login, goes to grant page - $content = $this->twig->render('security/grant.html.twig', [ - 'scopes' => $event->getScopes(), - 'client' => $event->getClient(), - 'grant' => OAuth2Grants::AUTHORIZATION_CODE, - // very simple way to ensure user gets to this point in the - // flow when granting or denying is to pre-add their credentials - 'email' => $request->request->get('email'), - 'password' => $request->request->get('password'), - ]); - - $response = new Response(200, [], $content); - $event->setResponse($response); - } else { - // 2. grant operation, either grants or denies - if ($request->request->get('action') == OAuth2Grants::AUTHORIZATION_CODE) { - $event->resolveAuthorization(AuthorizationRequestResolveEvent::AUTHORIZATION_APPROVED); - } else { + try { + $user = Common::ensureLoggedIn(); + // get requests will be intercepted and shown the login form + // other verbs we will handle as an authorization denied + // and this implementation ensures a user is set at this point already + if ($request->getMethod() !== 'POST') { $event->resolveAuthorization(AuthorizationRequestResolveEvent::AUTHORIZATION_DENIED); + return; + } else { + if (!$request->request->has('action')) { + // 1. successful login, goes to grant page + $content = $this->twig->render('security/grant.html.twig', [ + 'scopes' => $event->getScopes(), + 'client' => $event->getClient(), + 'grant' => OAuth2Grants::AUTHORIZATION_CODE, + // very simple way to ensure user gets to this point in the + // flow when granting or denying is to pre-add their credentials + 'email' => $request->request->get('email'), + 'password' => $request->request->get('password'), + ]); + $response = new Response(200, [], $content); + $event->setResponse($response); + } else { + // 2. grant operation, either grants or denies + if ($request->request->get('action') === OAuth2Grants::AUTHORIZATION_CODE) { + $event->setUser($user); + $event->resolveAuthorization(AuthorizationRequestResolveEvent::AUTHORIZATION_APPROVED); + } else { + $event->resolveAuthorization(AuthorizationRequestResolveEvent::AUTHORIZATION_DENIED); + } + } } - } - - if (\is_null($event->getUser())) { + // Whoops! + throw new BadRequestException(); + } catch (NoLoggedInUser) { $event->setResponse(new Response(302, [ 'Location' => Router::url('security_login', [ 'returnUrl' => $request->getUri(), ]), ])); - - return; } - - $event->resolveAuthorization(AuthorizationRequestResolveEvent::AUTHORIZATION_APPROVED); } /** @@ -348,7 +348,7 @@ class GNUsocial implements EventSubscriberInterface KernelEvents::REQUEST => 'onKernelRequest', 'console.command' => 'onCommand', OAuth2Events::USER_RESOLVE => 'userResolve', - OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE => 'authRequestResolve', + OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE => 'authorizeRequestResolve', ]; } } diff --git a/src/Security/Authenticator.php b/src/Security/Authenticator.php index 0daf9df958..e7c12a7ae0 100644 --- a/src/Security/Authenticator.php +++ b/src/Security/Authenticator.php @@ -158,8 +158,7 @@ class Authenticator extends AbstractFormLoginAuthenticator implements Authentica ); // TODO: Fix the Open Redirect security flaw here. - $targetPath = $request->request->get('returnUrl'); - + $targetPath = $request->query->get('returnUrl'); if ($targetPath ??= $this->getTargetPath($request->getSession(), $providerKey)) { return new RedirectResponse($targetPath); } diff --git a/src/Util/Common.php b/src/Util/Common.php index 9523229738..9a0c005bd1 100644 --- a/src/Util/Common.php +++ b/src/Util/Common.php @@ -135,17 +135,20 @@ abstract class Common public static function userNickname(): ?string { - return self::ensureLoggedIn()->getNickname(); + return self::ensureLoggedIn()?->getNickname(); } public static function userId(): ?int { - return self::ensureLoggedIn()->getId(); + return self::ensureLoggedIn()?->getId(); } + /** + * @throws NoLoggedInUser + */ public static function ensureLoggedIn(): LocalUser { - if (($user = self::user()) == null) { + if (\is_null($user = self::user())) { throw new NoLoggedInUser(); // TODO Maybe redirect to login page and back } else { @@ -160,7 +163,7 @@ abstract class Common */ public static function isLoggedIn(): bool { - return self::user() != null; + return !\is_null(self::user()); } /** @@ -337,6 +340,6 @@ abstract class Common public static function base64url_decode(string $data): string { - return base64_decode(str_pad(strtr(strtr($data, '_', '/'), '-', '+'), \mb_strlen($data) % 4, '=', \STR_PAD_RIGHT)); + return base64_decode(str_pad(strtr(strtr($data, '_', '/'), '-', '+'), mb_strlen($data) % 4, '=', \STR_PAD_RIGHT)); } }