pallets-eco/flask-principal

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...

  1. Flask.preprocess_request() calls the 'refore_request' functions
  2. Principal._on_before_request() loads the user identity from the session and calls set_identity()
  3. Principal.set_identity() raises an identity_loaded event
  4. MyApp.on_identity_loaded() reads the user account status from some database and decides to log-out the user.
  5. MyApp.on_identity_loaded() raises an identity_changed event with AnonymousIdentity()
  6. Principal._on_identity_changed() calls set_identity()
  7. Principal.set_identity() raises an identity_loaded event (Note, we are still in the MyApp.on_identity_loaded() handler from step 4)
  8. MyApp.on_identity_loaded() configures the App for anonymous access.
  9. 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