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?
In precedenza, aggiravamo questa limitazione SSO componendo la chiamata a /admin/users/sync_sso con un’altra chiamata all’endpoint /u/{username} solo per svuotare il nome (se il nuovo valore per il nome era vuoto).
Tuttavia, anche questo sembra aver smesso di funzionare in qualche versione recente, forse perché controlla se sso_overrides_name = true prima di aggiornare il nome.
Quindi, così com’è, quando si utilizza SSO e sso_overrides_name = true, ora sembra impossibile per il provider SSO svuotare il campo del nome su Discourse tramite API.
Puoi vedere una soluzione alternativa a questo, @sam?
Immagino che un parametro aggiuntivo per la nostra route sync_sso sia in ordine? Qualcosa come &clear_name non sono sicuro. Questo sembra un caso limite. Qual è il caso d’uso per i nomi vuoti? Forse impostalo semplicemente su username se non ne hai nessuno e l’interfaccia utente può sopprimere le duplicazioni.
Mi confonde il fatto che tu consideri questo un caso limite, quindi immagino che tu sia abituato a istanze in cui il nome è il campo obbligatorio.
Da noi è il contrario: lo username è ciò che tutti devono avere, e il nome è un campo opzionale (usiamo prioritize_username_in_ux = true e full name required = false). Pensa agli account di Twitter, dove tutti devono avere uno username/handle e possono opzionalmente avere anche un nome. Non credo che voler cancellare il nome (o altri dati personali) sia uno scenario limite.
Al momento, una volta inserito un nome, è impossibile rimuoverlo. Questa era una limitazione di sync_sso che abbiamo aggirato con una chiamata API aggiuntiva per aggiornare l’utente, ma ora nemmeno quella funziona più.
Ci abbiamo pensato, ma induce alcune persone a credere che lo username di una persona sia anche il suo vero nome: gestiamo un forum internazionale e spesso non è chiaro cosa sia il nome di una persona e cosa sia uno username (oltre alla loro posizione nell’interfaccia).
Dovrei menzionare che, se la memoria non mi inganna, esattamente lo stesso problema si verifica con la rimozione dell’avatar tramite sync_sso, poiché credo che nemmeno quello funzioni — lo aggiriamo fornendo il nostro URL per un avatar predefinito.
Se ci sono più campi che non si possono cancellare in qualche modo, forse un array (o un elenco csv) di quali campi cancellare/resettare?
Cosa servirebbe per raggiungere un accordo su un modo per realizzarlo? Sono felice di implementare qualsiasi cosa da parte mia: un nuovo parametro, un valore speciale, una seconda chiamata API. Ma la situazione attuale, dal mio punto di vista, non è un caso limite. Come @mentalstring sopra, sto sincronizzando con una fonte di verità esterna in cui l’impostazione di un’immagine del profilo e di un nome visualizzato sono facoltative. L’utente è autorizzato a non averli. Discourse permette che non vengano impostati. Se non si utilizza l’SSO, è possibile impostarli e rimuoverli liberamente. DiscourseConnect rompe questo: una volta impostati, non possono mai essere rimossi, il che secondo me è un (molto piccolo) bug.
Capisco la preoccupazione riguardo alla modifica del protocollo in cui vuoto attualmente significa nessuna modifica. Personalmente non sono d’accordo, penso che sia un modo semplice e ragionevole per interpretare il protocollo e il rischio è minimo: sarebbe molto strano che i sistemi vengano implementati in modo tale da inviare sempre i valori di nome e avatar_url, ma a volte impostarli su vuoto, e aspettarsi che ciò significhi mantenere il vecchio valore. E se ce ne sono alcuni che lo fanno, la conseguenza è solo che nome e avatar vengono deselezionati, e dovrebbe essere una correzione facile.
Ma comunque, non voglio discutere su questo punto, ma voglio sostenere che questa è una funzionalità necessaria. Sono disposto a fare la PR, voglio solo sapere quale soluzione verrebbe accettata.
Molte grazie in anticipo.
Dato che questo accade almeno con name e avatar_url e potenzialmente altri (penso anche website?), forse un parametro reset_fields con un elenco di campi da cancellare invece di diversi clear_x individuali?
Per noi sarebbe già utile se potessimo risolverlo con chiamate aggiuntive all’endpoint /u/{username}, ma anche quello ha smesso di funzionare a un certo punto.