diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index e0386b365fb282f7295a2594d52c8e22d45947e1..a95545b4fe8ac74e9e2c505a6815f318af89394b 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,11 +1,11 @@ -image: docker:20.10.9 +image: docker:latest variables: DOCKER_TLS_CERTDIR: "/certs" - IMAGE_VER: 4.15-p1 + IMAGE_VER: 5.1.1-p1 services: - - docker:20.10.9-dind + - docker:latest-dind before_script: - docker login -u $CI_REGISTRY_USER -p $CI_REGISTRY_PASSWORD $CI_REGISTRY diff --git a/Dockerfile b/Dockerfile index 06b10e601e8e3be6803a27975ba279ece8ef1d2c..e190adcdcf5edef35f559933fc52b20c9d90caca 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM matomo:4.15 +FROM matomo:5.1.1 MAINTAINER Andrej Ramašeuski <andrej.ramaseuski@pirati.cz> COPY LoginOIDC /var/www/html/plugins/LoginOIDC diff --git a/LoginOIDC/CHANGELOG.md b/LoginOIDC/CHANGELOG.md index 929bdbd47378ed47a9646ccd5ddbf5bcce612551..41adfc7eff7e30d517163a13203b6daac228cd79 100644 --- a/LoginOIDC/CHANGELOG.md +++ b/LoginOIDC/CHANGELOG.md @@ -1,5 +1,17 @@ ## Changelog +### 4.1.2 +* Fix disabling OIDC for superusers not having any effect (#68). + +### 4.1.1 +* Hotfix warning about session variable not being set when signing in with username/password. + +### 4.1.0 +* Add option to skip password confirmation requests when user has signed in via LoginOIDC (requires Matomo >4.12.0) (#72). +* Add option to automatically link existing users when IdP user id matches Matomos user id (#44). +* Fix logout redirect (#64). +* Improve db table creation (#31). + ### 4.0.0 * Prepare plugin for Matomo 4. * Linking accounts has been moved to the users security settings. @@ -18,22 +30,17 @@ * Add option to bypass second factor when sign in with OIDC. ### 0.1.4 - * Add option to automatically create unknown users. ### 0.1.3 - * Add an option to override the redirect URI. ### 0.1.2 - * Fix oauth flow for [Keycloak](https://github.com/keycloak/keycloak). * Improve FAQ. ### 0.1.1 - * Lowered the required Matomo version for this plugin. ### 0.1.0 - * Initial version. diff --git a/LoginOIDC/Controller.php b/LoginOIDC/Controller.php index 1c7c07efc1633c5414081858b97bd78cf749af91..4891524be96aa23933a1626a49aeb28e87d05a65 100644 --- a/LoginOIDC/Controller.php +++ b/LoginOIDC/Controller.php @@ -20,6 +20,7 @@ use Piwik\Nonce; use Piwik\Piwik; use Piwik\Plugins\UsersManager\API as UsersManagerAPI; use Piwik\Plugins\UsersManager\Model; +use Piwik\Request; use Piwik\Session\SessionFingerprint; use Piwik\Session\SessionInitializer; use Piwik\Url; @@ -193,13 +194,13 @@ class Controller extends \Piwik\Plugin\Controller throw new Exception(Piwik::translate("LoginOIDC_ExceptionNotConfigured")); } - if ($_SESSION["loginoidc_state"] !== Common::getRequestVar("state")) { + if ($_SESSION["loginoidc_state"] !== Request::fromGet()->getStringParameter("state")) { throw new Exception(Piwik::translate("LoginOIDC_ExceptionStateMismatch")); } else { unset($_SESSION["loginoidc_state"]); } - if (Common::getRequestVar("provider") !== "oidc") { + if (Request::fromGet()->getStringParameter("provider") !== "oidc") { throw new Exception(Piwik::translate("LoginOIDC_ExceptionUnknownProvider")); } @@ -207,10 +208,10 @@ class Controller extends \Piwik\Plugin\Controller $data = array( "client_id" => $settings->clientId->getValue(), "client_secret" => $settings->clientSecret->getValue(), - "code" => Common::getRequestVar("code"), + "code" => Request::fromGet()->getStringParameter("code"), "redirect_uri" => $this->getRedirectUri(), "grant_type" => "authorization_code", - "state" => Common::getRequestVar("state") + "state" => Request::fromGet()->getStringParameter("state") ); $dataString = http_build_query($data); @@ -284,7 +285,7 @@ class Controller extends \Piwik\Plugin\Controller } else { // users identity has been successfully confirmed by the remote oidc server if (Piwik::isUserIsAnonymous()) { - if ($settings->disableSuperuser->getValue() && Piwik::hasTheUserSuperUserAccess($user["login"])) { + if ($settings->disableSuperuser->getValue() && $this->hasTheUserSuperUserAccess($user["login"])) { throw new Exception(Piwik::translate("LoginOIDC_ExceptionSuperUserOauthDisabled")); } else { $this->signinAndRedirect($user, $settings); @@ -300,6 +301,30 @@ class Controller extends \Piwik\Plugin\Controller } } + /** + * Check whether the given user has superuser access. + * The function in Piwik\Core cannot be used because it requires an admin user being signed in. + * It was used as a template for this function. + * See: {@link \Piwik\Core::hasTheUserSuperUserAccess($theUser)} method. + * See: {@link \Piwik\Plugins\UsersManager\Model::getUsersHavingSuperUserAccess()} method. + * + * @param string $theUser A username to be checked for superuser access + * @return bool + */ + private function hasTheUserSuperUserAccess(string $theUser) + { + $userModel = new Model(); + $superUsers = $userModel->getUsersHavingSuperUserAccess(); + + foreach ($superUsers as $superUser) { + if ($theUser === $superUser['login']) { + return true; + } + } + + return false; + } + /** * Create a link between the remote user and the currently signed in user. * diff --git a/LoginOIDC/LoginOIDC.php b/LoginOIDC/LoginOIDC.php index 6e5789ee66fd331014da5023aeb19856ab6ce7ae..b1ea03d2389bbf15c96c692948d450e319ba9258 100644 --- a/LoginOIDC/LoginOIDC.php +++ b/LoginOIDC/LoginOIDC.php @@ -17,6 +17,7 @@ use Piwik\DbHelper; use Piwik\FrontController; use Piwik\Plugins\LoginOIDC\SystemSettings; use Piwik\Plugins\LoginOIDC\Url; +use Piwik\Request; use Piwik\Session; class LoginOIDC extends \Piwik\Plugin @@ -35,7 +36,8 @@ class LoginOIDC extends \Piwik\Plugin "Template.userSecurity.afterPassword" => "renderLoginOIDCUserSettings", "Template.loginNav" => "renderLoginOIDCMod", "Template.confirmPasswordContent" => "renderConfirmPasswordMod", - "Login.logout" => "logoutMod" + "Login.logout" => "logoutMod", + "Login.userRequiresPasswordConfirmation" => "userRequiresPasswordConfirmation" ); } @@ -61,8 +63,8 @@ class LoginOIDC extends \Piwik\Plugin */ private function shouldHandleRememberMe() : bool { - $module = Common::getRequestVar("module", false); - $action = Common::getRequestVar("action", false); + $module = Request::fromGet()->getStringParameter("module", ""); + $action = Request::fromGet()->getStringParameter("action", ""); return ($module == "LoginOIDC") && ($action == "callback"); } @@ -120,6 +122,8 @@ class LoginOIDC extends \Piwik\Plugin /** * Append login oauth button layout. + * The password confirmation modal is not consistent enough to rely on this modification. + * It is recommended to use the `userRequiresPasswordConfirmation` event instead * * @param string $out * @param string|null $payload @@ -145,9 +149,11 @@ class LoginOIDC extends \Piwik\Plugin $settings = new SystemSettings(); $endSessionUrl = $settings->endSessionUrl->getValue(); if (!empty($endSessionUrl) && $_SESSION["loginoidc_auth"]) { + // make sure we properly unset the plugins session variable + unset($_SESSION['loginoidc_auth']); $endSessionUrl = new Url($endSessionUrl); - if (isset($_SESSION[loginoidc_idtoken])) { - $endSessionUrl->setQueryParameter("id_token_hint", $_SESSION[loginoidc_idtoken]); + if (isset($_SESSION["loginoidc_idtoken"])) { + $endSessionUrl->setQueryParameter("id_token_hint", $_SESSION["loginoidc_idtoken"]); } $originalLogoutUrl = Config::getInstance()->General['login_logout_url']; if ($originalLogoutUrl) { @@ -157,6 +163,22 @@ class LoginOIDC extends \Piwik\Plugin } } + /** + * Disable password confirmation when user signed up with LoginOIDC. + * This feature requires Matomo >4.12.0 + * + * @return void + */ + public function userRequiresPasswordConfirmation(&$requiresPasswordConfirmation, $login) : void + { + $settings = new SystemSettings(); + $disablePasswordConfirmation = $settings->disablePasswordConfirmation->getValue(); + if ($disablePasswordConfirmation) { + // require password confirmation when user has not signed in with the plugin + $requiresPasswordConfirmation = !($_SESSION["loginoidc_auth"] ?? false); + } + } + /** * Extend database. * diff --git a/LoginOIDC/README.md b/LoginOIDC/README.md index b5e18f7fbd8ec3c216632a8637ebd0a796920d24..e27643115162c2c8ec2ba4e11e9ab01824934989 100644 --- a/LoginOIDC/README.md +++ b/LoginOIDC/README.md @@ -1,3 +1,4 @@ + # Matomo LoginOIDC Plugin ## Description diff --git a/LoginOIDC/SystemSettings.php b/LoginOIDC/SystemSettings.php index f78a008a36f4f1940d7a8d6947c57316f8794803..08f2d2e16f10e571535fcdfec14d5f099d57e683 100644 --- a/LoginOIDC/SystemSettings.php +++ b/LoginOIDC/SystemSettings.php @@ -27,6 +27,13 @@ class SystemSettings extends \Piwik\Settings\Plugin\SystemSettings */ public $disableSuperuser; + /** + * The disable password confirmation setting. + * + * @var bool + */ + public $disablePasswordConfirmation; + /** * Whether the login procedure has to be initiated from the Matomo login page * @@ -140,6 +147,7 @@ class SystemSettings extends \Piwik\Settings\Plugin\SystemSettings protected function init() { $this->disableSuperuser = $this->createDisableSuperuserSetting(); + $this->disablePasswordConfirmation = $this->createDisablePasswordConfirmationSetting(); $this->disableDirectLoginUrl = $this->createDisableDirectLoginUrlSetting(); $this->allowSignup = $this->createAllowSignupSetting(); $this->bypassTwoFa = $this->createBypassTwoFaSetting(); @@ -171,6 +179,20 @@ class SystemSettings extends \Piwik\Settings\Plugin\SystemSettings }); } + /** + * Add disable password confirmation setting. + * + * @return SystemSetting + */ + private function createDisablePasswordConfirmationSetting() : SystemSetting + { + return $this->makeSetting("disablePasswordConfirmation", $default = false, FieldConfig::TYPE_BOOL, function(FieldConfig $field) { + $field->title = Piwik::translate("LoginOIDC_SettingDisablePasswordConfirmation"); + $field->description = Piwik::translate("LoginOIDC_SettingDisablePasswordConfirmationHelp"); + $field->uiControl = FieldConfig::UI_CONTROL_CHECKBOX; + }); + } + /** * Add disable direct login url setting. * diff --git a/LoginOIDC/Url.php b/LoginOIDC/Url.php index d97dc43e07fd43f1f0128744d1f3b0efcff947e9..c86efb9fac40991da79d6d6749b24919737e964e 100644 --- a/LoginOIDC/Url.php +++ b/LoginOIDC/Url.php @@ -76,9 +76,10 @@ class Url { $urlParts = parse_url($url); - $this->scheme = $urlParts["scheme"]; - $this->host = $urlParts["host"]; - $this->path = $urlParts["path"]; + $this->scheme = $urlParts["scheme"] ?? null; + $this->host = $urlParts["host"] ?? null; + $this->port = $urlParts["port"] ?? null; + $this->path = $urlParts["path"] ?? null; if (isset($urlParts["query"])) { parse_str($urlParts["query"], $this->query); @@ -87,7 +88,7 @@ class Url /** * Build a full url string based on the parts. - * + * * @return string */ public function buildString() : string @@ -125,7 +126,7 @@ class Url * * @return void */ - public function setQueryParameter(string $parameter, string $value) + public function setQueryParameter(string $parameter, string $value) : void { $this->query[$parameter] = $value; } diff --git a/LoginOIDC/lang/de.json b/LoginOIDC/lang/de.json index e2a1dc7d7e8bd8018e11c7470f4193829dd85f31..3d1d44c27f0226eaf639e7739da3d0783d099180 100644 --- a/LoginOIDC/lang/de.json +++ b/LoginOIDC/lang/de.json @@ -2,6 +2,8 @@ "LoginOIDC": { "SettingDisableSuperuser": "Deaktiviere externen Login-Service für Superuser", "SettingDisableSuperuserHelp": "", + "SettingDisablePasswordConfirmation": "Deaktiviere Passwortbestätigung", + "SettingDisablePasswordConfirmationHelp": "Einige administrative Aufgaben erfordern eine explizite Bestätigung des Benutzerpasswortes. Wenn Benutzer ohne gesetztes Passwort Matomo benutzen kann über diese Einstellung der Bestätigungsdialog überspringbar gemacht werden.", "SettingDisableDirectLoginUrl": "Deaktiviere direkten Login Link", "SettingDisableDirectLoginUrlHelp": "Wenn der Haken gesetzt ist muss der Login über die Login-Seite initiiert werden, anderenfalls lässt sich die Loginschaltfläche auch von dritten Seiten einbinden.", "SettingAllowSignup": "Erstelle automatisch einen neuen Account, wenn sich ein unbekannter neuer Benutzer einloggt", diff --git a/LoginOIDC/lang/en.json b/LoginOIDC/lang/en.json index 43b6373f770a17896068d08d8ba905f7ce7dffda..411697cf15dd1a7edc8ed76bb3849ec489be73a3 100644 --- a/LoginOIDC/lang/en.json +++ b/LoginOIDC/lang/en.json @@ -2,6 +2,8 @@ "LoginOIDC": { "SettingDisableSuperuser": "Disable external login for superusers", "SettingDisableSuperuserHelp": "", + "SettingDisablePasswordConfirmation": "Disable password confirmation", + "SettingDisablePasswordConfirmationHelp": "Certain administrational tasks require a password confirmation. When administrators are using this plugin and do not have a password they could confirm, this option can be used to skip the dialogue, when signed in via LoginOIDC", "SettingDisableDirectLoginUrl": "Disable direct login url", "SettingDisableDirectLoginUrlHelp": "When checked, users have to inititate the login via the Matomo login page. Otherwise the login button can be embedded in third-party services.", "SettingAllowSignup": "Create new users when users try to log in with unknown OIDC accounts", diff --git a/LoginOIDC/plugin.json b/LoginOIDC/plugin.json index 30ffa9b7a3b2a0b67459effe5298b2375e72dedb..69d37e7a8cdc5e9a0da9746609ce447eea8d90f9 100644 --- a/LoginOIDC/plugin.json +++ b/LoginOIDC/plugin.json @@ -1,6 +1,6 @@ { "name": "LoginOIDC", - "version": "4.0.0", + "version": "5.0.0", "description": "Adds support for integrating external authentication services", "keywords": ["authentication", "login", "oauth", "openid", "connect", "sso"], "license": "GPL v3+", @@ -13,7 +13,7 @@ } ], "require": { - "piwik": ">=4.0.0-b1,<5.0.0-b1", + "piwik": ">=5.0.0-b1,<6.0.0-b1", "php": ">=7.0.0" }, "donate": { diff --git a/LoginOIDC/tests/Unit/UrlTest.php b/LoginOIDC/tests/Unit/UrlTest.php new file mode 100644 index 0000000000000000000000000000000000000000..ccdd80f7d3b91ef7589468c9afad527cb465234c --- /dev/null +++ b/LoginOIDC/tests/Unit/UrlTest.php @@ -0,0 +1,76 @@ +<?php +/** + * Matomo - free/libre analytics platform + * + * @link https://matomo.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + */ + +namespace Piwik\Plugins\LoginOIDC\tests\Unit; + +use Piwik\Plugins\LoginOIDC\Url; + +/** + * @group LoginOIDC + * @group UrlTest + * @group Plugins + */ +class UrlTest extends \PHPUnit\Framework\TestCase +{ + /** + * Default url string to test with. + * + * @var string + */ + private string $urlString; + + /** + * Default url object to test with. + * + * @var Url + */ + private Url $url; + + public function setUp() : void + { + parent::setUp(); + $this->urlString = "https://example.com/test?key=val&second=2&third=three"; + $this->url = new Url($this->urlString); + } + + public function testRebuildingUrl() : void + { + $this->assertEquals($this->url->buildString(), $this->urlString); + } + + public function testReadingQueryParameters() : void + { + $this->assertEquals($this->url->getQueryParameter("key"), "val"); + $this->assertEquals($this->url->getQueryParameter("second"), "2"); + $this->assertEquals($this->url->getQueryParameter("third"), "three"); + } + + public function testUpdatingQueryParameters() : void + { + $this->url->setQueryParameter("key", "one"); + $this->url->setQueryParameter("second", "two"); + $this->url->setQueryParameter("third", "3"); + $this->url->setQueryParameter("fourth", "4"); + + $this->assertEquals($this->url->getQueryParameter("key"), "one"); + $this->assertEquals($this->url->getQueryParameter("second"), "two"); + $this->assertEquals($this->url->getQueryParameter("third"), "3"); + $this->assertEquals($this->url->getQueryParameter("fourth"), "4"); + } + + public function testRebuilingModifiedUrl() : void + { + $targetUrlString = "https://example.com/test?key=one&second=two&third=3&fourth=4"; + $this->url->setQueryParameter("key", "one"); + $this->url->setQueryParameter("second", "two"); + $this->url->setQueryParameter("third", "3"); + $this->url->setQueryParameter("fourth", "4"); + + $this->assertEquals($this->url->buildString(), $targetUrlString); + } +} diff --git a/Provider b/Provider index e7ff61fdc9aad1c475aaa0e8cd6a5241c16c82f3..b83dabe984a06034b6fbd16fcc43cb839ef3ed49 160000 --- a/Provider +++ b/Provider @@ -1 +1 @@ -Subproject commit e7ff61fdc9aad1c475aaa0e8cd6a5241c16c82f3 +Subproject commit b83dabe984a06034b6fbd16fcc43cb839ef3ed49