diff --git a/extension.json b/extension.json index e68d4a2..17340e9 100644 --- a/extension.json +++ b/extension.json @@ -128,11 +128,15 @@ }, "AutoloadClasses": { "CookieWarningHooks": "includes/CookieWarningHooks.php", + "CookieWarningDecisions": "includes/CookieWarningDecisions.php", "CookieWarningTestTemplate": "tests/phpunit/includes/CookieWarningTestTemplate.php", "GeoLocation": "includes/GeoLocation.php" }, "ConfigRegistry": { "cookiewarning": "GlobalVarConfig::newInstance" }, + "ServiceWiringFiles": [ + "includes/ServiceWiring.php" + ], "manifest_version": 1 } diff --git a/includes/CookieWarningDecisions.php b/includes/CookieWarningDecisions.php new file mode 100644 index 0000000..d5fe9cc --- /dev/null +++ b/includes/CookieWarningDecisions.php @@ -0,0 +1,76 @@ +config = $config; + $this->geoLocation = $geoLocation; + } + + /** + * Checks, if the CookieWarning information bar should be visible to this user on + * this page. + * + * @param IContextSource $context + * @return bool Returns true, if the cookie warning should be visible, false otherwise. + * @throws ConfigException + * @throws MWException + */ + public function shouldShowCookieWarning( IContextSource $context ) { + $user = $context->getUser(); + + return $this->config->get( 'CookieWarningEnabled' ) && + !$user->getBoolOption( 'cookiewarning_dismissed', false ) && + !$context->getRequest()->getCookie( 'cookiewarning_dismissed' ) && + ( $this->config->get( 'CookieWarningGeoIPLookup' ) === 'js' || + $this->isInConfiguredRegion( $context ) ); + } + + /** + * Checks, if the user is in one of the configured regions. + * + * @TODO: This function or the function users should set the cookie or user option, if this + * function returns false to avoid a location lookup on each request. + * @param IContextSource $context + * @return bool + * @throws ConfigException + * @throws MWException + */ + private function isInConfiguredRegion( IContextSource $context ) { + if ( !$this->config->get( 'CookieWarningForCountryCodes' ) || + $this->config->get( 'CookieWarningGeoIPLookup' ) === 'none' ) { + wfDebugLog( 'CookieWarning', 'IP geolocation not configured, skipping.' ); + + return true; + } + + wfDebugLog( 'CookieWarning', 'Try to locate the user\'s IP address.' ); + $located = $this->geoLocation->locate( $context->getRequest()->getIP() ); + if ( !$located ) { + wfDebugLog( 'CookieWarning', + 'Locating the user\'s IP address failed or is' . ' configured false.' ); + + return true; + } + wfDebugLog( 'CookieWarning', 'Locating the user was successful, located' . ' region: ' . + $this->geoLocation->getCountryCode() ); + + return array_key_exists( $this->geoLocation->getCountryCode(), + $this->config->get( 'CookieWarningForCountryCodes' ) ); + } + + /** + * @return bool + * @throws ConfigException + */ + public function shouldAddResourceLoaderComponents() { + return $this->config->get( 'CookieWarningGeoIPLookup' ) === 'js' && + is_array( $this->config->get( 'CookieWarningForCountryCodes' ) ); + } +} diff --git a/includes/CookieWarningHooks.php b/includes/CookieWarningHooks.php index ce2e275..ca87704 100644 --- a/includes/CookieWarningHooks.php +++ b/includes/CookieWarningHooks.php @@ -1,8 +1,8 @@ redirect( $request->getRequestURL() ); } + /** * SkinTemplateOutputPageBeforeExec hook handler. * @@ -38,17 +40,21 @@ class CookieWarningHooks { * * @param SkinTemplate &$sk * @param QuickTemplate &$tpl + * @throws ConfigException + * @throws MWException */ public static function onSkinTemplateOutputPageBeforeExec( SkinTemplate &$sk, QuickTemplate &$tpl ) { - // if the cookiewarning should not be visible to the user, exit. - if ( !self::showWarning( $sk->getContext() ) ) { + /** @var CookieWarningDecisions $cookieWarningDecisions */ + $cookieWarningDecisions = MediaWikiServices::getInstance() + ->getService( 'CookieWarning.Decisions' ); + + if ( !$cookieWarningDecisions->shouldShowCookieWarning( $sk->getContext() ) ) { return; } $moreLink = self::getMoreLink(); - // if a "more information" URL was configured, add a link to it in the cookiewarning - // information bar + if ( $moreLink ) { $moreLink = ' ' . Html::element( 'a', @@ -108,21 +114,24 @@ class CookieWarningHooks { * - the interface message MediaWiki:Cookie-policy-link (bc T145781) * * @return string|null The url or null if none set + * @throws ConfigException */ private static function getMoreLink() { - // Config instance of CookieWarning - $conf = ConfigFactory::getDefaultInstance()->makeConfig( 'cookiewarning' ); + $conf = self::getConfig(); if ( $conf->get( 'CookieWarningMoreUrl' ) ) { return $conf->get( 'CookieWarningMoreUrl' ); } + $cookieWarningMessage = wfMessage( 'cookiewarning-more-link' ); if ( $cookieWarningMessage->exists() && !$cookieWarningMessage->isDisabled() ) { return $cookieWarningMessage->escaped(); } + $cookiePolicyMessage = wfMessage( 'cookie-policy-link' ); if ( $cookiePolicyMessage->exists() && !$cookiePolicyMessage->isDisabled() ) { return $cookiePolicyMessage->escaped(); } + return null; } @@ -132,42 +141,49 @@ class CookieWarningHooks { * Adds the required style and JS module, if cookiewarning is enabled. * * @param OutputPage $out + * @throws ConfigException + * @throws MWException */ public static function onBeforePageDisplay( OutputPage $out ) { - if ( self::showWarning( $out->getContext() ) ) { - $conf = self::getConfig(); - if ( - ExtensionRegistry::getInstance()->isLoaded( 'MobileFrontend' ) && - MobileContext::singleton()->shouldDisplayMobileView() - ) { - $moduleStyles = [ 'ext.CookieWarning.mobile.styles' ]; - } else { - $moduleStyles = [ 'ext.CookieWarning.styles' ]; - } - $modules = [ 'ext.CookieWarning' ]; - if ( - $conf->get( 'CookieWarningGeoIPLookup' ) === 'js' && - is_array( $conf->get( 'CookieWarningForCountryCodes' ) ) - ) { - $modules[] = 'ext.CookieWarning.geolocation'; - $moduleStyles[] = 'ext.CookieWarning.geolocation.styles'; - } - $out->addModules( $modules ); - $out->addModuleStyles( $moduleStyles ); + /** @var CookieWarningDecisions $cookieWarningDecisions */ + $cookieWarningDecisions = MediaWikiServices::getInstance() + ->getService( 'CookieWarning.Decisions' ); + + if ( !$cookieWarningDecisions->shouldShowCookieWarning( $out->getContext() ) ) { + return; } + + if ( + ExtensionRegistry::getInstance()->isLoaded( 'MobileFrontend' ) && + MobileContext::singleton()->shouldDisplayMobileView() + ) { + $moduleStyles = [ 'ext.CookieWarning.mobile.styles' ]; + } else { + $moduleStyles = [ 'ext.CookieWarning.styles' ]; + } + $modules = [ 'ext.CookieWarning' ]; + + if ( $cookieWarningDecisions->shouldAddResourceLoaderComponents() ) { + $modules[] = 'ext.CookieWarning.geolocation'; + $moduleStyles[] = 'ext.CookieWarning.geolocation.styles'; + } + $out->addModules( $modules ); + $out->addModuleStyles( $moduleStyles ); } /** * ResourceLoaderGetConfigVars hook handler. * * @param array &$vars + * @throws ConfigException */ public static function onResourceLoaderGetConfigVars( array &$vars ) { + /** @var CookieWarningDecisions $cookieWarningDecisions */ + $cookieWarningDecisions = MediaWikiServices::getInstance() + ->getService( 'CookieWarning.Decisions' ); $conf = self::getConfig(); - if ( - $conf->get( 'CookieWarningGeoIPLookup' ) === 'js' && - is_array( $conf->get( 'CookieWarningForCountryCodes' ) ) - ) { + + if ( $cookieWarningDecisions->shouldAddResourceLoaderComponents() ) { $vars += [ 'wgCookieWarningGeoIPServiceURL' => $conf->get( 'CookieWarningGeoIPServiceURL' ), 'wgCookieWarningForCountryCodes' => $conf->get( 'CookieWarningForCountryCodes' ), @@ -181,73 +197,7 @@ class CookieWarningHooks { * @return Config */ private static function getConfig() { - return ConfigFactory::getDefaultInstance()->makeConfig( 'cookiewarning' ); - } - - /** - * Checks, if the CookieWarning information bar should be visible to this user on - * this page. - * - * @param IContextSource $context - * @return bool Returns true, if the cookie warning should be visible, false otherwise. - */ - private static function showWarning( IContextSource $context ) { - $user = $context->getUser(); - $conf = self::getConfig(); - if ( - // if enabled in LocalSettings.php - $conf->get( 'CookieWarningEnabled' ) && - // if not already dismissed by this user (and saved in the user prefs) - !$user->getBoolOption( 'cookiewarning_dismissed', false ) && - // if not already dismissed by this user (and saved in the browser cookies) - !$context->getRequest()->getCookie( 'cookiewarning_dismissed' ) && - ( - $conf->get( 'CookieWarningGeoIPLookup' ) === 'js' || - self::inConfiguredRegion( $context, $conf ) - ) - ) { - return true; - } - return false; - } - - /** - * Checks, if the user is in one of the configured regions. - * - * @TODO: This function or the function users should set the cookie or user option, if this - * function returns false to avoid a location lookup on each request. - * @param IContextSource $context - * @param Config $conf - * @return bool - */ - private static function inConfiguredRegion( IContextSource $context, Config $conf ) { - if ( self::$inConfiguredRegion === null ) { - if ( - !$conf->get( 'CookieWarningForCountryCodes' ) || - $conf->get( 'CookieWarningGeoIPLookup' ) === 'none' - ) { - wfDebugLog( 'CookieWarning', 'IP geolocation not configured, skipping.' ); - self::$inConfiguredRegion = true; - } else { - wfDebugLog( 'CookieWarning', 'Try to locate the user\'s IP address.' ); - $geoLocation = new GeoLocation; - $located = $geoLocation - ->setConfig( $conf ) - ->setIP( $context->getRequest()->getIP() ) - ->locate(); - if ( !$located ) { - wfDebugLog( 'CookieWarning', 'Locating the user\'s IP address failed or is' . - ' configured false.' ); - self::$inConfiguredRegion = true; - } else { - wfDebugLog( 'CookieWarning', 'Locating the user was successful, located' . - ' region: ' . $geoLocation->getCountryCode() ); - self::$inConfiguredRegion = array_key_exists( $geoLocation->getCountryCode(), - $conf->get( 'CookieWarningForCountryCodes' ) ); - } - } - } - return self::$inConfiguredRegion; + return MediaWikiServices::getInstance()->getService( 'CookieWarning.Config' ); } /** diff --git a/includes/GeoLocation.php b/includes/GeoLocation.php index bfce87c..63cd837 100644 --- a/includes/GeoLocation.php +++ b/includes/GeoLocation.php @@ -7,44 +7,14 @@ * Implements the GeoLocation class, which allows to locate the user based on the IP address. */ class GeoLocation { - private $ip; private $config; private $countryCode; /** - * Set the IP address you want to locate. - * - * @param string $ip A valid IP address - * @return $this - */ - public function setIP( $ip ) { - if ( !IP::isValid( $ip ) ) { - throw new InvalidArgumentException( "$ip is not a valid IP address." ); - } - $this->ip = $ip; - - return $this; - } - - /** - * Returns the IP address. - * - * @return null|string NULL if the address is not set so far, string otherwise. - */ - public function getIP() { - return $this->ip; - } - - /** - * Sets the Config object used by this class. - * * @param Config $config - * @return $this */ - public function setConfig( Config $config ) { + public function __construct( Config $config ) { $this->config = $config; - - return $this; } /** @@ -63,17 +33,14 @@ class GeoLocation { * other problem occures which resulted in a failed locating process, this function returns * false, otherwise it returns true. * + * @param string $ip The IP address to lookup * @return bool|null NULL if no geolocation service configured, false on error, true otherwise. + * @throws ConfigException */ - public function locate() { + public function locate( $ip ) { $this->countryCode = null; - if ( $this->ip === null ) { - throw new RuntimeException( - 'No IP address set, locating now would return the servers location.' ); - } - if ( $this->config === null ) { - throw new RuntimeException( - 'You need to set the Config object first, before you can locate an IP address.' ); + if ( !IP::isValid( $ip ) ) { + throw new InvalidArgumentException( "$ip is not a valid IP address." ); } if ( !$this->config->get( 'CookieWarningGeoIPServiceURL' ) ) { return null; @@ -82,7 +49,7 @@ class GeoLocation { if ( substr( $requestUrl, -1 ) !== '/' ) { $requestUrl .= '/'; } - $json = Http::get( $requestUrl . $this->getIP(), [ + $json = Http::get( $requestUrl . $ip, [ 'timeout' => '2' ] ); if ( !$json ) { diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php new file mode 100644 index 0000000..b328fa2 --- /dev/null +++ b/includes/ServiceWiring.php @@ -0,0 +1,17 @@ + function ( MediaWikiServices $services ) { + return $services->getService( 'ConfigFactory' ) + ->makeConfig( 'cookiewarning' ); + }, + 'GeoLocation' => function ( MediaWikiServices $services ) { + return new GeoLocation( $services->getService( 'CookieWarning.Config' ) ); + }, + 'CookieWarning.Decisions' => function ( MediaWikiServices $services ) { + return new CookieWarningDecisions( $services->getService( 'CookieWarning.Config' ), + $services->getService( 'GeoLocation' ) ); + }, +]; diff --git a/tests/phpunit/includes/CookieWarningHooksTest.php b/tests/phpunit/includes/CookieWarningHooksTest.php index 4e1a57b..95d86e1 100644 --- a/tests/phpunit/includes/CookieWarningHooksTest.php +++ b/tests/phpunit/includes/CookieWarningHooksTest.php @@ -124,12 +124,12 @@ class CookieWarningHooksTest extends MediaWikiLangTestCase { public function testOnSkinTemplateOutputPageBeforeExecGeoLocation( $ipAddress, $countryCodes, $expected ) { - $this->resetCookieWarningHooks(); $this->setMwGlobals( [ 'wgCookieWarningEnabled' => true, 'wgCookieWarningGeoIPLookup' => is_array( $countryCodes ) ? 'php' : 'none', 'wgCookieWarningForCountryCodes' => $countryCodes, ] ); + $this->mockGeoLocationService(); $request = new FauxRequest(); $request->setIP( $ipAddress ); @@ -166,13 +166,12 @@ class CookieWarningHooksTest extends MediaWikiLangTestCase { ]; } - private function resetCookieWarningHooks() { - // reset the inConfiguredRegion value to retrigger a location lookup, if called again - $singleton = CookieWarningHooks::class; - $reflection = new ReflectionClass( $singleton ); - $instance = $reflection->getProperty( 'inConfiguredRegion' ); - $instance->setAccessible( true ); - $instance->setValue( null, null ); - $instance->setAccessible( false ); + private function mockGeoLocationService() { + $geoLocation = $this->getMockBuilder( GeoLocation::class ) + ->disableOriginalConstructor() + ->getMock(); + $geoLocation->method( 'locate' )->willReturn( true ); + $geoLocation->method( 'getCountryCode' )->willReturn( 'US' ); + $this->setService( 'GeoLocation', $geoLocation ); } }