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

gapi.auth.authorize callback doesn't get called when user closes window #25

Open
wonderfly opened this issue Jan 7, 2015 · 27 comments
Open

Comments

@wonderfly
Copy link
Contributor

From phenome on March 27, 2012 11:47:59

What steps will reproduce the problem? 1. Fire gapi.auth.authorize(). Wait for popup to open
2. Close the window What is the expected output? What do you see instead? Callback should be called with null parameter, or a token object with error, as per API.

When you DENY the authorization, the callback gets called with null.
::: Reading the API I assumed it would get called with a token object with the error parameter set. Is it really misleading or my interpretation is lacking?

I actually have no problem with it being called with null, but since there are two cases (the user actively DENYING authorization, or simply closing the window), it would be nice to get a token object with an error describing what happened.

Original issue: http://code.google.com/p/google-api-javascript-client/issues/detail?id=23

@wonderfly
Copy link
Contributor Author

From obr...@google.com on May 07, 2012 11:47:56

Status: Accepted
Owner: bsittler@google.com

@wonderfly
Copy link
Contributor Author

From 0xED....@gmail.com on July 20, 2012 09:54:21

This is just to be polite and say goodbye to a customer who just slammed the door - I mean "closed the window".

@wonderfly
Copy link
Contributor Author

From obr...@google.com on August 02, 2012 09:52:38

Labels: Milestone-GA

@wonderfly
Copy link
Contributor Author

From tableYou...@gmail.com on September 30, 2013 02:36:25

There are 2 issues here:

  • The event of closing the windows is not fetched at all. The callback should get called with authResult.error=window_closed
  • the documentation says "(If authorization fails, an error message is returned.)" - Really? The authResult is null in two important cases: if unauthorized and if the users denies the autorization. Instead the callback should get called with error=unauthorized and error=denied or similar

Someone at google willing to help and get this fixed after over a year? Or where is the source code of this stuff?

BTW: the docs are 'a bit' outdated and referring to this blog here (?) http://google-api-javascript-client.blogspot.de/

@wonderfly
Copy link
Contributor Author

From vishal.4...@gmail.com on November 23, 2013 15:24:00

I think issue #74 and #23 are related.

@wonderfly
Copy link
Contributor Author

From KingRobe...@gmail.com on May 29, 2014 20:36:14

2 years later and no word.
Anyway I don't think this feature is needed. If a user closes the login popup they can just click the login button again (just don't hide the login button until you have a user token). Also if they click the login button multiple times it just focuses the single popup.

@wonderfly
Copy link
Contributor Author

From rafalfel...@gmail.com on May 30, 2014 00:06:06

I would rather say that this future is nice to have. Why do some workarounds, not always nice and often not giving what we really want to achieve.

@wonderfly wonderfly removed their assignment Jan 7, 2015
@i386
Copy link

i386 commented Jan 19, 2015

This doesn't make for a great experience. Would be awesome to see this one fixed.

@gguuss
Copy link

gguuss commented Jan 20, 2015

Currently you can detect when the user clicks cancel by checking the error message in the authorization response. Demo here: http://wheresgus.com/secondchance/

You cannot detect when the user closes the window.

@broady
Copy link

broady commented Jan 29, 2015

Checking whether the window has closed turns out to be pretty trivial.

Here's a demo: http://jsbin.com/wuxinoruze/1/

@broady
Copy link

broady commented Jan 29, 2015

And here's a workaround. It'll fire whenever any opened window is closed. It works in the authSample.html page.

You could check the url contains google.com if your page opens windows for some other reason. (or hold a reference to the real window.open)

(function(wrapped) {

window.open = function() {
  var win = wrapped.apply(this, arguments);
  var i = setInterval(function() {
    if (win.closed) {
      console.log('closed!');
      clearInterval(i);
    }
  }, 100);
};

})(window.open);

@i386
Copy link

i386 commented Feb 12, 2015

Hi @broady, I appreciate the help here! Its great that we can detect the window close though Im not sure if this can detect the difference between a successful closing of the dialog when auth succeeds or when the user closes the page.

@lukx
Copy link

lukx commented Mar 2, 2015

I have enhanced the workaround by @broady to match @i386 's issue.
Instead of reacting to the "window.closed" state directly, I simply cancel the
deferred object. Cancelling an already-resolved promise will not have any effect.
However, it does reject the deferred and therefore invokes the onReject handler.

var authorizeDeferred;

function onClickLogin() {
    (function(wrapped) {
        window.open = function() {
            // re-assign the original window.open after one usage
            window.open = wrapped;

            var win = wrapped.apply(this, arguments);
            var i = setInterval(function() {
                if (win.closed) {
                    clearInterval(i);
                    // cancel has no effect when the promise is already resolved, e.g. by the success handler
                    // see http://docs.closure-library.googlecode.com/git/class_goog_Promise.html#goog.Promise.prototype.cancel
                    authorizeDeferred.cancel();
                }
            }, 100);
            return win;
        };
    })(window.open);

    authorizeDeferred = gapi.auth.authorize({
    // ...
    }).then(onAuthResult, onRejected);
}

@Hengjie
Copy link

Hengjie commented Aug 31, 2015

@lukx the problem with that approach is that it assumes a successful authorization from Google gets back to us before the interval timer is run. So basically the code has two issues: First, we check at a resolution of 100ms but there is no guarantee that we check 100ms after the window closes. Second, it assumes Google gets back to us quickly.

The solution for first is to debounce the cancel() call by some reasonable amount of time (say 1 second?) so that Google has a chance to resolve the promise before we reject it.

The solution to the second would require some internal knowledge of when user successfully clicks before the network reqs to Google happens for the authorization.

@alalonde
Copy link

alalonde commented Mar 4, 2016

This issue still exists in the auth2 client (https://developers.google.com/identity/sign-in/web/reference), which uses promises. I would expect that clicking 'deny' or closing the window would resolve the promise with an error. However, neither the success or error methods of the promise are called.

FWIW, @lukx's workaround seems to work pretty well.

@neilsoult
Copy link

Yea, why fix bugs when we can just bloat our code with workarounds!

@DarkSilence
Copy link

The bug is still present :(

@bsittler
Copy link
Contributor

bsittler commented Jan 4, 2017

Please try the newer gapi.auth2 API and let us know whether the problem persists

@DarkSilence
Copy link

@bsittler yep, the problem persists :(
My code is pretty simple:

gapi.load('auth2', () => { gapi.auth2.init({ scope: this.scope, client_id: this.clientId, cookie_policy: this.cookiePolicy, fetch_basic_profile: this._fetchBasicProfile, hosted_domain: this.hostedDomain, openid_realm: this.openidRealm }); });

On click event handler:

gapi.auth2.getAuthInstance().grantOfflineAccess( {'redirect_uri': 'postmessage'} ).then( ( authResult?: GoogleSignInSSAAuthResult ) => this.onSignIn( authResult ), () => this.onSignInError() );

No matter what I do (click Deny button, close window) error handler never fires so as onSignIn. If I click Approve button onSignIn fires as expected.

@bsittler
Copy link
Contributor

bsittler commented Jan 5, 2017

Thanks for the report, I've passed it on to those working on gapi.auth2.

@renarsvilnis
Copy link

Just noticed exactly the same behavior as @DarkSilence

@TMSCH
Copy link
Contributor

TMSCH commented Feb 3, 2017

Hi @DarkSilence, we are working on a fix, I will update this thread when it is released.

@TMSCH TMSCH self-assigned this Feb 14, 2017
@TMSCH TMSCH added the auth label Feb 14, 2017
@TMSCH
Copy link
Contributor

TMSCH commented Feb 17, 2017

Hi @DarkSilence @renarsvilnis @neilsoult @alalonde @Hengjie @lukx @i386 @broady @gguuss, the fix has now been released for gapi.auth2! signIn and grantOfflineAccess promises will be rejected if:

  • the user denies to consent the scopes
  • the user closes the popup

Thanks for your patience! Could one of you make sure you observe that it works, so I can close this issue?

@bernatfortet
Copy link

bernatfortet commented Jul 22, 2017

Hello @TMSCH I don't see this working for me.

  GoogleAuth.grantOfflineAccess()
    .then( response => {
      dispatch( signUp() )
    })
    .catch( error => console.log(`ERROR —— GAPI AUTH —— ${error}`) )

I get an error saying that there's no catch function. is that normal?

@TMSCH
Copy link
Contributor

TMSCH commented Jul 24, 2017

@bernatfortet it is not normal, the catch function should exist. What does dispatch(signUp()) do? Can you post the full stack trace of the error?

@bernatfortet
Copy link

Dispatch is part of Redux, it returns a promise, could that be problematic?

I've solved the issue by doing the following:

  GoogleAuth.grantOfflineAccess()
    .then( response => {
       myFunction()
    }, error => {
      console.log(`ERROR —— GAPI AUTH —— ${error}`)
    })

@TMSCH
Copy link
Contributor

TMSCH commented Jul 25, 2017

@bernatfortet it could be... The best way to know would be to not use it in the resolution callback of the grantOfflineAccess returned Promise (i.e. completely remove the dispatch( signUp() ) line and try again)

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

No branches or pull requests