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

Add Twitch to the supported providers #2142

Merged
merged 2 commits into from
Jul 30, 2024
Merged

Conversation

mbaumanndev
Copy link
Contributor

Hello !

I have not played much with it, tested with the Console and ASP.NET Core sandboxes, log-in and refresh token worked fine.

Is there anything else to do/check that is not in the login flow ?

@kevinchalet
Copy link
Member

Salut 👋🏻

Thanks for your PR!

Is there anything else to do/check that is not in the login flow ?

Did you test the client credentials grant?

I see you added a quirk to amend the grant types to also include the device grant type. But since the device authorization endpoint is not listed in https://id.twitch.tv/oauth2/.well-known/openid-configuration, I'd assume it's not working properly. Can you please test it?

@mbaumanndev
Copy link
Contributor Author

Salut 👋🏻

Salut ! 👋🏻

Thanks for your PR!

Your welcome !

Did you test the client credentials grant?

I see you added a quirk to amend the grant types to also include the device grant type. But since the device authorization endpoint is not listed in https://id.twitch.tv/oauth2/.well-known/openid-configuration, I'd assume it's not working properly. Can you please test it?

I don't think I tested it, I added it because Twitch listed this grant on their docs, I'll try to figure how to do that properly, I'll update the PR when I'm done, it may take a couple of days as I don't do .NET or OIDC on a daily basis, I do it for a side project to check what can be done with twitch APIs.

@kevinchalet
Copy link
Member

I'll update the PR when I'm done, it may take a couple of days as I don't do .NET or OIDC on a daily basis, I do it for a side project to check what can be done with twitch APIs.

Haha, no worries. Let me know if you need help 😃

@mbaumanndev
Copy link
Contributor Author

After checking Twitch docs, it seems that we must at some point validate the token for some use cases (https://dev.twitch.tv/docs/authentication/validate-tokens/), is there a file where I should override some behaviour to do it ?

I'm also having some difficulties with the device authorization endpoint, but I think I missied something with the scopes in the flow

@kevinchalet
Copy link
Member

After checking Twitch docs, it seems that we must at some point validate the token for some use cases (https://dev.twitch.tv/docs/authentication/validate-tokens/), is there a file where I should override some behaviour to do it ?

That really looks like a stupid design to me (and I strongly doubt most implementations do that, it's an insanely inefficient process).

We could treat that endpoint as an introspection endpoint, but it's not a standard flavor so we'd need some mapping. Not sure it's really worth it. Let's just ignore it? 😄

I'm also having some difficulties with the device authorization endpoint, but I think I missied something with the scopes in the flow

If https://dev.twitch.tv/docs/authentication/getting-tokens-oauth/#device-code-grant-flow correctly reflects how they implemented things, it doesn't seem to diverge much from the standard OAuth 2.0 device authorization grant. What error are you seeing?

@mbaumanndev
Copy link
Contributor Author

After checking Twitch docs, it seems that we must at some point validate the token for some use cases (https://dev.twitch.tv/docs/authentication/validate-tokens/), is there a file where I should override some behaviour to do it ?

That really looks like a stupid design to me (and I strongly doubt most implementations do that, it's an insanely inefficient process).

We could treat that endpoint as an introspection endpoint, but it's not a standard flavor so we'd need some mapping. Not sure it's really worth it. Let's just ignore it? 😄

For now we may ignore it, I'll try later to see if it's really needed for long during connections, and if that's the case I'll do a new contrib.

I'm also having some difficulties with the device authorization endpoint, but I think I missied something with the scopes in the flow

If https://dev.twitch.tv/docs/authentication/getting-tokens-oauth/#device-code-grant-flow correctly reflects how they implemented things, it doesn't seem to diverge much from the standard OAuth 2.0 device authorization grant. What error are you seeing?

I just updated the PR with the code I have for now, I'm not sure about the scope encoding and if I should force the removal of the openid scope when doing device auth.

Here is the setup I have in the program for testing:

 .AddTwitch(options =>
 {
     // For client credentials : https://github.com/Moerty/Twitch.MediatR?tab=readme-ov-file#prerequisites---twitch-credentials
     options.SetClientId("xxx")
              .SetClientSecret("xxx")
              .SetRedirectUri("callback/login/twitch")
              .AddScopes("user:read:email");
 });

And here is the error I'm getting:

An error occurred while trying to authenticate the user:
OpenIddict.Abstractions.OpenIddictExceptions+ProtocolException: An error occurred while authenticating the user.
  Error: invalid_request
  Error description: A generic 400 error was returned by the remote authorization server.
  Error URI: https://documentation.openiddict.com/errors/ID2161
  at async ValueTask<DeviceChallengeResult> OpenIddict.Client.OpenIddictClientService.ChallengeUsingDeviceAsync(
     DeviceChallengeRequest request) in \openiddict-core\src\OpenIddict.Client\
     OpenIddictClientService.cs:832
  at void System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
  at async ValueTask<DeviceChallengeResult> OpenIddict.Client.OpenIddictClientService.ChallengeUsingDeviceAsync(
     DeviceChallengeRequest request) in \openiddict-core\src\OpenIddict.Client\
     OpenIddictClientService.cs:857
  at void System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
  at void System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
  at TResult System.Threading.Tasks.ValueTask`1.get_Result()
  at async Task OpenIddict.Sandbox.Console.Client.InteractiveService.ExecuteAsync(CancellationToken stoppingToken) in
     \openiddict-core\sandbox\OpenIddict.Sandbox.Console.Client\InteractiveService.cs:
     155

I did not had this error the other day, I had another one because I didn't use the right endpoint... I'll try to take a further look on thursday or friday to check which 400 I get there.

@mbaumanndev
Copy link
Contributor Author

By the way, I just checked the URL given in the error (https://documentation.openiddict.com/errors/ID2161), and it's a 404 page, I can do a contrib on the docs in the next few weeks to help fix that

@kevinchalet
Copy link
Member

I did not had this error the other day, I had another one because I didn't use the right endpoint... I'll try to take a further look on thursday or friday to check which 400 I get there.

Take a look at the logs to see if there is a more useful error 😃

By the way, I just checked the URL given in the error (https://documentation.openiddict.com/errors/ID2161), and it's a 404 page, I can do a contrib on the docs in the next few weeks to help fix that

Yeah, adding a proper page for each error code is tracked by openiddict/openiddict-documentation#30. It's a huge task, tho' 🤣

@kevinchalet kevinchalet removed this from the 5.8.0 milestone Jul 19, 2024
@mbaumanndev
Copy link
Contributor Author

I made some progress :D

When I go with step by step debug, the flow give me a link to authenticate to twitch with the code. If I open it and enter the code while the program is stuck in debug, it works.

As soon as I disable the breakpoints, the link is displayed in the console and it fails instantly, no time to click on the link to validate the code, even if the code expires in five minutes.

I'll try to debug more, but if you encountered this issue already, I'm interested for some guidance

@kevinchalet
Copy link
Member

I'll try to debug more, but if you encountered this issue already, I'm interested for some guidance

Looks like their authorization_pending error response is not standard, so OpenIddict doesn't even retry to send the token request until the demand is approved or expires:

image

(the parameter must be named error, not message)

If you want to support Twitch's non-standard flow, you'll need to tweak this handler to map the non-standard parameter to its standard equivalent:

/// <summary>
/// Contains the logic responsible for mapping non-standard response parameters
/// to their standard equivalent for the providers that require it.
/// </summary>
public sealed class MapNonStandardResponseParameters : IOpenIddictClientHandler<ExtractTokenResponseContext>
{
/// <summary>
/// Gets the default descriptor definition assigned to this handler.
/// </summary>
public static OpenIddictClientHandlerDescriptor Descriptor { get; }
= OpenIddictClientHandlerDescriptor.CreateBuilder<ExtractTokenResponseContext>()
.UseSingletonHandler<MapNonStandardResponseParameters>()
.SetOrder(int.MaxValue - 50_000)
.SetType(OpenIddictClientHandlerType.BuiltIn)
.Build();
/// <inheritdoc/>
public ValueTask HandleAsync(ExtractTokenResponseContext context)
{
if (context is null)
{
throw new ArgumentNullException(nameof(context));
}
if (context.Response is null)
{
return default;
}
// Note: when using the client credentials grant, Dailymotion returns a "refresh_token"
// node with a JSON null value, which isn't allowed by OpenIddict (that requires a string).
//
// To work around that, the "refresh_token" node is removed when it is set to a null value .
if (context.Registration.ProviderType is ProviderTypes.Dailymotion && (JsonElement?)
context.Response[Parameters.RefreshToken] is { ValueKind: JsonValueKind.Null })
{
context.Response.RefreshToken = null;
}
// Note: Deezer doesn't return a standard "expires_in" parameter
// but returns an equivalent "expires" integer parameter instead.
if (context.Registration.ProviderType is ProviderTypes.Deezer)
{
context.Response[Parameters.ExpiresIn] = context.Response["expires"];
context.Response["expires"] = null;
}
// Note: Exact Online returns a non-standard "expires_in"
// parameter formatted as a string instead of a numeric type.
else if (context.Registration.ProviderType is ProviderTypes.ExactOnline &&
long.TryParse((string?) context.Response[Parameters.ExpiresIn],
NumberStyles.Integer, CultureInfo.InvariantCulture, out long value))
{
context.Response.ExpiresIn = value;
}
// Note: Huawei returns a non-standard "error" parameter as a numeric value, which is not allowed
// by OpenIddict (that requires a string). Huawei also returns a non-standard "sub_error" parameter
// that contains additional error information, with which the error code can demonstrate a specific
// meaning. To work around that, the "error" parameter is replaced with a standard error code.
// When the error code is "1101", the sub-error code of "20411" indicates that the device code
// authorization request is still waiting for the user to access the authorization page; the
// sub-error code of "20412" indicates that the user has not performed the device code authorization;
// the sub-error code of "20414" indicates that the user has denied the device code authorization.
// For more information about the error codes, sub-error codes, and their meanings, see:
// https://developer.huawei.com/consumer/en/doc/HMSCore-Guides/open-platform-error-0000001053869182#section6581130161218
else if (context.Registration.ProviderType is ProviderTypes.Huawei)
{
context.Response[Parameters.Error] =
((long?) context.Response[Parameters.Error], (long?) context.Response["sub_error"]) switch
{
(1101, 20404) => Errors.ExpiredToken,
(1101, 20411 or 20412) => Errors.AuthorizationPending,
(1101, 20414) => Errors.AccessDenied,
(not null, _) => Errors.InvalidRequest,
_ => null,
};
}
// Note: Tumblr returns a non-standard "id_token: false" node that collides
// with the standard id_token parameter used in OpenID Connect. To ensure
// the response is not rejected, the "id_token" node is manually removed.
else if (context.Registration.ProviderType is ProviderTypes.Tumblr)
{
context.Response["id_token"] = null;
}
return default;
}
}

@mbaumanndev
Copy link
Contributor Author

I'll try to debug more, but if you encountered this issue already, I'm interested for some guidance

Looks like their authorization_pending error response is not standard, so OpenIddict doesn't even retry to send the token request until the demand is approved or expires:

I just found this too before seeing your comment, thanks for your response, I'll check the mappings. I begin to figure out how it's working, but not knowing the standard make it hard, but that's fun 😄

@mbaumanndev
Copy link
Contributor Author

I can't figure why, even with a breakpoint in MapNonStandardResponseParameters, I don't enter its HandleAsync method, I kept falling in ValidateHttpResponse own HandleAsync first.

I'll stop for today, I'll give a try tomorrow to check why it happens.

@kevinchalet
Copy link
Member

kevinchalet commented Jul 19, 2024

I can't figure why, even with a breakpoint in MapNonStandardResponseParameters, I don't enter its HandleAsync method, I kept falling in ValidateHttpResponse own HandleAsync first.

Ah crap, do they return a non-200 response code?! (so the status code would be returned as a proper HTTP status AND as a custom status parameter? 🤣)

If so, we'll need a new MapNonStandardError handler that executes before ValidateHttpResponse (ValidateHttpResponse<ExtractTokenResponseContext>.Descriptor.Order - 500) 😅

@mbaumanndev
Copy link
Contributor Author

I can't figure why, even with a breakpoint in MapNonStandardResponseParameters, I don't enter its HandleAsync method, I kept falling in ValidateHttpResponse own HandleAsync first.

Ah crap, do they return a non-200 response code?! (so the status code would be returned as a proper HTTP status AND as a custom status parameter? 🤣)

If so, we'll need a new MapNonStandardError handler that executes before ValidateHttpResponse (ValidateHttpResponse<ExtractTokenResponseContext>.Descriptor.Order - 500) 😅

Oh, it make sense, I'll check tomorrow but I'm pretty sure they send a 400, as it's catched as a bad request by the handler

@kevinchalet
Copy link
Member

Thanks! 👍🏻

@mbaumanndev
Copy link
Contributor Author

I began to check for today, I don't know where to put the new handler : should it belong to WebIntergration as it's specific to Twitch ? If so, how do I check that I have a 400 at that level.

Or should I put it in SystemNetHttp to catch the 400 ?, If so, how should I make it specific as I don't have access to the provider types here ?

I'll try to dig more into the problem in the meantime

@kevinchalet
Copy link
Member

kevinchalet commented Jul 20, 2024

Thanks for asking 😃

I began to check for today, I don't know where to put the new handler : should it belong to WebIntergration as it's specific to Twitch ?

The rule is that any provider-specific quirk must live in the OpenIddict.Client.WebIntegration package (that itself depends on the OpenIddict.Client.SystemNetHttp package). Only fully standard things are allowed in the OpenIddict.Client or OpenIddict.Client.SystemNetHttp packages 😃

If so, how do I check that I have a 400 at that level.

You can access the HttpResponseMessage using context.Transaction.GetHttpResponseMessage(). Here's an example in the same project:

/// <summary>
/// Contains the logic responsible for normalizing the returned content
/// type of userinfo responses for the providers that require it.
/// </summary>
public sealed class NormalizeContentType : IOpenIddictClientHandler<ExtractUserinfoResponseContext>
{
/// <summary>
/// Gets the default descriptor definition assigned to this handler.
/// </summary>
public static OpenIddictClientHandlerDescriptor Descriptor { get; }
= OpenIddictClientHandlerDescriptor.CreateBuilder<ExtractUserinfoResponseContext>()
.UseSingletonHandler<NormalizeContentType>()
.SetOrder(ExtractUserinfoTokenHttpResponse.Descriptor.Order - 250)
.SetType(OpenIddictClientHandlerType.BuiltIn)
.Build();
/// <inheritdoc/>
public ValueTask HandleAsync(ExtractUserinfoResponseContext context)
{
if (context is null)
{
throw new ArgumentNullException(nameof(context));
}
// This handler only applies to System.Net.Http requests. If the HTTP response cannot be resolved,
// this may indicate that the request was incorrectly processed by another client stack.
var response = context.Transaction.GetHttpResponseMessage() ??
throw new InvalidOperationException(SR.GetResourceString(SR.ID0173));
if (response.Content is null)
{
return default;
}
// Some providers are known to return invalid or incorrect media types, which prevents
// OpenIddict from extracting userinfo responses. To work around that, the declared
// content type is replaced by the correct value for the providers that require it.
response.Content.Headers.ContentType = context.Registration.ProviderType switch
{
// Mixcloud returns JSON-formatted contents declared as "text/javascript".
ProviderTypes.Mixcloud when string.Equals(
response.Content.Headers.ContentType?.MediaType,
"text/javascript", StringComparison.OrdinalIgnoreCase)
=> new MediaTypeHeaderValue(MediaTypes.Json)
{
CharSet = Charsets.Utf8
},
// Wikimedia returns JSON-formatted contents declared as "text/html".
ProviderTypes.Wikimedia when string.Equals(
response.Content.Headers.ContentType?.MediaType,
"text/html", StringComparison.OrdinalIgnoreCase)
=> new MediaTypeHeaderValue(MediaTypes.Json)
{
CharSet = Charsets.Utf8
},
_ => response.Content.Headers.ContentType
};
return default;
}
}

@kevinchalet
Copy link
Member

Let me know if you still need help (or we can just pretend Twitch doesn't support the device authorization flow and make our lives easier 😄)

@mbaumanndev
Copy link
Contributor Author

I did not had the time to check longer this week-end, I may have some time today or tomorrow :)

@mbaumanndev
Copy link
Contributor Author

@kevinchalet Sorry for the delay, I had some plans that changed a few things in my life recently, I'm not sure I will have some time to code as a hobby or to contribute more in the next few months (if so, I'll be happy to contribute, I really want to do this device code flow ^^)

For now we can do as you suggested : drop the device flow. I tested with the console, everything works fine.

I'll push my wip on my fork if anyone want to take a look at it, but I'll try to open a PR for the device flow before the end of october :)

@kevinchalet
Copy link
Member

No problem, @mbaumanndev. Hope you're doing well.

Let's do that 👍🏻

@mbaumanndev
Copy link
Contributor Author

No problem, @mbaumanndev. Hope you're doing well.

I'm doing well, thanks ! I'll just be a little more busy than expected with work 😅

@kevinchalet kevinchalet merged commit 19b69d5 into openiddict:dev Jul 30, 2024
3 checks passed
@kevinchalet kevinchalet added this to the 5.8.0 milestone Jul 30, 2024
@kevinchalet
Copy link
Member

Merged. Merci beaucoup pour ta contribution ! 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants