fix: Get user first name and last name from Apple #2331
Conversation
|
|
JiggyDown
changed the title
Codecov Report
@@ Coverage Diff @@
## master #2331 +/- ##
==========================================
- Coverage 76.62% 76.61% -0.01%
==========================================
Files 315 315
Lines 17358 17379 +21
==========================================
+ Hits 13300 13315 +15
- Misses 3122 3128 +6
Partials 936 936
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thank you, this looks great! Since this is very difficult to test, could you please create a short screencast of this feature working and post it here?
Also can you confirm that https://www.ory.sh/docs/kratos/guides/sign-in-with-github-google-facebook-linkedin#apple is correct? There were a few people who had issues setting up Apple / cc @vinckr
| // https://developer.apple.com/documentation/sign_in_with_apple/sign_in_with_apple_js/configuring_your_webpage_for_sign_in_with_apple#3331292 | ||
| // First name and last name are passed only once in an additional parameter to the redirect URL. | ||
| // There's no way to make sure they haven't been tampered with. Blame Apple. |
| LastName *string `json:"lastName"` | ||
| } `json:"name"` | ||
| } | ||
| if err := json.Unmarshal([]byte(query.Get("user")), &user); err == nil { |
There was a problem hiding this comment.
Could you please add a test for this?
There was a problem hiding this comment.
@JiggyDown if you could add a small test for this, we can merge this right away :)
There was a problem hiding this comment.
Test added. Thank you for your patience!
|
@JiggyDown would you be open to make the requested changes? |
|
Hey @aeneasr, sorry I didn't have time to revisit this PR. I'll try to finish it by the end of this week. |
Apple doesn't accept any other value
|
Hello @JiggyDown |
Currently we can't get user first name and last name from Apple as the names directly to the callback URL, as opposed to embedding them in the ID token like other OIDC providers. This PR makes sure we handle that behavior properly.
Related issue(s)
N/A
Checklist
introduces a new feature.
contributing code guidelines.
vulnerability. If this pull request addresses a security. vulnerability, I
confirm that I got green light (please contact
security@ory.sh) from the maintainers to push
the changes.
works.
Further Comments
This PR requires an E2E test, but I can't find an existing one for OIDC. Can you give me some pointers on how the test should be done?