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

Updated TOKEN_ISSUER to 'accounts.google.com' #6836

Merged
merged 5 commits into from
Jul 29, 2020
Merged

Updated TOKEN_ISSUER to 'accounts.google.com' #6836

merged 5 commits into from
Jul 29, 2020

Conversation

arjun3396
Copy link
Contributor

Hi, I was getting this issue from today morning parse-server/Adapters/Auth/google.js was expecting the TOKEN_ISSUER to be prefixed with https:// but on debugging the original value was not having the prefix, removing https:// from TOKEN_ISSUER solved this bug. This issue is introduced in 4.3.0 as in 4.2.0 it is working fine currently I have downgraded the version to 4.2.0 for it to work properly and suggesting the changes please merge this PR.

Hi, I was getting this issue from today morning parse-server/Adapters/Auth/google.js was expecting the TOKEN_ISSUER to be prefixed with https:// but on debugging the original value was not having the prefix, removing https:// from TOKEN_ISSUER solved this bug. This issue is introduced in 4.3.0 as in 4.2.0 it is working fine currently I have downgraded the version to 4.2.0 for it to work properly and suggesting the changes please merge this PR.
@dplewis
Copy link
Member

dplewis commented Jul 28, 2020

The Travis Build is failing. Can you update the test cases?

@dplewis
Copy link
Member

dplewis commented Jul 28, 2020

@SebC99 Can you look at this? I see you worked on #6734

@SebC99
Copy link
Contributor

SebC99 commented Jul 28, 2020

As per Google official documentation, all examples are using https prefix, but it seems they have added this note: The value of iss in the ID token is equal to accounts.google.com or https://accounts.google.com.
So removing https:// from the TOKEN_ISSUER will fail too if Google sends it in various format.

In our cases, we always receive the https:// version.
I suggest to test both versions, with and without https.

@arjun3396
Copy link
Contributor Author

As per the discussion, I have added check for both https and without https and updated the test-cases please review or suggest if any other changes needed

@arjun3396
Copy link
Contributor Author

The Travis Build is failing. Can you update the test cases?

@dplewis I have updated the test case but this one below is failing, any suggestions on how to fix this?
ParseGraphQLServer Auto API Schema Relay Spec Object Identification Id inputs should work either with global id or object id
Message:
Expected 'U2Vjb25kYXJ5T2JqZWN0OmVJWHBxZXZCUnY=' to be less than 'U2Vjb25kYXJ5T2JqZWN0Om9WN2Uxd09jdlY='.
Stack:
Error: Expected 'U2Vjb25kYXJ5T2JqZWN0OmVJWHBxZXZCUnY=' to be less than 'U2Vjb25kYXJ5T2JqZWN0Om9WN2Uxd09jdlY='.
at
at UserContext.it_only_db (/home/travis/build/parse-community/parse-server/spec/ParseGraphQLServer.spec.js:2794:19)
at process._tickCallback (internal/process/next_tick.js:68:7)

@dplewis
Copy link
Member

dplewis commented Jul 29, 2020

@arjun3396 Thats a flaky test that you can ignore.

Copy link
Member

@dplewis dplewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@codecov
Copy link

codecov bot commented Jul 29, 2020

Codecov Report

Merging #6836 into master will decrease coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6836      +/-   ##
==========================================
- Coverage   93.88%   93.85%   -0.04%     
==========================================
  Files         169      169              
  Lines       12209    12210       +1     
==========================================
- Hits        11463    11460       -3     
- Misses        746      750       +4     
Impacted Files Coverage Δ
src/Adapters/Auth/google.js 92.85% <100.00%> (+0.10%) ⬆️
src/Adapters/Storage/Mongo/MongoStorageAdapter.js 92.92% <0.00%> (-0.67%) ⬇️
src/RestWrite.js 93.64% <0.00%> (-0.33%) ⬇️
src/Adapters/Auth/httpsRequest.js 100.00% <0.00%> (+4.76%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 92afcca...14d5ae9. Read the comment docs.

@dplewis dplewis merged commit 42f75d6 into parse-community:master Jul 29, 2020
therealjmj pushed a commit to therealjmj/SC-parse-server that referenced this pull request Aug 20, 2020
* Updated TOKEN_ISSUER to 'accounts.google.com'

Hi, I was getting this issue from today morning parse-server/Adapters/Auth/google.js was expecting the TOKEN_ISSUER to be prefixed with https:// but on debugging the original value was not having the prefix, removing https:// from TOKEN_ISSUER solved this bug. This issue is introduced in 4.3.0 as in 4.2.0 it is working fine currently I have downgraded the version to 4.2.0 for it to work properly and suggesting the changes please merge this PR.

* Update google.js

* Update AuthenticationAdapters.spec.js

* Update google.js

* Update google.js
@RodrigoSMarques
Copy link

Hello guys. When will a new version of Parse Server be released with this fix?
I upgraded to version 4.3 but had to go back to 4.2 because login with Google didn't work anymore.

Thank you.

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

Successfully merging this pull request may close these issues.

4 participants