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.

Sign up for GitHub

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

Jump to bottom

ensures JWT leeway is restored after we set it #1122

Merged
merged 1 commit into from
Jan 6, 2017
Merged

ensures JWT leeway is restored after we set it #1122

merged 1 commit into from
Jan 6, 2017

Conversation

bshaffer
Copy link
Contributor

@bshaffer bshaffer commented Jan 4, 2017

No description provided.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 4, 2017
@bshaffer
Copy link
Contributor Author

bshaffer commented Jan 4, 2017

addresses concerns in #828 (comment) regarding JWT::$leeway's global scope. cc @Martinfree

@bshaffer bshaffer merged commit 2a35d62 into master Jan 6, 2017
@bshaffer bshaffer deleted the do-not-pollute-global-scope-with-leeway-fix branch January 6, 2017 00:09
@calebdelnay
Copy link

calebdelnay commented Jan 6, 2017 •
edited
Loading

There is a bug in this PR. The code adds a 'jwt' config key but then uses $this->jwt instead of the correct $this->config['jwt'] when creating the token verifier. A PHP notice is raised:

Notice: Undefined property: Google_Client::$jwt in Google_Client->verifyIdToken() (line 697 of .../vendor/google/apiclient/src/Google/Client.php).

@bshaffer
Copy link
Contributor Author

bshaffer commented Jan 6, 2017

@calebdelnay thank you for catching this!

@bshaffer
Copy link
Contributor Author

bshaffer commented Jan 6, 2017

fixed in 8e3ac1a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bshaffer @calebdelnay @googlebot