Skip to content

Commit

Permalink
update
Browse files Browse the repository at this point in the history
  • Loading branch information
ncou committed Dec 23, 2020
1 parent 5894b57 commit 89f6aad
Show file tree
Hide file tree
Showing 6 changed files with 171 additions and 142 deletions.
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
"extra": {
"chiron": {
"bootloaders": [
"Chiron\\Csrf\\Bootloader\\PublishMaintenanceBootloader",
"Chiron\\Csrf\\Bootloader\\PublishCsrfBootloader",
"Chiron\\Csrf\\Bootloader\\CsrfTokenMiddlewareBootloader"
]
}
Expand Down
33 changes: 30 additions & 3 deletions config/csrf.php.dist
Original file line number Diff line number Diff line change
@@ -1,7 +1,34 @@
<?php

return [
'cookie' => 'csrf-token',
'length' => 16,
'lifetime' => 86400,
/*
|--------------------------------------------------------------------------
| CSRF Cookie Name
|--------------------------------------------------------------------------
| Default: 'csrf-token'
|
| The name of the cookie to use for the CSRF authentication token.
|
| This can be whatever you want (as long as it’s not empty and different
| from the other cookie names in your application).
|
| Use any US-ASCII characters, except control characters, spaces, or tabs.
| It also must not contain a separator character like the following:
| ( ) < > @ , ; : \ " / [ ] ? = { }
*/
'cookie_name' => 'csrf-token',
/*
|--------------------------------------------------------------------------
| CSRF Cookie Lifetime
|--------------------------------------------------------------------------
| Default: 31449600 (approximately 1 year, in seconds)
|
| The age of CSRF cookies, in seconds (minimal value: 0).
|
| The reason for setting a long-lived expiration time is to avoid
| problems in the case of a user closing a browser or bookmarking a page
| and then loading that page from a browser cache.
| Without persistent cookies, the form submission would fail in this case.
*/
'cookie_age' => 31449600,
];
58 changes: 10 additions & 48 deletions src/Config/CsrfConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
use Nette\Schema\Expect;
use Nette\Schema\Schema;

// EXEMPLE DJANGO Documentation : https://docs.djangoproject.com/en/3.1/ref/settings/#std:setting-CSRF_HEADER_NAME

//https://github.com/codeigniter4/CodeIgniter4/blob/2d9d652c1eada3aad7c17705df1dd99aa0c837b3/app/Config/App.php#L308
//https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite
//https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#samesite-cookie-attribute
Expand All @@ -18,66 +20,26 @@ final class CsrfConfig extends AbstractInjectableConfig

protected function getConfigSchema(): Schema
{
// TODO : il faudrait pas rajouter un booléen pour savoir si on active ou non la partie protection CSRF ????
// TODO : renommer le champ 'cookie' en 'cookieName'.
// TODO : virer "length".
// TODO : ajouter l'attribut "path" et "domain" dans cette config.
// TODO : attention il faut aussi ajouter les valeurs pour : "secure" et "samesite"
// TODO : limiter les valeurs de "samesite" à "Lax" et "Strict". avec la valeur par défaut à Lax !!!! Eventuellement 'None' mais ce n'est pas recommandé
// TODO : ajouter un controle pour que le lifetime soit supérieur ou égal à 0 (et dans ce cas le cookie est valable durant la durée de la session).
return Expect::structure([
'cookie' => Expect::string()->default('csrf-token'),
'length' => Expect::int()->default(16), // TODO : à virer !!!!
'lifetime' => Expect::int()->default(86400),
'cookie_name' => Expect::string()->default('csrf-token'),
'cookie_age' => Expect::int()->min(0)->default(31449600),
]);

/*
// enable/disable CSRF protection for this form
'csrf_protection' => true,
// the name of the hidden HTML field that stores the token
'csrf_field_name' => '_token',
*/
}

/**
* @return int
*/
public function getTokenLength(): int
{
return $this->get('length');
}

/**
* @return string
*/
public function getCookie(): string
// TODO : utiliser une regex pour valider le nom du cookie => https://github.com/yiisoft/cookies/blob/master/src/Cookie.php#L35 / https://developer.mozilla.org/fr/docs/Web/HTTP/Headers/Set-Cookie
public function getCookieName(): string
{
return $this->get('cookie');
return $this->get('cookie_name');
}

/**
* @return int|null
*/
public function getCookieLifetime(): int
{
return $this->get('lifetime');
}

/**
* @return bool
*/
// TODO : code temporaire
public function isCookieSecure(): bool
{
return false;
}

/**
* @return string|null
* @return int
*/
// TODO : code temporaire
public function getSameSite(): ?string
public function getCookieAge(): int
{
return null;
return $this->get('cookie_age');
}
}
44 changes: 26 additions & 18 deletions src/Middleware/CsrfProtectionMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,19 @@
* An antiforgery token is required for HTTP methods other than GET, HEAD, OPTIONS, and TRACE.
*
* @see https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#double-submit-cookie
* @see https://tools.ietf.org/html/rfc7231#section-4.2.1
*/
final class CsrfProtectionMiddleware implements MiddlewareInterface
{
/**
* Header to check for token instead of POST/GET data.
*/
public const HEADER = 'X-CSRF-Token';
public const HEADER = 'X-CSRF-Token'; // TODO : paramétrer ces valeurs dans le fichier csrf.php avec un champ headerName/fieldName ????

/**
* Parameter name used to represent client token in POST data.
*/
public const PARAMETER = 'csrf-token';
public const PARAMETER = 'csrf-token'; // TODO : paramétrer ces valeurs dans le fichier csrf.php avec un champ headerName/fieldName ???? Attention vérifier si ce champ n'est pas utilisé dans la méthode csrf_token ou dans le twig extension, car si on enléve le format de constante public on devra récupérer l'objet csrfConfig et ca peut vite complexifier le code !!!!

/**
* {@inheritdoc}
Expand All @@ -49,7 +50,7 @@ final class CsrfProtectionMiddleware implements MiddlewareInterface
*/
public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
{
// Verify CSRF token if the request method is considered "unsafe".
// Verify token if the request method is "unsafe" and require protection.
if ($this->needsProtection($request) && ! $this->tokensMatch($request)) {
// Throw an http error 412 "pre-condition failed" exception.
throw new TokenMismatchException();
Expand All @@ -59,15 +60,17 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
}

/**
* Assume that anything not defined as 'safe' by RFC7231 needs protection.
* Assume that any method not defined as 'safe' by RFC7231 needs protection.
*
* @see https://tools.ietf.org/html/rfc7231#section-4.2.1
*
* @param ServerRequestInterface $request
*
* @return bool
*/
private function needsProtection(ServerRequestInterface $request): bool
{
return Method::isSafe($request->getMethod()) === false;
return Method::isSafe($request->getMethod()) === false; // Permettre de passer aux méthodes isSafe/isUnsafe soit une string pour la méthode soit directement un objet ServerRequestInterface pour récupérer la méthode via un appel ->getMethod() et ensuite faire la vérification. Ca simplifierai les choses non ????
}

/**
Expand All @@ -79,30 +82,30 @@ private function needsProtection(ServerRequestInterface $request): bool
*/
private function tokensMatch(ServerRequestInterface $request): bool
{
$expectedToken = $this->getToken($request);
$providedToken = $this->getTokenFromRequest($request);
$expected = $this->fetchToken($request);
$provided = $this->getTokenFromRequest($request);

return hash_equals($expectedToken, $providedToken);
return hash_equals($expected, $provided);
}

/**
* Retrieve the token value present in the request attribute.
* Fetch the token value present in the request attribute.
* Token come from the previous middleware "CsrfTokenMiddleware".
*
* @param ServerRequestInterface $request
*
* @return string
*/
private function getToken(ServerRequestInterface $request): string
private function fetchToken(ServerRequestInterface $request): string
{
$token = $request->getAttribute(CsrfTokenMiddleware::ATTRIBUTE);

// TODO : il faudrait pas vérifier qu'il fait bien la taille attendue ??? style créer une méthode isValidToken( qui fait la vérif suivante) : is_string($token) && ctype_alnum($token) && strlen($token) === CsrfTokenMiddleware::TOKEN_LENGTH;
if (! $token || ! is_string($token)) {
throw new LogicException('Unable to prepare CSRF protection, token attribute is missing or invalid.');
// Ensure the token stored previously by the CsrfTokenMiddleware is present and has a valid format.
if (is_string($token) && ctype_alnum($token) && strlen($token) === CsrfTokenMiddleware::TOKEN_LENGTH) {
return $token;
}

return $token;
throw new LogicException('Unable to prepare CSRF protection, token attribute is missing or invalid.');
}

/**
Expand All @@ -112,19 +115,24 @@ private function getToken(ServerRequestInterface $request): string
*
* @return string
*/
// TODO : appeller la méthode isValidToken() pour vérifier qui la valeur est bien une string+alnum+de taille 40 caractéres ??? EDIT : cet appel ne me semble pas nécessaire car va surement allourdir le code !!!
// TODO : vérifier si la méthode de la request est POST dans ce cas vérifier dans le body, si le token n'est pas trouvé alors regarder dans le header.
// https://github.com/django/django/blob/master/django/middleware/csrf.py#L295
// TODO : rendre ce code plus propre ????
private function getTokenFromRequest(ServerRequestInterface $request): string
{
//$provided = $request->getParsedBody()['csrfToken'] ?? $request->getHeaderLine('X-CSRF-Token');

if ($request->hasHeader(self::HEADER)) {
return (string) $request->getHeaderLine(self::HEADER);
return (string) $request->getHeaderLine(self::HEADER); // TODO : attention ca va pas poser un soucis si il y a plusieurs headers ??? on va surement avoir une string avec un ";" comme séparateur !!!!
}

// Handle the case for a POST form.
$body = $request->getParsedBody();

if (is_array($body) && isset($body[self::PARAMETER]) && is_string($body[self::PARAMETER])) {
return $body[self::PARAMETER];
}

return '';
// TODO : initialiser plutot en début de méthode une variable $token = '' et ensuite la remplir soit avec le header soit avec le body (donc virer les 2 return) et finir par un return $token; en fin de méthode.
return ''; // TODO : créer une constante privé style self::EMPTY_TOKEN qui serait une chaine vide ????
}
}
Loading

0 comments on commit 89f6aad

Please sign in to comment.