Using /admin/users/sync_sso endpoint to sync SSO data, it is not possible to remove someone’s name from the account, even if it is not required. In other other words, it’s not possible to change an account’s name from something to nothing. This is happening with full_name_required = false (and sso_overrides_name = true).
I believe the issue is here:
but I’m afraid my knowledge of Ruby/Discourse isn’t that hot to submit a PR.
This feels like a feature request to me. At the moment you can use the admin API to clear names in cases like this. See:
Sorry, but I disagree. I don’t see how this can be a feature request and not a bug.
I understand there’s other API endpoints which can be used. But the main point of /admin/users/sync_sso is precisely to keep this sort of data, well, in sync. It already allows to set the account name field — that works. But, it assumes name is always a required field and doesn’t allow it to be set it to blank. So it can’t be used to keep data in sync.
The code below seems to make it work as expected on my sandbox, but again, I don’t feel confident enough to submit a PR at this point. Feel free to use it/adapt it, etc.
- if SiteSetting.sso_overrides_name && user.name != name && name.present?
- user.name = name || User.suggest_name(username.blank? ? email : username)
+ if SiteSetting.sso_overrides_name && user.name != name
+ if SiteSetting.full_name_required && name.present?
+ user.name = name || User.suggest_name(username.blank? ? email : username)
+ else
+ user.name = name
+ end
Its a feature request, the semantics are ambiguous. Lack of name could mean
You are asking for a protocol change here. I usually prefer explicitness so we don’t have a confusing api.
I see your point now.
Once again, I don’t know the internals (or ruby) enough… but there’s no way to know if the field name was provided on that request? Not present at all, don’t touch the name field of the account. Field name provided, set it (if blank, clear it). Wouldn’t this respect that protocol/behavior where only the provided fields are synched?
Sorry if this makes no sense — just trying to grasp from what I understand.
The problem is that the protocol semantics now of “name” param is there and set to blank, is the same as no “name” param. It simply does not touch it.
A change here is a change to protocol semantics. We could I guess change is so name= becomes name is blanked, and absence of name means “don’t touch name”, but people already rely on the old behavior so this is technically a breaking change.
Can you explain why you are removing names?
We are not removing everyone’s names — the users are updating their own data whenever they update their account on our website (SSO provider). We rely on /admin/users/sync_sso to keep data in sync (username, name, avatar, etc) in their Discourse account. Name is an optional field and can be set to blank/empty.
I just realized the issue happens not just with name but also with updating bio, avatar, etc: it’s not possible to keep those SSO records in sync through /admin/users/sync_sso if they need to be updated to blank/empty, regardless whether they are mandatory fields or not.
I understand the point that people may be relying on existing behavior (although nobody reported this issue before?), but if this is the protocol, it seems it has significant limitations for its purpose of synchronizing SSO records.
I’m also getting bitten by this, individual users are unable to remove their own personal information (name, avatars, bio, custom fields etc) from Discourse which has ramifications as you can imagine.
Agree with not changing the semantics of how it works now but could you not allow us to set these attributes as false in the SSO payload or something similar to be explicit about removing them?
Auparavant, nous contournions cette limitation SSO en composant l’appel à /admin/users/sync_sso avec un autre appel au point de terminaison /u/{username} juste pour effacer le nom (si la nouvelle valeur pour le nom était vide).
Cependant, cela semble également avoir cessé de fonctionner à un moment donné dans une version récente, peut-être parce qu’il vérifie si sso_overrides_name = true avant de mettre à jour le nom.
Ainsi, tel quel, lors de l’utilisation de SSO et sso_overrides_name = true, il semble désormais impossible pour le fournisseur SSO d’effacer le champ nom sur Discourse via l’API.
Voyez-vous une solution de contournement, @sam ?
Je suppose qu’un paramètre supplémentaire pour notre route sync_sso est nécessaire ? Quelque chose comme &clear_name pas sûr. Cela ressemble à un cas limite. Quel est le cas d’utilisation pour les noms vides ? Peut-être le définir simplement sur le nom d’utilisateur si vous n’en avez pas et que l’interface utilisateur peut supprimer les doublons.
Cela me trouble que vous considériez cela comme un cas limite, j’imagine donc que vous êtes habitué à des cas où le nom est le champ requis.
Nous avons l’inverse : le nom d’utilisateur est ce que tout le monde doit avoir, et le nom est un champ facultatif (nous utilisons prioritize_username_in_ux = true et full name required = false). Pensez aux comptes Twitter, où tout le monde doit avoir un nom d’utilisateur/pseudo, et peut éventuellement avoir un nom aussi. Je ne pense pas que vouloir effacer le champ nom (ou d’autres données personnelles) soit un scénario limite.
Actuellement, une fois qu’un nom a été saisi, il est impossible de le supprimer. C’était une limitation de sync_sso que nous avons contournée avec un appel API supplémentaire pour mettre à jour l’utilisateur, mais maintenant cela ne fonctionne plus non plus.
Nous l’avons envisagé, mais cela amène certaines personnes à supposer que le nom d’utilisateur est aussi le nom réel : nous gérons un forum international et il n’est souvent pas clair de savoir ce qui est le nom d’une personne et ce qui est un nom d’utilisateur (autre que leur position dans l’interface).
Je dois mentionner que si ma mémoire est bonne, le même problème exact se produit avec la suppression de l’avatar via sync_sso, car je pense que cela ne fonctionne pas non plus — nous contournons cela en fournissant notre propre URL pour un avatar par défaut.
S’il y a plusieurs champs qu’on ne peut pas effacer d’une manière ou d’une autre, peut-être un tableau (ou une liste csv) des champs à effacer/réinitialiser ?
Qu’est-ce qu’il faudrait pour parvenir à un accord sur la manière d’y parvenir ? Je suis heureux de mettre en œuvre quoi que ce soit de mon côté : un nouveau paramètre, une valeur spéciale, un deuxième appel API. Mais la situation actuelle, de mon point de vue, n’est pas un cas extrême. Comme @mentalstring ci-dessus, je synchronise avec une source de vérité externe où la définition d’une photo de profil et d’un nom d’affichage sont facultatives. L’utilisateur est autorisé à ne pas les avoir. Discourse leur permet de ne pas être définis. Si vous n’utilisez pas le SSO, vous pouvez les définir et les supprimer librement. DiscourseConnect brise cela : une fois qu’ils sont définis, ils ne peuvent jamais être supprimés, ce qui est, à mon avis, un (très petit) bug.
Je comprends la préoccupation concernant la modification du protocole où vide signifie actuellement aucun changement. Personnellement, je ne suis pas d’accord, je pense que c’est une manière simple et raisonnable d’interpréter le protocole et le risque est minime : il serait très étrange que des systèmes soient implémentés de telle sorte qu’ils envoient toujours les valeurs name et avatar_url, mais les définissent parfois à vide, et s’attendent à ce que cela signifie conserver l’ancienne valeur. Et s’il y en a qui le font, la conséquence est seulement que le nom et l’avatar sont désactivés, et ce devrait être une correction facile.
Mais bref, je ne veux pas argumenter sur ce point, mais je veux soutenir que c’est une fonctionnalité nécessaire. Je suis prêt à faire le PR, je veux juste savoir quelle solution serait acceptée.
Merci d’avance.
Étant donné que cela se produit avec au moins name et avatar_url et potentiellement d’autres (je pense aussi à website ?), peut-être un paramètre reset_fields avec une liste de champs à effacer au lieu de plusieurs clear_x individuels ?
Pour nous, il serait déjà utile de pouvoir corriger cela avec des appels supplémentaires au point de terminaison /u/{username}, mais cela aussi a cessé de fonctionner à un moment donné.