singingwolfboy / flask-dance

Doing the OAuth dance with style using Flask, requests, and oauthlib.
https://pypi.python.org/pypi/Flask-Dance/
MIT License
1k stars 157 forks source link

Support for PKCE? #293

Open xscheck opened 4 years ago

xscheck commented 4 years ago

I have a working version of a Flask app using FD that authenticates my REST APIs successfully when the APIs are accessed from a browser. I am using Google OAuth2. Much appreciate what FD brings to the table.

I am trying to do the same now with a mobile app: it uses a subset of the REST APIs but user needs to be authenticated first. I believe I have to have PKCE flow, not Implicit flow. (I want the API server to get the auth token, not the user's device - the app on the device should not have the Secrets)

I've googled, checked the FD docs and the open-closed issues here but could not find anything specific on the topic. Closest is issue #265. Is that same as PKCE support (or isn't that what was meant there? Yes, I am second guessing)? I don't see code-verifier/code-challenge and code-challenge-method in the Providers' doc.

OR, there is some other way to manage the authentication from FD client to a mobile app?

Sorry if newbie question. I am also trying to implement (for example using AppAuth on Android) but am not seeing all the pieces (yet) to make this work.

Thanks in advance for any insights.

daenney commented 4 years ago

265 is not PKCE. Authorization Code Grant is the most common flow for oauth2, so yes we support that. PKCE is an extension: https://oauth.net/2/pkce/

PKCE (RFC 7636) is an extension to the Authorization Code flow to prevent certain attacks and to be able to securely perform the OAuth exchange from public clients. It is primarily used by mobile and JavaScript apps, but the technique can be applied to any client as well.

Flask-Dance doesn't do much with oauth itself, we use requests-oauthlib and thus oauthlib instead which does support PKCE. The problem is we currently don't expose that ability in FD, and we'd have to override https://github.com/oauthlib/oauthlib/blob/794c9fb1305141903f4cd182d7d0aab959e86219/oauthlib/oauth2/rfc6749/request_validator.py#L579 by construction our own RequestValidator. It doesn't look like requests-oauthlib currenctly allows for that, so we'd have to start with a change in that library first.

@singingwolfboy Do you have a sense of what we would have to do here? I think supporting PKCE makes a lot of sense and is something I'd like to see us have.

singingwolfboy commented 4 years ago

It would be nice to support PKCE. It would be nice to support OIDC. It would be nice to support a lot of things: OAuth is a very, very broad standard that has a lot of different optional pieces.

Unfortunately, I think the fundamental problem is requests-oauthlib. The library does not have a dedicated maintainer, only a few people who are trying to keep the whole thing from falling over. (I am one of those people.) The code is messy and confusing, and it doesn't expose all the functionality that oauthlib offers, which makes it a pain to use.

Maybe the best path forward is to rewrite Flask-Dance so that it doesn't use requests-oauthlib at all, but instead depends on requests and oauthlib directly. That would be a lot of work, though, and since I'm not actively using Flask-Dance in my own work these days, I'm not very motivated to do so. If someone else is interested in taking this on, I'm happy to provide assistance and explain the architecture of the project as much as I can.

I'm sorry I don't have better news for you, @xscheck.

xscheck commented 4 years ago

Appreciate the quick responses. So fair to say that FD has not been used with OAuth2 REST APIs & mobile apps? Clearly not with the prescribed path.

How much of a heavy lift is updating requests-oauthlib with PKCE? Given this library is used in a number of projects, would it not be beneficial to adding it here? But I also note your concern about current state and ownership issues.

The other route, of dropping it from FD and rewriting sounds daunting to me (but that's just me).

daenney commented 4 years ago

Appreciate the quick responses. So fair to say that FD has not been used with OAuth2 REST APIs & mobile apps? Clearly not with the prescribed path.

No. That's not at all fair to say. And it's quite the contrary. PKCE is an extension that was added later to improve the security in case of a mobile client on JS app, that can't keep the client secret secret. It doesn't mean you can't do oauth with mobile or JS apps without it. But you don't have to have a client secret in the first place, you can do the flows purely with a client id. It just depends on what your oauth provider supports.

xscheck commented 4 years ago

I would like to revive this issue. As provided on this thread, Flask-Dance correctly explains the OAuth2 workflow here: https://flask-dance.readthedocs.io/en/latest/how-oauth-works.html#oauth-2.

Here's what I am experiencing, where provider is Google, consumer is my python server with Flask-Dance, and client is my Android app -- apologies in advance given I am still discovering the nuances of this flow (and TL/DR)

  1. From Android app, I call an REST API, say /dashboard (a GET method), which is wrapped in @login_required: this does cause the @oauth_before_login.connect signal to be triggered, and eventually (Flask-Login?) returns HTML body with, amongst other things, consumer's client_id (and state) to the Android app.

  2. Android app sends this client_id via GoogleSignInOptions, requesting AuthCode, and does the necessary local calls leading to successfully getting the authcode from Google. (BTW, I haven't been able to figure out how to send "state" (no such field/option in GoogleSignInOptions)).

  3. This is where I am stuck: how to call /dashboard from Android app again with the authcode.

To "continue the flow", I coded the Android app to send the authcode via POST to a different REST API on the python server, which uses oauth2client.client.credentials_from_code, and successfully retrieves profile and email id of user, which is provisioned on the server and therefore can "confirm login". (However, none of the @login_required APIs work - as expected - because haven't truly gone thru the dance.)

Is the authcode sent to /dashboard endpoint in a HTTP header?
And what about "state"?

(BTW, separately from a browser, I can call /dashboard, which pops the login request and then proceeds to return result of the /dashboard API back to the browser: so this flow is successful and does call blueprint login endpoint and the @oauth_authorized.connect_via(blueprint_google) signal, matching the authenticated user to the one provisioned in the server.)

Any pointers?

I have other questions: like the redirect, client id, state, etc in (1) is buried in the HTML response, which I assume is coming from Google - anyway to get JSON instead.

Appreciate your comments in advance.

xscheck commented 4 years ago

I have made some progress ... sorry to bug you folks. At the end of all this, I'll be sure to post my findings and conclusions.

First of all, I implemented all redirects myself (now using Android - Volley, and turning off auto-redirect).

So relatively same step (3) in my previous email: I am now redirecting to Google to get auth code, am getting a login popup (browser) on Android, after which Google is directly calling consumer at /login/google/authorized?...

However, the dance is failing with oauthlib.oauth2.rfc6749.errors.MismatchingStateError

The odd thing is that the redirect URL (/login/google/authorized?...) to consumer from Google does have the correct "state" received by Android and redirected to Google to log in, but the line 246 in oauth2.py: state = flask.session[state_key] always returns 'WDWsoDGHHrk4sAZn1y1I5c2Xm3fTRp' for state, leading to the MismatchStateError.

What gives? Or am I way off here?

In the meantime, continuing to investigate.

xscheck commented 4 years ago

Another update, with debug output Before redirect : DEBUG:flask_dance.consumer.oauth2:client_id = ... DEBUG:requests_oauthlib.oauth2_session:Generated new state mmMg0Y23gPV9J7OL6dkZdfYLCkQ0wp. DEBUG:flask_dance.consumer.oauth2:state = mmMg0Y23gPV9J7OL6dkZdfYLCkQ0wp DEBUG:flask_dance.consumer.oauth2:redirect URL = https://accounts.google.com/o/oauth2/auth?response_type=code&client_id=...&redirect_uri=http%3A%2F%2Frdfakedomain.com%3A5000%2Flogin%2Fgoogle%2Fauthorized&scope=profile+email&state=mmMg0Y23gPV9J7OL6dkZdfYLCkQ0wp&access_type=offline&prompt=consent

And after the redirect to /login/google/authorized, just before the MismatchingStateError: DEBUG:flask_dance.consumer.oauth2:next_url = /login/success DEBUG:flask_dance.consumer.oauth2:state = WDWsoDGHHrk4sAZn1y1I5c2Xm3fTRp DEBUG:flask_dance.consumer.oauth2:client_id = ... DEBUG:flask_dance.consumer.oauth2:client_secret = ...

JonathanHuot commented 4 years ago

Hi @xscheck, I'd like to remove some confusion here.

PKCE is useful if you want to do the OAuth2.0 Authorization Code Dance from Desktop/Mobile Apps and use the access tokens directly from the app. No backend are involved in PKCE. You don't need flask-dance to support PKCE because you will never install flask-dance in a mobile. You can still need PKCE if you install a Desktop Application written in Python, but that's not a common use-case.

It all depends on what are the OAuth actors here:

The Resource Server does not care about flows, they only care about Access Token and their validation. If the Client is a mobile App, then use a Javascript PKCE library. If the Client is a python backend, then use flask-dance.

PKCE client support is almost never needed for python because python is rarely used on Desktop side (we may found some Desktop Apps written in Python, but that's not a common choice).

xscheck commented 4 years ago

Thanx @JonathanHuot. I believe I have moved past the PKCE issue. I am using plain old OAuth2 authorization flow as documented in RFC6749 and also Flask Dance.

Consumer: My python (server for REST APIs) uses Flask Dance. Client: My Android mobile coded using Java, leveraging Volley - so directly processing HTTP requests and responses. Provider: Google auth.

Status is as provided in my previous update on this thread: I get a state mismatch error. I have checked multiple times, that the state mismatch always has WDWsoDGHHrk4sAZn1y1I5c2Xm3fTRp on one side of the match, which is quite odd. There could be a "session" issue: flask.session vs session in requests_oauthlib/flask_dance, generate_token, new_state(), ...: investigating.

JonathanHuot commented 4 years ago

Maybe you should open another ticket to have more attention to the thread. So in your use case, the flask.session is the one between Client and Consumer, and requests_oauthlib is between Consumer and Provider. The Token is only for Consumer provided by Provider.

mkmojo commented 4 years ago

Hi @xscheck, I'd like to remove some confusion here.

PKCE is useful if you want to do the OAuth2.0 Authorization Code Dance from Desktop/Mobile Apps and use the access tokens directly from the app. No backend are involved in PKCE. You don't need flask-dance to support PKCE because you will never install flask-dance in a mobile. You can still need PKCE if you install a Desktop Application written in Python, but that's not a common use-case.

It all depends on what are the OAuth actors here:

  • Resource Server ? (i.e. APIs protected by Access Tokens)
  • Clients ? (i.e. the Client is some apps wanting to use a Resource Server protected by Access Tokens).

The Resource Server does not care about flows, they only care about Access Token and their validation. If the Client is a mobile App, then use a Javascript PKCE library. If the Client is a python backend, then use flask-dance.

PKCE client support is almost never needed for python because python is rarely used on Desktop side (we may found some Desktop Apps written in Python, but that's not a common choice).

Thanks @JonathanHuot, this is really insightful. Some follow up question. Say my Android app is done with PKCE. How will my Flask server know that some user has logged it?

For example, if there are endpoints to user's profile picture which my Flask server can retrieve from database only if the user has logged in on the Android app. How is PKCE letting the Flask server know a user has signed in via Google?