Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: username may not be of the email type #10

Merged
merged 2 commits into from
May 23, 2024

Conversation

this-is-tobi
Copy link
Contributor

Le nom d'utilisateur peut ne pas être un email dans Keycloak, forcer le type email dans le champs username empêche l'utilisation d'un nom d'utilisateur qui n'est pas un email.

@garronej
Copy link
Collaborator

Salut @this-is-tobi,
Merci pour la pr.
En fait ça dépend de si emailAsUsername est activé dans la configuration du realm.
Je suis ok pour merger ça mais uniquemement avec une logique conditionelle.
La propriété est sur le kcContext

@this-is-tobi
Copy link
Contributor Author

this-is-tobi commented May 22, 2024

Alors j'ai réfléchi / regardé et emailAsUsername permet de se connecter à l'aide de son adresse email mais ne l'oblige pas (j'ai le cas sur plusieurs Keycloak que je maintiens où j'ai bien cette option activée, ce qui permet aux utilisateurs d'utiliser leurs adresses emails OU leur noms d'utilisateur, à leur convenance).

ps: merci pour la réponse rapide 🪻

Edit: J'ai dit des bêtises j'ai confondu avec loginWithEmailAllowed, tu as raison registrationEmailAsUsername force l'email en tant que nom d'utilisateur, je corrige dans ce sens.

@this-is-tobi
Copy link
Contributor Author

Au passage, si tu me l'accordes j'en profiterai bien pour rendre conditionnel l'affichage du Ou utiliser vos identifiants, dans notre cas nous n'avons pas d'autre fournisseurs d'identité (pour l'instant en tout cas) donc l'affichage sonne un peu bizarre :

Capture d’écran 2024-05-22 à 20 05 07

Si tu le veux bien je remplacerais bien :

// src/login/pages/Login.tsx

<h5>{msgStr("selfCredentials")}</h5>

Par :

// src/login/pages/Login.tsx

{(() => {
    if (social.providers.length) {
        return (
            <h5>{msgStr("selfCredentials")}</h5>
        );
    }
})()}

Si je comprends bien ce que je lis dans le kcContext, c'est ça qui détermine si il y a ou non d'autres fournisseurs d'identité.

@garronej
Copy link
Collaborator

Oui c'est bien ça!

Merci pour ta contribution!

Au passag le kcContext n'est pas propre a Keycloakify, c'est le context FreeMarker de Keycloak qui est automatiquement converti en un object javascript par Keycloakify. Donc ce qui est présent dans le kcContext est authoritatif.

Au passage, je vais bientôt mettre a jour ce theme pour le rendre compatible avec Keycloak 24.
Je le ferais quand j'aurais enfin livré Keycloakify v10: keycloakify/keycloakify#538

@this-is-tobi
Copy link
Contributor Author

Trop chouette @garronej, on est pas encore passé sur Keycloak v24 de notre côté mais je vais suivre tes travaux du coup, merci pour le boulot en cas !

Je viens de pousser la deuxième modification, n'hésites pas à me dire si tu souhaites des modifications.

src/login/pages/Login.tsx Outdated Show resolved Hide resolved
@garronej garronej merged commit ba9c3c9 into codegouvfr:main May 23, 2024
@garronej
Copy link
Collaborator

Thank you!

garronej added a commit that referenced this pull request May 23, 2024
Signed-off-by: Joseph Garrone <[email protected]>
@this-is-tobi
Copy link
Contributor Author

You're welcome, btw thank you for the awesome work !

@this-is-tobi
Copy link
Contributor Author

Oh @garronej ! https://github.com/codegouvfr/keycloak-theme-dsfr/actions/runs/9212447695/job/25344077778 Do you want me to fix this with a ? in social.providers?.length

garronej added a commit that referenced this pull request May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants