Skip to content

feat: add ability to disable nonce - #52

Merged
Salakar merged 2 commits into
invertase:masterfrom
phound-inc:master
Jun 10, 2020
Merged

feat: add ability to disable nonce#52
Salakar merged 2 commits into
invertase:masterfrom
phound-inc:master

Conversation

@dougkeen

Copy link
Copy Markdown
Contributor

This is useful if you use a platform like Auth0, whose Apple token exchange flow doesn't currently support nonces.

With this change, you can disable nonces like so:

await appleAuth.performRequest({
  nonceEnabled: false,
  requestedOperation: AppleAuthRequestOperation.LOGIN,
  requestedScopes: [AppleAuthRequestScope.EMAIL, AppleAuthRequestScope.FULL_NAME],
})

@CLAassistant

CLAassistant commented Mar 23, 2020

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@mikehardy mikehardy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't see why not - I know plumbing nonce through was something everyone had to do semi-unexpectedly for it to work and caused a PR to the expo apple auth as well (firebase requires nonce, so that drove it). So I can see the need and the code looks fine to me. I'll leave for @Salakar to merge in case he knows a reason not to

@Salakar

Salakar commented Mar 30, 2020

Copy link
Copy Markdown
Contributor

@dougkeen I'm happy to add this in so thanks for this PR - it does though look like you're missing nonceEnabled: true as a default value in JS; the same way requestOperation defaults, e.g.;

https://github.com/invertase/react-native-apple-authentication/blob/master/lib/AppleAuthModule.js#L60
and
https://github.com/invertase/react-native-apple-authentication/blob/master/lib/AppleAuthModule.js#L66-L72

Without this I think it will cause a native crash here: https://github.com/invertase/react-native-apple-authentication/pull/52/files#diff-c1c850227210d2e412283742bab2ec03R44 as it'd try to call isEqual on a nil value

@dougkeen

dougkeen commented Mar 30, 2020

Copy link
Copy Markdown
Contributor Author

@Salakar Good call. I definitely missed that bit in the javascript where the defaults are set and type-checked.

FYI, in Obj-C messages sent to nil simply return nil, which is probably why I didn't see any native crashes in my testing.

@mikehardy

Copy link
Copy Markdown
Collaborator

@Salakar I believe this is ready for merge if your concerns are addressed? (they seem to be)

@Salakar Salakar changed the title Add ability to disable nonce behavior feat: add ability to disable nonce Jun 10, 2020
@Salakar
Salakar merged commit d6d22d4 into invertase:master Jun 10, 2020
@Salakar

Salakar commented Jun 10, 2020

Copy link
Copy Markdown
Contributor

Thank you :)

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.

4 participants