Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Guests can see user profile regardless of the permission "view user list" #1680

Closed
MichaelBelgium opened this issue Dec 4, 2018 · 4 comments

Comments

@MichaelBelgium
Copy link

MichaelBelgium commented Dec 4, 2018

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:
7. 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
@tobyzerner
Copy link
Contributor

tobyzerner commented Dec 4, 2018

This is a regression caused by 40e4c0a. 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:

  • Change viewDiscussions permission to viewForum
  • Revert 40e4c0a

@tobyzerner tobyzerner added this to the 0.1.0-beta.9 milestone Dec 4, 2018
@luceos
Copy link
Member

luceos commented Dec 5, 2018

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 👏

@luceos
Copy link
Member

luceos commented Jan 9, 2019

@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?

@askvortsov1
Copy link
Member

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

@askvortsov1 askvortsov1 removed this from the 0.1 milestone Oct 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants