Open bradygaster opened 2 years ago
Assessing if we need to add support for the OAuth2 endpoints for Twitter (our docs specifically call out that a 3rd party NuGet package can be used for this, but that package fails to work with ASP.NET Core identity, so customers who want to support Twitter auth are in a broken state; they lack support for the OAuth2 features with our provider but can't authenticate customers with the OSS provider.
Assuming you're referring to the AspNet.Security.OAuth.Twitter
package @martincostello and I maintain as part of the aspnet-contrib
initiative, it's based on the ASP.NET Core OAuth 2.0 base handler and thus should work with ASP.NET Core Identity like any other provider.
Should that package not work with ASP.NET Core Identity, this would be a major issue that would deserve a quick fix so I'm very surprised you didn't open a ticket to inform us about that 😕
I'd like to kindly remind you that this provider was requested by the ASP.NET team: the least you can do in return is to inform us ASAP when you think something is wrong with it.
/cc @Tratcher
I just tested locally and I haven't encountered any issue with ASP.NET Core Identity.
@martincostello do you mind giving it a try to confirm it works fine for you too?
Oh i apologize - just saw this. :) In fact I tried with both the one we ship and the one from your side, in an experiment with Blazor WebAssembly auth. I hadn't filed an issue against your repo because I hadn't validated it wasn't my own code doing something awry. I'd love to sync up with you and show you what I ran into when I tried your libs.
Blazor is an untested scenario on our side, mainly through lack of experience. We often get questions about it though for our OAuth providers in general (not specifically Twitter).
If it’s a scenario that should work with Blazor and it’s fairly trivial to fix whatever’s wrong, then I think we’d be happy to work with you and the team to light it up.
Absolutely appreciate that @martincostello and @kevinchalet - I've used a few of your other providers for projects over the years so I was eager when the docs pointed me in your direction. Once you see my code/attempt-at-code, if we find something I can tweak, I'd be eager to update the doc with something more tangible than a link showing folks who want to use the oauth2 approaches you provide that we don't, so I'd be stoked to work on a deeper article highlighting some of what's possible with it once we figure out what else I've done wrong in the sample.
I can't speak for Kévin's availability, but I'd be happy to do a call with you any time between 1600-1800 BST (0800-1000 PDT?) any weekday to try and work through what you have and do some troubleshooting.
You can get some contact details off my GH profile.
@bradygaster sent me a repro: it's a classical Blazor WASM+Identity Server app with a custom ExternalLogin.cshtml
that uses the popular Tweetinvi library to retrieve the profile image and the email address (in this case, the Twitter authentication part is purely handled server-side and the Blazor WASM client gets local tokens from IdSrv after a transparent authorization dance).
There are 2 distinct issues:
The Tweetinvi API used to achieve that - Users.GetAuthenticatedUserAsync()
- targets the v1 API endpoints. Yet, these endpoints don't work with the Twitter OAuth 2.0 integration, that requires using the v2 endpoints. Once you use the UsersV2.GetUserByNameAsync("me")
API to get the profile image (AFAIK, you can't retrieve the email address using the v2 user endpoints), things work fine.
From time to time, too much data is stored in the AuthenticationProperties
and the handler ends up generating a state
bag that is too large for Twitter. Empirically, I was able to determine that states longer than 500 chars cause a generic error. I'm fairly sure this potential issue was discussed during the review. Fixing that won't be exactly trivial as it will require storing the state elsewhere (e.g in a distributed cache or in a database, as the OpenIddict client does)
Ah cool, glad one of them was a fairly easy fix.
The second issue sounds like the problem I had with the first implementation that seemed to fix itself: https://github.com/dotnet/aspnetcore/issues/39664#issuecomment-1018598236
Oh that's very interesting - adding @ReubenBond as you said "distributed caching" - an idea he and the Orleans team and I are discussing.
@kevinchalet - to be clear, I only used the Tweetinvi client because I couldn't figure out how to use the claims to get all the properties directly from Twitter. If I could make that work, I'd probably have no use for that client package.
I'm not at a computer right now to try it myself by running the code, but maybe there's addition fields you can request in the user payload, and then map those to claims?
https://github.com/aspnet-contrib/AspNet.Security.OAuth.Providers/blob/dev/docs/twitter.md
@bradygaster by default, the v2 user API returns very few data: the ID, the username and the name, nothing more: https://oauth-playground.glitch.me/?id=findMyUser.
If you need additional data, you need to use a special parameter called fields
(many providers do that). You can configure that via TwitterOptions.UserFields
and add a claim action to map the profile image field to a specific claim.
Crap, @martincostello is too fast for me 🤣
I'm not at a computer right now to try it myself by running the code, but maybe there's addition fields you can request in the user payload, and then map those to claims?
https://github.com/aspnet-contrib/AspNet.Security.OAuth.Providers/blob/dev/docs/twitter.md
I think I tried that and the authentication part began to fail after switching over but I can surely try again.
I haven't looked at getting the email yet, but adding these two lines of configuration for the provider using our repo's sample app gets me a claim containing the profile image's URL.
.AddTwitter(options =>
{
options.ClientId = Configuration["Twitter:ClientId"];
options.ClientSecret = Configuration["Twitter:ClientSecret"];
options.UserFields.Add("profile_image_url");
options.ClaimActions.MapJsonSubKey("urn:twitter:avatar", "data", "profile_image_url");
})
The available fields are documented here.
It appears that it isn't possible to get the user's email address using the v2 Twitter API: twittercommunity.com
I haven't run into the "state too long" problem at all locally with our sample - probably because we don't do anything additional with the AuthenticationProperties
.
I'll have a think about how we could provide a way to work around that that isn't tied to much to a specific solution and/or is extensible.
It appears that it isn't possible to get the user's email address using the v2 Twitter API: twittercommunity.com
Given ASP.NET identity requires email, this seems bad. Any workarounds you'd advise?
@martincostello that twittercommunity thread was about looking up users by e-mail, not retrieving their e-mail address.
Does the old approach work with the new OAuth2 flow? https://github.com/dotnet/aspnetcore/blob/4ea0f60dfbaf576e6ced5a107bc876b0784423d4/src/Security/Authentication/Twitter/src/TwitterHandler.cs#L313
I’ll give it a try tomorrow, but I assumed not as it’s part of the 1.1 API.
Nope, that doesn't work. Using the access token as-is gets you an HTTP 403 error.
I looked at the code for ExecuteRequestAsync()
, but that needs a token secret to generate the relevant signature to call the API which you don't have in the OAuth 2 flow.
Assuming I've understood it all correctly, you can only get the email by using the ASP.NET Core provider that uses the 1.1 API. Otherwise you have to mix and match the two API versions to use the different capabilities.
The snippet I added to test the above was this:
options.Events.OnCreatingTicket = async context =>
{
context.Backchannel.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue(
"Bearer",
context.AccessToken);
using var response = await context.Backchannel.GetAsync(
"https://api.twitter.com/1.1/account/verify_credentials.json?include_email=true",
context.HttpContext.RequestAborted);
response.EnsureSuccessStatusCode();
using var content = await response.Content.ReadAsStreamAsync();
using var document = await JsonDocument.ParseAsync(content);
if (document.RootElement is { } root)
{
// Do stuff
}
};
According to the Twitter documentation for the endpoint there's no guarantee an email would be returned so even with the ASP.NET Core Twitter provider an application using ASP.NET Core Identity would need to handle the case of their being no email address when the user logs in:
If the user does not have an email address on their account, or if the email address is not verified, null will be returned.
Assuming I've understood it all correctly, you can only get the email by using the ASP.NET Core provider that uses the 1.1 API. Otherwise you have to mix and match the two API versions to use the different capabilities.
This was my presumption at first glance. Eesh. Kinda sub-optimal. Really appreciate the diligence looking through and debugging it.
@bradygaster not sure if it works for you, but you always can ask the users for their email address using a good old form 😃 (it's what the default Identity UI does).
@bradygaster Can you put this issue into a milestone please?
[This item was copied from the planning repo]
Through Cu, work with partners in AAD and internally within ASP.NET to collaborate on user research and experience measurement to find ways we can ease the burden of configuring popular auth providers like Microsoft Identity Platform or and 2 popular 3rd party providers.
Specifically concentrate on easing the friction and effort (keystrokes, clicks, and internet searches required) for one or all of the following scenarios:
As we improve the scenario with product work, we should accompany that product work with documentation for our testing team so the experience of configuring auth for these key scenarios can be tested repeatedly and so we know when service or API changes result in our needing to update the docs or the experience.