flask-dance: Wrong session usage or possible security issue

Working according to the basic documentation, I’m hitting a serious problem where one user login session in one browser is propogated to another browser with no login credentials.

Here’s my relevant server code:

from os import environ

from flask import Flask, redirect, url_for, render_template
from flask_sqlalchemy import SQLAlchemy
from flask_migrate import Migrate
from flask_dance.contrib.twitter import make_twitter_blueprint, twitter
from flask_dance.consumer.backend.sqla import SQLAlchemyBackend, OAuthConsumerMixin
from werkzeug.contrib.fixers import ProxyFix


app = Flask(__name__)
app.secret_key = environ.get('FLASK_SECRET_KEY')
app.wsgi_app = ProxyFix(app.wsgi_app)
app.config.from_object('config.Config')

db = SQLAlchemy(app)
migrate = Migrate(app, db)

class OAuth(OAuthConsumerMixin, db.Model):
    pass

twitter_blueprint = make_twitter_blueprint(
    api_key=app.config['TWITTER_CONSUMER_KEY'],
    api_secret=app.config['TWITTER_CONSUMER_SECRET'],
)
twitter_blueprint.backend = SQLAlchemyBackend(OAuth, db.session)
app.register_blueprint(twitter_blueprint, url_prefix='/login')


@app.route('/')
def index():
    username = None
    if twitter.authorized:
        resp = twitter.get('account/settings.json')
        username = resp.json()['screen_name']
    return render_template('index.html', username=username)

Steps:

Open two separate browser sessions
In both sessions, navigate to server:5000, homepage shows a login link {{ url_for('twitter.login') }}
On browser A, perform Twitter authentication dance
Redirect back to homepage, username is rendered correctly, inspecting the SQL database, the oauth tokens are indeed saved correctly for the user
On browser B refresh homepage, username is now populated with the login session from browser A

Other notes:

Happens on all environments, both with and without flask debug mode, as well as running through gunicorn
Backend database is a postgreSQL instance
Flask 1.0.2, Flask-dance 1.0.0, Python 3.7
Adding user_required=False as a param to SQLAlchemyBackend doesn’t change this behavior

This is no doubt a serious potential security bug. Either the library is behaving in an unexpected way, or I’m doing something wrong, and hitting a pitfall, in which case the documentation probably should be updated to warn about this behavior.

About this issue

Original URL
State: closed
Created 6 years ago
Comments: 20 (13 by maintainers)

Most upvoted comments

I think my problem relates to me running with gunicorn and threaded workers which seems like a no go when using flask-dance since some state gets shared on the blueprint and not in the app or requests contexts. If I were to run without threads, it’d probably be fine.

A good example highlighting the problem, but is certainly not the only thing, is the following:

self.session._state = state

Since the blueprint gets shared between all threads/requests, the above code will result in problems when using the proxy that gets created by each provider. Actually, using threads is a security risk as suggested by the original poster and as I mentioned in my original post.

I think the ideal solution for this problem, other than just not using threads, would be to continue creating a proxy that is in the app context similar to the following:

    @google_bp.before_app_request
    def set_applocal_session():
        ctx = stack.top
        ctx.google_oauth = google_bp.session

    return google_bp

google = LocalProxy(partial(_lookup_app_object, "google_oauth"))

Perhaps a more generic name similar to Flask-Login’s current_user (e.g. current_oauth_session) and make sure every reference, even in the blueprint, refers to that instantiation. Basically, using current_oauth_session just like you use flask.session or flask.request, etc.

Also, I really hate all the @lazy decorators going on but to each their own.

Honestly, the fact that there are proxies made available by each provider gave me the impression this was thread/request safe since that is typically the case with the other flask proxies like flask.g, flask.session, etc.

awiddersheim on Sep 13, 2018

@singingwolfboy Hrm, reading the same docs you are from Flask and I get the opposite impression. Especially the parts in Storing Data.

The g name stands for “global”, but that is referring to the data being global within a context. The data on g is lost after the context ends, and it is not an appropriate place to store data between requests. Use the session or a database to store data across requests.

awiddersheim on Sep 10, 2018

I found this while looking at what I think is a similarly related issue. Hopefully I don’t conflate the two too badly. Anyway, I am using Google as my OAuth provider with the default session backend and have this in my before_request() hook.

                  if not google.authorized:
                      return redirect(url_for('google.login'))

                  response = google.get('/oauth2/v2/userinfo')
                  response.raise_for_status()

                  g.user_data['email'] = response.json()['email']

What seems to be happening is that g.user_data['email'] can interleave and I think it is related to Flask-Dance. So, for example, if I open two browsers and log in as separate users, one of those requests will get a users email twice. My understanding is that google is a context local specific to that request and therefore that user’s session so I’m not sure why that might be happening. My guess is I don’t fully understand the flow and multi-user setups when using Flask-Dance.

awiddersheim on Sep 5, 2018

Interesting, yeah I guess that’s reasonable. I’ll see if I can propose a documentation fix that makes this issue clear. Thanks for the quick response!

yuvadm on Aug 20, 2018