Skip to content

Fix: ValidateIdToken method and unit tests #58

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

Merged
merged 2 commits into from
Oct 22, 2019

Conversation

heykevin
Copy link
Contributor

I was attempting to increase code coverage of the validateIdToken test when I realized the tests were not working as expected which then led me to realizing the method itself was not working as expected.

Changes:

  • Cast response status to number similar to AuthResponse
  • Check clientID in the list of audience, it was comparing ID to an array before
  • mockIdPayload in the tests that matches what we are expecting in tests

@coveralls
Copy link

Coverage Status

Coverage increased (+3.3%) to 91.389% when pulling 4ca5afc on heykevin:broken_validateIdToken into 6352c75 on intuit:develop.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+3.3%) to 91.389% when pulling 4ca5afc on heykevin:broken_validateIdToken into 6352c75 on intuit:develop.

@abisalehalliprasan
Copy link
Collaborator

Thanks @heykevin for the changes. Good catch 👍

@abisalehalliprasan abisalehalliprasan merged commit a2d0ab3 into intuit:develop Oct 22, 2019
abisalehalliprasan added a commit that referenced this pull request Jan 2, 2020
* Coveralls Badge Fix

* Coveralls Badge Fix:Final

* Coveralls Badge Fix : istanbul package added

* Coveralls Badge Fix + snyk added

* Coveralls Badge Fix + snyk added

* Snyk removed from Makefile

* Snyk removed for timebeing

* Add more code coverage for OAuthClient (#54)

* Add better code coverage in OAuthClient

* Fix: ValidateIdToken method and unit tests (#58)

* Fix validateIdToken tests

* Pointing README Badge to Develop

* Pointing README Badge to Develop

* Update Develop Coverage Badge

* README.md corrections

* Version Bump

Co-authored-by: Oscar Rabasa <[email protected]>
Co-authored-by: Kevin Tang <[email protected]>
abisalehalliprasan added a commit that referenced this pull request Apr 10, 2020
* Coveralls Badge Fix

* Coveralls Badge Fix:Final

* Coveralls Badge Fix : istanbul package added

* Coveralls Badge Fix + snyk added

* Coveralls Badge Fix + snyk added

* Snyk removed from Makefile

* Snyk removed for timebeing

* Add more code coverage for OAuthClient (#54)

* Add better code coverage in OAuthClient

* Fix: ValidateIdToken method and unit tests (#58)

* Fix validateIdToken tests

* Pointing README Badge to Develop

* Pointing README Badge to Develop

* Update Develop Coverage Badge

* Fix: handle not JSON content in response parsing

Some intuit API as invoice download return not JSON content (PDF in this
case). `makeApiCall` wasn't working with it because of mandatory
response body parsing.

So if the response is not JSON, we don't want to parse the body.
And simply let the caller decide what to do with it.

Co-authored-by: abisalehalliprasan <[email protected]>
Co-authored-by: Oscar Rabasa <[email protected]>
Co-authored-by: Kevin Tang <[email protected]>
Co-authored-by: abisalehalliprasan <[email protected]>
abisalehalliprasan added a commit that referenced this pull request Apr 10, 2020
* Accept HTTP status codes between 199 and 300 as successes (#78)

* Fix: handle not JSON content in response parsing (#59)

* Coveralls Badge Fix

* Coveralls Badge Fix:Final

* Coveralls Badge Fix : istanbul package added

* Coveralls Badge Fix + snyk added

* Coveralls Badge Fix + snyk added

* Snyk removed from Makefile

* Snyk removed for timebeing

* Add more code coverage for OAuthClient (#54)

* Add better code coverage in OAuthClient

* Fix: ValidateIdToken method and unit tests (#58)

* Fix validateIdToken tests

* Pointing README Badge to Develop

* Pointing README Badge to Develop

* Update Develop Coverage Badge

* Fix: handle not JSON content in response parsing

Some intuit API as invoice download return not JSON content (PDF in this
case). `makeApiCall` wasn't working with it because of mandatory
response body parsing.

So if the response is not JSON, we don't want to parse the body.
And simply let the caller decide what to do with it.

Co-authored-by: abisalehalliprasan <[email protected]>
Co-authored-by: Oscar Rabasa <[email protected]>
Co-authored-by: Kevin Tang <[email protected]>
Co-authored-by: abisalehalliprasan <[email protected]>

* Release : 2.1.0

* Release : 2.1.0

* Release : 2.1.0 : Revert back dependencies

* Release : 2.1.0 : Revert back dependencies

Co-authored-by: Zina Schroeder <[email protected]>
Co-authored-by: Sébastien Boulle <[email protected]>
Co-authored-by: Oscar Rabasa <[email protected]>
Co-authored-by: Kevin Tang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants