Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sane handling of identity_changed event raised inside identity_loaded event handler #93

Open
sean102 opened this issue Aug 10, 2024 · 0 comments

Comments

@sean102
Copy link

sean102 commented Aug 10, 2024

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant