From 81d269dec1c0c8739c76bcdcde626f9151dcaef9 Mon Sep 17 00:00:00 2001 From: Yassine Guedidi Date: Sat, 23 Nov 2024 11:27:18 +0100 Subject: [PATCH] Move site config from property to parameter in LoginFormAuthenticator --- src/Guzzle/AuthenticatorSubscriber.php | 12 +++---- .../Authenticator/Authenticator.php | 7 +++-- src/SiteConfig/Authenticator/Factory.php | 4 +-- .../Authenticator/LoginFormAuthenticator.php | 31 +++++++------------ .../LoginFormAuthenticatorTest.php | 24 +++++++------- 5 files changed, 35 insertions(+), 43 deletions(-) diff --git a/src/Guzzle/AuthenticatorSubscriber.php b/src/Guzzle/AuthenticatorSubscriber.php index bbcb31e24..ff4e9acc6 100644 --- a/src/Guzzle/AuthenticatorSubscriber.php +++ b/src/Guzzle/AuthenticatorSubscriber.php @@ -62,14 +62,14 @@ class AuthenticatorSubscriber implements SubscriberInterface, LoggerAwareInterfa } $client = $event->getClient(); - $authenticator = $this->authenticatorFactory->buildFromSiteConfig($config); + $authenticator = $this->authenticatorFactory->buildFromSiteConfig(); - if (!$authenticator->isLoggedIn($client)) { + if (!$authenticator->isLoggedIn($config, $client)) { $this->logger->debug('loginIfRequired> user is not logged in, attach authenticator'); $emitter = $client->getEmitter(); $emitter->detach($this); - $authenticator->login($client); + $authenticator->login($config, $client); $emitter->attach($this); } } @@ -94,8 +94,8 @@ class AuthenticatorSubscriber implements SubscriberInterface, LoggerAwareInterfa return; } - $authenticator = $this->authenticatorFactory->buildFromSiteConfig($config); - $isLoginRequired = $authenticator->isLoginRequired($body); + $authenticator = $this->authenticatorFactory->buildFromSiteConfig(); + $isLoginRequired = $authenticator->isLoginRequired($config, $body); $this->logger->debug('loginIfRequested> retry #' . $this->retries . ' with login ' . ($isLoginRequired ? '' : 'not ') . 'required'); @@ -104,7 +104,7 @@ class AuthenticatorSubscriber implements SubscriberInterface, LoggerAwareInterfa $emitter = $client->getEmitter(); $emitter->detach($this); - $authenticator->login($client); + $authenticator->login($config, $client); $emitter->attach($this); $event->retry(); diff --git a/src/SiteConfig/Authenticator/Authenticator.php b/src/SiteConfig/Authenticator/Authenticator.php index bb919a348..9623e39ab 100644 --- a/src/SiteConfig/Authenticator/Authenticator.php +++ b/src/SiteConfig/Authenticator/Authenticator.php @@ -3,6 +3,7 @@ namespace Wallabag\SiteConfig\Authenticator; use GuzzleHttp\ClientInterface; +use Wallabag\SiteConfig\SiteConfig; interface Authenticator { @@ -11,14 +12,14 @@ interface Authenticator * * @return self */ - public function login(ClientInterface $guzzle); + public function login(SiteConfig $siteConfig, ClientInterface $guzzle); /** * Checks if we are logged into the site, but without calling the server (e.g. do we have a Cookie). * * @return bool */ - public function isLoggedIn(ClientInterface $guzzle); + public function isLoggedIn(SiteConfig $siteConfig, ClientInterface $guzzle); /** * Checks from the HTML of a page if authentication is requested by a grabbed page. @@ -27,5 +28,5 @@ interface Authenticator * * @return bool */ - public function isLoginRequired($html); + public function isLoginRequired(SiteConfig $siteConfig, $html); } diff --git a/src/SiteConfig/Authenticator/Factory.php b/src/SiteConfig/Authenticator/Factory.php index 408690128..cf4267768 100644 --- a/src/SiteConfig/Authenticator/Factory.php +++ b/src/SiteConfig/Authenticator/Factory.php @@ -14,8 +14,8 @@ class Factory * * @throw \OutOfRangeException if there are no credentials for this host */ - public function buildFromSiteConfig(SiteConfig $siteConfig) + public function buildFromSiteConfig() { - return new LoginFormAuthenticator($siteConfig); + return new LoginFormAuthenticator(); } } diff --git a/src/SiteConfig/Authenticator/LoginFormAuthenticator.php b/src/SiteConfig/Authenticator/LoginFormAuthenticator.php index 8714f1e2a..811f98fdd 100644 --- a/src/SiteConfig/Authenticator/LoginFormAuthenticator.php +++ b/src/SiteConfig/Authenticator/LoginFormAuthenticator.php @@ -11,37 +11,28 @@ use Wallabag\SiteConfig\SiteConfig; class LoginFormAuthenticator implements Authenticator { - /** @var SiteConfig */ - private $siteConfig; - - public function __construct(SiteConfig $siteConfig) - { - // @todo OptionResolver - $this->siteConfig = $siteConfig; - } - - public function login(ClientInterface $guzzle) + public function login(SiteConfig $siteConfig, ClientInterface $guzzle) { $postFields = [ - $this->siteConfig->getUsernameField() => $this->siteConfig->getUsername(), - $this->siteConfig->getPasswordField() => $this->siteConfig->getPassword(), + $siteConfig->getUsernameField() => $siteConfig->getUsername(), + $siteConfig->getPasswordField() => $siteConfig->getPassword(), ] + $this->getExtraFields($guzzle); $guzzle->post( - $this->siteConfig->getLoginUri(), + $siteConfig->getLoginUri(), ['body' => $postFields, 'allow_redirects' => true, 'verify' => false] ); return $this; } - public function isLoggedIn(ClientInterface $guzzle) + public function isLoggedIn(SiteConfig $siteConfig, ClientInterface $guzzle) { if (($cookieJar = $guzzle->getDefaultOption('cookies')) instanceof CookieJar) { /** @var \GuzzleHttp\Cookie\SetCookie $cookie */ foreach ($cookieJar as $cookie) { // check required cookies - if ($cookie->getDomain() === $this->siteConfig->getHost()) { + if ($cookie->getDomain() === $siteConfig->getHost()) { return true; } } @@ -50,13 +41,13 @@ class LoginFormAuthenticator implements Authenticator return false; } - public function isLoginRequired($html) + public function isLoginRequired(SiteConfig $siteConfig, $html) { // need to check for the login dom element ($options['not_logged_in_xpath']) in the HTML try { $crawler = new Crawler((string) $html); - $loggedIn = $crawler->evaluate((string) $this->siteConfig->getNotLoggedInXpath()); + $loggedIn = $crawler->evaluate((string) $siteConfig->getNotLoggedInXpath()); } catch (\Throwable $e) { return false; } @@ -70,17 +61,17 @@ class LoginFormAuthenticator implements Authenticator * * @return array */ - private function getExtraFields(ClientInterface $guzzle) + private function getExtraFields(SiteConfig $siteConfig, ClientInterface $guzzle) { $extraFields = []; - foreach ($this->siteConfig->getExtraFields() as $fieldName => $fieldValue) { + foreach ($siteConfig->getExtraFields() as $fieldName => $fieldValue) { if ('@=' === substr($fieldValue, 0, 2)) { $expressionLanguage = $this->getExpressionLanguage($guzzle); $fieldValue = $expressionLanguage->evaluate( substr($fieldValue, 2), [ - 'config' => $this->siteConfig, + 'config' => $siteConfig, ] ); } diff --git a/tests/SiteConfig/Authenticator/LoginFormAuthenticatorTest.php b/tests/SiteConfig/Authenticator/LoginFormAuthenticatorTest.php index 31c979b55..d429b72a0 100644 --- a/tests/SiteConfig/Authenticator/LoginFormAuthenticatorTest.php +++ b/tests/SiteConfig/Authenticator/LoginFormAuthenticatorTest.php @@ -35,8 +35,8 @@ class LoginFormAuthenticatorTest extends TestCase 'password' => 'unkn0wn', ]); - $auth = new LoginFormAuthenticator($siteConfig); - $res = $auth->login($guzzle); + $auth = new LoginFormAuthenticator(); + $res = $auth->login($siteConfig, $guzzle); $this->assertInstanceOf(LoginFormAuthenticator::class, $res); } @@ -65,8 +65,8 @@ class LoginFormAuthenticatorTest extends TestCase 'password' => 'unkn0wn', ]); - $auth = new LoginFormAuthenticator($siteConfig); - $res = $auth->login($guzzle); + $auth = new LoginFormAuthenticator(); + $res = $auth->login($siteConfig, $guzzle); $this->assertInstanceOf(LoginFormAuthenticator::class, $res); } @@ -128,8 +128,8 @@ class LoginFormAuthenticatorTest extends TestCase 'password' => 'unkn0wn', ]); - $auth = new LoginFormAuthenticator($siteConfig); - $res = $auth->login($client); + $auth = new LoginFormAuthenticator(); + $res = $auth->login($siteConfig, $client); $this->assertInstanceOf(LoginFormAuthenticator::class, $res); } @@ -194,8 +194,8 @@ class LoginFormAuthenticatorTest extends TestCase 'password' => 'unkn0wn', ]); - $auth = new LoginFormAuthenticator($siteConfig); - $res = $auth->login($client); + $auth = new LoginFormAuthenticator(); + $res = $auth->login($siteConfig, $client); $this->assertInstanceOf(LoginFormAuthenticator::class, $res); } @@ -210,8 +210,8 @@ class LoginFormAuthenticatorTest extends TestCase 'password' => 'unkn0wn', ]); - $auth = new LoginFormAuthenticator($siteConfig); - $loginRequired = $auth->isLoginRequired(file_get_contents(__DIR__ . '/../../fixtures/nextinpact-login.html')); + $auth = new LoginFormAuthenticator(); + $loginRequired = $auth->isLoginRequired($siteConfig, file_get_contents(__DIR__ . '/../../fixtures/nextinpact-login.html')); $this->assertFalse($loginRequired); } @@ -227,8 +227,8 @@ class LoginFormAuthenticatorTest extends TestCase 'notLoggedInXpath' => '//h2[@class="title_reserve_article"]', ]); - $auth = new LoginFormAuthenticator($siteConfig); - $loginRequired = $auth->isLoginRequired(file_get_contents(__DIR__ . '/../../fixtures/nextinpact-article.html')); + $auth = new LoginFormAuthenticator(); + $loginRequired = $auth->isLoginRequired($siteConfig, file_get_contents(__DIR__ . '/../../fixtures/nextinpact-article.html')); $this->assertTrue($loginRequired); }