nhosoya / omniauth-apple

OmniAuth strategy for Sign In with Apple
MIT License
261 stars 99 forks source link

Add rspec test cases #40

Closed btalbot closed 4 years ago

btalbot commented 4 years ago

These currently cover about 97% of the code. They also expect other pending PR fixes to be merged before they will pass. These PR include fixes for RoR extentions, nonce-valdation, name in info hash, and default-scope.

Error handling and various other corner cases are missing from these tests and should be added when possible.

btalbot commented 4 years ago

I should note that I'm still learning about how apple handles OAuth2 / OIDC vs how it is done by google or facebook. These tests are based on some tests from omniauth-google-oauth2 which seems to be widely used and stable but also different enough that there could be some misunderstanding.

At the very least, test coverage would help avoid RoR extensions sneaking in and various syntax errors.

The RSA key signing complexity at the top of the tests is due to how JWT.decode is used as it requires RSA keys to be available when decoding and validating the id_token. As real apple keys cannot be used (we don't have the private keys) test keys are injected and used instead.

nhosoya commented 4 years ago

Awesome!! I actually ran this test and found it using the RoR extension 😭 https://github.com/nhosoya/omniauth-apple/blob/3715b1043951b21c57555f8f1302f95f9c9f2bde/lib/omniauth/strategies/apple.rb#L100

nhosoya commented 4 years ago

I merged #41 and all the tests passed.