Auth issue: User isn't denied if deleted after session is created

[GiantCrocodile]

If you are authorized and while you are logged in your account gets deleted from the database, there is no logout/error happening. You can still browse /account/profile etc.

I deleted the entry directly in the database and not with weppy's DAL. I don't know if this issue can occur if you do it the normal productive way.

[gi0baro]

@GiantCrocodile this is a bit complicated. The fact is that the authorization session is intended to avoid the selection of the user from the database. This is done for two reasons:

• Performance: selecting the user from the db in every request is a overhead of SELECT operations
• Logic: selecting the user from the db in every request makes the session storage kind of useless, since you can store everything on the db and just use it

Considered these, the issue comes from the fact that if you remove the user from the database, weppy doesn't have any way to notify the session is expired, because, in case of using cookies, it doesn't have access to the session data since they are on the user cookie, and in case of server storage, weppy doesn't know which sessions belongs to the destroyed user, since they are identified by uuid numbers generated on fly. So, the only way to check if the user is still existing, is to check in the db every time the user makes a request, and we get back to the performance consideration. The idea behind the session is that edits will be reflected with its expiration.

I may consider to add a check at the db non-per-request but only at certain time intervals, but these need further considerations and design.

I would appreciate some share of thoughts about this.

[GiantCrocodile]

So @gi0baro just to think further: If we want to ban an user. What would be the right way? Of course we have to have a way to get this done because sessions can last very long if you have someone abusing the system like spamming or posting TOS violating stuff.

[gi0baro]

Actually you can ban users setting the registration_key attribute to "blocked", so for example, once you have your user selected from db:

user.update_record(registration_key="blocked")

I think I should make this more accessible and better documented. Maybe a method in Auth module like:

auth.ban_user(user_to_be_banned)

what do you think?

[GiantCrocodile]

Setting the value to "blocked" seem to cause an unwanted side effect then? I looked how it is used and it is used in this 2 files:

https://github.com/gi0baro/weppy/blob/master/weppy/tools/auth/handlers.py#L82 and https://github.com/gi0baro/weppy/blob/master/weppy/tools/auth/exposer.py#L323

In first case it will throw self.auth.messages.login_disabled like expected but in second file it will throw

               flash(self.messages.registration_pending)
                redirect(self.auth.url('request_reset_password'))

I think here "blocked" should be removed because it shouldn't trigger a registration pending message and no password reset request? Banned user shouldn't be able to do this.

[gi0baro]

@GiantCrocodile yes, that's a bug. Can you please open up another issue dedicated to it?

[GiantCrocodile]

@gi0baro back to original question: If the user is blocked, the user is still able to interact with the website because only on next log in blocked will be recognized by system right?

[gi0baro]

@GiantCrocodile exactly, actually won't notice the fact until session will expire.

[GiantCrocodile]

@gi0baro regarding the auth.ban_user(user_to_be_banned) suggestion: I think it would be cooler to have a function like auth.change_status(affected_user, new_status) so you can use one function to set the user pending, disabled or blocked.

I will think about a good way for the rest of this trouble.

[gi0baro]

Back to the issue, I think I will add a periodical check to the database status of the users when loading sessions. The idea is to perform the check on predefined time intervals (something between 300 and 600 seconds) when the sessions are loaded. This will need:

• to store in the session the last check timestamp
• the check for the timedelta between the last check and the current operation
• the re-load of the data from the database to the session if the timedelta was greater than the chosen interval

This will obviously slow down the session load, but should not impact the performance too much.