Simplify oauth_authorized with redirect

[cancan101]

trafficstars

repost from https://github.com/singingwolfboy/flask-dance/issues/203#issuecomment-486333141 (CC @daenney @singingwolfboy) Right now the expectation is that writers of oauth_authorized callbacks handle saving the token if they want a redirect. This leads to more complicated logic to handle redirects (see linked comment), reposted here: why push that responsibility on the user? Seems like there is a non trivial amount of logic in setting the token (ie handling errors) that I would guess most consumers would still want to use.

I think in the base case of just wanting to customize redirects (ie to send the user back to the where they originally came from) where the logic is:

    next_url = flask.session["next_url"]
    return flask.redirect(next_url)

having to also set the token seems unnatural.

I suggest improving the DX here to allow returning the a response and still re-using the existing set token logic.

[singingwolfboy]

It's not so hard to set the token in the backend using the blueprint.token property:

@oauth_authorized.connect
def logged_in(blueprint, token):
    blueprint.token = token
    next_url = flask.session["next_url"]
    return flask.redirect(next_url)

Maybe we need to make this clearer in the documentation?

[jmprovencher]

So if I understand correctly, the "next" parameter is no longer supported in the new version of Flask-Dance?

[daenney]

Not quite, the undocumented automatic fallback/usage of next as a URL parameter has been removed since 2.0: https://github.com/singingwolfboy/flask-dance/commit/7701b72d153265d88312ca038d0854f0ce08b813 and https://github.com/singingwolfboy/flask-dance/pull/230.

It's perfectly fine to use a next URL param yourself, but you'll have to implement the logic you want around it.

[singingwolfboy]

Would #269 make this clearer?

[daenney]

I'm not sure there's much further need to discuss this. It's very clear from the examples that it's not at all a "non trivial amount of logic in setting the token" that needs to be implemented. It's 3 very trivial lines of code, no complicated logic or error handling required. The new example should make it easy for anyone who wants to do this.

There's also plenty of reasons we don't want to magically do this, b/c people might not always want to follow the next_url regardless of whether it's set. This is something that the person building their web app should have full control over and explicitly express the desired behaviour.

[jmprovencher]

The part I'm missing (maybe it's trivial !) is how do we set the next_url in the flask session?

Currently If a user is not authenticated In a specific frontend of my API, I redirect him like that

return redirect(
            url_for(
                "google.login",
                next=url_for(
                    "my_custom_blueprint"
                )
            )
)

I guess now I should simply do something like:

flask.session["next_url"] = "url_to_my_custom_blueprint"
return redirect(url_for("google.login") )

Is that correct?

[daenney]

Yup, that ought to do the trick. One additional tip, when doing the redirect it's probably good to do:

next_url = flask.session.pop("next_url", "")
if next_url:
  return redirect...
return redirect(url_for("/")) # any named route, homepage etc

Alternatively, you can set the default argument to .pop("next_url", "app.index") to avoid the if next_url check and always redirect.

By using .pop you remove the value from the session. There's no need to keep it around anymore and avoids it still being accidentally set and causing surprises if/when a user re-authenticates.

[jmprovencher]

If I'm using the multi-user account (creating user account on the end of the OAuth dance) do we still need to return False from the @oauth_authorized function to disable the default behaviour for saving the token?

[daenney]

That's pretty different. Lets discuss that in a separate issue instead of lobbing all questions together here.

[jmprovencher]

ok. My concern relating to this issue was also how can we redirect using the new method and return False at the same time.

[singingwolfboy]

The part I'm missing (maybe it's trivial !) is how do we set the next_url in the flask session?

Doing it in your view code is one way. You could also hook into the oauth_before_login signal:

import flask
from flask_dance.consumer import oauth_before_login

@oauth_before_login.connect
def before_login(blueprint, url):
    flask.session["next_url"] = flask.request.args.get("next_url")

[jmprovencher]

Thanks for this info @singingwolfboy ! The documentation in the signal section is pretty clear on how to do this ! I just never went throught it when searching for my problem! Thanks

[singingwolfboy]

@jmprovencher If you didn't find it when you were searching for your problem, then the docs can still be improved. 😄 Where were you searching? Could you send a pull request to add a link from the place you were searching to the place where the information was?

[jmprovencher]

I was looking in the multi-user setup page. I think maybe we could do a specific example on how to do dynamic redirection (without using redirect_url in the blueprint).