Changed state and email fields to optional to match Apple's responses

[intonarumori]

In this PR we updated the model objects to match Apple's responses.

The Issue

We were getting errors with the library if we omitted the state parameter in our call to getIDCredential or we were hiding our email address in the Apple Sign-in dialog. The error was pretty obscure:

TypeError: Instance of '_TypeError': type '_TypeError' is not a subtype of type 'JSObject'

What was actually happening

• We omitted the state parameter when calling getIDCredential and then hid the email in the Apple Sign-in dialog
• The state and email fields were omitted in the JS response from Apple
• toDart here could not parse the JS object because the state and email fields were defined as required fields and not optionals in UserI and AuthorizationI
• An informative parsing error was raised:

  TypeError: null: type 'Null' is not a subtype of type 'String'

• The catch block is only accounting for errors coming from Apple when trying to convert the error to SignInErrorI here
• This failed, so another error was raised:

  TypeError: Instance of '_TypeError': type '_TypeError' is not a subtype of type 'JSObject'

• This obscured the original informative error message

The solution

• state field has been updated to be optional: state is defined as optional when calling getIDCredential here. When not passing it, Apple will omit it in their response, thus it should be an optional field on AuthorizationI.

• email field has been updated to optional: In various circumstances (ex. when the user uses Apple sign-in with their email hidden) this field might be omitted in the response from Apple. Thus this should also be defined as optional in UserI.

Couple of questions remain

• Should we do the same for name.firstName and name.lastName? So far we were not about to clear any of these fields so it seems they are always provided, however it's hard to know we really covered all edge cases.
• Should we improve the code in the catch block here so if parsing errors in the future arise we get more informative error messages and can fix them quicker?

[tp]

@intonarumori Thanks for the thorough investigation.

Surely I was always testing with a state parameter, so it might be that the types here are wrong.

I will verify this locally and if it fixes the state-less and/or "second login" flow as you describe, will prepare a release ASAP.

[tp]

Should we do the same for name.firstName and name.lastName?

I think that is already working fine because the whole name object is optional, and thus won't error when this is omitted.

[tp]

Just tested this on web without any requested scopes (no name or mail), and without passing a nonce or state either.

Before (on master) it failed, and with this fix it worked fine.

So thanks a ton @intonarumori!

[tp]

Should we improve the code in the catch block here so if parsing errors in the future arise we get more informative error messages and can fix them quicker?

Yes, that would be a great follow-up. The code should not assume that it's only that specific error type. Will have to check whether an is check would work on there, and if it doesn't match forward the "raw" error.