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?
Zuvor umgingen wir diese SSO-Beschränkung, indem wir den Aufruf an /admin/users/sync_sso mit einem weiteren Aufruf an den Endpunkt /u/{username} kombinierten, nur um den Namen zu leeren (falls der neue Wert für den Namen leer war).
Dies scheint jedoch in einer neueren Version aus irgendeinem Grund nicht mehr zu funktionieren, möglicherweise weil geprüft wird, ob sso_overrides_name = true ist, bevor der Name aktualisiert wird.
Wenn also SSO und sso_overrides_name = true verwendet werden, scheint es jetzt unmöglich zu sein, das Namensfeld in Discourse über die API vom SSO-Anbieter leeren zu lassen.
Sehen Sie eine Möglichkeit, dies zu umgehen, @sam?
Ich vermute, ein zusätzlicher Parameter für unsere sync_sso-Route ist angebracht? So etwas wie &clear_name, bin mir nicht sicher. Das fühlt sich wie ein solcher Grenzfall an. Was ist der Anwendungsfall für leere Namen? Vielleicht einfach auf den Benutzernamen setzen, wenn Sie keinen haben, und die Benutzeroberfläche kann Duplikate unterdrücken.
Es ist für mich verwirrend, dass Sie dies als Sonderfall betrachten. Ich nehme an, Sie sind also an Fälle gewöhnt, in denen der Name das erforderliche Feld ist.
Wir haben es umgekehrt: Der Benutzername ist das, was jeder haben muss, und der Name ist ein optionales Feld (wir verwenden prioritize_username_in_ux = true und full name required = false). Denken Sie an Twitter-Konten, bei denen jeder einen Benutzernamen/ein Handle haben muss und optional auch einen Namen haben kann. Ich glaube nicht, dass es ein Sonderfall ist, wenn man das Namensfeld (oder andere persönliche Daten) löschen möchte.
Derzeit ist es unmöglich, einen einmal eingegebenen Namen zu entfernen. Dies war eine Einschränkung von sync_sso, die wir mit einem zusätzlichen API-Aufruf zur Aktualisierung des Benutzers umgangen haben, aber jetzt funktioniert auch das nicht mehr.
Wir haben es in Betracht gezogen, aber es verleitet einige Leute zu der Annahme, dass der Benutzername auch ihr richtiger Name ist: Wir betreiben ein internationales Forum und es ist oft nicht klar, was der Name einer Person und was ein Benutzername ist (außer durch ihre Position in der Benutzeroberfläche).
Ich sollte erwähnen, dass, wenn mich meine Erinnerung nicht trügt, genau das gleiche Problem beim Entfernen des Avatars über sync_sso auftritt, da dies meiner Meinung nach auch nicht funktioniert – wir umgehen dies, indem wir unsere eigene URL für einen Standardavatar bereitstellen.
Wenn es mehrere Felder gibt, die man auf irgendeine Weise nicht löschen kann, vielleicht ein Array (oder eine CSV-Liste) von Feldern, die gelöscht/zurückgesetzt werden sollen?
Was wäre nötig, um eine Einigung über einen Weg zur Erreichung dieses Ziels zu erzielen? Ich bin gerne bereit, auf meiner Seite alles zu implementieren: einen neuen Parameter, einen speziellen Wert, einen zweiten API-Aufruf. Aber die Situation ist aus meiner Sicht kein Grenzfall. Wie @mentalstring oben, synchronisiere ich mich mit einer externen Wahrheitsquelle, bei der das Festlegen eines Profilbildes und eines Anzeigenamens optional ist. Der Benutzer darf sie nicht haben. Discourse erlaubt es, sie nicht festzulegen. Wenn Sie kein SSO verwenden, können Sie sie frei festlegen und entfernen. DiscourseConnect bricht dies: Sobald sie festgelegt sind, können sie nie wieder entfernt werden, was meiner Meinung nach ein (sehr kleiner) Fehler ist.
Ich verstehe die Bedenken hinsichtlich der Änderung des Protokolls, bei dem leer derzeit keine Änderung bedeutet. Persönlich stimme ich dem nicht zu, ich denke, es ist eine unkomplizierte und vernünftige Art, das Protokoll zu interpretieren, und das Risiko ist winzig: Es wäre sehr seltsam, wenn Systeme so implementiert würden, dass sie immer Namens- und Avatar-URL-Werte senden, sie aber manchmal auf leer setzen und erwarten, dass dies bedeutet, den alten Wert beizubehalten. Und wenn es welche gibt, die das tun, ist die Folge nur, dass Name und Avatar gelöscht werden, und es sollte eine einfache Korrektur sein.
Aber wie auch immer, ich möchte diesen Punkt nicht diskutieren, aber ich möchte die Argumentation darlegen, dass dies eine notwendige Funktion ist. Ich bin bereit, den PR zu machen, ich möchte nur wissen, welche Lösung akzeptiert würde.
Vielen Dank im Voraus.
Da dies bei mindestens name und avatar_url und möglicherweise anderen (ich glaube auch website?) vorkommt, vielleicht ein reset_fields-Parameter mit einer Liste von Feldern, die gelöscht werden sollen, anstelle mehrerer einzelner clear_x-Felder?
Für uns wäre es schon nützlich, wenn wir es mit zusätzlichen Aufrufen an den /u/{username}-Endpunkt beheben könnten, aber auch das hat irgendwann aufgehört zu funktionieren.