maj: sémantique, révision vérification password #3

Scalone
cmsassot scala 6 commity/ów z maj do develop 2023-01-05 14:09:41 +01:00
Właściciel

Ajout d'un customHasher pour le password qui implémente un salt éventuellement récupéré si configuré et si présent
ainsi qu'un pepper si non null dans les variables d'environnement.

Révision sémantique namespace et noms de classe
PDO pour SQLLogin

Ajout d'un customHasher pour le password qui implémente un salt éventuellement récupéré si configuré et si présent ainsi qu'un pepper si non null dans les variables d'environnement. Révision sémantique namespace et noms de classe PDO pour SQLLogin
rmasson dodał(-a) 1 commit 2022-12-14 16:46:00 +01:00
maj: sémantique, révision vérification ppassword
Niektóre etapy zgłosiły ostrzeżenia
Cadoles/hydra-sql/pipeline/head This commit is unstable
Cadoles/hydra-sql/pipeline/pr-develop This commit is unstable
441c0f563c
Właściciel

Symfony Security Check Report

1 package has known vulnerabilities.

twig/twig (v3.3.10)

  • CVE-2022-39261: Possibility to load a template outside a configured directory when using the filesystem loader

Note that this checker can only detect vulnerabilities that are referenced in the security advisories database.
Execute this command regularly to check the newly discovered vulnerabilities.

Symfony Security Check Report ============================= 1 package has known vulnerabilities. twig/twig (v3.3.10) ------------------- * [CVE-2022-39261][]: Possibility to load a template outside a configured directory when using the filesystem loader [CVE-2022-39261]: https://symfony.com/blog/twig-security-release-possibility-to-load-a-template-outside-a-configured-directory-when-using-the-filesystem-loader Note that this checker can only detect vulnerabilities that are referenced in the security advisories database. Execute this command regularly to check the newly discovered vulnerabilities.
Właściciel

Test report

PHP CS Fixer

Overview

State Total
Passed 0
Skipped 0
Failed 5
Error 0

Total duration: 0s

See details
Status Name Class
Cadoles_hydra-sql_PR-3/src/Service/SQLLoginService
Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLPasswordException
Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLLoginException
Cadoles_hydra-sql_PR-3/src/SQLLogin/SQLLoginConnect
Cadoles_hydra-sql_PR-3/src/Security/SQLLoginUserAuthenticator

Errors

`Cadoles_hydra-sql_PR-3/src/Service/SQLLoginService`

Output

applied fixers:
---------------
* global_namespace_import
* no_unused_imports```

</details>


<details>
  <summary>`Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLPasswordException`</summary>

**Output** 

applied fixers:

  • global_namespace_import
  • no_unused_imports
  • no_extra_blank_lines```
`Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLLoginException`

Output

applied fixers:
---------------
* global_namespace_import
* no_unused_imports
* no_extra_blank_lines```

</details>


<details>
  <summary>`Cadoles_hydra-sql_PR-3/src/SQLLogin/SQLLoginConnect`</summary>

**Output** 

applied fixers:

  • global_namespace_import
  • phpdoc_separation
  • no_unused_imports```
`Cadoles_hydra-sql_PR-3/src/Security/SQLLoginUserAuthenticator`

Output

applied fixers:
---------------
* global_namespace_import
* no_unused_imports```

</details>





# Test report ## PHP CS Fixer ### Overview | State | Total | |-------|-------| | Passed | 0 | | Skipped | 0 | | Failed | 5 | | Error | 0 | **Total duration**: 0s <details> <summary>See details</summary> | Status | Name | Class | |--------|------|-------| | &#10799; | `Cadoles_hydra-sql_PR-3/src/Service/SQLLoginService` || | &#10799; | `Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLPasswordException` || | &#10799; | `Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLLoginException` || | &#10799; | `Cadoles_hydra-sql_PR-3/src/SQLLogin/SQLLoginConnect` || | &#10799; | `Cadoles_hydra-sql_PR-3/src/Security/SQLLoginUserAuthenticator` || </details> <br /> #### Errors <details> <summary>`Cadoles_hydra-sql_PR-3/src/Service/SQLLoginService`</summary> **Output** ``` applied fixers: --------------- * global_namespace_import * no_unused_imports``` </details> <details> <summary>`Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLPasswordException`</summary> **Output** ``` applied fixers: --------------- * global_namespace_import * no_unused_imports * no_extra_blank_lines``` </details> <details> <summary>`Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLLoginException`</summary> **Output** ``` applied fixers: --------------- * global_namespace_import * no_unused_imports * no_extra_blank_lines``` </details> <details> <summary>`Cadoles_hydra-sql_PR-3/src/SQLLogin/SQLLoginConnect`</summary> **Output** ``` applied fixers: --------------- * global_namespace_import * phpdoc_separation * no_unused_imports``` </details> <details> <summary>`Cadoles_hydra-sql_PR-3/src/Security/SQLLoginUserAuthenticator`</summary> **Output** ``` applied fixers: --------------- * global_namespace_import * no_unused_imports``` </details>
Właściciel

Rapport PHPStan

 ------ ---------------------------------------------- 
  Line   Security/SQLLoginUserProvider.php             
 ------ ---------------------------------------------- 
  19     Access to an undefined property               
         App\Security\SQLLoginUserProvider::$session.  
 ------ ---------------------------------------------- 


 [ERROR] Found 1 error                                                          


## Rapport PHPStan ``` ------ ---------------------------------------------- Line Security/SQLLoginUserProvider.php ------ ---------------------------------------------- 19 Access to an undefined property App\Security\SQLLoginUserProvider::$session. ------ ---------------------------------------------- [ERROR] Found 1 error ```
wpetit zażądał(-a) zmian 2022-12-15 21:34:58 +01:00
@@ -0,0 +24,4 @@
if ($this->isPasswordTooLong($plainPassword)) {
throw new InvalidPasswordException();
}
$hash = hash($this->hashAlgo, $plainPassword.$salt.$this->pepper);
Właściciel

Je pense que ce code ne sera pas compatible avec les applications ayant choisies d'utiliser les algorithmes argon2id, scrypt et bcrypt (qui sont d'ailleurs les recommandations OWASP 1 aujourd'hui).

Normalement, en PHP on utilise les méthodes password_hash() 2 et password_verify() 3 pour utiliser ces algorithmes (et d'ailleurs le salt est directement stocké dans le hash final 4 ).

Autre chose: la méthode assume un certain format pour la concaténation des différents éléments à hacher ($plainPassword.$salt.$this->pepper). Il est fort peu probable que cette séquence soit toujours respectée dans toutes les applications. À mon avis il serait certainement préférable d'utiliser un patron (avec la méthode strstr() 5 par exemple) pour permettre à l'utilisateur de spécifiquer le format de concaténation.

Je pense que ce code ne sera pas compatible avec les applications ayant choisies d'utiliser les algorithmes `argon2id`, `scrypt` et `bcrypt` (qui sont d'ailleurs les recommandations OWASP [^1] aujourd'hui). Normalement, en PHP on utilise les méthodes `password_hash()` [^2] et `password_verify()` [^3] pour utiliser ces algorithmes (et d'ailleurs le salt est directement stocké dans le hash final [^4]). Autre chose: la méthode assume un certain format pour la concaténation des différents éléments à hacher (`$plainPassword.$salt.$this->pepper`). Il est fort peu probable que cette séquence soit toujours respectée dans toutes les applications. À mon avis il serait certainement préférable d'utiliser un patron (avec la méthode `strstr()` [^5] par exemple) pour permettre à l'utilisateur de spécifiquer le format de concaténation. [^1]: https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html [^2]: https://www.php.net/manual/en/function.password-hash.php [^3]: https://www.php.net/manual/en/function.password-verify.php [^4]: https://stackoverflow.com/questions/40993645/understanding-bcrypt-salt-as-used-by-php-password-hash [^5]: https://www.php.net/manual/en/function.strtr.php
Author
Właściciel

j'ai utilisé les fonction password_algos() et hash_algos() pour faire la différence entre les algos, pour utiliser hash() ou password_hash()

Mais effectivement le pattern est pas pris en compte

j'ai utilisé les fonction password_algos() et hash_algos() pour faire la différence entre les algos, pour utiliser hash() ou password_hash() Mais effectivement le pattern est pas pris en compte
rmasson oznaczył(-a) tę rozmowę jako rozwiązaną
@@ -0,0 +35,4 @@
return false;
}
if ($this->hash($plainPassword, $salt) === $hashedPassword) {
Właściciel

Il ne faut pas utiliser l'opérateur === pour faire des comparaisons de mots de passe (ça ouvre à des failles du type "Timing attack" 1 ).

Il serait mieux d'utiliser https://www.php.net/manual/en/function.hash-equals.php je pense dans ce cas.

Il ne faut pas utiliser l'opérateur `===` pour faire des comparaisons de mots de passe (ça ouvre à des failles du type "Timing attack" [^1]). Il serait mieux d'utiliser https://www.php.net/manual/en/function.hash-equals.php je pense dans ce cas. [^1]: https://www.chosenplaintext.ca/articles/beginners-guide-constant-time-cryptography.html
wpetit zmieniono tytuł z maj: sémantique, révision vérification ppassword na maj: sémantique, révision vérification password 2022-12-15 21:35:10 +01:00
wpetit zażądał(-a) zmian 2022-12-15 21:41:43 +01:00
config/services.yaml Nieaktualny
@@ -25,2 +25,4 @@
locales: '%env(APP_LOCALES)%'
app.supported_locales: ~
env(PEPPER): "257d62c24cd352c21b51c26dba678c8ff05011a89022aec106185bf67c69aa8b"
Właściciel

Hmm, je pense que par défaut le pepper devrait être vide.

Hmm, je pense que par défaut le pepper devrait être vide.
@@ -0,0 +28,4 @@
public const ERROR_PDO = 'error_pdo';
protected string $baseUrl;
private SQLLoginService $pdoService;
Właściciel

Je pense que la variable n'a pas été renommée $pdoService en accord avec le changement de nom du service.

Je pense que la variable n'a pas été renommée `$pdoService` en accord avec le changement de nom du service.
@@ -0,0 +32,4 @@
private UrlGeneratorInterface $router;
private PasswordEncoder $passwordHasher;
public function __construct(string $baseUrl, SQLLoginService $pdoService, UrlGeneratorInterface $router, PasswordEncoder $passwordHasher)
Właściciel
Idem, cf. [commentaire précédent](https://forge.cadoles.com/Cadoles/hydra-sql/pulls/3/files#issuecomment-55979)
@@ -0,0 +35,4 @@
public function __construct(string $baseUrl, SQLLoginService $pdoService, UrlGeneratorInterface $router, PasswordEncoder $passwordHasher)
{
$this->baseUrl = $baseUrl;
$this->pdoService = $pdoService;
Właściciel
Idem, cf. [commentaire précédent](https://forge.cadoles.com/Cadoles/hydra-sql/pulls/3/files#issuecomment-55979)
@@ -0,0 +72,4 @@
public function getConnection()
{
// Appel du singleton
$pdo = SQLLoginConnect::getInstance();
Właściciel
Idem, cf. [commentaire précédent](https://forge.cadoles.com/Cadoles/hydra-sql/pulls/3/files#issuecomment-55979)
@@ -1 +1 @@
123
134
Właściciel

Ce fichier doit il vraiment être commit dans le dépôt ? Attention avec les git add -A :P

Ce fichier doit il vraiment être commit dans le dépôt ? Attention avec les `git add -A` :P
rmasson dodał(-a) 1 commit 2022-12-16 15:00:22 +01:00
ajout de l'updatde hashage selon algo indiqué en ver env, fix typo
Niektóre etapy zgłosiły ostrzeżenia
Cadoles/hydra-sql/pipeline/pr-develop This commit is unstable
bd1b035f1e
Właściciel

Symfony Security Check Report

1 package has known vulnerabilities.

twig/twig (v3.3.10)

  • CVE-2022-39261: Possibility to load a template outside a configured directory when using the filesystem loader

Note that this checker can only detect vulnerabilities that are referenced in the security advisories database.
Execute this command regularly to check the newly discovered vulnerabilities.

Symfony Security Check Report ============================= 1 package has known vulnerabilities. twig/twig (v3.3.10) ------------------- * [CVE-2022-39261][]: Possibility to load a template outside a configured directory when using the filesystem loader [CVE-2022-39261]: https://symfony.com/blog/twig-security-release-possibility-to-load-a-template-outside-a-configured-directory-when-using-the-filesystem-loader Note that this checker can only detect vulnerabilities that are referenced in the security advisories database. Execute this command regularly to check the newly discovered vulnerabilities.
Właściciel

Test report

PHP CS Fixer

Overview

State Total
Passed 0
Skipped 0
Failed 5
Error 0

Total duration: 0s

See details
Status Name Class
Cadoles_hydra-sql_PR-3/src/Service/SQLLoginService
Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLLoginAlgoException
Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLLoginConfigurationException
Cadoles_hydra-sql_PR-3/src/SQLLogin/SQLLoginConnect
Cadoles_hydra-sql_PR-3/src/Security/SQLLoginUserAuthenticator

Errors

`Cadoles_hydra-sql_PR-3/src/Service/SQLLoginService`

Output

applied fixers:
---------------
* global_namespace_import
* no_unused_imports```

</details>


<details>
  <summary>`Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLLoginAlgoException`</summary>

**Output** 

applied fixers:

  • global_namespace_import
  • no_unused_imports
  • no_extra_blank_lines```
`Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLLoginConfigurationException`

Output

applied fixers:
---------------
* global_namespace_import
* no_unused_imports
* no_extra_blank_lines```

</details>


<details>
  <summary>`Cadoles_hydra-sql_PR-3/src/SQLLogin/SQLLoginConnect`</summary>

**Output** 

applied fixers:

  • global_namespace_import
  • phpdoc_separation
  • no_unused_imports```
`Cadoles_hydra-sql_PR-3/src/Security/SQLLoginUserAuthenticator`

Output

applied fixers:
---------------
* global_namespace_import
* no_unused_imports```

</details>





# Test report ## PHP CS Fixer ### Overview | State | Total | |-------|-------| | Passed | 0 | | Skipped | 0 | | Failed | 5 | | Error | 0 | **Total duration**: 0s <details> <summary>See details</summary> | Status | Name | Class | |--------|------|-------| | &#10799; | `Cadoles_hydra-sql_PR-3/src/Service/SQLLoginService` || | &#10799; | `Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLLoginAlgoException` || | &#10799; | `Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLLoginConfigurationException` || | &#10799; | `Cadoles_hydra-sql_PR-3/src/SQLLogin/SQLLoginConnect` || | &#10799; | `Cadoles_hydra-sql_PR-3/src/Security/SQLLoginUserAuthenticator` || </details> <br /> #### Errors <details> <summary>`Cadoles_hydra-sql_PR-3/src/Service/SQLLoginService`</summary> **Output** ``` applied fixers: --------------- * global_namespace_import * no_unused_imports``` </details> <details> <summary>`Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLLoginAlgoException`</summary> **Output** ``` applied fixers: --------------- * global_namespace_import * no_unused_imports * no_extra_blank_lines``` </details> <details> <summary>`Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLLoginConfigurationException`</summary> **Output** ``` applied fixers: --------------- * global_namespace_import * no_unused_imports * no_extra_blank_lines``` </details> <details> <summary>`Cadoles_hydra-sql_PR-3/src/SQLLogin/SQLLoginConnect`</summary> **Output** ``` applied fixers: --------------- * global_namespace_import * phpdoc_separation * no_unused_imports``` </details> <details> <summary>`Cadoles_hydra-sql_PR-3/src/Security/SQLLoginUserAuthenticator`</summary> **Output** ``` applied fixers: --------------- * global_namespace_import * no_unused_imports``` </details>
Właściciel

Rapport PHPStan

 ------ ----------------------------------------------------------------------- 
  Line   Security/Hasher/PasswordEncoder.php                                    
 ------ ----------------------------------------------------------------------- 
  107    Method App\Security\Hasher\PasswordEncoder::getValidAlgo() should      
         return App\SQLLogin\Exception\InvalidSQLLoginAlgoException|string but  
         return statement is missing.                                           
 ------ ----------------------------------------------------------------------- 


 [ERROR] Found 1 error                                                          


## Rapport PHPStan ``` ------ ----------------------------------------------------------------------- Line Security/Hasher/PasswordEncoder.php ------ ----------------------------------------------------------------------- 107 Method App\Security\Hasher\PasswordEncoder::getValidAlgo() should return App\SQLLogin\Exception\InvalidSQLLoginAlgoException|string but return statement is missing. ------ ----------------------------------------------------------------------- [ERROR] Found 1 error ```
rmasson dodał(-a) 1 commit 2022-12-16 15:39:50 +01:00
fix faille twig, coding standard
Niektóre etapy nie powiodły się
Cadoles/hydra-sql/pipeline/pr-develop There was a failure building this commit
906d8edf82
rmasson dodał(-a) 1 commit 2022-12-16 16:49:32 +01:00
supression des fonctions liées à l'update du hashage de mot de passe
Niektóre etapy nie powiodły się
Cadoles/hydra-sql/pipeline/pr-develop There was a failure building this commit
422e97d329
rmasson poprosił o recenzję wpetit 2022-12-16 16:49:48 +01:00
rmasson force-pushed maj from 73eae4176e to b8fb01da5c 2023-01-05 12:13:24 +01:00 Porównaj
Właściciel

Symfony Security Check Report

No packages have known vulnerabilities.

Note that this checker can only detect vulnerabilities that are referenced in the security advisories database.
Execute this command regularly to check the newly discovered vulnerabilities.

Symfony Security Check Report ============================= No packages have known vulnerabilities. Note that this checker can only detect vulnerabilities that are referenced in the security advisories database. Execute this command regularly to check the newly discovered vulnerabilities.
Właściciel

Test report

PHP CS Fixer

Overview

State Total
Passed 0
Skipped 0
Failed 1
Error 0

Total duration: 0s

See details
Status Name Class
Cadoles_hydra-sql_PR-3/src/Security/SQLLoginUserAuthenticator

Errors

`Cadoles_hydra-sql_PR-3/src/Security/SQLLoginUserAuthenticator`

Output

applied fixers:
---------------
* global_namespace_import
* no_unused_imports```

</details>





# Test report ## PHP CS Fixer ### Overview | State | Total | |-------|-------| | Passed | 0 | | Skipped | 0 | | Failed | 1 | | Error | 0 | **Total duration**: 0s <details> <summary>See details</summary> | Status | Name | Class | |--------|------|-------| | &#10799; | `Cadoles_hydra-sql_PR-3/src/Security/SQLLoginUserAuthenticator` || </details> <br /> #### Errors <details> <summary>`Cadoles_hydra-sql_PR-3/src/Security/SQLLoginUserAuthenticator`</summary> **Output** ``` applied fixers: --------------- * global_namespace_import * no_unused_imports``` </details>
Właściciel

Rapport PHPStan

 ------ ---------------------------------------------------------------------- 
  Line   SQLLogin/SQLLoginRequest.php                                          
 ------ ---------------------------------------------------------------------- 
  19     Deprecated in PHP 8.0: Required parameter $dsn follows optional       
         parameter $config.                                                    
  19     Deprecated in PHP 8.0: Required parameter $password follows optional  
         parameter $config.                                                    
  19     Deprecated in PHP 8.0: Required parameter $user follows optional      
         parameter $config.                                                    
 ------ ---------------------------------------------------------------------- 


 [ERROR] Found 3 errors                                                         


## Rapport PHPStan ``` ------ ---------------------------------------------------------------------- Line SQLLogin/SQLLoginRequest.php ------ ---------------------------------------------------------------------- 19 Deprecated in PHP 8.0: Required parameter $dsn follows optional parameter $config. 19 Deprecated in PHP 8.0: Required parameter $password follows optional parameter $config. 19 Deprecated in PHP 8.0: Required parameter $user follows optional parameter $config. ------ ---------------------------------------------------------------------- [ERROR] Found 3 errors ```
rmasson dodał(-a) 1 commit 2023-01-05 13:22:41 +01:00
maj: correction phpstan, optional param config must be behind
Niektóre etapy zgłosiły ostrzeżenia
Cadoles/hydra-sql/pipeline/pr-develop This commit looks good
Cadoles/hydra-sql/pipeline/head This commit is unstable
03e15da862
Właściciel

Symfony Security Check Report

No packages have known vulnerabilities.

Note that this checker can only detect vulnerabilities that are referenced in the security advisories database.
Execute this command regularly to check the newly discovered vulnerabilities.

Symfony Security Check Report ============================= No packages have known vulnerabilities. Note that this checker can only detect vulnerabilities that are referenced in the security advisories database. Execute this command regularly to check the newly discovered vulnerabilities.
Właściciel

Test report

PHP CS Fixer

Overview

State Total
Passed 1
Skipped 0
Failed 0
Error 0

Total duration: 0s

See details
Status Name Class
All OK

# Test report ## PHP CS Fixer ### Overview | State | Total | |-------|-------| | Passed | 1 | | Skipped | 0 | | Failed | 0 | | Error | 0 | **Total duration**: 0s <details> <summary>See details</summary> | Status | Name | Class | |--------|------|-------| | &#10003; | `All OK` || </details> <br />
Właściciel

Rapport PHPStan


 [OK] No errors                                                                 


## Rapport PHPStan ``` [OK] No errors ```
cmsassot merged commit 84b72ccf18 into develop 2023-01-05 14:09:41 +01:00
Zaloguj się, aby dołączyć do tej rozmowy.
No Reviewers
Brak etykiety
Uczestnicy 3
Powiadomienia
Termin realizacji
Brak ustawionego terminu realizacji.
Zależności

No dependencies set.

Reference: Cadoles/hydra-sql#3
No description provided.