search

mindstellar / Osclass

With Osclass, get your own classifieds site for free. Build your own Osclass installation and start advertising real estate, jobs or whatever you want- in minutes!
https://docs.mindstellar.com/
GNU General Public License v3.0
90 stars 49 forks source link

Inactive User Can Post New Ads #460

Closed eurobank closed 2 years ago

eurobank commented 2 years ago

Osclass 5.x dev

Yesterday a new user registered, posted some scam ads and i DE-ACTIVATED HIM.

Screenshot

Today he posted some new ads from that INACTIVE account.

Screenshot (2)

eurobank commented 2 years ago

Also that specific user has managed to REACTIVATE his account but still remained blocked.

Why a Deactivated and Blocked user, can re-activate his account? How did he do that?

Tangol commented 2 years ago

@eurobank You're a genius!

I found 2 bugs here:

  1. When you Deactivate the user from the admin, he doesn't get logged out of the website. So if he's still logged in, he can post listings, even if the admin disabled his account. 2. When you have Users need to validate their account enabled, and a user activated his account by mail, if you Deactivate his account from the admin, he can re-activate it by clicking on the validation link in the previous email.

This happens in all Osclass versions.

EDIT 1: The only thing that works for stopping bad users is the Blocking feature. When you block him, he gets logged out from the website, and receives an error when he tries to re-validate the account by mail: Your account has already been validated.

So actually it's not a bug. Activation-Deactivation is related to the mail system.

EDIT 2: Actually there is a bug here - the listing publishing system, doesn't take into account if the user is Deactivated (maybe Blocked also). So, if a user activated his account by mail and then you proceed to Deactivate his account from the admin, he will still be able to post listings even if his account is deactivated.

What is the fix? In my opinion, to prevent possible exploits, we need to add an additional check when posting listings, to see if the user is Deactivated/Blocked or not, and if true, his posted listings will get Deactivated/Blocked status also.

navjottomer commented 2 years ago

Thanks both of you. I'll work on this asap may we could create a hotfix for this for immediate release.

eurobank commented 2 years ago

@navjottomer I don't think an urgent hotfix is needed. Not that important, not exploited from what i see. I get around 100-150 registrations per day, i only saw this a couple of times (while some other cases may have been present, without me knowing it).

Tangol commented 2 years ago

After giving some more thought to this, I think that the correct and easier solution to this, would be to apply the same de-logging mechanism to the user Deactivation, as we have at user Blocking.

So basically, when the admin Deactivates a user account, the user gets logged out and any action he may try to take, would be accompanied by the standard unactivated account mesaage: The user has not been validated yet. Would you like to re-send your activation?

Actually this is the main bug in this report: when manually Deactivated, the user doesn't get logged out from the website, like he does when Blocked.

eurobank commented 2 years ago

I have given some more though in this.

A Deactivated (Inactive) user, actually mean two things: He hasn't yet activated his account OR some admin deactivated his account.

It is normal that somehow (not sure how) he CAN activate his account (again). The logic is correct.

So the Admin should BLOCK the account and not deactivate the account. Or maybe if an admin deactivates an account, a block should be also be enabled.

Tangol commented 2 years ago

That's true, but keep in mind that the Deactivation feature is related only to the email activation setting in the admin user settings area. So basically, Deactivation/Activation serves no other purpose than the one stated above.

As I mentioned in my initial comment (point 2 - Strikethrough), a user can and should be able to always activate/re-activate his account by using the activation link in the email. This is how the system was built, and it's fine.

If the admin really wants to stop a user, then he should use the Block feature. Deactivating ≠ Blocking nor should it be.

Now, the ony real problem/bug is that when for any reason, the admin Deactivates a user, the user can stil remain logged in and post ACTIVE listings. Actually this is a minor one, from what I tested (might also be something more serious), but I hope that @navjottomer will provide some more insight on this.

~~EDIT: or if we want to give an actual use to the admin user Deactivation, maybe we can transform it into a user email address re-confirmation system, like when we manually deactivate the user, he automatically gets logged out and receives another email: Hello user, Due to security reasons, your account was deactivated. Please confirm your account using the following link...~~ Sorry, this is actally implemented as a notification, I forgot about it.

~~This would allow us to make sure that the suspicious user account, still has a valid email address and not a temporary one. Just an idea.~~

eurobank commented 2 years ago

@Tangol A USER can't deactivate his account. Where is that option? I only know a Delete Account.

Tangol commented 2 years ago

@Tangol A USER can't deactivate his account. Where is that option? I only know a Delete Account.

Where did you get this from? I never said this. The user can only Activate his account by mail, or the admin can Activate/Deactivate it.

eurobank commented 2 years ago

@Tangol A USER can't deactivate his account. Where is that option? I only know a Delete Account.

Where did you get this from? I never said this. The user can only Activate his account by mail, or the admin can Activate/Deactivate it.

My mistake. You write RE-activate, i got it as DE-activate. Apologies.

navjottomer commented 2 years ago

Only key thing which was missing is adding a user activation check with user enabled check while requesting WebSecBaseModal class. @Tangol has accurately summarized the activate/deactivate system and how does it work. Please check the recent fix.

Tangol commented 2 years ago

From what I've tested, it seems to be working fine now. @eurobank ?

eurobank commented 2 years ago

From what I've tested, it seems to be working fine now. @eurobank ?

Well it will take some time for me to check this.

eurobank commented 2 years ago

I did a quick test myself with two computers and seems work fine (or i'm not that talented as the users/posters).