Core: Guests can see user profile regardless of the permission "view user list"

Created on 4 Dec 2018  路  4Comments  路  Source: flarum/core

Bug Report

Related to #1676 and https://discuss.flarum.org/d/17835-the-requested-resource-was-not-found-on-refresh and cuz toby asked me to 馃槢

Current Behavior
Guests can view user profiles regardless of the permission "view user list". It only triggers the permission on refreshing the page of a user or when directly navigating to a user profile url.

Steps to Reproduce

  1. Login and go the the permission page in the admin section
  2. Set the permission "View user list" to "Members"
  3. Log out
  4. Navigate to a discussion
  5. Click on someone's name
  6. You can view their profile.

Optional:

  1. Refresh the page or navigate directly to a user profile and see it working nonetheless, but displaying a vague error.

Expected Behavior

  • A more clear error message after step 7. You get a The requested resource was not found and not some kind of Permission denied error
  • Guests can't browse through a discussion to a user profile if the permission is set to members only

Environment

  • Flarum version: beta 8
  • Website URL: local
  • Webserver: nginx
  • Hosting environment: Laravel homestead
  • PHP version: 7.2.9
  • Browser: latest version of chrome
Flarum core 0.1.0-beta.8
PHP version: 7.2.9-1+ubuntu18.04.1+deb.sury.org+1
Loaded extensions: Core, date, libxml, openssl, pcre, zlib, filter, hash, pcntl, Reflection, SPL, sodium, session, standard, mysqlnd, PDO, xml, bcmath, calendar, ctype, curl, dom, mbstring, fileinfo, ftp, gd, gettext, iconv, igbinary, imap, intl, json, ldap, exif, msgpack, mysqli, pdo_mysql, pdo_pgsql, pdo_sqlite, pgsql, Phar, posix, readline, shmop, SimpleXML, soap, sockets, sqlite3, sysvmsg, sysvsem, sysvshm, tokenizer, wddx, xmlreader, xmlwriter, xsl, zip, memcached, blackfire, Zend OPcache, xdebug
+------------------------------+-------------------+------------------------------------------+
| Flarum Extensions            |                   |                                          |
+------------------------------+-------------------+------------------------------------------+
| ID                           | Version           | Commit                                   |
+------------------------------+-------------------+------------------------------------------+
| flarum-emoji                 | v0.1.0-beta.8     |                                          |
| flarum-lang-english          | v0.1.0-beta.8     |                                          |
| flarum-flags                 | v0.1.0-beta.8     |                                          |
| flarum-likes                 | v0.1.0-beta.8     |                                          |
| flarum-lock                  | v0.1.0-beta.8     |                                          |
| flarum-mentions              | v0.1.0-beta.8     |                                          |
| flarum-statistics            | v0.1.0-beta.8     |                                          |
| flarum-sticky                | v0.1.0-beta.8     |                                          |
| flarum-subscriptions         | v0.1.0-beta.8     |                                          |
| flarum-suspend               | v0.1.0-beta.8     |                                          |
| flarum-tags                  | v0.1.0-beta.8.1   |                                          |
| flarum-bbcode                | v0.1.0-beta.8     |                                          |
| michaelbelgium-profile-views | dev-flarum-beta-8 | 3350771156941554978c4692e3eef54b0adbda38 |
+------------------------------+-------------------+------------------------------------------+
Base URL: http://flarum.devel
Installation path: /home/vagrant/flarum
Debug mode: ON
needs-discussion typbug

Most helpful comment

This is a regression caused by https://github.com/flarum/core/commit/40e4c0acddd5c29dec322e6588ecd84a89544fa4. It turns out that the viewDiscussions permission is more like a global viewForum permission to restrict access from your whole forum (including discussions and user profiles), which is why we had been using it in UserPolicy::find. Whereas viewUserList is literally just for viewing the GET /api/users endpoint, not individual user profiles (because you can indeed access them anyway if you can see discussions).

So plan of attack from here:

All 4 comments

This is a regression caused by https://github.com/flarum/core/commit/40e4c0acddd5c29dec322e6588ecd84a89544fa4. It turns out that the viewDiscussions permission is more like a global viewForum permission to restrict access from your whole forum (including discussions and user profiles), which is why we had been using it in UserPolicy::find. Whereas viewUserList is literally just for viewing the GET /api/users endpoint, not individual user profiles (because you can indeed access them anyway if you can see discussions).

So plan of attack from here:

New permissions should be thoroughly scrutinised in the future, in my opinion the viewUserList permission caused too much confusion anyway, I'm glad we can revert this and apply something more transparent 馃憦

@tobscure what are we going to do with the viewUserList permission?

What I've considered is that the permission (even if named incorrectly) was introduced because it was so easy to scrape for users. If we'd tackle the #1356 issue, the permission could probably be modified to suit the needs better?

This was resolved in #2305, the remainder is superceded by #2202

Was this page helpful?
0 / 5 - 0 ratings

Related issues

luceos picture luceos  路  4Comments

ardacebi picture ardacebi  路  4Comments

webpigeon picture webpigeon  路  3Comments

luceos picture luceos  路  3Comments

jordanjay29 picture jordanjay29  路  3Comments