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)

في السابق، كنا نتجاوز هذا القيد الخاص بتسجيل الدخول الموحد (SSO) عن طريق دمج استدعاء إلى نقطة النهاية /admin/users/sync_sso مع استدعاء آخر إلى نقطة النهاية /u/{username} لمجرد مسح الاسم (إذا كانت القيمة الجديدة للاسم فارغة).

ومع ذلك، يبدو أن هذا توقف عن العمل أيضًا في وقت ما في إصدار حديث، ربما لأنه يتحقق مما إذا كان sso_overrides_name = true قبل تحديث الاسم.

لذلك، كما هو الحال، عند استخدام تسجيل الدخول الموحد (SSO) و sso_overrides_name = true، يبدو الآن أنه من المستحيل على موفر تسجيل الدخول الموحد (SSO) مسح حقل الاسم في Discourse عبر واجهة برمجة التطبيقات (API).

هل يمكنك رؤية أي حل بديل لهذا، @sam؟

أعتقد أن هناك حاجة لمعامل إضافي لمسار sync_sso الخاص بنا؟ شيء مثل &clear_name لست متأكدًا. يبدو هذا حالة هامشية جدًا. ما هي حالة الاستخدام للأسماء الفارغة؟ ربما قم بتعيينها إلى اسم المستخدم إذا لم يكن لديك أي شيء ويمكن لواجهة المستخدم قمع التكرار.

من المحير بالنسبة لي أنك تعتبر هذه حالة هامشية، لذا أفترض أنه يجب أن تكون معتادًا على الحالات التي يكون فيها الاسم هو الحقل المطلوب.

لدينا العكس: اسم المستخدم هو ما يجب أن يمتلكه الجميع، والاسم هو حقل اختياري (نستخدم prioritize_username_in_ux = true و full name required = false). فكر في حسابات تويتر، حيث يجب أن يمتلك الجميع اسم مستخدم/معرف، ويمكنهم اختياريًا إضافة اسم أيضًا. لا أعتقد أن رغبة المرء في مسح حقل الاسم (أو بيانات شخصية أخرى) هي سيناريو هامشي.

في الوقت الحالي، بمجرد أن يملأ المرء اسمًا، يصبح من المستحيل إزالته. كان هذا قيدًا من sync_sso قمنا بالتحايل عليه بمكالمة API إضافية لتحديث المستخدم، ولكن الآن لم يعد هذا يعمل أيضًا.

لقد نظرنا في الأمر، ولكنه يدفع بعض الأشخاص إلى افتراض أن اسم المستخدم الخاص بالشخص هو أيضًا اسمه الحقيقي: نحن ندير منتدى دوليًا وغالبًا ما لا يكون من الواضح ما هو اسم الشخص وما هو اسم المستخدم (بخلاف موضعهما في الواجهة).

يجب أن أذكر أنه إذا لم تخونني الذاكرة، فإن نفس المشكلة بالضبط تحدث مع إزالة صورة الرمز الشخصي للمرء من خلال sync_sso لأنني أعتقد أن هذا لا يعمل أيضًا - نحن نتحايل على ذلك من خلال توفير عنوان URL الخاص بنا لصورة رمزية افتراضية.

إذا كان هناك حقول متعددة قد لا يتمكن المرء من مسحها بطريقة ما، فربما مصفوفة (أو قائمة CSV) للحقول التي يجب مسحها/إعادة تعيينها؟

ما الذي سيتطلبه الأمر للتوصل إلى اتفاق بشأن طريقة لإنجاز هذا؟ يسعدني تنفيذ أي شيء من جانبي: معلمة جديدة، قيمة خاصة، استدعاء API ثانٍ. لكن الوضع الحالي، من وجهة نظري، ليس حالة هامشية. مثل @mentalstring أعلاه، أقوم بالمزامنة مع مصدر حقيقة خارجي حيث يكون تعيين صورة الملف الشخصي واسم العرض اختياريين. يُسمح للمستخدم بعدم امتلاكها. يسمح Discourse بعدم تعيينها. إذا لم تستخدم SSO، يمكنك تعيينها وإزالتها بحرية. DiscourseConnect يكسر هذا: بمجرد تعيينها، لا يمكن إزالتها أبدًا، وهو في رأيي خطأ (صغير جدًا).

أتفهم القلق بشأن تغيير البروتوكول حيث يعني الفارغ حاليًا عدم وجود تغيير. شخصيًا، أختلف، أعتقد أنه طريقة واضحة ومعقولة لتفسير البروتوكول والخطر ضئيل: سيكون من الغريب جدًا أن يتم تنفيذ الأنظمة بحيث ترسل دائمًا قيم الاسم و avatar_url، ولكن في بعض الأحيان تعيينها إلى فارغة، وتتوقع أن يعني ذلك الاحتفاظ بالقيمة القديمة. وإذا كان هناك أي منها يفعل ذلك، فإن النتيجة هي فقط إلغاء تعيين الاسم والصورة الرمزية، ويجب أن يكون إصلاحًا سهلاً.

ولكن على أي حال، لا أريد الجدال في هذه النقطة، ولكني أريد تقديم الحجة بأن هذه ميزة ضرورية. أنا على استعداد لتقديم طلب السحب (PR)، وأريد فقط معرفة الحل الذي سيتم قبوله.

شكرًا جزيلاً مقدمًا.

إعجاب واحد (1)

نظرًا لأن هذا يحدث مع name و avatar_url على الأقل وربما مع حقول أخرى (أعتقد website أيضًا؟)، فربما يكون هناك معلمة reset_fields مع قائمة بالحقول لمسحها بدلاً من عدة حقول clear_x فردية؟

بالنسبة لنا، سيكون من المفيد بالفعل إذا تمكنا من إصلاحه باستدعاءات إضافية لنقطة النهاية /u/{username}، ولكن هذا أيضًا توقف عن العمل في وقت ما.

إعجاب واحد (1)