Discussion :

[tchize_]

Aujourd'hui, je développais une application spring (ouais ouais, je sais), plus particulièrement, je configurais la sécurité et le hashage des mots de passe.

Je me loggue + pas à pas: les hash sont les mêmes, et pourtant je suis jeté. Bizarre me dis-je, j'ai du rater qqch. Je réessaie et là: plus de hash, le hash stocké de l'utilisateur a disparu de la mémoire (je précise que c'est par rapport à ce hash qu'on vérifie ce que l'utilisateur a entré). Je fouille et vla ti pas que je trouve le coupable dans spring security:

Code java :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
 
    /**
     * @deprecated Use the exception message or use a custom exception if you really need additional information.
     */
    @Deprecated
    public AuthenticationException(String msg, Object extraInformation) {
        super(msg);
        if (extraInformation instanceof CredentialsContainer) {
            ((CredentialsContainer) extraInformation).eraseCredentials();
        }
        this.extraInformation = extraInformation;
    }

Deux anti-pattern pour le prix d'une:
-> Logique buisness / effet de bord dans une Exception (alors que cette logique devrais se trouver dans le catch correspondant)
-> logique buisness dans une constructeur (autant faire les choses en beautés).

Oui, vous avez bien lu, si l'utilisateur tappe un mauvais mot de passe, on efface tout ce qu'on a de lui en mémoire. Et tant pis si on stockait les données uniquement en mémoire

On rajoute à ça:
- comportement non documenté
- Passage d'un "Object" très typé
- Appel d'un constructeur déprécié

Notez qu'on est bien conscient là qu'on est occupé d'effacer un password du store puisque l'appel est

Code java :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
        if (!passwordEncoder.isPasswordValid(userDetails.getPassword(), presentedPassword, salt)) {
            logger.debug("Authentication failed: password does not match stored value");
 
            throw new BadCredentialsException(messages.getMessage(
                    "AbstractUserDetailsAuthenticationProvider.badCredentials", "Bad credentials"),
                    includeDetailsObject ? userDetails : null);
        }

(presentedPassword étant ce que j'ai tappé au prompt)
Notez aussi le flag ambigu "includeDetailsObject" qui, à true, a in fine l'effet de supprimer des détails

[ternel]

Adorable, en effet.

[Médinoc]

Je suis d'accord sur le fait que ça ne devrait pas se trouver dans le constructeur, mais d'un autre côté je trouve normal de supprimer les credentials le plus tôt possible (logguer un mauvais mot de passe peut donner des informations sur le vrai mdp s'il y a eu typo, etc.).

En fait, le problème n'est pas tant qu'on efface les credentials, mais qu'on le fasse sur l'original (effet de bord). La chose à faire serait de cloner le extraInformation et filtrer la copie.

[tchize_]

En fait, le problème n'est pas tant qu'on efface les credentials, mais qu'on le fasse sur l'original (effet de bord). La chose à faire serait de cloner le extraInformation et filtrer la copie.

extraInformation etant de type "inconnu", impossible à cloner.

La chose propre aurait été de prendre un type bien précis, d'extraire les informations voulue dans le constructeur et de ne stocker que ces informations

Bref, rien de ce qui a été fait