Site option to prevent display of usernames

Bug #1353516 reported by Tony Butler on 2014-08-06
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
Wishlist
Robert Lyon

Bug Description

There are a few places in Mahara where usernames are displayed to ordinary users (e.g. user/find.php and user/online.php). This may be a privacy issue for sites where email addresses are used as usernames.

This patch adds a site option enabling the admin to prevent usernames being displayed to ordinary users anywhere in the site.

Robert Lyon (robertl-9) on 2014-08-07
Changed in mahara:
status: New → In Progress
assignee: nobody → Robert Lyon (robertl-9)
importance: Undecided → Medium

Actually, there shouldn't be an option at all. Regular users shouldn't see the username, but only staff and admins.

Tony Butler (tonyjbutler) wrote :

Thanks Kristina. So does that mean that the $username parameter of display_name() is redundant, or that the original logic was incorrect? In other words, should the username always be displayed for admins and staff, or only when $username is true?
The original logic,

$addusername = $username || !empty($userto->admin) || !empty($userto->staff);

implies that there are situations where all users should be able to see usernames. Should this logic actually be more like

$addusername = $username && (!empty($userto->admin) || !empty($userto->staff));

or simply

$addusername = !empty($userto->admin) || !empty($userto->staff);

?

 ...

I've had another look, and the documentation for display_name() says:

@param boolean $username show the user's username even if the viewer is not an admin

I suppose the third possibility is that usernames should always be displayed to admins, but only to staff when $username is true (and never to ordinary users).

Let me know what you think.

Cheers,
Tony

Mahara Bot (dev-mahara) wrote :

Patch for "master" branch: https://reviews.mahara.org/4202

Tony Butler (tonyjbutler) wrote :

I've added another commit to this issue, after receiving a report from a student that notification emails were displaying the sender's username anyway.

Mahara Bot (dev-mahara) wrote :

Patch for "master" branch: https://reviews.mahara.org/4208

Tony Butler (tonyjbutler) wrote :

I've now added a third commit to prevent searching on usernames if the 'nousernames' option is enforced (except for site admins and site staff who can always see usernames anyway, and should therefore also be able to always search on them).

I've also amended the first commit to fix a couple of areas I spotted where the 'searchusernames' option was being used to decide whether or not to *display* usernames. The 'nousernames' option seems a more logical choice so is now used instead.

Robert Lyon (robertl-9) wrote :

This patch involves adding in a new user search setting so we end up with three settings relating to user search, which are:

[A] Users can hide real names: If checked, users who have set a display name may choose to be searchable only by their display name and will not be found in searches for their real name. (In the site administration section of the site, users are always searchable by their real names).

[B] Never display usernames: If checked, ordinary users will not be able to see the username of any other user.

[C] Search usernames: If checked, allow usernames to be searched on as part of "Search users".

So I believe there are 8 different permutations in play

1) [A] ticked
    [B] unticked
    [C] unticked

2) [A] ticked
    [B] ticked
    [C] unticked

3) [A] ticked
    [B] ticked
    [C] ticked

4) [A] ticked
    [B] unticked
    [C] ticked

5) [A] unticked
    [B] ticked
    [C] ticked

6) [A] unticked
    [B] ticked
    [C] unticked

7) [A] unticked
    [B] unticked
    [C] ticked

8) [A] unticked
    [B] unticked
    [C] unticked

Problems found when logged in as a normal user with admins settings in:

State 1,2,3, and 4:
- Can't actually search by displayname - even though the admin field description says we can
- Can search by real name - and real name is displayed in brackets next to displayname - so not actually hidden for those using displayname

State 5, 6, 7, 8:
- Worked as expected - thought being able to search by username and then not being able to display username (and visa versa) could be a bit confusing

Tony Butler (tonyjbutler) wrote :
Download full text (3.2 KiB)

I get completely different results for these tests, which I've summarised below (thanks to http://www.tablesgenerator.com/).

These are exactly the results I would expect, logged in as a normal (student) user, searching for another normal user who has set a display name and a preference to hide their real name (where applicable).

However it has occurred to me that, since 'Search usernames' has no effect if 'Never display usernames' is enforced (and nor should it, as you pointed out above), perhaps these two settings would be better combined into one. What do you think (and if you agree, what do you think it should it look like / how should it be worded)?

+-------------------+------------------+-----------+---------------+------------+---------------+------------+---------------+------------+
| Users can hide RN | Never display UN | Search UN | DN searchable | DN visible | RN searchable | RN visible | UN searchable | UN visible |
+-------------------+------------------+-----------+---------------+------------+---------------+------------+---------------+------------+
| / | | | Y | Y | N | N | N | Y |
+-------------------+------------------+-----------+---------------+------------+---------------+------------+---------------+------------+
| / | / | | Y | Y | N | N | N | N |
+-------------------+------------------+-----------+---------------+------------+---------------+------------+---------------+------------+
| / | / | / | Y | Y | N | N | N | N |
+-------------------+------------------+-----------+---------------+------------+---------------+------------+---------------+------------+
| / | | / | Y | Y | N | N | Y | Y |
+-------------------+------------------+-----------+---------------+------------+---------------+------------+---------------+------------+
| | / | / | Y | Y | Y | Y | N | N |
+-------------------+------------------+-----------+---------------+------------+---------------+------------+---------------+------------+
| | / | | Y | Y | Y | Y | N | N |
+-------------------+------------------+-----------+---------------+------------+---------------+------------+---------------+------------+
| | | / | Y | Y | Y | Y | Y | Y |
+-------------------+------------------+-----------+---------------+------------+---------------+------------+---------------+------------+
| | | | Y | Y | Y | Y | N | Y |
+--------------...

Read more...

Tony Butler (tonyjbutler) wrote :
  • results.ods Edit (17.6 KiB, application/vnd.oasis.opendocument.spreadsheet)

OK, the text table obviously didn't work out so I've attached the original spreadsheet. ;-)

Aaron Wells (u-aaronw) wrote :

Hi Tony,

Looking at the git history, it appears the $username parameter of display_username() was added back in 2010 in order to support the $cfg->searchusernames option, i.e. Administration -> Site options -> User settings -> Search usernames. This option is on by default, and its help text: "If checked, allow usernames to be search on as part of 'Search users'."

I think we could use some consistency in our approach, though. As it currently stands, usernames are... semi-private.

- Admins & staff can always see usernames
- Users can see their own username, on their account settings page
- The username is displayed on the user search page, if you don't disable "Search usernames"
- If you have clean URLs turned on, the username is a default part of a user's cleanurl.

If we're adding a sitewide option to hide usernames (and apparently there is a use-case for it), then we should just eliminate "Search usernames" as a separate option. Instead, we say that if usernames are hidden, you can't search by them. Also if usernames are hidden, we should use a different method to the default Clean URL. Maybe by passing the user's name (or preferred name) through the url generator, instead of their username.

Changed in mahara:
milestone: none → 15.04.0
importance: Medium → Wishlist
Mahara Bot (dev-mahara) wrote :

Patch for "master" branch: https://reviews.mahara.org/4306

Tony Butler (tonyjbutler) wrote :

Thanks for the feedback Aaron.

I've added a commit which consolidates these two options into one, hopefully resolving the inconsistencies you mention above. If usernames are not displayed, Clean URLs for profile pages will be generated using the preferred name if one is supplied, or the real firstname and lastname concatenated together.

If everything is OK please let me know and I'll squash all the commits into one.

Cheers,
Tony

Aaron Wells (u-aaronw) wrote :

Thanks for the patch, Tony! I'll try to take a look at it in the next few days.

Cheers,
Aaron

Tony Butler (tonyjbutler) wrote :

Thanks for reviewing Aaron. I've addressed the points you raised (the de-duping code was in there already).

Cheers,
Tony

Did someone already test what the display is like in Elasticsearch? Will it be the same as with internal search when displaying users?

Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/4202
Committed: http://gitorious.org/mahara/mahara/commit/bd02e40b6e0b2886120680bcbe0fe85e281b33d5
Submitter: Aaron Wells (<email address hidden>)
Branch: master

commit bd02e40b6e0b2886120680bcbe0fe85e281b33d5
Author: Tony Butler <email address hidden>
Date: Tue Jan 20 11:04:10 2015 +0000

Make notification emails respect 'nousernames' option (Bug #1353516)

Change-Id: I272dab1e9119f6f2f464957547f1a7d3df61cc28
Signed-off-by: Tony Butler <email address hidden>

Reviewed: https://reviews.mahara.org/3581
Committed: http://gitorious.org/mahara/mahara/commit/2447fe3dc14f619c38154c46352855f26844ffdd
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit 2447fe3dc14f619c38154c46352855f26844ffdd
Author: Tony Butler <email address hidden>
Date: Wed Aug 6 15:09:32 2014 +0100

Add site option to prevent display of usernames (Bug #1353516)

Change-Id: I0c2db9c1fefcea2dfd7e442729ee0d46e822a781
Signed-off-by: Tony Butler <email address hidden>

Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/4208
Committed: http://gitorious.org/mahara/mahara/commit/2f82ef796e9c0b8f946f16d100bd452b60056693
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit 2f82ef796e9c0b8f946f16d100bd452b60056693
Author: Tony Butler <email address hidden>
Date: Fri Jan 23 11:05:09 2015 +0000

Prevent searching by username if 'nousernames' is enabled (Bug #1353516)

Change-Id: Iabae07ea9ca90510064311e0947ef4ad84a67b14

Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/4306
Committed: http://gitorious.org/mahara/mahara/commit/5f14d7ae577e7cbdd72a9fe2efa0f1af85ebe73d
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit 5f14d7ae577e7cbdd72a9fe2efa0f1af85ebe73d
Author: Tony Butler <email address hidden>
Date: Fri Feb 20 17:24:30 2015 +0000

Consolidate 'searchusernames' and 'nousernames' options (Bug #1353516)

Change-Id: Ib0d6028fd81ac0b66f0c9d49e201825c07c41d98

Robert Lyon (robertl-9) on 2015-03-09
Changed in mahara:
status: In Progress → Fix Committed
tags: added: nominatedfeature
Robert Lyon (robertl-9) on 2015-04-17
Changed in mahara:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Bug attachments