diff --git a/src/Guzzle/AuthenticatorSubscriber.php b/src/Guzzle/AuthenticatorSubscriber.php index bbcb31e24..4b565076e 100644 --- a/src/Guzzle/AuthenticatorSubscriber.php +++ b/src/Guzzle/AuthenticatorSubscriber.php @@ -9,7 +9,7 @@ use GuzzleHttp\Message\RequestInterface; use Psr\Log\LoggerAwareInterface; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; -use Wallabag\SiteConfig\Authenticator\Factory; +use Wallabag\SiteConfig\LoginFormAuthenticator; use Wallabag\SiteConfig\SiteConfig; use Wallabag\SiteConfig\SiteConfigBuilder; @@ -23,8 +23,8 @@ class AuthenticatorSubscriber implements SubscriberInterface, LoggerAwareInterfa /** @var SiteConfigBuilder */ private $configBuilder; - /** @var Factory */ - private $authenticatorFactory; + /** @var LoginFormAuthenticator */ + private $authenticator; /** @var LoggerInterface */ private $logger; @@ -32,10 +32,10 @@ class AuthenticatorSubscriber implements SubscriberInterface, LoggerAwareInterfa /** * AuthenticatorSubscriber constructor. */ - public function __construct(SiteConfigBuilder $configBuilder, Factory $authenticatorFactory) + public function __construct(SiteConfigBuilder $configBuilder, LoginFormAuthenticator $authenticator) { $this->configBuilder = $configBuilder; - $this->authenticatorFactory = $authenticatorFactory; + $this->authenticator = $authenticator; $this->logger = new NullLogger(); } @@ -62,14 +62,13 @@ class AuthenticatorSubscriber implements SubscriberInterface, LoggerAwareInterfa } $client = $event->getClient(); - $authenticator = $this->authenticatorFactory->buildFromSiteConfig($config); - if (!$authenticator->isLoggedIn($client)) { + if (!$this->authenticator->isLoggedIn($config, $client)) { $this->logger->debug('loginIfRequired> user is not logged in, attach authenticator'); $emitter = $client->getEmitter(); $emitter->detach($this); - $authenticator->login($client); + $this->authenticator->login($config, $client); $emitter->attach($this); } } @@ -94,8 +93,7 @@ class AuthenticatorSubscriber implements SubscriberInterface, LoggerAwareInterfa return; } - $authenticator = $this->authenticatorFactory->buildFromSiteConfig($config); - $isLoginRequired = $authenticator->isLoginRequired($body); + $isLoginRequired = $this->authenticator->isLoginRequired($config, $body); $this->logger->debug('loginIfRequested> retry #' . $this->retries . ' with login ' . ($isLoginRequired ? '' : 'not ') . 'required'); @@ -104,7 +102,7 @@ class AuthenticatorSubscriber implements SubscriberInterface, LoggerAwareInterfa $emitter = $client->getEmitter(); $emitter->detach($this); - $authenticator->login($client); + $this->authenticator->login($config, $client); $emitter->attach($this); $event->retry(); diff --git a/src/SiteConfig/Authenticator/Authenticator.php b/src/SiteConfig/Authenticator/Authenticator.php deleted file mode 100644 index bb919a348..000000000 --- a/src/SiteConfig/Authenticator/Authenticator.php +++ /dev/null @@ -1,31 +0,0 @@ -siteConfig = $siteConfig; - } - - public function login(ClientInterface $guzzle) + /** + * Logs the configured user on the given Guzzle client. + * + * @return self + */ + public function login(SiteConfig $siteConfig, ClientInterface $guzzle) { $postFields = [ - $this->siteConfig->getUsernameField() => $this->siteConfig->getUsername(), - $this->siteConfig->getPasswordField() => $this->siteConfig->getPassword(), - ] + $this->getExtraFields($guzzle); + $siteConfig->getUsernameField() => $siteConfig->getUsername(), + $siteConfig->getPasswordField() => $siteConfig->getPassword(), + ] + $this->getExtraFields($siteConfig, $guzzle); $guzzle->post( - $this->siteConfig->getLoginUri(), + $siteConfig->getLoginUri(), ['body' => $postFields, 'allow_redirects' => true, 'verify' => false] ); return $this; } - public function isLoggedIn(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(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; } } @@ -53,13 +50,20 @@ class LoginFormAuthenticator implements Authenticator return false; } - public function isLoginRequired($html) + /** + * Checks from the HTML of a page if authentication is requested by a grabbed page. + * + * @param string $html + * + * @return bool + */ + 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; } @@ -73,17 +77,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/Guzzle/AuthenticatorSubscriberTest.php b/tests/Guzzle/AuthenticatorSubscriberTest.php index 207c9c283..8ffb99208 100644 --- a/tests/Guzzle/AuthenticatorSubscriberTest.php +++ b/tests/Guzzle/AuthenticatorSubscriberTest.php @@ -14,16 +14,19 @@ use Monolog\Logger; use PHPUnit\Framework\TestCase; use Wallabag\Guzzle\AuthenticatorSubscriber; use Wallabag\SiteConfig\ArraySiteConfigBuilder; -use Wallabag\SiteConfig\Authenticator\Authenticator; -use Wallabag\SiteConfig\Authenticator\Factory; +use Wallabag\SiteConfig\LoginFormAuthenticator; class AuthenticatorSubscriberTest extends TestCase { public function testGetEvents() { + $authenticator = $this->getMockBuilder(LoginFormAuthenticator::class) + ->disableOriginalConstructor() + ->getMock(); + $subscriber = new AuthenticatorSubscriber( new ArraySiteConfigBuilder(), - new Factory() + $authenticator ); $events = $subscriber->getEvents(); @@ -35,8 +38,12 @@ class AuthenticatorSubscriberTest extends TestCase public function testLoginIfRequiredNotRequired() { + $authenticator = $this->getMockBuilder(LoginFormAuthenticator::class) + ->disableOriginalConstructor() + ->getMock(); + $builder = new ArraySiteConfigBuilder(['example.com' => []]); - $subscriber = new AuthenticatorSubscriber($builder, new Factory()); + $subscriber = new AuthenticatorSubscriber($builder, $authenticator); $logger = new Logger('foo'); $handler = new TestHandler(); @@ -64,7 +71,7 @@ class AuthenticatorSubscriberTest extends TestCase public function testLoginIfRequiredWithNotLoggedInUser() { - $authenticator = $this->getMockBuilder(Authenticator::class) + $authenticator = $this->getMockBuilder(LoginFormAuthenticator::class) ->disableOriginalConstructor() ->getMock(); @@ -75,16 +82,8 @@ class AuthenticatorSubscriberTest extends TestCase $authenticator->expects($this->once()) ->method('login'); - $factory = $this->getMockBuilder(Factory::class) - ->disableOriginalConstructor() - ->getMock(); - - $factory->expects($this->once()) - ->method('buildFromSiteConfig') - ->willReturn($authenticator); - $builder = new ArraySiteConfigBuilder(['example.com' => ['requiresLogin' => true]]); - $subscriber = new AuthenticatorSubscriber($builder, $factory); + $subscriber = new AuthenticatorSubscriber($builder, $authenticator); $logger = new Logger('foo'); $handler = new TestHandler(); @@ -124,8 +123,12 @@ class AuthenticatorSubscriberTest extends TestCase public function testLoginIfRequestedNotRequired() { + $authenticator = $this->getMockBuilder(LoginFormAuthenticator::class) + ->disableOriginalConstructor() + ->getMock(); + $builder = new ArraySiteConfigBuilder(['example.com' => []]); - $subscriber = new AuthenticatorSubscriber($builder, new Factory()); + $subscriber = new AuthenticatorSubscriber($builder, $authenticator); $logger = new Logger('foo'); $handler = new TestHandler(); @@ -153,7 +156,7 @@ class AuthenticatorSubscriberTest extends TestCase public function testLoginIfRequestedNotRequested() { - $authenticator = $this->getMockBuilder(Authenticator::class) + $authenticator = $this->getMockBuilder(LoginFormAuthenticator::class) ->disableOriginalConstructor() ->getMock(); @@ -161,19 +164,11 @@ class AuthenticatorSubscriberTest extends TestCase ->method('isLoginRequired') ->willReturn(false); - $factory = $this->getMockBuilder(Factory::class) - ->disableOriginalConstructor() - ->getMock(); - - $factory->expects($this->once()) - ->method('buildFromSiteConfig') - ->willReturn($authenticator); - $builder = new ArraySiteConfigBuilder(['example.com' => [ 'requiresLogin' => true, 'notLoggedInXpath' => '//html', ]]); - $subscriber = new AuthenticatorSubscriber($builder, $factory); + $subscriber = new AuthenticatorSubscriber($builder, $authenticator); $logger = new Logger('foo'); $handler = new TestHandler(); @@ -210,7 +205,7 @@ class AuthenticatorSubscriberTest extends TestCase public function testLoginIfRequestedRequested() { - $authenticator = $this->getMockBuilder(Authenticator::class) + $authenticator = $this->getMockBuilder(LoginFormAuthenticator::class) ->disableOriginalConstructor() ->getMock(); @@ -221,19 +216,11 @@ class AuthenticatorSubscriberTest extends TestCase $authenticator->expects($this->once()) ->method('login'); - $factory = $this->getMockBuilder(Factory::class) - ->disableOriginalConstructor() - ->getMock(); - - $factory->expects($this->once()) - ->method('buildFromSiteConfig') - ->willReturn($authenticator); - $builder = new ArraySiteConfigBuilder(['example.com' => [ 'requiresLogin' => true, 'notLoggedInXpath' => '//html', ]]); - $subscriber = new AuthenticatorSubscriber($builder, $factory); + $subscriber = new AuthenticatorSubscriber($builder, $authenticator); $logger = new Logger('foo'); $handler = new TestHandler(); @@ -276,7 +263,7 @@ class AuthenticatorSubscriberTest extends TestCase public function testLoginIfRequestedRedirect() { - $factory = $this->getMockBuilder(Factory::class) + $authenticator = $this->getMockBuilder(LoginFormAuthenticator::class) ->disableOriginalConstructor() ->getMock(); @@ -284,7 +271,7 @@ class AuthenticatorSubscriberTest extends TestCase 'requiresLogin' => true, 'notLoggedInXpath' => '//html', ]]); - $subscriber = new AuthenticatorSubscriber($builder, $factory); + $subscriber = new AuthenticatorSubscriber($builder, $authenticator); $logger = new Logger('foo'); $handler = new TestHandler(); diff --git a/tests/SiteConfig/Authenticator/LoginFormAuthenticatorTest.php b/tests/SiteConfig/LoginFormAuthenticatorTest.php similarity index 87% rename from tests/SiteConfig/Authenticator/LoginFormAuthenticatorTest.php rename to tests/SiteConfig/LoginFormAuthenticatorTest.php index 31c979b55..a1bac9207 100644 --- a/tests/SiteConfig/Authenticator/LoginFormAuthenticatorTest.php +++ b/tests/SiteConfig/LoginFormAuthenticatorTest.php @@ -1,13 +1,13 @@ '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); } @@ -80,7 +80,7 @@ class LoginFormAuthenticatorTest extends TestCase $response->expects($this->any()) ->method('getBody') - ->willReturn(file_get_contents(__DIR__ . '/../../fixtures/aoc.media.html')); + ->willReturn(file_get_contents(__DIR__ . '/../fixtures/aoc.media.html')); $response->expects($this->any()) ->method('getStatusCode') @@ -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); } @@ -142,7 +142,7 @@ class LoginFormAuthenticatorTest extends TestCase $response->expects($this->any()) ->method('getBody') - ->willReturn(file_get_contents(__DIR__ . '/../../fixtures/nextinpact-login.html')); + ->willReturn(file_get_contents(__DIR__ . '/../fixtures/nextinpact-login.html')); $response->expects($this->any()) ->method('getStatusCode') @@ -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); }