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:

https://github.com/discourse/discourse/blob/88ecb650a98921e5e3cd345e29f84d9dd204b777/app/models/discourse_single_sign_on.rb#L268

but I’m afraid my knowledge of Ruby/Discourse isn’t that hot to submit a PR.

4 个赞

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 个赞

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 个赞

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?

1 个赞

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?

1 个赞

以前,我们通过将对 /admin/users/sync_sso 的调用与对 /u/{username} 端点的另一个调用组合来绕过此 SSO 限制,只是为了清除名称(如果名称的新值为空)。

然而,这似乎在最近某个版本中也停止工作了,可能是因为它在更新名称之前会检查 sso_overrides_name = true

因此,目前在使用 SSO 和 sso_overrides_name = true 时,似乎无法通过 API 让 SSO 提供商清除 Discourse 上的名称字段。

@sam,您能看到任何绕过此的方法吗?

我猜我们的 sync_sso 路由需要一个额外的参数?比如 &clear_name 不确定。这感觉是个极端情况。空白名字的用例是什么?也许如果没有名字就设置为用户名,然后 UI 可以抑制重复。

对我来说,你认为这是一个极端情况(edge case)是很令人困惑的,所以我猜你一定习惯了名字是必填字段的情况。

我们的情况正好相反:用户名是每个人都必须有的,而名字是可选字段(我们使用 prioritize_username_in_ux = truefull name required = false)。想想 Twitter 账户,每个人都必须有一个用户名/句柄,并且可以选择性地添加一个名字。我不认为清除名字字段(或其他个人数据)是一种极端情况。

目前,一旦填写了名字,就无法将其删除。这是 sync_sso 的一个限制,我们通过额外的 API 调用来更新用户来解决,但现在这也不起作用了。

我们考虑过,但这会让一些人认为用户名也是他们的真实姓名:我们运营一个国际论坛,人们常常不清楚什么是姓名,什么是用户名(除了在界面上的位置)。

我应该提到,如果我没记错的话,通过 sync_sso 删除头像也会出现完全相同的问题,因为我认为那也不起作用——我们通过提供自己的默认头像 URL 来解决这个问题。

如果存在多个无法以某种方式清除的字段,也许可以提供一个数组(或 CSV 列表)来指定要清除/重置的字段?

要达成一致并找到完成此任务的方法需要什么?我很乐意在我的这一侧实现任何操作:一个新参数、一个特殊值、第二次 API 调用。但从我的角度来看,目前的情况并非边缘情况。就像上面 @mentalstring 所说,我正在与外部事实来源同步,其中设置个人资料图片和显示名称是可选的。用户可以不设置它们。Discourse 允许它们不被设置。如果您不使用 SSO,您可以自由地设置和删除它们。DiscourseConnect 会破坏这一点:一旦设置了它们,就永远无法删除它们,我认为这是一个(非常小的)错误。

我理解关于更改协议的担忧,在协议中空值目前意味着不更改。我个人不同意,我认为这是一种直接且合理的解释协议的方式,风险很小:系统发送名称和 avatar_url 值,但有时将其设置为空,并期望这表示保留旧值,这种情况会非常奇怪。而且,如果有任何系统这样做,后果只是名称和头像被取消设置,这应该是一个容易修复的问题。

但无论如何,我不想争论这一点,但我确实想说明这是一个必要的功能。我愿意提交 PR,我只是想知道什么解决方案会被接受。

提前表示感谢。

1 个赞

鉴于这种情况至少会影响 nameavatar_url,甚至可能影响其他字段(我认为也包括 website?),是否可以提供一个 reset_fields 参数,其中包含一个要清除的字段列表,而不是多个单独的 clear_x

对我们来说,如果能通过对 /u/{username} 端点的附加调用来修复它就已经很有用了,但该端点在某个时候也停止工作了。

1 个赞