Allow name removal using SSO

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.

3 Likes

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:

1 Like

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

  1. Clear the name
  2. Leave the name what it was

You are asking for a protocol change here. I usually prefer explicitness so we don’t have a confusing api.

2 Likes

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?