AuthCodeURL() modifies flow's URL instead of creating a new object about oauth2 HOT 13 CLOSED

Yes this was intentional. We are still hacking on this package. The new
design has was just committed but we haven't documented it yet.

On Wed, 12 Nov 2014 at 07:51, korya [email protected] wrote:

The most recent commit (0cf6f9b
0cf6f9b)
introduced an undocumented change to AuthCodeURL() semantics. I expected
it just to generate a new url string based on the information stored in
flow, but it actually changes the state of the flow object otself.

In order to allow users to login with their Google accounts, I use the
following approach:

// Upon initialization of the web serverconfig, err := oauth2.NewConfig(...)
...// For each incoming connect requestauthUrl := config.AuthCodeURL(...)
...// Handle the oauth2callback, etc.

That is, I've created a global config just once, and then used it for
different connection requests for Google provider.

Recently, after updating my app and the dependencies, I've discovered that
the approach does not work (actually, it works, but not as I expected):

// Upon initialization of the web serverflow, err := oauth2.New(...)
...// For each incoming connect requestauthUrl := flow.AuthCodeURL(...)
...// Handle the oauth2callback, etc.

Again, I create once a global variable flow for google provider, and
for every incoming connection request, I execute flow.AuthCodeURL() to
generate a new url.

The code works for the first request. However starting from the second
request the code won't work.

The reason for this behavior is that every time AuthCodeURL() it modifies
the flow's AuthURL field. In previous implementation, the original flow's
authURL remained unchanged. For more deatails see 0cf6f9b#diff-
eb12cee32bcf605b68363a07e95e0174R149
0cf6f9b#diff-eb12cee32bcf605b68363a07e95e0174R149

My questions are:

• was this change done intentionally?
• if yes, why was it necessary? and shouldn't the fact that the state
  of flow object is changed be documented?

—
Reply to this email directly or view it on GitHub
#53.

from oauth2.

Ok. I'm curious what is the motivation for doing it, and what should be the right way to use the library in my case. Should I create a new flow object on every request as follows?

// For each incoming connect request
flow, err := oauth2.New(...)
authUrl := flow.AuthCodeURL(...)

// Later on for each incoming oauth2callback request
flow, err := oauth2.New(...)
flow.NewTransportFromCode(code)

from oauth2.

Yes. That's what you should do.
On Wed, 12 Nov 2014 at 08:36, korya [email protected] wrote:

Ok. I'm curious what is the motivation for doing it, and what should be
the right way to use the library in my case. Should I create a new flow
object on every request as follows?

// For each incoming connect request

flow, err := oauth2.New(...)authUrl := flow.AuthCodeURL(...)
// Later on for each incoming oauth2callback requestflow, err := oauth2.New(...)
flow.NewTransportFromCode(code)

—
Reply to this email directly or view it on GitHub
#53 (comment).

from oauth2.

OK. I played a little bit, and faced the same problem.

I create the flow object as follows:

func getFlow(config *Cfg) (*oauth2.Flow, error) {
    return oauth2.New(
        oauth2.Client(config.ClientID, config.ClientSecret),
        oauth2.RedirectURL(config.RedirectURL),
        oauth2.Scope(config.Scopes...),
        google.Endpoint(),
    )
}

The problem here is google.Endpoint() (see https://github.com/golang/oauth2/blob/master/google/google.go#L51). It always returns the exact same url object, which is then used referenced in flow's options, and later modified by AuthCodeURL().

from oauth2.

Ahhh.. that's a bug.

This line in AuthCodeURL:

u := f.opts.AuthURL

should be

u := *f.opts.AuthURL

from oauth2.

Fixing it.

from oauth2.

So AuthCodeURL actually should not modify the state of the flow object... So after this fix, I can continue using oauth2 lib in previous manner:

1. create a global flow object flow
2. call flow.AuthCodeURL(...) on every incoming request

// Upon initialization of the web server
flow, err := oauth2.New(...)
...
// For each incoming connect request
authUrl := flow.AuthCodeURL(...)
...
// Handle the oauth2callback, etc.

from oauth2.

https://codereview.appspot.com/173080043/

from oauth2.

I'm not sure about your commit procedures/workflows. When is the fix going to be committed in this repo?

from oauth2.

We'll update this thread once it has.

On Wed Nov 12 2014 at 10:14:40 AM korya [email protected] wrote:

I'm not sure about your commit procedures/workflows. When is the fix going
to be committed in this repo?

—
Reply to this email directly or view it on GitHub
#53 (comment).

from oauth2.

I mean how long is it going to take? Can I expect it to be merged in this repo in the next 12 hours? 24 hours?

from oauth2.

2d3ce25 fixed the issue.

from oauth2.

Thanks, guys. You're really fast. Although very secretive :)

from oauth2.