Sane handling of identity_changed event raised inside identity_loaded event handler
sean102 opened this issue · 0 comments
Consider the case of a user account being disabled while that user has a session open.
Receiving a request from that session immediately after the account is disabled will be processed like...
- Flask.preprocess_request() calls the 'refore_request' functions
- Principal._on_before_request() loads the user identity from the session and calls set_identity()
- Principal.set_identity() raises an identity_loaded event
- MyApp.on_identity_loaded() reads the user account status from some database and decides to log-out the user.
- MyApp.on_identity_loaded() raises an identity_changed event with AnonymousIdentity()
- Principal._on_identity_changed() calls set_identity()
- Principal.set_identity() raises an identity_loaded event (Note, we are still in the MyApp.on_identity_loaded() handler from step 4)
- MyApp.on_identity_loaded() configures the App for anonymous access.
- Functions return, stack unwinds.
We see that changing identity inside the before_request cascade has resulted in a recursive call to Principal.set_identity()
Consider that the identity_loaded event may have multiple subscribers. Lets call them 'on_identity_loaded_A()' and 'on_identity_loaded_B()'. Now suppose on_identity_loaded_A() wants to reject the user.
The call sequence will look like:
set_identity(session_account)
on_identity_loaded_A(session_account)
set_identity(anonymous)
on_identity_loaded_A(anonymous)
on_identity_loaded_B(anonymous)
on_identity_loaded_B(session_account) # BIG Problem!
We see than on_identity_loaded_B() has been called with the session_account AFTER the identity has been changed to anonymous. Nothing good could come of this.
The fundamental problem is that blinker Signal.send() will send the session_account to all subscribers of the identity_loaded event, even AFTER the first subscriber has rejected that identity.
PROPOSAL
The solution I propose is to detect recursive calls to Principal.set_identity() and delay any changes to the identity until the current call returns.
In my code, I have sub-classed Principal and overridden the set_identity() method. I offer my code as a potential solution, but there are of course many ways of doing this. I'm not a Python wiz. I'm sure someone more experienced could improve on my work.
def set_identity(self, identity):
try:
pending = self.set_identity_pending # throw exception if NOT recursion
self.set_identity_pending = identity # take the most recent setting
return # delay the identity change until the current one plays out
except:
pass
self.set_identity_pending = identity
while True:
super().set_identity( identity )
if self.set_identity_pending == identity:
break
identity = self.set_identity_pending
del self.set_identity_pending