-
-
Notifications
You must be signed in to change notification settings - Fork 584
Adds Linear, Miro, and Webflow web providers #2284
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
Changes from all commits
d602bdb
ac458d1
9a4268a
65781c8
eea9fd9
d8a2e1d
45ec137
50f538c
67bff41
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,8 +5,11 @@ | |
| */ | ||
|
|
||
| using System.Collections.Immutable; | ||
| using System.Diagnostics; | ||
| using System.Net.Http; | ||
| using System.Net.Http.Headers; | ||
| using System.Net.Http.Json; | ||
| using OpenIddict.Client.SystemNetHttp; | ||
| using static OpenIddict.Client.SystemNetHttp.OpenIddictClientSystemNetHttpHandlerFilters; | ||
| using static OpenIddict.Client.SystemNetHttp.OpenIddictClientSystemNetHttpHandlers; | ||
| using static OpenIddict.Client.WebIntegration.OpenIddictClientWebIntegrationConstants; | ||
|
|
@@ -25,12 +28,64 @@ public static class Revocation | |
| MapNonStandardRequestParameters.Descriptor, | ||
| OverrideHttpMethod.Descriptor, | ||
| AttachBearerAccessToken.Descriptor, | ||
| AttachNonStandardRequestPayload.Descriptor, | ||
|
|
||
| /* | ||
| * Revocation response extraction: | ||
| */ | ||
| NormalizeContentType.Descriptor | ||
| ]; | ||
|
|
||
| /// <summary> | ||
| /// Contains the logic responsible for attaching a non-standard payload for the providers that require it. | ||
| /// </summary> | ||
| public sealed class AttachNonStandardRequestPayload : IOpenIddictClientHandler<PrepareRevocationRequestContext> | ||
| { | ||
| /// <summary> | ||
| /// Gets the default descriptor definition assigned to this handler. | ||
| /// </summary> | ||
| public static OpenIddictClientHandlerDescriptor Descriptor { get; } | ||
| = OpenIddictClientHandlerDescriptor.CreateBuilder<PrepareRevocationRequestContext>() | ||
| .AddFilter<RequireHttpUri>() | ||
| .UseSingletonHandler<AttachNonStandardRequestPayload>() | ||
| .SetOrder(AttachHttpParameters<PrepareRevocationRequestContext>.Descriptor.Order + 500) | ||
| .SetType(OpenIddictClientHandlerType.BuiltIn) | ||
| .Build(); | ||
|
|
||
| /// <inheritdoc/> | ||
| public ValueTask HandleAsync(PrepareRevocationRequestContext context) | ||
| { | ||
| if (context is null) | ||
| { | ||
| throw new ArgumentNullException(nameof(context)); | ||
| } | ||
|
|
||
| Debug.Assert(context.Transaction.Request is not null, SR.GetResourceString(SR.ID4008)); | ||
|
|
||
| // This handler only applies to System.Net.Http requests. If the HTTP request cannot be resolved, | ||
| // this may indicate that the request was incorrectly processed by another client stack. | ||
| var request = context.Transaction.GetHttpRequestMessage() ?? | ||
| throw new InvalidOperationException(SR.GetResourceString(SR.ID0173)); | ||
|
|
||
| request.Content = context.Registration.ProviderType switch | ||
| { | ||
| // The token revocation endpoints exposed by these providers | ||
| // requires sending the request parameters as a JSON payload: | ||
| ProviderTypes.Miro => JsonContent.Create( | ||
| context.Transaction.Request, | ||
| OpenIddictSerializer.Default.Request, | ||
| new MediaTypeHeaderValue(OpenIddictClientSystemNetHttpConstants.MediaTypes.Json) | ||
| { | ||
| CharSet = OpenIddictClientSystemNetHttpConstants.Charsets.Utf8 | ||
| }), | ||
|
|
||
| _ => request.Content | ||
| }; | ||
|
|
||
| return default; | ||
| } | ||
| } | ||
|
|
||
|
|
||
| /// <summary> | ||
| /// Contains the logic responsible for mapping non-standard request parameters | ||
|
|
@@ -56,13 +111,37 @@ public ValueTask HandleAsync(PrepareRevocationRequestContext context) | |
| throw new ArgumentNullException(nameof(context)); | ||
| } | ||
|
|
||
| // Weibo, VK ID and Yandex don't support the standard "token" parameter and | ||
| // These providers don't support the standard "token" parameter and | ||
| // require using the non-standard "access_token" parameter instead. | ||
| if (context.Registration.ProviderType is ProviderTypes.Weibo or ProviderTypes.VkId or ProviderTypes.Yandex) | ||
| if (context.Registration.ProviderType is | ||
| ProviderTypes.VkId or ProviderTypes.Webflow or | ||
| ProviderTypes.Weibo or ProviderTypes.Yandex) | ||
| { | ||
| context.Request.AccessToken = context.Token; | ||
| context.Request.Token = null; | ||
| context.Request.TokenTypeHint = null; | ||
| } | ||
|
|
||
| // Linear requires only the access_token and no other parameters. | ||
| else if (context.Registration.ProviderType is ProviderTypes.Linear) | ||
| { | ||
| context.Request.AccessToken = context.Token; | ||
| context.Request.Token = null; | ||
| context.Request.TokenTypeHint = null; | ||
| context.Request.ClientId = null; | ||
| } | ||
|
|
||
| // Miro uses a JSON payload that expects the non-standard | ||
| // "accessToken", "clientId" and "clientSecret" properties. | ||
| else if (context.Registration.ProviderType is ProviderTypes.Miro) | ||
| { | ||
| context.Request["accessToken"] = context.Token; | ||
| context.Request["clientId"] = context.Request.ClientId; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting, I don't see these parameters mentioned in https://developers.linear.app/docs/oauth/authentication#id-6.-revoke-an-access-token. Are we sure their non-standard revocation endpoint supports client authentication?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, Linear is an interesting one. Before I updated the parameters according to their docs, I tested the revocation and it worked: So, in the initial PR, I did not make the code changes according to their docs. But, it is clearly risky doing this as it is not documented to work this way, and they can easily change it in the future so it does not work correctly anymore. I have now updated the code to conform to their docs: |
||
| context.Request["clientSecret"] = context.Request.ClientSecret; | ||
| context.Request.Token = null; | ||
| context.Request.TokenTypeHint = null; | ||
| context.Request.ClientId = null; | ||
| context.Request.ClientSecret = null; | ||
| } | ||
|
|
||
| return default; | ||
|
|
@@ -148,6 +227,15 @@ public ValueTask HandleAsync(PrepareRevocationRequestContext context) | |
| context.Request.Token = null; | ||
| } | ||
|
|
||
| // Miro requires using bearer authentication with the token that is going to be revoked. | ||
| // | ||
| // Note: the token property CANNOT be used here as the token parameter is mapped to "accessToken". | ||
| else if (context.Registration.ProviderType is ProviderTypes.Miro && | ||
| (string?) context.Request["accessToken"] is { Length: > 0 } token) | ||
| { | ||
| request.Headers.Authorization = new AuthenticationHeaderValue(Schemes.Bearer, token); | ||
| } | ||
|
|
||
| return default; | ||
| } | ||
| } | ||
|
|
||


Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you're going to send the access token in the
Authorizationheader, there's no real point mapping it here. And if you don't nullifycontext.Request.Tokenhere, you can reuse the same branch as theZendeskprovider in theAttachBearerAccessTokenhandler 😃There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Miro requires both the
accessTokenproperty in the JSON payload and the Bearer token. If I send just the Bearer token without theaccessTokenproperty, I get an error:Likewise, if I omit the Bearer token and send just the
accessTokenproperty, it fails as well:I need to supply both to get a successful response:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeah, I thought that branch was for the Linear provider... that uses a single
access_tokenparameter 🤣