e107inc / e107

e107 Bootstrap CMS (Content Management System) v2 with PHP, MySQL, HTML5, jQuery and Twitter Bootstrap. Issue Discussion Room: https://gitter.im/e107inc/e107
https://e107.org
GNU General Public License v3.0
318 stars 212 forks source link

[Bug]: Users can opt-in to userclasses but cannot opt-out of them. #5159

Open HatchlingByHeart opened 6 months ago

HatchlingByHeart commented 6 months ago

What e107 version are you using?

v2.3.3

Bug description

Once a user has assigned themselves any custom userclass on the settings page, they cannot unassign it. In the database, I have noticed in the "e107_user" table that the "user_class" column is completely overwritten with the id of the userclass instead of it being appended to it. This happens on my current production install and a fresh test install.

How to reproduce

  1. Install e107 2.3.3
  2. Log In
  3. Create a custom userclass in the User Classes section of the admin panel.
  4. Go to User Settings page and assign the new class to your account.
  5. Attempt to unassign that same class.

Expected behavior

Users should be able to unassign a userclass from their account.

What browser(s) are you seeing the problem on?

Firefox, Chrome / Brave

PHP Version

PHP 8.2.13

Deltik commented 6 months ago

I'm not able to reproduce your issue using the steps that you provided. See the video below for what I did:

2023-12-20 18-30-32.webm

What did you do differently from what I did to make the user unable to unassign a userclass that they have permission to unassign?

HatchlingByHeart commented 6 months ago

I'm not able to reproduce your issue using the steps that you provided. See the video below for what I did: 2023-12-20.18-30-32.webm

What did you do differently from what I did to make the user unable to unassign a userclass that they have permission to unassign?

I think the only thing I did different was name the database info something other then "e107". I am currently investigating what other differences there could be in the environment I've installed it on compared to everyone else's.

Are you also running PHP 8.2.13? What OS is your server using?

There is a discussion regarding this issue here: https://github.com/e107inc/e107/discussions/5162

HatchlingByHeart commented 6 months ago

https://github.com/e107inc/e107/assets/6427368/24350cf5-0233-48e5-9b38-b64f250b1bf1

Here is what I did. @Deltik

Deltik commented 6 months ago

I was able to reproduce the bug as a normal user (not admin) by unchecking all the "Subscribed to" items.

The cause is that the class[] array being empty is not sent to the server for validation, so the server assumes that the client did not want to change the userclasses.

As for the "Manager" and "Visibility" changing from "Everyone (public)" to "Admin" in /e107_admin/userclass2.php?mode=main&action=edit, it's because "Everyone (public)" is saved as 0, which is interpreted as no value, so the default will be "Admin" when the page loads again. It is actually saved as "Everyone (public)", but a subsequent save will actually change it to "Admin".

So there are two bugs.

HatchlingByHeart commented 6 months ago

I was able to reproduce the bug as a normal user (not admin) by unchecking all the "Subscribed to" items.

The cause is that the class[] array being empty is not sent to the server for validation, so the server assumes that the client did not want to change the userclasses.

As for the "Manager" and "Visibility" changing from "Everyone (public)" to "Admin" in /e107_admin/userclass2.php?mode=main&action=edit, it's because "Everyone (public)" is saved as 0, which is interpreted as no value, so the default will be "Admin" when the page loads again. It is actually saved as "Everyone (public)", but a subsequent save will actually change it to "Admin".

So there are two bugs.

I'm impressed you found this out that fast. You may have just saved my website. :)

HatchlingByHeart commented 6 months ago

I have discovered while debugging that the userclass string seems to get wiped out by the validation process whenever you submit the form and only the ID of the newly toggled userclass remains.

I have 3 visible userclass checkboxes: Administrator, Moderator, and a Custom Class I created with the ID# 2. Administrator and Moderator were already checked. This image is after I checked class ID# 2 and submitted.

Screenshot 2023-12-22 070137

HatchlingByHeart commented 6 months ago

I have worked around this issue by doing adding a dummy userclass that is otherwise not used for anything (ID# 1 in my case), and making the following changes:

/e107_core/shortcodes/batch/userclasses_shortcodes.php Screenshot 2023-12-23 074609

/e107_handlers/userclass_class.php Screenshot 2023-12-23 074649

Vodhin commented 6 months ago

I came across this issue a while ago. I added a bit of code to usersettings.php around line 540 to ensure that userclass was never empty by adding a useless value;


// Update Userclass - only if its the user changing their own data (admins can do it another way)
$allData['data']['user_class'] = ($allData['data']['user_class'] ? $allData['data']['user_class'] : -256);
if (isset($allData['data']['user_class']))
{ ...