Add support for extended profile using SIWA token exchange #322

asmclean · 2019-11-11T16:40:28Z

Changes

Add support for getting name properties from the fullName property in the credential when using Sign In with Apple and token exchange. This will extract and send only values for the first and last name, similar to what's provided in the user parameter by the SIWA for websites.

References

Testing

Please describe how this can be tested by reviewers. Be specific about anything not tested and reasons why. If this library has unit and/or integration testing, tests should be added for new functionality and existing tests should complete without errors.

This change adds unit test coverage
This change has been tested on the latest version of the platform/language or why not

Checklist

I have read the Auth0 general contribution guidelines
All existing and new tests complete without errors
All active GitHub checks have passed

lbalmaceda

This is a particular use case / scenario. I don't think the mapping of user object values is responsibility of the Authentication#tokenExchange method. cc @cocojoe

cocojoe · 2019-11-11T18:30:27Z

Agreed it's not the responsibility of the core tokenExchange method. However, is fine to happen inside the apple specific wrapper of this method.

cocojoe · 2019-11-11T18:31:28Z

Auth0/Authentication.swift

@@ -911,21 +911,38 @@ public extension Authentication {
    ```
    Auth0
       .authentication(clientId: clientId, domain: "samples.auth0.com")
-       .tokenExchange(withAppleAuthorizationCode: authCode, scope: "openid profile offline_access", audience: "https://myapi.com/api)
+       .tokenExchange(withAppleAuthorizationCode: authCode, fullName: credentials.fullName, scope: "openid profile offline_access", audience: "https://myapi.com/api)


please change parm order to match the method

cocojoe · 2019-11-11T18:35:30Z

Auth0Tests/AuthenticationSpec.swift

@@ -215,33 +215,40 @@ class AuthenticationSpec: QuickSpec {
            }

        }
-


What reformatting did you apply to this file?

Not sure: I based this off of a spike that appears to have already included the whitespace changes. I've reverted them, but let me know if anything still looks off.

cocojoe · 2019-11-11T18:36:03Z

Cartfile.resolved

@@ -1,4 +1,4 @@
 github "AliSoftware/OHHTTPStubs" "8.0.0"
-github "Quick/Nimble" "v8.0.2"
-github "Quick/Quick" "v2.1.0"
+github "Quick/Nimble" "v8.0.4"


I would prefer deps are updated in a separate PR. However, I'm okay with it.

This was unintentional on my part, so I removed this change.

cocojoe · 2019-11-11T18:37:53Z

Auth0Tests/AuthenticationSpec.swift

@@ -283,7 +290,22 @@ class AuthenticationSpec: QuickSpec {
                    }
                }
            }
-
+
+            it("should exchange apple auth code for credentials with fullName") {


are there any cases it will fail e.g. if first name is missing or last name is missing?

No, there are not. The values can be missing or null and it won't cause any issues. Now, I've filtered them out if there is no value.

cocojoe · 2019-11-11T20:05:03Z

Auth0/Authentication.swift

+              "subject_token": authCode,
+              "subject_token_type": "http://auth0.com/oauth/token-type/apple-authz-code",
+              "scope": scope ?? "openid profile offline_access" ]
+        if fullName != nil {


You can tweak this a little

if let fullName = fullName, let jsonData = try? String(

This also lets you remove the optionals in the JSON Object.

cocojoe · 2019-11-11T20:06:22Z

Auth0/Authentication.swift

+        if fullName != nil {
+            if let jsonData = try? String(
+                data: JSONSerialization.data(
+                    withJSONObject: [


Just to check the endpoint is fine to accept null values?

{\"lastName\":\"Walsh\",\"firstName\":null}}

Yes, null values are okay.

cocojoe · 2019-11-12T14:14:43Z

@Widcket can you take a look as well, please

Widcket · 2019-11-12T15:40:47Z

Auth0Tests/AuthenticationSpec.swift

-        }
+            it("should exchange apple auth code for credentials with fullName") {
+                var fullName = PersonNameComponents()
+                fullName.givenName = "John"


Can you please add test cases for when the name and/or surname are nil?

crs2020 · 2019-11-13T07:25:39Z

This is fraud please contact me at 480-506-8408

…

On November 12, 2019 at 9:43 AM, Drew ***@***.***>wrote: @asmcleancommented on this pull request. InAuth0Tests/AuthenticationSpec.swift(#322 (comment)): >@@ -284,8 +292,24 @@ class AuthenticationSpec: QuickSpec { } } - } + it("should exchange apple auth code for credentials with fullName") { + var fullName = PersonNameComponents() + fullName.givenName = "John" Added. — You are receiving this because you are subscribed to this thread. Reply to this email directly,view it on GitHub(#322?email_source=notifications&email_token=ANMQPERWM5BLKSRNOBXOV7LQTLMLDA5CNFSM4JLX67Q2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCLIOIIY#discussion_r345318171), orunsubscribe(https://github.com/notifications/unsubscribe-auth/ANMQPER2DRTGB4WBQY5BDSTQTLMLDANCNFSM4JLX67QQ).

cocojoe · 2019-11-13T13:04:22Z

@asmclean Please can you rebase against master and I'll approve this. Thanks

cocojoe · 2019-11-13T13:05:14Z

Never mind I thought update was blocked, I should be good to approve in 5 mins 😄

* Add support for extended profile using SIWA token exchange * Fix indenting/whitespace * Revert accidental Cartfile.resolved changes * Reorder SIWA parameters, and stop sending missing values * Expand test cases for SIWA

…uth0#322)" This reverts commit 61598dd.

asmclean requested a review from a team November 11, 2019 16:40

asmclean force-pushed the IPS-772 branch from b7fa343 to 8e0893d Compare November 11, 2019 16:57

Add support for extended profile using SIWA token exchange

567cbdc

asmclean force-pushed the IPS-772 branch from 8e0893d to 567cbdc Compare November 11, 2019 17:36

lbalmaceda reviewed Nov 11, 2019

View reviewed changes

cocojoe requested changes Nov 11, 2019

View reviewed changes

Drew McLean added 3 commits November 11, 2019 15:38

Fix indenting/whitespace

7a6733e

Revert accidental Cartfile.resolved changes

cb9abbc

Reorder SIWA parameters, and stop sending missing values

71128a5

cocojoe requested a review from Widcket November 12, 2019 14:16

Widcket reviewed Nov 12, 2019

View reviewed changes

Expand test cases for SIWA

341cf65

cocojoe added the CH: Added label Nov 13, 2019

cocojoe added this to the vNext milestone Nov 13, 2019

Merge branch 'master' into IPS-772

5da6d4c

cocojoe self-requested a review November 13, 2019 13:20

cocojoe approved these changes Nov 13, 2019

View reviewed changes

cocojoe merged commit 5e6a256 into auth0:master Nov 13, 2019

cocojoe mentioned this pull request Nov 14, 2019

Release 1.19.2 #323

Merged

sam-w added a commit to LoungeBuddy/Auth0.swift that referenced this pull request Mar 11, 2020

Revert "Add support for extended profile using SIWA token exchange (a…

ef6702e

…uth0#322)" This reverts commit 61598dd.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for extended profile using SIWA token exchange #322

Add support for extended profile using SIWA token exchange #322

asmclean commented Nov 11, 2019 •

edited by cocojoe

Loading

lbalmaceda left a comment

cocojoe commented Nov 11, 2019 •

edited

Loading

cocojoe Nov 11, 2019

asmclean Nov 12, 2019

cocojoe Nov 11, 2019

asmclean Nov 12, 2019

cocojoe Nov 11, 2019

asmclean Nov 12, 2019

cocojoe Nov 11, 2019

asmclean Nov 12, 2019

cocojoe Nov 11, 2019

cocojoe Nov 11, 2019

asmclean Nov 12, 2019

cocojoe commented Nov 12, 2019 •

edited

Loading

Widcket Nov 12, 2019

asmclean Nov 12, 2019

crs2020 commented Nov 13, 2019 via email

cocojoe commented Nov 13, 2019

cocojoe commented Nov 13, 2019

		@@ -215,33 +215,40 @@ class AuthenticationSpec: QuickSpec {
		}

		}

Add support for extended profile using SIWA token exchange #322

Add support for extended profile using SIWA token exchange #322

Conversation

asmclean commented Nov 11, 2019 • edited by cocojoe Loading

Changes

References

Testing

Checklist

lbalmaceda left a comment

Choose a reason for hiding this comment

cocojoe commented Nov 11, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cocojoe commented Nov 12, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crs2020 commented Nov 13, 2019 via email

cocojoe commented Nov 13, 2019

cocojoe commented Nov 13, 2019

asmclean commented Nov 11, 2019 •

edited by cocojoe

Loading

cocojoe commented Nov 11, 2019 •

edited

Loading

cocojoe commented Nov 12, 2019 •

edited

Loading