fix(android): pass the specified scope when creating configuration #363
Conversation
|
|
| @@ -150,7 +150,7 @@ public void configure(ReadableMap configObject) { | |||
| .clientId(clientId) | |||
| .redirectUri(redirectUri) | |||
| .responseType(SignInWithAppleConfiguration.ResponseType.ALL) | |||
There was a problem hiding this comment.
| .responseType(SignInWithAppleConfiguration.ResponseType.ALL) | |
| .responseType(responseType) |
as long as we're in here - appears the exact same oversight happened with responseType
- it gets a default value at the top in var responseType
- the config is checked to see if it exists, and if it exists then the config value is set into responseType
- then all that is completely ignored and
ALLis used
worth fixing? What do you think?
There was a problem hiding this comment.
I explained here my reasoning related to responseType. Fixing it is not as simple as that...
There was a problem hiding this comment.
Even if we ignore the fact that appleAuthAndroid.ResponseType.ID_TOKEN is not valid, and fix that single line of code (i.e., passing .responseType(responseType)), it would still be a breaking change as existing code that is currently explicitly setting the responseType to SignInWithAppleConfiguration.ResponseType.ID_TOKEN would all of a sudden break because passing just id_token in the request to Apple would result in an error during Sign in with Apple.
There was a problem hiding this comment.
Oh I'm sorry! Only saw that comment you left with your reasoning just now. Agreed, leave for now
|
https://www.npmjs.com/package/@invertase/react-native-apple-authentication?activeTab=versions published, thanks for the help |
Fixes #362 (except for
responseType, which is left out as explained here)