From 30c493ae555f91d76a87340cadef3f3f2ae34e42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Dufraisse?= Date: Fri, 11 Feb 2022 21:35:20 +0100 Subject: [PATCH 1/9] feat(crsftokenmanager): install it --- composer.json | 5 +- composer.lock | 312 +++++++++++++++++++++++---- includes/YesWikiInit.php | 2 + includes/services.yaml | 4 + includes/services/TemplateEngine.php | 16 +- 5 files changed, 298 insertions(+), 41 deletions(-) diff --git a/composer.json b/composer.json index ded240958..997315c9d 100644 --- a/composer.json +++ b/composer.json @@ -34,7 +34,8 @@ "symfony/yaml": "^5.1", "tamtamchik/simple-flash": "^2.0", "twig/twig": "^3.0", - "yeswiki/theme-margot": "dev-master" + "yeswiki/theme-margot": "dev-master", + "symfony/security-csrf": "^5.0" }, "conflict": { "psr/cache": ">=2.0", @@ -45,6 +46,8 @@ "symfony/error-handler": ">=6.0", "symfony/event-dispatcher": ">=6.0", "symfony/filesystem": ">=6.0", + "symfony/password-hasher": ">=6.0", + "symfony/security-core": ">=6.0", "symfony/string": ">=6.0" }, "extra": { diff --git a/composer.lock b/composer.lock index 03164085f..59876ea75 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "215040317134baa2df64f1754c381c80", + "content-hash": "5fe1db9baef4f17aaa55a1aa32d1d0ad", "packages": [ { "name": "caxy/php-htmldiff", @@ -1712,6 +1712,79 @@ ], "time": "2022-01-29T18:08:07+00:00" }, + { + "name": "symfony/password-hasher", + "version": "v5.4.3", + "source": { + "type": "git", + "url": "https://github.com/symfony/password-hasher.git", + "reference": "b5ed59c4536d8386cd37bb86df2b7bd5fbbd46d4" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/password-hasher/zipball/b5ed59c4536d8386cd37bb86df2b7bd5fbbd46d4", + "reference": "b5ed59c4536d8386cd37bb86df2b7bd5fbbd46d4", + "shasum": "" + }, + "require": { + "php": ">=7.2.5", + "symfony/polyfill-php80": "^1.15" + }, + "conflict": { + "symfony/security-core": "<5.3" + }, + "require-dev": { + "symfony/console": "^5", + "symfony/security-core": "^5.3|^6.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "Symfony\\Component\\PasswordHasher\\": "" + }, + "exclude-from-classmap": [ + "/Tests/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Robin Chalas", + "email": "robin.chalas@gmail.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Provides password hashing utilities", + "homepage": "https://symfony.com", + "keywords": [ + "hashing", + "password" + ], + "support": { + "source": "https://github.com/symfony/password-hasher/tree/v5.4.3" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2022-01-02T09:53:40+00:00" + }, { "name": "symfony/polyfill-ctype", "version": "v1.24.0", @@ -1825,12 +1898,12 @@ } }, "autoload": { - "psr-4": { - "Symfony\\Polyfill\\Intl\\Grapheme\\": "" - }, "files": [ "bootstrap.php" - ] + ], + "psr-4": { + "Symfony\\Polyfill\\Intl\\Grapheme\\": "" + } }, "notification-url": "https://packagist.org/downloads/", "license": [ @@ -1906,12 +1979,12 @@ } }, "autoload": { - "psr-4": { - "Symfony\\Polyfill\\Intl\\Normalizer\\": "" - }, "files": [ "bootstrap.php" ], + "psr-4": { + "Symfony\\Polyfill\\Intl\\Normalizer\\": "" + }, "classmap": [ "Resources/stubs" ] @@ -2070,12 +2143,12 @@ } }, "autoload": { - "psr-4": { - "Symfony\\Polyfill\\Php73\\": "" - }, "files": [ "bootstrap.php" ], + "psr-4": { + "Symfony\\Polyfill\\Php73\\": "" + }, "classmap": [ "Resources/stubs" ] @@ -2149,12 +2222,12 @@ } }, "autoload": { - "psr-4": { - "Symfony\\Polyfill\\Php80\\": "" - }, "files": [ "bootstrap.php" ], + "psr-4": { + "Symfony\\Polyfill\\Php80\\": "" + }, "classmap": [ "Resources/stubs" ] @@ -2232,12 +2305,12 @@ } }, "autoload": { - "psr-4": { - "Symfony\\Polyfill\\Php81\\": "" - }, "files": [ "bootstrap.php" ], + "psr-4": { + "Symfony\\Polyfill\\Php81\\": "" + }, "classmap": [ "Resources/stubs" ] @@ -2373,6 +2446,171 @@ ], "time": "2022-01-02T09:53:40+00:00" }, + { + "name": "symfony/security-core", + "version": "v5.4.3", + "source": { + "type": "git", + "url": "https://github.com/symfony/security-core.git", + "reference": "b26a44457a4d1a60c79f1c23273e812c4077ce85" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/security-core/zipball/b26a44457a4d1a60c79f1c23273e812c4077ce85", + "reference": "b26a44457a4d1a60c79f1c23273e812c4077ce85", + "shasum": "" + }, + "require": { + "php": ">=7.2.5", + "symfony/deprecation-contracts": "^2.1|^3", + "symfony/event-dispatcher-contracts": "^1.1|^2|^3", + "symfony/password-hasher": "^5.3|^6.0", + "symfony/polyfill-php80": "^1.16", + "symfony/service-contracts": "^1.1.6|^2|^3" + }, + "conflict": { + "symfony/event-dispatcher": "<4.4", + "symfony/http-foundation": "<5.3", + "symfony/ldap": "<4.4", + "symfony/security-guard": "<4.4", + "symfony/validator": "<5.2" + }, + "require-dev": { + "psr/cache": "^1.0|^2.0|^3.0", + "psr/container": "^1.0|^2.0", + "psr/log": "^1|^2|^3", + "symfony/cache": "^4.4|^5.0|^6.0", + "symfony/event-dispatcher": "^4.4|^5.0|^6.0", + "symfony/expression-language": "^4.4|^5.0|^6.0", + "symfony/http-foundation": "^5.3|^6.0", + "symfony/ldap": "^4.4|^5.0|^6.0", + "symfony/translation": "^4.4|^5.0|^6.0", + "symfony/validator": "^5.2|^6.0" + }, + "suggest": { + "psr/container-implementation": "To instantiate the Security class", + "symfony/event-dispatcher": "", + "symfony/expression-language": "For using the expression voter", + "symfony/http-foundation": "", + "symfony/ldap": "For using LDAP integration", + "symfony/validator": "For using the user password constraint" + }, + "type": "library", + "autoload": { + "psr-4": { + "Symfony\\Component\\Security\\Core\\": "" + }, + "exclude-from-classmap": [ + "/Tests/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Fabien Potencier", + "email": "fabien@symfony.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Symfony Security Component - Core Library", + "homepage": "https://symfony.com", + "support": { + "source": "https://github.com/symfony/security-core/tree/v5.4.3" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2022-01-02T09:53:40+00:00" + }, + { + "name": "symfony/security-csrf", + "version": "v5.4.3", + "source": { + "type": "git", + "url": "https://github.com/symfony/security-csrf.git", + "reference": "57c1c252ca756289c2b61327e08fb10be3936956" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/security-csrf/zipball/57c1c252ca756289c2b61327e08fb10be3936956", + "reference": "57c1c252ca756289c2b61327e08fb10be3936956", + "shasum": "" + }, + "require": { + "php": ">=7.2.5", + "symfony/polyfill-php80": "^1.16", + "symfony/security-core": "^4.4|^5.0|^6.0" + }, + "conflict": { + "symfony/http-foundation": "<5.3" + }, + "require-dev": { + "symfony/http-foundation": "^5.3|^6.0" + }, + "suggest": { + "symfony/http-foundation": "For using the class SessionTokenStorage." + }, + "type": "library", + "autoload": { + "psr-4": { + "Symfony\\Component\\Security\\Csrf\\": "" + }, + "exclude-from-classmap": [ + "/Tests/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Fabien Potencier", + "email": "fabien@symfony.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Symfony Security Component - CSRF Library", + "homepage": "https://symfony.com", + "support": { + "source": "https://github.com/symfony/security-csrf/tree/v5.4.3" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2022-01-02T09:53:40+00:00" + }, { "name": "symfony/service-contracts", "version": "v2.5.0", @@ -2851,12 +3089,12 @@ "source": { "type": "git", "url": "https://github.com/YesWiki/yeswiki-theme-margot.git", - "reference": "0c9fc2d4155dad994656637db0ee3df7bc12f23c" + "reference": "06b61539b7ec0d3f3d19bfb527c22e4e41689672" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/YesWiki/yeswiki-theme-margot/zipball/0c9fc2d4155dad994656637db0ee3df7bc12f23c", - "reference": "0c9fc2d4155dad994656637db0ee3df7bc12f23c", + "url": "https://api.github.com/repos/YesWiki/yeswiki-theme-margot/zipball/06b61539b7ec0d3f3d19bfb527c22e4e41689672", + "reference": "06b61539b7ec0d3f3d19bfb527c22e4e41689672", "shasum": "" }, "require": { @@ -2873,9 +3111,9 @@ ], "support": { "issues": "https://github.com/YesWiki/yeswiki-theme-margot/issues", - "source": "https://github.com/YesWiki/yeswiki-theme-margot/tree/master" + "source": "https://github.com/YesWiki/yeswiki-theme-margot/tree/margot-fun" }, - "time": "2022-01-27T18:06:16+00:00" + "time": "2022-02-08T11:31:22+00:00" } ], "packages-dev": [ @@ -3121,16 +3359,16 @@ }, { "name": "phar-io/version", - "version": "3.1.0", + "version": "3.1.1", "source": { "type": "git", "url": "https://github.com/phar-io/version.git", - "reference": "bae7c545bef187884426f042434e561ab1ddb182" + "reference": "15a90844ad40f127afd244c0cad228de2a80052a" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phar-io/version/zipball/bae7c545bef187884426f042434e561ab1ddb182", - "reference": "bae7c545bef187884426f042434e561ab1ddb182", + "url": "https://api.github.com/repos/phar-io/version/zipball/15a90844ad40f127afd244c0cad228de2a80052a", + "reference": "15a90844ad40f127afd244c0cad228de2a80052a", "shasum": "" }, "require": { @@ -3166,9 +3404,9 @@ "description": "Library for handling version information and constraints", "support": { "issues": "https://github.com/phar-io/version/issues", - "source": "https://github.com/phar-io/version/tree/3.1.0" + "source": "https://github.com/phar-io/version/tree/3.1.1" }, - "time": "2021-02-23T14:00:09+00:00" + "time": "2022-02-07T21:56:48+00:00" }, { "name": "phpdocumentor/reflection-common", @@ -3777,11 +4015,11 @@ } }, "autoload": { - "classmap": [ - "src/" - ], "files": [ "src/Framework/Assert/Functions.php" + ], + "classmap": [ + "src/" ] }, "notification-url": "https://packagist.org/downloads/", @@ -4324,16 +4562,16 @@ }, { "name": "sebastian/global-state", - "version": "5.0.3", + "version": "5.0.4", "source": { "type": "git", "url": "https://github.com/sebastianbergmann/global-state.git", - "reference": "23bd5951f7ff26f12d4e3242864df3e08dec4e49" + "reference": "19c519631c5a511b7ed0ad64a6713fdb3fd25fe4" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/sebastianbergmann/global-state/zipball/23bd5951f7ff26f12d4e3242864df3e08dec4e49", - "reference": "23bd5951f7ff26f12d4e3242864df3e08dec4e49", + "url": "https://api.github.com/repos/sebastianbergmann/global-state/zipball/19c519631c5a511b7ed0ad64a6713fdb3fd25fe4", + "reference": "19c519631c5a511b7ed0ad64a6713fdb3fd25fe4", "shasum": "" }, "require": { @@ -4376,7 +4614,7 @@ ], "support": { "issues": "https://github.com/sebastianbergmann/global-state/issues", - "source": "https://github.com/sebastianbergmann/global-state/tree/5.0.3" + "source": "https://github.com/sebastianbergmann/global-state/tree/5.0.4" }, "funding": [ { @@ -4384,7 +4622,7 @@ "type": "github" } ], - "time": "2021-06-11T13:31:12+00:00" + "time": "2022-02-10T07:01:19+00:00" }, { "name": "sebastian/lines-of-code", diff --git a/includes/YesWikiInit.php b/includes/YesWikiInit.php index 2f74b4dac..2e5defb2c 100644 --- a/includes/YesWikiInit.php +++ b/includes/YesWikiInit.php @@ -44,6 +44,7 @@ use Symfony\Component\Routing\Loader\AnnotationDirectoryLoader; use Symfony\Component\Routing\Route; use Symfony\Component\Routing\RouteCollection; +use Symfony\Component\Security\Csrf\CsrfTokenManager; // TODO put elsewhere // https://github.com/sensiolabs/SensioFrameworkExtraBundle/blob/master/src/Routing/AnnotatedRouteControllerLoader.php @@ -310,6 +311,7 @@ public function initCoreServices($wiki) $containerBuilder->set(Wiki::class, $wiki); $containerBuilder->set(ParameterBagInterface::class, $containerBuilder->getParameterBag()); + $containerBuilder->set(CsrfTokenManager::class, new CsrfTokenManager()); $loader = new YamlFileLoader($containerBuilder, new FileLocator(__DIR__)); $loader->load('services.yaml'); diff --git a/includes/services.yaml b/includes/services.yaml index a6837f874..dd56038d4 100644 --- a/includes/services.yaml +++ b/includes/services.yaml @@ -8,6 +8,10 @@ services: Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface: synthetic: true + # Manually set inside the initCoreService method + Symfony\Component\Security\Csrf\CsrfTokenManager: + synthetic: true + # Manually set inside the initCoreService method # TODO remove this object when refactoring will be finished YesWiki\Wiki: diff --git a/includes/services/TemplateEngine.php b/includes/services/TemplateEngine.php index 5dce207d0..32d8e7e94 100644 --- a/includes/services/TemplateEngine.php +++ b/includes/services/TemplateEngine.php @@ -3,6 +3,7 @@ namespace YesWiki\Core\Service; use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface; +use Symfony\Component\Security\Csrf\CsrfTokenManager; use YesWiki\Wiki; class TemplateNotFound extends \Exception @@ -15,11 +16,17 @@ class TemplateEngine protected $twigLoader; protected $twig; protected $assetsManager; - - public function __construct(Wiki $wiki, ParameterBagInterface $config, AssetsManager $assetsManager) - { + protected $csrfTokenManager; + + public function __construct( + Wiki $wiki, + ParameterBagInterface $config, + AssetsManager $assetsManager, + CsrfTokenManager $csrfTokenManager + ) { $this->wiki = $wiki; $this->assetsManager = $assetsManager; + $this->csrfTokenManager = $csrfTokenManager; // Default path (main namespace) is the root of the project. There are no templates // there, but it's needed to call relative path like render('tools/bazar/templates/...') $this->twigLoader = new \Twig\Loader\FilesystemLoader('./'); @@ -98,6 +105,9 @@ public function __construct(Wiki $wiki, ParameterBagInterface $config, AssetsMan $this->addTwigHelper('include_css', function ($file) { $this->assetsManager->AddCSSFile($file); }); + $this->addTwigHelper('crsfToken', function ($tokenId) { + $this->csrfTokenManager->getToken($tokenId); + }); } private function addTwigHelper($name, $callback) From 8a92be76ef1a4067cf648bfa62f94317cc581f12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Dufraisse?= Date: Sat, 12 Feb 2022 17:24:42 +0100 Subject: [PATCH 2/9] feat(csrftokenmanager): add doc --- docs/csrf.md | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 docs/csrf.md diff --git a/docs/csrf.md b/docs/csrf.md new file mode 100644 index 000000000..dc51efbe1 --- /dev/null +++ b/docs/csrf.md @@ -0,0 +1,55 @@ +# YesWiki's anti CSRF Documentation + +## What is it ? + +CRSF (Cross-site request forgery) is a method of web attack in one click described for example in [Wikipedia](https://en.wikipedia.org/wiki/Cross-site_request_forgery). + +## Usage + +`symfony/security-csrf` is used to manage tokens needed to prevent csrf. It is installed bt default with composer and `YesWiki::loadExtensions()`. + + 1. Get a token and use it into the concerned form or link for dangerous actions reserved to admins or owners (like `deletepage`, delete a user, change password) + - in php code, get a token with this code for example : + ``` + use Symfony\Component\Security\Csrf\CsrfTokenManager; + ... + $token = $this->wiki->services->get(CsrfTokenManager::class)->getToken('tokenId'); + ``` + - in `twig` template, use `{{ crsfToken('tokenId') }}` + 2. when processing a request with the token, check if it is right inspiring of this example : + ``` + use Symfony\Component\Security\Csrf\CsrfToken; + use Symfony\Component\Security\Csrf\CsrfTokenManager; + ... + $csrfTokenManager = $this->wiki->services->get(CsrfTokenManager::class); + $token = new CsrfToken('tokenId', filter_input(INPUT_POST,'token', FILTER_SANITIZE_STRING)); + // replace 'token' by the used name in form's input + if ($csrfTokenManager->isTokenValid($token)) { + ... + $csrfTokenManager->removeToken('tokenId'); // remove it if you want only one usage + } + ``` + +## Refreshing a token + +You can refresh a token to delete its previous value and replace it by a new one. +``` +$token = $this->wiki->services->get(CsrfTokenManager::class)->refreshToken('tokenId'); +``` +Previous token will be considered as invalid after calling `refreshToken`. + +## Rules to name 'tokenId' + +We propose the following rule to name token and avoid trouble between methods. +`\\`. +For api we can have +`METHOD api/route/id`. + +Examples: + - delete a page `handler\deletepage\MyPageTag` + - delete a user `action\usertable\deleteuser\UserName` + - password update `action\usersettings\changepass` + - password update `DELETE api/pages/MyPageTag` + +It is possible to add the tool's name like `login\action\usersettings\changepass`. + From 47025e9b845c9cba6d3df0d464cdf7016012fae3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Dufraisse?= Date: Sat, 12 Feb 2022 17:47:09 +0100 Subject: [PATCH 3/9] feat(usersettingsAction): use csrf token to change password --- tools/login/actions/usersettings.php | 35 ++++++++++++++++++++++------ tools/login/lang/login_ca.inc.php | 7 ++++++ tools/login/lang/login_en.inc.php | 6 +++++ tools/login/lang/login_es.inc.php | 6 +++++ tools/login/lang/login_fr.inc.php | 6 +++++ tools/login/lang/login_nl.inc.php | 6 +++++ tools/login/lang/login_pt.inc.php | 6 +++++ 7 files changed, 65 insertions(+), 7 deletions(-) diff --git a/tools/login/actions/usersettings.php b/tools/login/actions/usersettings.php index 294e90ef4..529cbbefa 100644 --- a/tools/login/actions/usersettings.php +++ b/tools/login/actions/usersettings.php @@ -4,6 +4,9 @@ Software under AGPL Licence */ +use Symfony\Component\Security\Csrf\CsrfToken; +use Symfony\Component\Security\Csrf\CsrfTokenManager; + if (!defined('WIKINI_VERSION')) { die('accès direct interdit'); } @@ -84,13 +87,30 @@ if (!$this->user->checkPassword($_POST['oldpass'])) { // check password first $error = $this->user->error; } else { // user properly typed his old password in - $password = $_POST['password']; - if ($this->user->updatePassword($password)) { - $this->session->setMessage(_t('USER_PASSWORD_CHANGED').' !'); - $this->user->logIn(); - $this->Redirect($this->href()); - } else { // Something when wrong when updating the user in DB - $this->session->setMessage($this->user->error); + // check token + $csrfTokenManager = $this->services->get(CsrfTokenManager::class); + try { + $postToken = filter_input(INPUT_POST, 'crsf-token', FILTER_SANITIZE_STRING); + if (is_null($postToken) || $postToken === false) { + throw new \Exception(_t('USERSTTINGS_CHANGE_PASS_NO_TOKEN_ERROR')); + } + $token = new CsrfToken('login\action\usersettings\changepass', $postToken); + if (!$csrfTokenManager->isTokenValid($token)) { + $csrfTokenManager->removeToken('login\action\usersettings\changepass'); + throw new \Exception(_t('USERSTTINGS_CHANGE_PASS_TOKEN_FAIL_ERROR')); + } + $csrfTokenManager->removeToken('login\action\usersettings\changepass'); + + $password = $_POST['password']; + if ($this->user->updatePassword($password)) { + $this->session->setMessage(_t('USER_PASSWORD_CHANGED').' !'); + $this->user->logIn(); + $this->Redirect($this->href()); + } else { // Something when wrong when updating the user in DB + $this->session->setMessage($this->user->error); + } + } catch (\Throwable $th) { + $error = $th->getMessage(); } } } // End of changepass action @@ -178,6 +198,7 @@
+
diff --git a/tools/login/lang/login_ca.inc.php b/tools/login/lang/login_ca.inc.php index 14eaa7132..a50684714 100755 --- a/tools/login/lang/login_ca.inc.php +++ b/tools/login/lang/login_ca.inc.php @@ -71,4 +71,11 @@ // actions/login.php // 'LOGIN_COOKIES_ERROR' => 'Vous devez accepter les cookies pour pouvoir vous connecter.', + + // actions/usersettins.php + 'USERSTTINGS_CHANGE_PASS_NO_TOKEN_ERROR' => 'Error de disseny del lloc: el nou formulari d\'enviament de contrasenyes '. + 'no contenia el testimoni d\'identificació únic necessari per als mecanismes de seguretat interns. '. + 'La contrasenya no ha canviat.', + 'USERSTTINGS_CHANGE_PASS_TOKEN_FAIL_ERROR' => 'La contrasenya no s\'ha canviat perquè és possible que aquesta pàgina s\'hagi obert per segona vegada. '. + 'Si us plau, renoveu la sol·licitud de canvi des d\'aquesta finestra (el testimoni de seguretat intern no era bo). ', ]; diff --git a/tools/login/lang/login_en.inc.php b/tools/login/lang/login_en.inc.php index 4ec9ba455..02344acf1 100755 --- a/tools/login/lang/login_en.inc.php +++ b/tools/login/lang/login_en.inc.php @@ -71,4 +71,10 @@ // actions/login.php // 'LOGIN_COOKIES_ERROR' => 'Vous devez accepter les cookies pour pouvoir vous connecter.', + + // actions/usersettins.php + 'USERSTTINGS_CHANGE_PASS_NO_TOKEN_ERROR' => 'Site design error: The new password submission form did not contain the unique identification token '. + 'needed for internal security mechanisms. Password not changed.', + 'USERSTTINGS_CHANGE_PASS_TOKEN_FAIL_ERROR' => 'Password not changed because this page may have been opened a second time. '. + 'Please renew the change request from this window (the internal security token was not good). ', ]; diff --git a/tools/login/lang/login_es.inc.php b/tools/login/lang/login_es.inc.php index 74a005878..ce91426a7 100755 --- a/tools/login/lang/login_es.inc.php +++ b/tools/login/lang/login_es.inc.php @@ -71,4 +71,10 @@ // actions/login.php // 'LOGIN_COOKIES_ERROR' => 'Vous devez accepter les cookies pour pouvoir vous connecter.', + + // actions/usersettins.php + 'USERSTTINGS_CHANGE_PASS_NO_TOKEN_ERROR' => 'Error de diseño del sitio: el nuevo formulario de envío de contraseña no contenía el token de identificación único '. + 'necesario para los mecanismos de seguridad internos. Contraseña no cambiada.', + 'USERSTTINGS_CHANGE_PASS_TOKEN_FAIL_ERROR' => 'La contraseña no se ha cambiado porque es posible que esta página se haya abierto por segunda vez. '. + 'Renueve la solicitud de cambio desde esta ventana (el token de seguridad interno no era bueno).', ]; diff --git a/tools/login/lang/login_fr.inc.php b/tools/login/lang/login_fr.inc.php index 9ea04daa6..fc35d3adc 100755 --- a/tools/login/lang/login_fr.inc.php +++ b/tools/login/lang/login_fr.inc.php @@ -71,4 +71,10 @@ // actions/login.php 'LOGIN_COOKIES_ERROR' => 'Vous devez accepter les cookies pour pouvoir vous connecter.', + + // actions/usersettins.php + 'USERSTTINGS_CHANGE_PASS_NO_TOKEN_ERROR' => 'Erreur de conception du site : Le formulaire de soumission du nouveau mot de passe ne contenait pas '. + 'le jeton d\'identification unique nécessaire aux mécanismes internes de sécurité. Mot de passe non modifié.', + 'USERSTTINGS_CHANGE_PASS_TOKEN_FAIL_ERROR' => 'Mot de passe non modifié car cette page a peut-être été ouverte une seconde fois. '. + 'Veuillez renouveler la demande de changement depuis cette fenêtre (le jeton interne de sécurité n\'était pas bon).', ]; diff --git a/tools/login/lang/login_nl.inc.php b/tools/login/lang/login_nl.inc.php index f29d0b96a..85d589e9b 100755 --- a/tools/login/lang/login_nl.inc.php +++ b/tools/login/lang/login_nl.inc.php @@ -71,4 +71,10 @@ // actions/login.php // 'LOGIN_COOKIES_ERROR' => 'Vous devez accepter les cookies pour pouvoir vous connecter.', + + // actions/usersettins.php + 'USERSTTINGS_CHANGE_PASS_NO_TOKEN_ERROR' => 'Fout bij siteontwerp: het nieuwe formulier voor het indienen van wachtwoorden bevatte niet het '. + 'unieke identificatietoken dat nodig is voor interne beveiligingsmechanismen. Wachtwoord niet gewijzigd.', + 'USERSTTINGS_CHANGE_PASS_TOKEN_FAIL_ERROR' => 'Wachtwoord niet gewijzigd omdat deze pagina mogelijk een tweede keer is geopend. '. + 'Verleng het wijzigingsverzoek vanuit dit venster (het interne beveiligingstoken was niet goed).', ]; diff --git a/tools/login/lang/login_pt.inc.php b/tools/login/lang/login_pt.inc.php index 3f375ac89..b288ee18f 100755 --- a/tools/login/lang/login_pt.inc.php +++ b/tools/login/lang/login_pt.inc.php @@ -71,4 +71,10 @@ // actions/login.php // 'LOGIN_COOKIES_ERROR' => 'Vous devez accepter les cookies pour pouvoir vous connecter.', + + // actions/usersettins.php + 'USERSTTINGS_CHANGE_PASS_NO_TOKEN_ERROR' => 'Erro de conceção do site: O novo formulário de submissão da palavra-passe não continha o símbolo de '. + 'identificação único necessário para os mecanismos de segurança interna. A palavra-passe não foi alterada.', + 'USERSTTINGS_CHANGE_PASS_TOKEN_FAIL_ERROR' => 'A palavra-passe não foi alterada porque esta página pode ter sido aberta uma segunda vez. '. + 'Por favor, renove o pedido de alteração desta janela (o sinal de segurança interna não foi bom).', ]; From 220e764767e551b5761139dbea343f5c12443e5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Dufraisse?= Date: Sat, 12 Feb 2022 18:04:45 +0100 Subject: [PATCH 4/9] feat(anti-csrf): also for usersettings deleteByAdmin and updateUser --- tools/login/actions/usersettings.php | 96 +++++++++++++++++++--------- 1 file changed, 67 insertions(+), 29 deletions(-) diff --git a/tools/login/actions/usersettings.php b/tools/login/actions/usersettings.php index 529cbbefa..b8cbbf58d 100644 --- a/tools/login/actions/usersettings.php +++ b/tools/login/actions/usersettings.php @@ -4,6 +4,7 @@ Software under AGPL Licence */ +use Symfony\Component\Security\Csrf\Exception\TokenNotFoundException; use Symfony\Component\Security\Csrf\CsrfToken; use Symfony\Component\Security\Csrf\CsrfTokenManager; @@ -11,6 +12,9 @@ die('accès direct interdit'); } +// get services +$csrfTokenManager = $this->services->get(CsrfTokenManager::class); + $userLoggedIn = false; $referrer=''; $isAdmin = $this->UserIsAdmin(); @@ -48,38 +52,68 @@ $this->Redirect($this->href()); } elseif ($adminIsActing || $userLoggedIn) { // Admin or user wants to manage the user if (substr($action, 0, 6) == 'update') { // Whoever it is tries to update the user - $OK = $this->user->setByAssociativeArray(array( - 'email' => isset($_POST['email']) ? $_POST['email'] : '', - 'motto' => isset($_POST['motto']) ? $_POST['motto'] : '', - 'revisioncount' => isset($_POST['revisioncount']) ? $_POST['revisioncount'] : '', - 'changescount' => isset($_POST['changescount']) ? $_POST['changescount'] : '', - 'doubleclickedit' => isset($_POST['doubleclickedit']) ? $_POST['doubleclickedit'] : '', - 'show_comments' => isset($_POST['show_comments']) ? $_POST['show_comments'] : '', - )); - if ($OK) { - $OK = $this->user->updateIntoDB('email, motto, revisioncount, changescount, doubleclickedit, show_comments'); - if ($userLoggedIn) { // In case it's the user trying to update oneself, need to reset the cooky - $this->user->logIn(); + try { + $postToken = filter_input(INPUT_POST, 'crsf-token', FILTER_SANITIZE_STRING); + if (is_null($postToken) || $postToken === false) { + throw new TokenNotFoundException(_t('USERSTTINGS_CHANGE_PASS_NO_TOKEN_ERROR')); + } + $token = new CsrfToken('login\action\usersettings\updateuser', $postToken); + if (!$csrfTokenManager->isTokenValid($token)) { + $csrfTokenManager->removeToken('login\action\usersettings\updateuser'); + throw new TokenNotFoundException(_t('USERSTTINGS_CHANGE_PASS_TOKEN_FAIL_ERROR')); } - // forward - $this->session->setMessage(_t('USER_PARAMETERS_SAVED').' !'); - if ($userLoggedIn) { // In case it's the usther trying to update oneself - $this->Redirect($this->href()); - } else { // That's the admin acting, we need to pass the user on - $this->Redirect($this->href('', '', 'user='.$_GET['user'].'&from='.$referrer, false)); + $csrfTokenManager->removeToken('login\action\usersettings\updateuser'); + + $OK = $this->user->setByAssociativeArray(array( + 'email' => isset($_POST['email']) ? $_POST['email'] : '', + 'motto' => isset($_POST['motto']) ? $_POST['motto'] : '', + 'revisioncount' => isset($_POST['revisioncount']) ? $_POST['revisioncount'] : '', + 'changescount' => isset($_POST['changescount']) ? $_POST['changescount'] : '', + 'doubleclickedit' => isset($_POST['doubleclickedit']) ? $_POST['doubleclickedit'] : '', + 'show_comments' => isset($_POST['show_comments']) ? $_POST['show_comments'] : '', + )); + if ($OK) { + $OK = $this->user->updateIntoDB('email, motto, revisioncount, changescount, doubleclickedit, show_comments'); + if ($userLoggedIn) { // In case it's the user trying to update oneself, need to reset the cooky + $this->user->logIn(); + } + // forward + $this->session->setMessage(_t('USER_PARAMETERS_SAVED').' !'); + if ($userLoggedIn) { // In case it's the usther trying to update oneself + $this->Redirect($this->href()); + } else { // That's the admin acting, we need to pass the user on + $this->Redirect($this->href('', '', 'user='.$_GET['user'].'&from='.$referrer, false)); + } + } else { // Unable to update + $this->session->setMessage($this->user->error); } - } else { // Unable to update - $this->session->setMessage($this->user->error); + } catch (TokenNotFoundException $th) { + $errorUpdate = $th->getMessage(); } } // End of update action if ($adminIsActing) { // Admin wants to manage the user if ($action == 'deleteByAdmin') { // Admin trying to delete user - $this->user->delete(); - // forward - $this->session->setMessage(_t('USER_DELETED').' !'); - $this->Redirect($this->href('', $referrer)); + try { + $postToken = filter_input(INPUT_POST, 'crsf-token', FILTER_SANITIZE_STRING); + if (is_null($postToken) || $postToken === false) { + throw new TokenNotFoundException(_t('USERSTTINGS_CHANGE_PASS_NO_TOKEN_ERROR')); + } + $token = new CsrfToken('login\action\usersettings\deleteByAdmin', $postToken); + if (!$csrfTokenManager->isTokenValid($token)) { + $csrfTokenManager->removeToken('login\action\usersettings\deleteByAdmin'); + throw new TokenNotFoundException(_t('USERSTTINGS_CHANGE_PASS_TOKEN_FAIL_ERROR')); + } + $csrfTokenManager->removeToken('login\action\usersettings\deleteByAdmin'); + + $this->user->delete(); + // forward + $this->session->setMessage(_t('USER_DELETED').' !'); + $this->Redirect($this->href('', $referrer)); + } catch (TokenNotFoundException $th) { + $errorUpdate = $th->getMessage(); + } } // End of delete by admin action } elseif ($userLoggedIn) { // Admin isn't acting therefore that's an already logged in user @@ -88,16 +122,15 @@ $error = $this->user->error; } else { // user properly typed his old password in // check token - $csrfTokenManager = $this->services->get(CsrfTokenManager::class); try { $postToken = filter_input(INPUT_POST, 'crsf-token', FILTER_SANITIZE_STRING); if (is_null($postToken) || $postToken === false) { - throw new \Exception(_t('USERSTTINGS_CHANGE_PASS_NO_TOKEN_ERROR')); + throw new TokenNotFoundException(_t('USERSTTINGS_CHANGE_PASS_NO_TOKEN_ERROR')); } $token = new CsrfToken('login\action\usersettings\changepass', $postToken); if (!$csrfTokenManager->isTokenValid($token)) { $csrfTokenManager->removeToken('login\action\usersettings\changepass'); - throw new \Exception(_t('USERSTTINGS_CHANGE_PASS_TOKEN_FAIL_ERROR')); + throw new TokenNotFoundException(_t('USERSTTINGS_CHANGE_PASS_TOKEN_FAIL_ERROR')); } $csrfTokenManager->removeToken('login\action\usersettings\changepass'); @@ -109,7 +142,7 @@ } else { // Something when wrong when updating the user in DB $this->session->setMessage($this->user->error); } - } catch (\Throwable $th) { + } catch (TokenNotFoundException $th) { $error = $th->getMessage(); } } @@ -122,6 +155,9 @@ if ($adminIsActing) { echo ' — '.$this->user->getProperty('name'); } ?> + +
+ href('', '', 'user='.$this->user->getProperty('name').'&from='.$referrer, false); @@ -149,6 +185,7 @@ ?>
+
+ FormClose(); @@ -198,7 +236,7 @@
- +
From e19385388c1ec006ca60f89c2bc7f1afebbb9242 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Dufraisse?= Date: Sun, 13 Feb 2022 12:10:46 +0100 Subject: [PATCH 5/9] feat(csrftokenmanager): define option for refresh directly from twig --- docs/csrf.md | 4 ++++ includes/services/TemplateEngine.php | 17 ++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/docs/csrf.md b/docs/csrf.md index dc51efbe1..138deb46d 100644 --- a/docs/csrf.md +++ b/docs/csrf.md @@ -33,9 +33,13 @@ CRSF (Cross-site request forgery) is a method of web attack in one click describ ## Refreshing a token You can refresh a token to delete its previous value and replace it by a new one. + - in PHP ``` $token = $this->wiki->services->get(CsrfTokenManager::class)->refreshToken('tokenId'); ``` + - in `twig` template, use `{{ crsfToken({id:'tokenId',refresh:true}) }}`; +``` + Previous token will be considered as invalid after calling `refreshToken`. ## Rules to name 'tokenId' diff --git a/includes/services/TemplateEngine.php b/includes/services/TemplateEngine.php index 32d8e7e94..8cd92059c 100644 --- a/includes/services/TemplateEngine.php +++ b/includes/services/TemplateEngine.php @@ -2,6 +2,7 @@ namespace YesWiki\Core\Service; +use Exception; use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface; use Symfony\Component\Security\Csrf\CsrfTokenManager; use YesWiki\Wiki; @@ -106,7 +107,21 @@ public function __construct( $this->assetsManager->AddCSSFile($file); }); $this->addTwigHelper('crsfToken', function ($tokenId) { - $this->csrfTokenManager->getToken($tokenId); + if (is_string($tokenId)) { + return $this->csrfTokenManager->getToken($tokenId); + } elseif (is_array($tokenId)) { + if (!isset($tokenId['id'])) { + throw new Exception("When array, `\$tokenId` should contain `id` key !"); + } else { + if (isset($tokenId['refresh']) && $tokenId['refresh'] === true) { + return $this->csrfTokenManager->grefreshToken($tokenId['id']); + } else { + return $this->csrfTokenManager->getToken($tokenId['id']); + } + } + } else { + throw new Exception("`\$tokenId` should be a string or an array !"); + } }); } From a0f2c41277b68cb20d82dcb777cda581f3b9e47d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Dufraisse?= Date: Sun, 13 Feb 2022 12:26:59 +0100 Subject: [PATCH 6/9] feat(csrftokencontroller): create it and use it in usersettings action --- docs/csrf.md | 16 +++++- includes/controllers/CsrfTokenController.php | 59 ++++++++++++++++++++ includes/services.yaml | 3 + lang/yeswiki_ca.php | 6 ++ lang/yeswiki_en.php | 6 ++ lang/yeswiki_es.php | 6 ++ lang/yeswiki_fr.php | 6 ++ lang/yeswiki_nl.php | 6 ++ lang/yeswiki_pt.php | 6 ++ tools/login/actions/usersettings.php | 42 +++----------- tools/login/lang/login_ca.inc.php | 10 ++-- tools/login/lang/login_en.inc.php | 9 ++- tools/login/lang/login_es.inc.php | 9 ++- tools/login/lang/login_fr.inc.php | 9 ++- tools/login/lang/login_nl.inc.php | 9 ++- tools/login/lang/login_pt.inc.php | 9 ++- 16 files changed, 145 insertions(+), 66 deletions(-) create mode 100644 includes/controllers/CsrfTokenController.php diff --git a/docs/csrf.md b/docs/csrf.md index 138deb46d..c53377d40 100644 --- a/docs/csrf.md +++ b/docs/csrf.md @@ -22,7 +22,7 @@ CRSF (Cross-site request forgery) is a method of web attack in one click describ use Symfony\Component\Security\Csrf\CsrfTokenManager; ... $csrfTokenManager = $this->wiki->services->get(CsrfTokenManager::class); - $token = new CsrfToken('tokenId', filter_input(INPUT_POST,'token', FILTER_SANITIZE_STRING)); + $token = new CsrfToken('tokenId', filter_input(INPUT_POST,'tokenNameInForm', FILTER_SANITIZE_STRING)); // replace 'token' by the used name in form's input if ($csrfTokenManager->isTokenValid($token)) { ... @@ -30,6 +30,20 @@ CRSF (Cross-site request forgery) is a method of web attack in one click describ } ``` + or with a controller throwing `TokenNotFoundException` : + ``` + use Symfony\Component\Security\Csrf\Exception\TokenNotFoundException; + use YesWiki\Core\Controller\CsrfTokenController; + $csrfTokenController = $this->wiki->services->get(CsrfTokenController::class); + try { + $csrfTokenController->checkTockenThenRemove('tokenId', 'POST', 'tokenNameInForm'); + ... code if OK + } catch (TokenNotFoundException $th) { + $errorMessage = $th->getMessage(); + ... // code if not OK + } + ``` + ## Refreshing a token You can refresh a token to delete its previous value and replace it by a new one. diff --git a/includes/controllers/CsrfTokenController.php b/includes/controllers/CsrfTokenController.php new file mode 100644 index 000000000..a506c1f37 --- /dev/null +++ b/includes/controllers/CsrfTokenController.php @@ -0,0 +1,59 @@ +csrfTokenManager = $csrfTokenManager; + } + + /** + * check if token is present and valid in input + * throw TokenNotFoundException or Exception + * + * @param string $name + * @param string $inputType "GET" or "POST" + * @param string $inputKey key in the input to use + * @return bool + */ + public function checkTockenThenRemove(string $name, string $inputType, string $inputKey): bool + { + if (empty($name)) { + throw new Exception("parameter `\$name` should not be empty !"); + } + switch ($inputType) { + case 'GET': + $inputToken = filter_input(INPUT_GET, $inputKey, FILTER_SANITIZE_STRING); + break; + + case 'POST': + $inputToken = filter_input(INPUT_POST, $inputKey, FILTER_SANITIZE_STRING); + break; + + default: + throw new Exception("Unknown type for parameter `\$inputType` !"); + return false; + } + if (is_null($inputToken) || $inputToken === false) { + throw new TokenNotFoundException(_t('NO_CSRF_TOKEN_ERROR')); + } + $token = new CsrfToken($name, $inputToken); + $isValid = $this->csrfTokenManager->isTokenValid($token); + $this->csrfTokenManager->removeToken($name); + if (!$isValid) { + throw new TokenNotFoundException(_t('CSRF_TOKEN_FAIL_ERROR')); + } + return true; + } +} diff --git a/includes/services.yaml b/includes/services.yaml index dd56038d4..739469668 100644 --- a/includes/services.yaml +++ b/includes/services.yaml @@ -19,3 +19,6 @@ services: YesWiki\Core\Service\: resource: 'services/*' + + YesWiki\Core\Controller\: + resource: 'controllers/*' diff --git a/lang/yeswiki_ca.php b/lang/yeswiki_ca.php index fa60bc186..cb9368c26 100755 --- a/lang/yeswiki_ca.php +++ b/lang/yeswiki_ca.php @@ -269,6 +269,12 @@ // actions/wantedpages.php 'NO_PAGE_TO_CREATE' => 'No hi ha cap pàgina per crear', + // includes/controllers/CrsfController.php + 'NO_CSRF_TOKEN_ERROR' => 'Error de disseny del lloc: el formulari d\'enviament no contenia el testimoni '. + 'd\'identificació únic necessari per als mecanismes de seguretat interns.', + 'CSRF_TOKEN_FAIL_ERROR' => 'Pot ser que aquesta pàgina s\'hagi obert per segona vegada. '. + 'Si us plau, renoveu la sol·licitud des d\'aquesta finestra (el testimoni de seguretat intern no era bo).', + // setup/header.php 'OK' => 'D\'acord', 'FAIL' => 'Error', diff --git a/lang/yeswiki_en.php b/lang/yeswiki_en.php index 7d75e188e..5ba21fe27 100755 --- a/lang/yeswiki_en.php +++ b/lang/yeswiki_en.php @@ -266,6 +266,12 @@ // actions/wantedpages.php 'NO_PAGE_TO_CREATE' => 'No page to create', + + // includes/controllers/CrsfController.php + 'NO_CSRF_TOKEN_ERROR' => 'Site design error: The submission form did not contain the unique '. + 'identification token needed for internal security mechanisms.', + 'CSRF_TOKEN_FAIL_ERROR' => 'This page may have been opened a second time. '. + 'Please renew the request from this window (the internal security token was not good).', // setup/header.php 'OK' => 'OK', diff --git a/lang/yeswiki_es.php b/lang/yeswiki_es.php index ae569bcc8..12bc3d8ea 100755 --- a/lang/yeswiki_es.php +++ b/lang/yeswiki_es.php @@ -271,6 +271,12 @@ // actions/wantedpages.php 'NO_PAGE_TO_CREATE' => 'Ninguna página para crear', + // includes/controllers/CrsfController.php + 'NO_CSRF_TOKEN_ERROR' => 'Error de diseño del sitio: el formulario de envío no contenía el token '. + 'de identificación único necesario para los mecanismos de seguridad internos.', + 'CSRF_TOKEN_FAIL_ERROR' => 'Es posible que esta página se haya abierto por segunda vez. '. + 'Renueve la solicitud desde esta ventana (el token de seguridad interno no era bueno).', + // setup/header.php 'OK' => 'OK', 'FAIL' => 'FRACASO', diff --git a/lang/yeswiki_fr.php b/lang/yeswiki_fr.php index b324bcf89..fdc03a2c0 100755 --- a/lang/yeswiki_fr.php +++ b/lang/yeswiki_fr.php @@ -267,6 +267,12 @@ // actions/wantedpages.php 'NO_PAGE_TO_CREATE' => 'Aucune page à créer', + // includes/controllers/CrsfController.php + 'NO_CSRF_TOKEN_ERROR' => 'Erreur de conception du site : Le formulaire de soumission ne contenait pas '. + 'le jeton d\'identification unique nécessaire aux mécanismes internes de sécurité.', + 'CSRF_TOKEN_FAIL_ERROR' => 'Cette page a peut-être été ouverte une seconde fois. '. + 'Veuillez renouveler la demande depuis cette fenêtre (le jeton interne de sécurité n\'était pas bon).', + // setup/header.php 'OK' => 'OK', 'FAIL' => 'ECHEC', diff --git a/lang/yeswiki_nl.php b/lang/yeswiki_nl.php index 9d9ee3b77..6c28448f3 100755 --- a/lang/yeswiki_nl.php +++ b/lang/yeswiki_nl.php @@ -268,6 +268,12 @@ // actions/wantedpages.php 'NO_PAGE_TO_CREATE' => 'Geen enkele pagina aan te maken', + // includes/controllers/CrsfController.php + 'NO_CSRF_TOKEN_ERROR' => 'Fout bij het ontwerpen van de site: het indieningsformulier bevatte niet het unieke identificatietoken '. + 'dat nodig is voor interne beveiligingsmechanismen.', + 'CSRF_TOKEN_FAIL_ERROR' => 'Deze pagina is mogelijk een tweede keer geopend. '. + 'Verleng het verzoek vanuit dit venster (het interne beveiligingstoken was niet goed).', + // setup/header.php 'OK' => 'OK', 'FAIL' => 'MISLUKT', diff --git a/lang/yeswiki_pt.php b/lang/yeswiki_pt.php index f2a78b821..e4209fa99 100755 --- a/lang/yeswiki_pt.php +++ b/lang/yeswiki_pt.php @@ -267,6 +267,12 @@ // actions/wantedpages.php 'NO_PAGE_TO_CREATE' => 'Nenhuma página para criar', + // includes/controllers/CrsfController.php + 'NO_CSRF_TOKEN_ERROR' => 'Erro de conceção do local: O formulário de submissão não continha o símbolo de identificação '. + 'único necessário para os mecanismos de segurança interna.', + 'CSRF_TOKEN_FAIL_ERROR' => 'Esta página pode ter sido aberta uma segunda vez. '. + 'Por favor, renove o pedido desta janela (o sinal de segurança interna não foi bom).', + // setup/header.php 'OK' => 'OK', 'FAIL' => 'FALHA', diff --git a/tools/login/actions/usersettings.php b/tools/login/actions/usersettings.php index b8cbbf58d..cd054d074 100644 --- a/tools/login/actions/usersettings.php +++ b/tools/login/actions/usersettings.php @@ -5,8 +5,8 @@ */ use Symfony\Component\Security\Csrf\Exception\TokenNotFoundException; -use Symfony\Component\Security\Csrf\CsrfToken; use Symfony\Component\Security\Csrf\CsrfTokenManager; +use YesWiki\Core\Controller\CsrfTokenController; if (!defined('WIKINI_VERSION')) { die('accès direct interdit'); @@ -14,6 +14,7 @@ // get services $csrfTokenManager = $this->services->get(CsrfTokenManager::class); +$csrfTokenController = $this->services->get(CsrfTokenController::class); $userLoggedIn = false; $referrer=''; @@ -53,16 +54,7 @@ } elseif ($adminIsActing || $userLoggedIn) { // Admin or user wants to manage the user if (substr($action, 0, 6) == 'update') { // Whoever it is tries to update the user try { - $postToken = filter_input(INPUT_POST, 'crsf-token', FILTER_SANITIZE_STRING); - if (is_null($postToken) || $postToken === false) { - throw new TokenNotFoundException(_t('USERSTTINGS_CHANGE_PASS_NO_TOKEN_ERROR')); - } - $token = new CsrfToken('login\action\usersettings\updateuser', $postToken); - if (!$csrfTokenManager->isTokenValid($token)) { - $csrfTokenManager->removeToken('login\action\usersettings\updateuser'); - throw new TokenNotFoundException(_t('USERSTTINGS_CHANGE_PASS_TOKEN_FAIL_ERROR')); - } - $csrfTokenManager->removeToken('login\action\usersettings\updateuser'); + $csrfTokenController->checkTockenThenRemove('login\action\usersettings\updateuser', 'POST', 'crsf-token'); $OK = $this->user->setByAssociativeArray(array( 'email' => isset($_POST['email']) ? $_POST['email'] : '', @@ -88,7 +80,7 @@ $this->session->setMessage($this->user->error); } } catch (TokenNotFoundException $th) { - $errorUpdate = $th->getMessage(); + $errorUpdate = _t('USERSETTINGS_EMAIL_NOT_CHANGED') .' '. $th->getMessage(); } } // End of update action @@ -96,23 +88,14 @@ if ($action == 'deleteByAdmin') { // Admin trying to delete user try { - $postToken = filter_input(INPUT_POST, 'crsf-token', FILTER_SANITIZE_STRING); - if (is_null($postToken) || $postToken === false) { - throw new TokenNotFoundException(_t('USERSTTINGS_CHANGE_PASS_NO_TOKEN_ERROR')); - } - $token = new CsrfToken('login\action\usersettings\deleteByAdmin', $postToken); - if (!$csrfTokenManager->isTokenValid($token)) { - $csrfTokenManager->removeToken('login\action\usersettings\deleteByAdmin'); - throw new TokenNotFoundException(_t('USERSTTINGS_CHANGE_PASS_TOKEN_FAIL_ERROR')); - } - $csrfTokenManager->removeToken('login\action\usersettings\deleteByAdmin'); + $csrfTokenController->checkTockenThenRemove('login\action\usersettings\deleteByAdmin', 'POST', 'crsf-token'); $this->user->delete(); // forward $this->session->setMessage(_t('USER_DELETED').' !'); $this->Redirect($this->href('', $referrer)); } catch (TokenNotFoundException $th) { - $errorUpdate = $th->getMessage(); + $errorUpdate = _t('USERSETTINGS_USER_NOT_DELETED') .' '. $th->getMessage(); } } // End of delete by admin action } elseif ($userLoggedIn) { // Admin isn't acting therefore that's an already logged in user @@ -123,16 +106,7 @@ } else { // user properly typed his old password in // check token try { - $postToken = filter_input(INPUT_POST, 'crsf-token', FILTER_SANITIZE_STRING); - if (is_null($postToken) || $postToken === false) { - throw new TokenNotFoundException(_t('USERSTTINGS_CHANGE_PASS_NO_TOKEN_ERROR')); - } - $token = new CsrfToken('login\action\usersettings\changepass', $postToken); - if (!$csrfTokenManager->isTokenValid($token)) { - $csrfTokenManager->removeToken('login\action\usersettings\changepass'); - throw new TokenNotFoundException(_t('USERSTTINGS_CHANGE_PASS_TOKEN_FAIL_ERROR')); - } - $csrfTokenManager->removeToken('login\action\usersettings\changepass'); + $csrfTokenController->checkTockenThenRemove('login\action\usersettings\changepass', 'POST', 'crsf-token'); $password = $_POST['password']; if ($this->user->updatePassword($password)) { @@ -143,7 +117,7 @@ $this->session->setMessage($this->user->error); } } catch (TokenNotFoundException $th) { - $error = $th->getMessage(); + $error = _t('USERSETTINGS_PASSWORD_NOT_CHANGED') .' '. $th->getMessage(); } } } // End of changepass action diff --git a/tools/login/lang/login_ca.inc.php b/tools/login/lang/login_ca.inc.php index a50684714..f28df7092 100755 --- a/tools/login/lang/login_ca.inc.php +++ b/tools/login/lang/login_ca.inc.php @@ -72,10 +72,8 @@ // actions/login.php // 'LOGIN_COOKIES_ERROR' => 'Vous devez accepter les cookies pour pouvoir vous connecter.', - // actions/usersettins.php - 'USERSTTINGS_CHANGE_PASS_NO_TOKEN_ERROR' => 'Error de disseny del lloc: el nou formulari d\'enviament de contrasenyes '. - 'no contenia el testimoni d\'identificació únic necessari per als mecanismes de seguretat interns. '. - 'La contrasenya no ha canviat.', - 'USERSTTINGS_CHANGE_PASS_TOKEN_FAIL_ERROR' => 'La contrasenya no s\'ha canviat perquè és possible que aquesta pàgina s\'hagi obert per segona vegada. '. - 'Si us plau, renoveu la sol·licitud de canvi des d\'aquesta finestra (el testimoni de seguretat intern no era bo). ', + // actions/usersettings.php + 'USERSETTINGS_EMAIL_NOT_CHANGED' => 'El correu electrònic no s\'ha modificat.', + 'USERSETTINGS_PASSWORD_NOT_CHANGED' => 'La contrasenya no ha canviat.', + 'USERSETTINGS_USER_NOT_DELETED' => 'Utilizador não eliminado.', ]; diff --git a/tools/login/lang/login_en.inc.php b/tools/login/lang/login_en.inc.php index 02344acf1..41e6298ae 100755 --- a/tools/login/lang/login_en.inc.php +++ b/tools/login/lang/login_en.inc.php @@ -72,9 +72,8 @@ // actions/login.php // 'LOGIN_COOKIES_ERROR' => 'Vous devez accepter les cookies pour pouvoir vous connecter.', - // actions/usersettins.php - 'USERSTTINGS_CHANGE_PASS_NO_TOKEN_ERROR' => 'Site design error: The new password submission form did not contain the unique identification token '. - 'needed for internal security mechanisms. Password not changed.', - 'USERSTTINGS_CHANGE_PASS_TOKEN_FAIL_ERROR' => 'Password not changed because this page may have been opened a second time. '. - 'Please renew the change request from this window (the internal security token was not good). ', + // actions/usersettings.php + 'USERSETTINGS_EMAIL_NOT_CHANGED' => 'Email not modified.', + 'USERSETTINGS_PASSWORD_NOT_CHANGED' => 'Password not changed.', + 'USERSETTINGS_USER_NOT_DELETED' => 'User not deleted.', ]; diff --git a/tools/login/lang/login_es.inc.php b/tools/login/lang/login_es.inc.php index ce91426a7..e1ba12d67 100755 --- a/tools/login/lang/login_es.inc.php +++ b/tools/login/lang/login_es.inc.php @@ -72,9 +72,8 @@ // actions/login.php // 'LOGIN_COOKIES_ERROR' => 'Vous devez accepter les cookies pour pouvoir vous connecter.', - // actions/usersettins.php - 'USERSTTINGS_CHANGE_PASS_NO_TOKEN_ERROR' => 'Error de diseño del sitio: el nuevo formulario de envío de contraseña no contenía el token de identificación único '. - 'necesario para los mecanismos de seguridad internos. Contraseña no cambiada.', - 'USERSTTINGS_CHANGE_PASS_TOKEN_FAIL_ERROR' => 'La contraseña no se ha cambiado porque es posible que esta página se haya abierto por segunda vez. '. - 'Renueve la solicitud de cambio desde esta ventana (el token de seguridad interno no era bueno).', + // actions/usersettings.php + 'USERSETTINGS_EMAIL_NOT_CHANGED' => 'Correo electrónico no cambiado.', + 'USERSETTINGS_PASSWORD_NOT_CHANGED' => 'Contraseña no cambiada.', + 'USERSETTINGS_USER_NOT_DELETED' => 'Usuario no eliminado.', ]; diff --git a/tools/login/lang/login_fr.inc.php b/tools/login/lang/login_fr.inc.php index fc35d3adc..9d2118bad 100755 --- a/tools/login/lang/login_fr.inc.php +++ b/tools/login/lang/login_fr.inc.php @@ -72,9 +72,8 @@ // actions/login.php 'LOGIN_COOKIES_ERROR' => 'Vous devez accepter les cookies pour pouvoir vous connecter.', - // actions/usersettins.php - 'USERSTTINGS_CHANGE_PASS_NO_TOKEN_ERROR' => 'Erreur de conception du site : Le formulaire de soumission du nouveau mot de passe ne contenait pas '. - 'le jeton d\'identification unique nécessaire aux mécanismes internes de sécurité. Mot de passe non modifié.', - 'USERSTTINGS_CHANGE_PASS_TOKEN_FAIL_ERROR' => 'Mot de passe non modifié car cette page a peut-être été ouverte une seconde fois. '. - 'Veuillez renouveler la demande de changement depuis cette fenêtre (le jeton interne de sécurité n\'était pas bon).', + // actions/usersettings.php + 'USERSETTINGS_EMAIL_NOT_CHANGED' => 'E-mail non modifié.', + 'USERSETTINGS_PASSWORD_NOT_CHANGED' => 'Mot de passe non modifié.', + 'USERSETTINGS_USER_NOT_DELETED' => 'Utilisateur non supprimé.', ]; diff --git a/tools/login/lang/login_nl.inc.php b/tools/login/lang/login_nl.inc.php index 85d589e9b..9e3b9dc40 100755 --- a/tools/login/lang/login_nl.inc.php +++ b/tools/login/lang/login_nl.inc.php @@ -72,9 +72,8 @@ // actions/login.php // 'LOGIN_COOKIES_ERROR' => 'Vous devez accepter les cookies pour pouvoir vous connecter.', - // actions/usersettins.php - 'USERSTTINGS_CHANGE_PASS_NO_TOKEN_ERROR' => 'Fout bij siteontwerp: het nieuwe formulier voor het indienen van wachtwoorden bevatte niet het '. - 'unieke identificatietoken dat nodig is voor interne beveiligingsmechanismen. Wachtwoord niet gewijzigd.', - 'USERSTTINGS_CHANGE_PASS_TOKEN_FAIL_ERROR' => 'Wachtwoord niet gewijzigd omdat deze pagina mogelijk een tweede keer is geopend. '. - 'Verleng het wijzigingsverzoek vanuit dit venster (het interne beveiligingstoken was niet goed).', + // actions/usersettings.php + 'USERSETTINGS_EMAIL_NOT_CHANGED' => 'E-mail niet gewijzigd.', + 'USERSETTINGS_PASSWORD_NOT_CHANGED' => 'Wachtwoord niet gewijzigd.', + 'USERSETTINGS_USER_NOT_DELETED' => 'Gebruiker niet verwijderd.', ]; diff --git a/tools/login/lang/login_pt.inc.php b/tools/login/lang/login_pt.inc.php index b288ee18f..e090e427b 100755 --- a/tools/login/lang/login_pt.inc.php +++ b/tools/login/lang/login_pt.inc.php @@ -72,9 +72,8 @@ // actions/login.php // 'LOGIN_COOKIES_ERROR' => 'Vous devez accepter les cookies pour pouvoir vous connecter.', - // actions/usersettins.php - 'USERSTTINGS_CHANGE_PASS_NO_TOKEN_ERROR' => 'Erro de conceção do site: O novo formulário de submissão da palavra-passe não continha o símbolo de '. - 'identificação único necessário para os mecanismos de segurança interna. A palavra-passe não foi alterada.', - 'USERSTTINGS_CHANGE_PASS_TOKEN_FAIL_ERROR' => 'A palavra-passe não foi alterada porque esta página pode ter sido aberta uma segunda vez. '. - 'Por favor, renove o pedido de alteração desta janela (o sinal de segurança interna não foi bom).', + // actions/usersettings.php + 'USERSETTINGS_PASSWORD_NOT_CHANGED' => 'Palavra-passe não alterada.', + 'USERSETTINGS_EMAIL_NOT_CHANGED' => 'E-mail não modificado.', + 'USERSETTINGS_USER_NOT_DELETED' => 'Utilizador não eliminado.', ]; From fb76150843289d15277d0a41d7064b2bdb6f3a23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Dufraisse?= Date: Sun, 13 Feb 2022 14:44:33 +0100 Subject: [PATCH 7/9] feat(csrftokenmanager): use it with deletepage --- handlers/page/deletepage.php | 45 ++++++++++++++++++++--- lang/yeswiki_ca.php | 1 + lang/yeswiki_en.php | 1 + lang/yeswiki_es.php | 1 + lang/yeswiki_fr.php | 1 + lang/yeswiki_nl.php | 1 + lang/yeswiki_pt.php | 1 + tools/tags/handlers/page/__deletepage.php | 20 ++++++++-- 8 files changed, 61 insertions(+), 10 deletions(-) diff --git a/handlers/page/deletepage.php b/handlers/page/deletepage.php index d95e3016c..913852541 100755 --- a/handlers/page/deletepage.php +++ b/handlers/page/deletepage.php @@ -21,11 +21,19 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ +use Symfony\Component\Security\Csrf\Exception\TokenNotFoundException; +use Symfony\Component\Security\Csrf\CsrfTokenManager; +use YesWiki\Core\Controller\CsrfTokenController; + // Vérification de sécurité if (!defined("WIKINI_VERSION")) { die("accès direct interdit"); } +// get services +$csrfTokenManager = $this->services->get(CsrfTokenManager::class); +$csrfTokenController = $this->services->get(CsrfTokenController::class); + // get the GET parameter 'incomingurl' for the incoming url if (!empty($_REQUEST['incomingurl'])) { $incomingurl = urldecode($_GET['incomingurl']); @@ -53,6 +61,7 @@ $msg .= '" method="post" style="display: inline">' . "\n"; $msg .= str_replace("{tag}", $this->Link($tag), _t('DELETEPAGE_CONFIRM')) . "\n"; $msg .= '

'; + $msg .= ''; $msg .= '' . "\n"; $msg .= "\n"; } else { - $this->DeleteOrphanedPage($tag); - $this->LogAdministrativeAction($this->GetUserName(), "Suppression de la page ->\"\"" . $tag . "\"\""); - $msg = str_replace("{tag}", $tag, _t('DELETEPAGE_MESSAGE')); + try { + $csrfTokenController->checkTockenThenRemove("handler\deletepage\\$tag", 'POST', 'crsf-token'); - $hasBeenDeleted = true; - // if $incomingurl has been defined and doesn't refer to the deleted page, redirect to it - $redirectToIncoming = !empty($incomingurl); + $this->DeleteOrphanedPage($tag); + $this->LogAdministrativeAction($this->GetUserName(), "Suppression de la page ->\"\"" . $tag . "\"\""); + $msg = str_replace("{tag}", $tag, _t('DELETEPAGE_MESSAGE')); + + $hasBeenDeleted = true; + // if $incomingurl has been defined and doesn't refer to the deleted page, redirect to it + $redirectToIncoming = !empty($incomingurl); + } catch (TokenNotFoundException $th) { + $msg = $this->render("@templates/alert-message-with-back.twig", [ + 'type' => 'danger', + 'message' => _t('DELETEPAGE_NOT_DELETED').' '.$th->getMessage() + ]); + } } } else { + if (isset($_GET['eraselink']) + && $_GET['eraselink'] === 'oui' + && isset($_GET['confirme']) + && ($_GET['confirme'] === 'oui')) { + // a trouble occured, invald token ? + try { + $csrfTokenController->checkTockenThenRemove("handler\deletepage\\{$this->tag}", 'POST', 'crsf-token'); + } catch (TokenNotFoundException $th) { + $msg .= $this->render("@templates/alert-message.twig", [ + 'type' => 'danger', + 'message' => _t('DELETEPAGE_NOT_DELETED').' '.$th->getMessage() + ]); + } + } $msg = "

" . _t('DELETEPAGE_NOT_ORPHEANED') . "

\n"; $linkedFrom = $this->LoadAll("SELECT DISTINCT from_tag " . "FROM " . $this->config["table_prefix"] . "links " . "WHERE to_tag = '" . $this->GetPageTag() . "'"); @@ -84,6 +116,7 @@ $msg .= '" method="post" style="display: inline">' . "\n"; $msg .= str_replace("{tag}", $this->Link($this->tag), _t('DELETEPAGE_CONFIRM_WHEN_BACKLINKS')) . "\n"; $msg .= '

'; + $msg .= 'tag}")) .'">'; $msg .= ' 'Cette page n\'est pas orpheline.', // 'DELETEPAGE_NOT_OWNER' => 'Vous n\'êtes pas le propriétaire de cette page.', // 'DELETEPAGE_PAGES_WITH_LINKS_TO' => 'Pages ayant un lien vers {tag} :', + 'DELETEPAGE_NOT_DELETED' => 'La pàgina no s\'ha suprimit.', // handlers/edit // 'EDIT_ALERT_ALREADY_SAVED_BY_ANOTHER_USER' => 'ALERTE : '. diff --git a/lang/yeswiki_en.php b/lang/yeswiki_en.php index 5ba21fe27..0e0ac3bb2 100755 --- a/lang/yeswiki_en.php +++ b/lang/yeswiki_en.php @@ -491,6 +491,7 @@ 'DELETEPAGE_NOT_ORPHEANED' => 'This page is not orpheaned.', 'DELETEPAGE_NOT_OWNER' => 'You are not owner of this page.', 'DELETEPAGE_PAGES_WITH_LINKS_TO' => 'Pages with a link to {tag} :', + 'DELETEPAGE_NOT_DELETED' => 'Not deleted page.', // handlers/edit 'EDIT_ALERT_ALREADY_SAVED_BY_ANOTHER_USER' => 'ALERT : '. diff --git a/lang/yeswiki_es.php b/lang/yeswiki_es.php index 12bc3d8ea..1ba9ada25 100755 --- a/lang/yeswiki_es.php +++ b/lang/yeswiki_es.php @@ -533,6 +533,7 @@ // 'DELETEPAGE_NOT_ORPHEANED' => 'Cette page n\'est pas orpheline.', // 'DELETEPAGE_NOT_OWNER' => 'Vous n\'êtes pas le propriétaire de cette page.', // 'DELETEPAGE_PAGES_WITH_LINKS_TO' => 'Pages ayant un lien vers {tag} :', + 'DELETEPAGE_NOT_DELETED' => 'Página no eliminada.', // handlers/edit // 'EDIT_ALERT_ALREADY_SAVED_BY_ANOTHER_USER' => 'ALERTE : '. diff --git a/lang/yeswiki_fr.php b/lang/yeswiki_fr.php index fdc03a2c0..6470580a9 100755 --- a/lang/yeswiki_fr.php +++ b/lang/yeswiki_fr.php @@ -528,6 +528,7 @@ 'DELETEPAGE_NOT_ORPHEANED' => 'Cette page n\'est pas orpheline.', 'DELETEPAGE_NOT_OWNER' => 'Vous n\'êtes pas le propriétaire de cette page.', 'DELETEPAGE_PAGES_WITH_LINKS_TO' => 'Pages ayant un lien vers {tag} :', + 'DELETEPAGE_NOT_DELETED' => 'Page non supprimée.', // handlers/edit 'EDIT_ALERT_ALREADY_SAVED_BY_ANOTHER_USER' => 'ALERTE : '. diff --git a/lang/yeswiki_nl.php b/lang/yeswiki_nl.php index 6c28448f3..00a4badc9 100755 --- a/lang/yeswiki_nl.php +++ b/lang/yeswiki_nl.php @@ -529,6 +529,7 @@ // 'DELETEPAGE_NOT_ORPHEANED' => 'Cette page n\'est pas orpheline.', // 'DELETEPAGE_NOT_OWNER' => 'Vous n\'êtes pas le propriétaire de cette page.', // 'DELETEPAGE_PAGES_WITH_LINKS_TO' => 'Pages ayant un lien vers {tag} :', + 'DELETEPAGE_NOT_DELETED' => 'Pagina niet verwijderd.', // handlers/edit // 'EDIT_ALERT_ALREADY_SAVED_BY_ANOTHER_USER' => 'ALERTE : '. diff --git a/lang/yeswiki_pt.php b/lang/yeswiki_pt.php index e4209fa99..82e41bc29 100755 --- a/lang/yeswiki_pt.php +++ b/lang/yeswiki_pt.php @@ -530,6 +530,7 @@ // 'DELETEPAGE_NOT_ORPHEANED' => 'Cette page n\'est pas orpheline.', // 'DELETEPAGE_NOT_OWNER' => 'Vous n\'êtes pas le propriétaire de cette page.', // 'DELETEPAGE_PAGES_WITH_LINKS_TO' => 'Pages ayant un lien vers {tag} :', + 'DELETEPAGE_NOT_DELETED' => 'Página não apagada.', // handlers/edit // 'EDIT_ALERT_ALREADY_SAVED_BY_ANOTHER_USER' => 'ALERTE : '. diff --git a/tools/tags/handlers/page/__deletepage.php b/tools/tags/handlers/page/__deletepage.php index 80212787c..36f359b7c 100755 --- a/tools/tags/handlers/page/__deletepage.php +++ b/tools/tags/handlers/page/__deletepage.php @@ -21,14 +21,26 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ +use Symfony\Component\Security\Csrf\CsrfToken; +use Symfony\Component\Security\Csrf\CsrfTokenManager; + // Vérification de sécurité if (!defined("WIKINI_VERSION")) { die("accès direct interdit"); } -if ($this->UserIsOwner() || $this->UserIsAdmin()) { - if (isset($_GET['eraselink']) && $_GET['eraselink'] == 'oui') { - $this->Query("DELETE FROM " . $this->config["table_prefix"] . "links WHERE to_tag = '" - . $this->GetPageTag() . "'"); +if (($this->UserIsOwner() || $this->UserIsAdmin()) + && isset($_GET['eraselink']) + && $_GET['eraselink'] === 'oui' + && isset($_GET['confirme']) + && ($_GET['confirme'] === 'oui') + ) { + $inputToken = filter_input(INPUT_POST, 'crsf-token', FILTER_SANITIZE_STRING); + if (!is_null($inputToken) && $inputToken !== false) { + $tag = $this->GetPageTag(); + $token = new CsrfToken("handler\deletepage\\$tag", $inputToken); + if ($this->services->get(CsrfTokenManager::class)->isTokenValid($token)) { + $this->Query("DELETE FROM {$this->config["table_prefix"]}links WHERE to_tag = '$tag'"); + } } } From 33d28eb28fed02818eba6270a72d388a37aa3cd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Dufraisse?= Date: Mon, 14 Feb 2022 14:08:43 +0100 Subject: [PATCH 8/9] doc(crsd): update text and twig examples --- docs/csrf.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/csrf.md b/docs/csrf.md index c53377d40..efcc39e6c 100644 --- a/docs/csrf.md +++ b/docs/csrf.md @@ -8,14 +8,17 @@ CRSF (Cross-site request forgery) is a method of web attack in one click describ `symfony/security-csrf` is used to manage tokens needed to prevent csrf. It is installed bt default with composer and `YesWiki::loadExtensions()`. - 1. Get a token and use it into the concerned form or link for dangerous actions reserved to admins or owners (like `deletepage`, delete a user, change password) + 1. Get a token and use it into the concerned form or link for any action that may alter data (create/update/delete) and that needs acl validation (like `deletepage`, delete a user, change password) - in php code, get a token with this code for example : ``` use Symfony\Component\Security\Csrf\CsrfTokenManager; ... $token = $this->wiki->services->get(CsrfTokenManager::class)->getToken('tokenId'); ``` - - in `twig` template, use `{{ crsfToken('tokenId') }}` + - in `twig` template, use `{{ crsfToken('tokenId') }}`. Example for a form (twig): + ``` + + ``` 2. when processing a request with the token, check if it is right inspiring of this example : ``` use Symfony\Component\Security\Csrf\CsrfToken; @@ -36,7 +39,7 @@ CRSF (Cross-site request forgery) is a method of web attack in one click describ use YesWiki\Core\Controller\CsrfTokenController; $csrfTokenController = $this->wiki->services->get(CsrfTokenController::class); try { - $csrfTokenController->checkTockenThenRemove('tokenId', 'POST', 'tokenNameInForm'); + $csrfTokenController->checkTocken('tokenId', 'POST', 'tokenNameInForm'); ... code if OK } catch (TokenNotFoundException $th) { $errorMessage = $th->getMessage(); From 3d280dce027b4d01ec40ca6d65019ae0608072da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Dufraisse?= Date: Mon, 14 Feb 2022 14:10:01 +0100 Subject: [PATCH 9/9] fix(CrsfTokenController): rename checkTockenThenRemove to checkTocken --- handlers/page/deletepage.php | 4 ++-- includes/controllers/CsrfTokenController.php | 6 ++++-- tools/login/actions/usersettings.php | 6 +++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/handlers/page/deletepage.php b/handlers/page/deletepage.php index 913852541..f483541e9 100755 --- a/handlers/page/deletepage.php +++ b/handlers/page/deletepage.php @@ -70,7 +70,7 @@ $msg .= "\n"; } else { try { - $csrfTokenController->checkTockenThenRemove("handler\deletepage\\$tag", 'POST', 'crsf-token'); + $csrfTokenController->checkTocken("handler\deletepage\\$tag", 'POST', 'crsf-token'); $this->DeleteOrphanedPage($tag); $this->LogAdministrativeAction($this->GetUserName(), "Suppression de la page ->\"\"" . $tag . "\"\""); @@ -93,7 +93,7 @@ && ($_GET['confirme'] === 'oui')) { // a trouble occured, invald token ? try { - $csrfTokenController->checkTockenThenRemove("handler\deletepage\\{$this->tag}", 'POST', 'crsf-token'); + $csrfTokenController->checkTocken("handler\deletepage\\{$this->tag}", 'POST', 'crsf-token'); } catch (TokenNotFoundException $th) { $msg .= $this->render("@templates/alert-message.twig", [ 'type' => 'danger', diff --git a/includes/controllers/CsrfTokenController.php b/includes/controllers/CsrfTokenController.php index a506c1f37..97029f050 100644 --- a/includes/controllers/CsrfTokenController.php +++ b/includes/controllers/CsrfTokenController.php @@ -20,14 +20,16 @@ public function __construct( /** * check if token is present and valid in input - * throw TokenNotFoundException or Exception * * @param string $name * @param string $inputType "GET" or "POST" * @param string $inputKey key in the input to use * @return bool + * + * @throws TokenNotFoundException + * @throws Exception */ - public function checkTockenThenRemove(string $name, string $inputType, string $inputKey): bool + public function checkTocken(string $name, string $inputType, string $inputKey): bool { if (empty($name)) { throw new Exception("parameter `\$name` should not be empty !"); diff --git a/tools/login/actions/usersettings.php b/tools/login/actions/usersettings.php index cd054d074..8e6d69017 100644 --- a/tools/login/actions/usersettings.php +++ b/tools/login/actions/usersettings.php @@ -54,7 +54,7 @@ } elseif ($adminIsActing || $userLoggedIn) { // Admin or user wants to manage the user if (substr($action, 0, 6) == 'update') { // Whoever it is tries to update the user try { - $csrfTokenController->checkTockenThenRemove('login\action\usersettings\updateuser', 'POST', 'crsf-token'); + $csrfTokenController->checkTocken('login\action\usersettings\updateuser', 'POST', 'crsf-token'); $OK = $this->user->setByAssociativeArray(array( 'email' => isset($_POST['email']) ? $_POST['email'] : '', @@ -88,7 +88,7 @@ if ($action == 'deleteByAdmin') { // Admin trying to delete user try { - $csrfTokenController->checkTockenThenRemove('login\action\usersettings\deleteByAdmin', 'POST', 'crsf-token'); + $csrfTokenController->checkTocken('login\action\usersettings\deleteByAdmin', 'POST', 'crsf-token'); $this->user->delete(); // forward @@ -106,7 +106,7 @@ } else { // user properly typed his old password in // check token try { - $csrfTokenController->checkTockenThenRemove('login\action\usersettings\changepass', 'POST', 'crsf-token'); + $csrfTokenController->checkTocken('login\action\usersettings\changepass', 'POST', 'crsf-token'); $password = $_POST['password']; if ($this->user->updatePassword($password)) {