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

GCE: Update generic auth error message #571

Closed
wants to merge 2 commits into
base: trunk
from

Conversation

Projects
None yet
2 participants
@mtekel
Contributor

mtekel commented Sep 2, 2015

Google authentication will fail if your time drift (difference between your local time and time on google servers) is bigger than 5 minutes. This often happens on the VM instances running in various cloud providers. Google itself doesn't provide any helpful indication of the issue, the response is just {u'error': u'invalid_grant'}.

There is also problem in the _get_error(self, body), line 205: the error response returned by google above will mean that err is a string, not a dictionary. err.get will then throw attribute error, which displays generic error message. I think it is better to display the generic error message than not so helpful 'invalid_grant' response from google. You might decide to fix or change that later, but for now I think it's important to hint about time drift to the user, as this happens quite often and it's hard to find, if you only check whether your credentials are correct (which they can be, but you'll still fail auth with big time drift).

google: Update generic auth error message
Google authentication will fail if your time drift (difference between your local time and time on google servers) is bigger than 5 minutes. This often happens on the VM instances running in various cloud providers. Google itself doesn't provide any helpful indication of the issue, the response is just {u'error': u'invalid_grant'}.

@mtekel mtekel changed the title from google: Update generic auth error message to GCE: Update generic auth error message Sep 2, 2015

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Sep 2, 2015

Member

It looks like it would also be good to document this in the docs.

Besides that, LGTM. Thanks.

/cc @erjohnso

Member

Kami commented Sep 2, 2015

It looks like it would also be good to document this in the docs.

Besides that, LGTM. Thanks.

/cc @erjohnso

@mtekel

This comment has been minimized.

Show comment
Hide comment
@mtekel

mtekel Sep 4, 2015

Contributor

Hi,

I am not sure what is the best place to put this information in the docs. Should that be a separate PR?

Contributor

mtekel commented Sep 4, 2015

Hi,

I am not sure what is the best place to put this information in the docs. Should that be a separate PR?

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Sep 4, 2015

Member

Ideally, it would be the same PR.

You could put it in the section where it's document how you retrieve the credentials.

Member

Kami commented Sep 4, 2015

Ideally, it would be the same PR.

You could put it in the section where it's document how you retrieve the credentials.

google: Update auth documentation with drift info
Mention that large time drift will cause authentication failures in the documentation for GCE.
@mtekel

This comment has been minimized.

Show comment
Hide comment
@mtekel

mtekel Sep 10, 2015

Contributor

Added doc update. Hopefully it's in the right place.

Contributor

mtekel commented Sep 10, 2015

Added doc update. Hopefully it's in the right place.

@mtekel

This comment has been minimized.

Show comment
Hide comment
@mtekel

mtekel Sep 23, 2015

Contributor

Hi,

have you had time to look at this? It's not a big change, but it can save some people a lot of time debugging issues...

Contributor

mtekel commented Sep 23, 2015

Hi,

have you had time to look at this? It's not a big change, but it can save some people a lot of time debugging issues...

@asfgit asfgit closed this in 3865e06 Sep 25, 2015

asfgit pushed a commit that referenced this pull request Sep 25, 2015

google: Update auth documentation with drift info
Mention that large time drift will cause authentication failures in the documentation for GCE.

Closes #571

Signed-off-by: Tomaz Muraus <tomaz@apache.org>
@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Sep 25, 2015

Member

Merged, thanks!

Member

Kami commented Sep 25, 2015

Merged, thanks!

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