Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User object not returned after Apple Authorize #765

Closed
pieperu opened this issue Mar 12, 2023 · 5 comments
Closed

User object not returned after Apple Authorize #765

pieperu opened this issue Mar 12, 2023 · 5 comments

Comments

@pieperu
Copy link

pieperu commented Mar 12, 2023

Describe the bug

Hi. I’m trying to retrieve the givenname and surname claims from the apple signin flow. My understanding is that this is only returned the very first time a user authenticates, and is returned in the same payload as the id_token, and would like something like this:

state=xxxstatexxx
&code=xxxxx.x.xx.xxxxx
&id_token=xxxxsomeidtokenxxxx
&user={"name":{"firstName":"Some","lastName":"User"},"email":"[email protected]"}

I’ve been diving around trying to work out why I can’t surface them in my Identity Server External Identity /callback endpoint and I wanted to check that the issue wasn’t in this Apple OAuth library.

I can see that as of 6.10.0 that the email is no longer taken from the User payload coming back from the apple authorise response, but also it seems that the user payload is somewhat discarded. I can see here:

var context = new OAuthCreatingTicketContext(principal, properties, Context, Scheme, Options, Backchannel, tokens, tokens.Response.RootElement);

That the User object is not included in the created ticket at all and doesn't appear to be available anywhere else. Could this be an oversight?

When creating the AuthenticationTicket, this User isn’t provided at all:

return new AuthenticationTicket(context.Principal!, context.Properties, Scheme.Name);

Could it be that these could be included in the context.Properties to be included in the AuthenticationTicket?

Side note, I notice that the entire root payload is passed into the OAuthCreatingTIcketContext, should it be this instead:

tokens.Response.RootElement.GetString("user")

var context = new OAuthCreatingTicketContext(principal, properties, Context, Scheme, Options, Backchannel, tokens, tokens.Response.RootElement.GetString("user"));

To return the user json object? As is expected by the OAuthCreaingTicketContext from the Owin repo:

https://github.com/pruiz/Owin.OAuthGeneric/blob/d1acc2983fd9624f3da576aaaf759ca615b1405d/Owin.OAuthGeneric/OAuthCreatingTicketContext.cs#L47

Steps To reproduce

Using Identity Server (Duende) logging in with Apple, the payload returned to the callback endpoint does not include any names (givenname or surname). It does not appear to be surfaced anywhere.

Expected behaviour

One first login, that in the callback endpoint we will have access to the User payload returned by apple's Authorize endpoint on first login.

Actual behaviour

None of this comes back to the Identity Server endpoint/

System information

dotnet 7
Duende Identity Server
Latest version of the apple oauth package

Additional context

For privacy reasons Apple does not provide an endpoint to retrieve users names after the initial login. The only opportunity to capture this information is on first sign in, so we need to ensure this is returned to the callback endpoint.

@pieperu pieperu added the bug label Mar 12, 2023
@martincostello
Copy link
Member

That’s correct in that the library does not use this information since 6.0.10. It’s also correct that there is exactly one opportunity to get these values from Apple.

The use of the user object was removed in response to GHSA-3893-h8qg-6h5f.

If you still want to access the raw payload, you can use the ticket creation event to do so. As the values can be spoofed as noted in the advisory, you should not use the data to make security-related decisions (e.g. authorisation checks).

@fipster-dev
Copy link

Thanks for the super quick response Martin, and the info. Much appreciated.

I'll look into using the ticket creation event as you've suggested 🙇‍♂️

@pieperu
Copy link
Author

pieperu commented Mar 13, 2023

Hey Martin, a followup when you have a mo.

I may have misunderstood when you said

If you still want to access the raw payload, you can use the ticket creation event to do so.

By raw payload did you mean the payload that was passed from apple in the request form?

Looking into using that ticket creation event I noticed that the value coming through for the User object on the context was pretty much the same as the Token param that gets passed into that same context.

I had created a draft PR

#767

Just to show that a change would be needed to surface that user object on the context in the event.

But just for clarity - Is surfacing the User payload that apples POSTs totally off the cards, even if it's on this context? (Therefore making GivenName and Surname totally inaccessible)

@martincostello
Copy link
Member

Sorry - I confused the properties variable, which is passed to you via the event's context, and the parameters variable (containing the user), which is not.

However, the user is still ultimately still available to you if you really really want to access it via the HttpContext. You can see where we get it from the put into parameters below:

IEnumerable<KeyValuePair<string, StringValues>> source;
// If form_post was used, then read the parameters from the form rather than the query string
if (string.Equals(Request.Method, HttpMethod.Post.Method, StringComparison.OrdinalIgnoreCase))
{
source = Request.Form;
}
else
{
source = Request.Query;
}
var parameters = new Dictionary<string, StringValues>(source);
return await HandleRemoteAuthenticateAsync(parameters);

You could use similar logic in your event handler to grab the user parameter of the current HTTP request to do what you would like to do with the values available.

@pieperu
Copy link
Author

pieperu commented Mar 13, 2023

@martincostello thank you so much for your help on this! You are a god send :)

I'd been looking at this so long I totally missed the HTTP Context was available in the event handler 🤦

Problem solved!

@pieperu pieperu closed this as completed Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants