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

SECURITY_ISSUE : Http request context (response-headers) not getting popped up from the global flask context on request completion #463

Closed
Mtushar22 opened this issue Dec 6, 2023 · 4 comments
Assignees

Comments

@Mtushar22
Copy link

Steps to reproduce :

  1. In the demo app code for python(flask) add the line app.app_context().push() after creating flask app .
  2. Run the server locally with Gunicorn as WSGI server .
  3. Gunicorn --bind 0.0.0.0:5000 --workers=1 app:app (Running with only 1 worker to easily reproduce the issue and to ensure consistent behaviour)
  4. Note the above command runs Gunicorn with single worker and uses default worker model (sync) . Issue might not be replicated with other worker models .
  5. Recipe used (email-password) , hit sign-in api so that request context has tokens in the response headers .
  6. Post sign in api hit login-methods api , this will also have the same tokens in the response headers .

This is a security concern as this is resulting in the following scenario :
User A logs in his device with his credentials , server responds with a token .
User B on a different device who goes to the Url to login gets logged with userA's credentials as the login-methods api responds with UserA's token (since the context from the 1st request was not popped up from the global stack) in the response headers hence the client logs User B without asking him to login .

Some speculations after checking library code and g-unicorn's functionality .

  1. The method used to set response header's in the request is somehow not able to pop out context from the global stack on completion of the http request lifecycle. One solution could be to pop out request context in the (Middleware which has @app.after_request)
  2. There is some bad interaction happening with Gunicorn environment (which uses a pre-fork worker model), the application context is automatically handled for each worker process. Manually pushing the application context(app.app_context().push()) in this situation might be leading to issue so need to have a workaround for this in the middleware to prevent this .
    As removing (app.app_context().push()) is not the right way cuz it would be needed in use-cases where we need to access current-app context .
@rishabhpoddar
Copy link
Contributor

I tried to follow the steps you mentioned, but I am unable to replicate the issue.

Are the steps missing something? For example, did you modify the backend in any way? Also, i see that in the steps you mentioned, you are starting the backend on port 5000, however, the frontend is querying port 3001.

Perhaps you could upload the generated demo code that has this issue on github and I can try that? Also, what version of python and gunicorn are you using? The versions I used are:

  • python 3.9.8
  • gunicorn (version 21.2.0)

The command I ran to start the backend is:

gunicorn --bind 0.0.0.0:3001 --workers=1 app:app

Finally, for the above to work, i had to modify the package.json in the root of the demo app to be as follows:

{
    "name": "my-app",
    "version": "0.0.1",
    "description": "",
    "main": "index.js",
    "scripts": {
        "start:frontend": "cd frontend && npm run start",
        "start:frontend-live-demo-app": "cd frontend && npx serve -s build",
        "start:backend": "cd backend && pip install virtualenv && virtualenv venv && chmod +x venv/bin/activate && . venv/bin/activate && pip install -r requirements.txt && python app.py",
        "start:backend-live-demo-app": "cd backend && ./startLiveDemoApp.sh",
        "start": "npm-run-all --parallel start:frontend",
        "start-live-demo-app": "npx npm-run-all --parallel start:frontend-live-demo-app start:backend-live-demo-app"
    },
    "keywords": [],
    "author": "",
    "license": "ISC",
    "dependencies": {
        "npm-run-all": "^4.1.5"
    }
}

(I essentially removed calling start:backend from the start script command.

@Mtushar22
Copy link
Author

I tested the behaviour just by using backend code and running it on 5000 port .
Also you will have to add the line app.app_context().push() post flask app creation in the backend code to replicate this behaviour

@rishabhpoddar
Copy link
Contributor

Also you will have to add the line app.app_context().push() post flask app creation in the backend code to replicate this behaviour

Why are you doing this? Cause without this, it works just fine.

@rishabhpoddar
Copy link
Contributor

This issue has been fixed in the python SDK version >= 0.18.3.

The issue was that g.supertokens was not being unset in the @after_request handler, which caused token changes from that session to be propagated to other requests. This was only an issue when using app.app_context().push() in your app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants