From f014ddb2f7adbf168dcb2a87b5b650b22f7bf350 Mon Sep 17 00:00:00 2001 From: martincostello Date: Thu, 21 Mar 2024 15:45:35 +0000 Subject: [PATCH 1/3] Use AdditionalAuthorizationParameters - Ensure that overrides of `BuildChallengeUrl()` that do not call the base implementation add the values from the new `AdditionalAuthorizationParameters` property. - Move all static challenge parameters into the `AdditionalAuthorizationParameters` property on the options. - Remove `BuildChallengeUrl()` overrides, where possible, to just use `AdditionalAuthorizationParameters` instead. --- .../AlipayAuthenticationHandler.cs | 9 ++++++++- .../AlipayAuthenticationOptions.cs | 2 ++ .../AppleAuthenticationHandler.cs | 12 ------------ .../AppleAuthenticationOptions.cs | 3 +++ .../DeezerAuthenticationHandler.cs | 8 ++++++++ .../LineAuthenticationHandler.cs | 9 ++++++++- .../LineAuthenticationOptions.cs | 2 ++ .../MixcloudAuthenticationHandler.cs | 9 ++++++++- .../MixcloudAuthenticationOptions.cs | 2 ++ .../RedditAuthenticationHandler.cs | 9 --------- .../RedditAuthenticationOptions.cs | 4 ++++ .../ShopifyAuthenticationHandler.cs | 8 ++++++++ .../WeixinAuthenticationHandler.cs | 9 ++++++++- .../WeixinAuthenticationOptions.cs | 2 ++ .../WorkWeixinAuthenticationHandler.cs | 8 ++++++++ 15 files changed, 71 insertions(+), 25 deletions(-) diff --git a/src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationHandler.cs index 513bb3bf5..ddb8cd0e5 100644 --- a/src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationHandler.cs @@ -210,10 +210,17 @@ protected override string BuildChallengeUrl([NotNull] AuthenticationProperties p { ["app_id"] = Options.ClientId, // Used instead of "client_id" ["scope"] = scope, - ["response_type"] = "code", ["redirect_uri"] = redirectUri, }; + if (Options.AdditionalAuthorizationParameters?.Count > 0) + { + foreach (var parameter in Options.AdditionalAuthorizationParameters) + { + parameters[parameter.Key] = parameter.Value; + } + } + if (Options.UsePkce) { var bytes = RandomNumberGenerator.GetBytes(256 / 8); diff --git a/src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationOptions.cs b/src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationOptions.cs index 08677ddd5..73867b698 100644 --- a/src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationOptions.cs +++ b/src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationOptions.cs @@ -22,6 +22,8 @@ public AlipayAuthenticationOptions() TokenEndpoint = AlipayAuthenticationDefaults.TokenEndpoint; UserInformationEndpoint = AlipayAuthenticationDefaults.UserInformationEndpoint; + AdditionalAuthorizationParameters["response_type"] = "code"; + Scope.Add("auth_user"); ClaimActions.MapJsonKey(Claims.Avatar, "avatar"); diff --git a/src/AspNet.Security.OAuth.Apple/AppleAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Apple/AppleAuthenticationHandler.cs index fb26c159e..de41b336e 100644 --- a/src/AspNet.Security.OAuth.Apple/AppleAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Apple/AppleAuthenticationHandler.cs @@ -9,7 +9,6 @@ using System.Text; using System.Text.Encodings.Web; using System.Text.Json; -using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; @@ -37,17 +36,6 @@ public partial class AppleAuthenticationHandler( set { base.Events = value; } } - /// - protected override string BuildChallengeUrl( - [NotNull] AuthenticationProperties properties, - [NotNull] string redirectUri) - { - var challengeUrl = base.BuildChallengeUrl(properties, redirectUri); - - // Apple requires the response mode to be form_post when the email or name scopes are requested - return QueryHelpers.AddQueryString(challengeUrl, "response_mode", "form_post"); - } - /// protected override Task CreateEventsAsync() => Task.FromResult(new AppleAuthenticationEvents()); diff --git a/src/AspNet.Security.OAuth.Apple/AppleAuthenticationOptions.cs b/src/AspNet.Security.OAuth.Apple/AppleAuthenticationOptions.cs index a02f9263c..8cfcc4bcd 100644 --- a/src/AspNet.Security.OAuth.Apple/AppleAuthenticationOptions.cs +++ b/src/AspNet.Security.OAuth.Apple/AppleAuthenticationOptions.cs @@ -29,6 +29,9 @@ public AppleAuthenticationOptions() Events = new AppleAuthenticationEvents(); + // Apple requires the response mode to be form_post when the email or name scopes are requested + AdditionalAuthorizationParameters["response_mode"] = "form_post"; + Scope.Add("openid"); Scope.Add("name"); Scope.Add("email"); diff --git a/src/AspNet.Security.OAuth.Deezer/DeezerAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Deezer/DeezerAuthenticationHandler.cs index 035531df5..f18c7429e 100644 --- a/src/AspNet.Security.OAuth.Deezer/DeezerAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Deezer/DeezerAuthenticationHandler.cs @@ -98,6 +98,14 @@ protected override string BuildChallengeUrl([NotNull] AuthenticationProperties p ["perms"] = scopes, }; + if (Options.AdditionalAuthorizationParameters?.Count > 0) + { + foreach (var parameter in Options.AdditionalAuthorizationParameters) + { + parameters[parameter.Key] = parameter.Value; + } + } + if (Options.UsePkce) { var bytes = RandomNumberGenerator.GetBytes(256 / 8); diff --git a/src/AspNet.Security.OAuth.Line/LineAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Line/LineAuthenticationHandler.cs index aa7eb6abe..0b8620bee 100644 --- a/src/AspNet.Security.OAuth.Line/LineAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Line/LineAuthenticationHandler.cs @@ -35,13 +35,20 @@ protected override async Task ExchangeCodeAsync([NotNull] OA { var tokenRequestParameters = new Dictionary { - ["grant_type"] = "authorization_code", ["code"] = context.Code, ["redirect_uri"] = context.RedirectUri, ["client_id"] = Options.ClientId, ["client_secret"] = Options.ClientSecret, }; + if (Options.AdditionalAuthorizationParameters?.Count > 0) + { + foreach (var parameter in Options.AdditionalAuthorizationParameters) + { + tokenRequestParameters[parameter.Key] = parameter.Value; + } + } + // PKCE https://tools.ietf.org/html/rfc7636#section-4.5, see BuildChallengeUrl if (context.Properties.Items.TryGetValue(OAuthConstants.CodeVerifierKey, out var codeVerifier)) { diff --git a/src/AspNet.Security.OAuth.Line/LineAuthenticationOptions.cs b/src/AspNet.Security.OAuth.Line/LineAuthenticationOptions.cs index 8696457bd..3163d35aa 100644 --- a/src/AspNet.Security.OAuth.Line/LineAuthenticationOptions.cs +++ b/src/AspNet.Security.OAuth.Line/LineAuthenticationOptions.cs @@ -23,6 +23,8 @@ public LineAuthenticationOptions() TokenEndpoint = LineAuthenticationDefaults.TokenEndpoint; UserInformationEndpoint = LineAuthenticationDefaults.UserInformationEndpoint; + AdditionalAuthorizationParameters["grant_type"] = "authorization_code"; + Scope.Add("profile"); Scope.Add("openid"); diff --git a/src/AspNet.Security.OAuth.Mixcloud/MixcloudAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Mixcloud/MixcloudAuthenticationHandler.cs index 6319f7f79..879727301 100644 --- a/src/AspNet.Security.OAuth.Mixcloud/MixcloudAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Mixcloud/MixcloudAuthenticationHandler.cs @@ -35,9 +35,16 @@ protected override string BuildChallengeUrl([NotNull] AuthenticationProperties p { ["client_id"] = Options.ClientId, ["scope"] = scope, - ["response_type"] = "code", }; + if (Options.AdditionalAuthorizationParameters?.Count > 0) + { + foreach (var parameter in Options.AdditionalAuthorizationParameters) + { + parameters[parameter.Key] = parameter.Value; + } + } + if (Options.UsePkce) { var bytes = RandomNumberGenerator.GetBytes(256 / 8); diff --git a/src/AspNet.Security.OAuth.Mixcloud/MixcloudAuthenticationOptions.cs b/src/AspNet.Security.OAuth.Mixcloud/MixcloudAuthenticationOptions.cs index ef19d956f..649342799 100644 --- a/src/AspNet.Security.OAuth.Mixcloud/MixcloudAuthenticationOptions.cs +++ b/src/AspNet.Security.OAuth.Mixcloud/MixcloudAuthenticationOptions.cs @@ -23,6 +23,8 @@ public MixcloudAuthenticationOptions() TokenEndpoint = MixcloudAuthenticationDefaults.TokenEndpoint; UserInformationEndpoint = MixcloudAuthenticationDefaults.UserInformationEndpoint; + AdditionalAuthorizationParameters["response_type"] = "code"; + ClaimActions.MapJsonKey(ClaimTypes.NameIdentifier, "key"); ClaimActions.MapJsonKey(ClaimTypes.Name, "username"); ClaimActions.MapJsonKey(Claims.FullName, "name"); diff --git a/src/AspNet.Security.OAuth.Reddit/RedditAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Reddit/RedditAuthenticationHandler.cs index 033bdba0d..c6bae4d30 100644 --- a/src/AspNet.Security.OAuth.Reddit/RedditAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Reddit/RedditAuthenticationHandler.cs @@ -58,15 +58,6 @@ protected override async Task CreateTicketAsync( return new AuthenticationTicket(context.Principal!, context.Properties, Scheme.Name); } - protected override string BuildChallengeUrl([NotNull] AuthenticationProperties properties, [NotNull] string redirectUri) - { - var challengeUrl = base.BuildChallengeUrl(properties, redirectUri); - - // Add duration=permanent to the authorization request to get an access token that doesn't expire after 1 hour. - // See https://github.com/reddit/reddit/wiki/OAuth2#authorization for more information. - return QueryHelpers.AddQueryString(challengeUrl, "duration", "permanent"); - } - /// protected override string FormatScope([NotNull] IEnumerable scopes) { diff --git a/src/AspNet.Security.OAuth.Reddit/RedditAuthenticationOptions.cs b/src/AspNet.Security.OAuth.Reddit/RedditAuthenticationOptions.cs index 95a25fc0b..5d5406c1c 100644 --- a/src/AspNet.Security.OAuth.Reddit/RedditAuthenticationOptions.cs +++ b/src/AspNet.Security.OAuth.Reddit/RedditAuthenticationOptions.cs @@ -24,6 +24,10 @@ public RedditAuthenticationOptions() TokenEndpoint = RedditAuthenticationDefaults.TokenEndpoint; UserInformationEndpoint = RedditAuthenticationDefaults.UserInformationEndpoint; + // Add duration=permanent to the authorization request to get an access token that doesn't expire after 1 hour. + // See https://github.com/reddit/reddit/wiki/OAuth2#authorization for more information. + AdditionalAuthorizationParameters["duration"] = "permanent"; + Scope.Add("identity"); ClaimActions.MapJsonKey(ClaimTypes.NameIdentifier, "id"); diff --git a/src/AspNet.Security.OAuth.Shopify/ShopifyAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Shopify/ShopifyAuthenticationHandler.cs index 9ed22158f..6ecb08cb2 100644 --- a/src/AspNet.Security.OAuth.Shopify/ShopifyAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Shopify/ShopifyAuthenticationHandler.cs @@ -116,6 +116,14 @@ protected override string BuildChallengeUrl([NotNull] AuthenticationProperties p ["redirect_uri"] = redirectUri, }; + if (Options.AdditionalAuthorizationParameters?.Count > 0) + { + foreach (var parameter in Options.AdditionalAuthorizationParameters) + { + parameters[parameter.Key] = parameter.Value; + } + } + if (Options.UsePkce) { var bytes = RandomNumberGenerator.GetBytes(256 / 8); diff --git a/src/AspNet.Security.OAuth.Weixin/WeixinAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Weixin/WeixinAuthenticationHandler.cs index 60d9650c9..e43f3053b 100644 --- a/src/AspNet.Security.OAuth.Weixin/WeixinAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Weixin/WeixinAuthenticationHandler.cs @@ -131,9 +131,16 @@ protected override string BuildChallengeUrl([NotNull] AuthenticationProperties p { ["appid"] = Options.ClientId, ["scope"] = scope, - ["response_type"] = "code", }; + if (Options.AdditionalAuthorizationParameters?.Count > 0) + { + foreach (var parameter in Options.AdditionalAuthorizationParameters) + { + parameters[parameter.Key] = parameter.Value; + } + } + if (Options.UsePkce) { var bytes = RandomNumberGenerator.GetBytes(256 / 8); diff --git a/src/AspNet.Security.OAuth.Weixin/WeixinAuthenticationOptions.cs b/src/AspNet.Security.OAuth.Weixin/WeixinAuthenticationOptions.cs index ecb81032d..7209a71fa 100644 --- a/src/AspNet.Security.OAuth.Weixin/WeixinAuthenticationOptions.cs +++ b/src/AspNet.Security.OAuth.Weixin/WeixinAuthenticationOptions.cs @@ -23,6 +23,8 @@ public WeixinAuthenticationOptions() TokenEndpoint = WeixinAuthenticationDefaults.TokenEndpoint; UserInformationEndpoint = WeixinAuthenticationDefaults.UserInformationEndpoint; + AdditionalAuthorizationParameters["response_type"] = "code"; + Scope.Add("snsapi_login"); Scope.Add("snsapi_userinfo"); diff --git a/src/AspNet.Security.OAuth.WorkWeixin/WorkWeixinAuthenticationHandler.cs b/src/AspNet.Security.OAuth.WorkWeixin/WorkWeixinAuthenticationHandler.cs index 5a6af03da..ac777cb03 100644 --- a/src/AspNet.Security.OAuth.WorkWeixin/WorkWeixinAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.WorkWeixin/WorkWeixinAuthenticationHandler.cs @@ -110,6 +110,14 @@ protected override string BuildChallengeUrl([NotNull] AuthenticationProperties p ["redirect_uri"] = redirectUri, }; + if (Options.AdditionalAuthorizationParameters?.Count > 0) + { + foreach (var parameter in Options.AdditionalAuthorizationParameters) + { + parameters[parameter.Key] = parameter.Value; + } + } + if (Options.UsePkce) { var bytes = RandomNumberGenerator.GetBytes(256 / 8); From 19ad9b207180b320c2f1a7931d2622a13a2b5e39 Mon Sep 17 00:00:00 2001 From: martincostello Date: Thu, 21 Mar 2024 17:21:22 +0000 Subject: [PATCH 2/3] Address feedback - Remove redundant null checks. - Revert changes to move static parameters into `AdditionalAuthorizationParameters`. --- .../AlipayAuthenticationHandler.cs | 3 ++- .../AlipayAuthenticationOptions.cs | 2 -- .../AppleAuthenticationHandler.cs | 12 ++++++++++++ .../AppleAuthenticationOptions.cs | 3 --- .../DeezerAuthenticationHandler.cs | 2 +- .../LineAuthenticationHandler.cs | 3 ++- .../LineAuthenticationOptions.cs | 2 -- .../MixcloudAuthenticationHandler.cs | 3 ++- .../MixcloudAuthenticationOptions.cs | 2 -- .../RedditAuthenticationHandler.cs | 9 +++++++++ .../RedditAuthenticationOptions.cs | 4 ---- .../ShopifyAuthenticationHandler.cs | 2 +- .../WeixinAuthenticationHandler.cs | 3 ++- .../WeixinAuthenticationOptions.cs | 2 -- .../WorkWeixinAuthenticationHandler.cs | 2 +- 15 files changed, 32 insertions(+), 22 deletions(-) diff --git a/src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationHandler.cs index ddb8cd0e5..f8617cffe 100644 --- a/src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationHandler.cs @@ -210,10 +210,11 @@ protected override string BuildChallengeUrl([NotNull] AuthenticationProperties p { ["app_id"] = Options.ClientId, // Used instead of "client_id" ["scope"] = scope, + ["response_type"] = "code", ["redirect_uri"] = redirectUri, }; - if (Options.AdditionalAuthorizationParameters?.Count > 0) + if (Options.AdditionalAuthorizationParameters.Count > 0) { foreach (var parameter in Options.AdditionalAuthorizationParameters) { diff --git a/src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationOptions.cs b/src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationOptions.cs index 73867b698..08677ddd5 100644 --- a/src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationOptions.cs +++ b/src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationOptions.cs @@ -22,8 +22,6 @@ public AlipayAuthenticationOptions() TokenEndpoint = AlipayAuthenticationDefaults.TokenEndpoint; UserInformationEndpoint = AlipayAuthenticationDefaults.UserInformationEndpoint; - AdditionalAuthorizationParameters["response_type"] = "code"; - Scope.Add("auth_user"); ClaimActions.MapJsonKey(Claims.Avatar, "avatar"); diff --git a/src/AspNet.Security.OAuth.Apple/AppleAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Apple/AppleAuthenticationHandler.cs index de41b336e..fb26c159e 100644 --- a/src/AspNet.Security.OAuth.Apple/AppleAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Apple/AppleAuthenticationHandler.cs @@ -9,6 +9,7 @@ using System.Text; using System.Text.Encodings.Web; using System.Text.Json; +using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; @@ -36,6 +37,17 @@ public partial class AppleAuthenticationHandler( set { base.Events = value; } } + /// + protected override string BuildChallengeUrl( + [NotNull] AuthenticationProperties properties, + [NotNull] string redirectUri) + { + var challengeUrl = base.BuildChallengeUrl(properties, redirectUri); + + // Apple requires the response mode to be form_post when the email or name scopes are requested + return QueryHelpers.AddQueryString(challengeUrl, "response_mode", "form_post"); + } + /// protected override Task CreateEventsAsync() => Task.FromResult(new AppleAuthenticationEvents()); diff --git a/src/AspNet.Security.OAuth.Apple/AppleAuthenticationOptions.cs b/src/AspNet.Security.OAuth.Apple/AppleAuthenticationOptions.cs index 8cfcc4bcd..a02f9263c 100644 --- a/src/AspNet.Security.OAuth.Apple/AppleAuthenticationOptions.cs +++ b/src/AspNet.Security.OAuth.Apple/AppleAuthenticationOptions.cs @@ -29,9 +29,6 @@ public AppleAuthenticationOptions() Events = new AppleAuthenticationEvents(); - // Apple requires the response mode to be form_post when the email or name scopes are requested - AdditionalAuthorizationParameters["response_mode"] = "form_post"; - Scope.Add("openid"); Scope.Add("name"); Scope.Add("email"); diff --git a/src/AspNet.Security.OAuth.Deezer/DeezerAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Deezer/DeezerAuthenticationHandler.cs index f18c7429e..ba15d9f06 100644 --- a/src/AspNet.Security.OAuth.Deezer/DeezerAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Deezer/DeezerAuthenticationHandler.cs @@ -98,7 +98,7 @@ protected override string BuildChallengeUrl([NotNull] AuthenticationProperties p ["perms"] = scopes, }; - if (Options.AdditionalAuthorizationParameters?.Count > 0) + if (Options.AdditionalAuthorizationParameters.Count > 0) { foreach (var parameter in Options.AdditionalAuthorizationParameters) { diff --git a/src/AspNet.Security.OAuth.Line/LineAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Line/LineAuthenticationHandler.cs index 0b8620bee..30f97d8da 100644 --- a/src/AspNet.Security.OAuth.Line/LineAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Line/LineAuthenticationHandler.cs @@ -35,13 +35,14 @@ protected override async Task ExchangeCodeAsync([NotNull] OA { var tokenRequestParameters = new Dictionary { + ["grant_type"] = "authorization_code", ["code"] = context.Code, ["redirect_uri"] = context.RedirectUri, ["client_id"] = Options.ClientId, ["client_secret"] = Options.ClientSecret, }; - if (Options.AdditionalAuthorizationParameters?.Count > 0) + if (Options.AdditionalAuthorizationParameters.Count > 0) { foreach (var parameter in Options.AdditionalAuthorizationParameters) { diff --git a/src/AspNet.Security.OAuth.Line/LineAuthenticationOptions.cs b/src/AspNet.Security.OAuth.Line/LineAuthenticationOptions.cs index 3163d35aa..8696457bd 100644 --- a/src/AspNet.Security.OAuth.Line/LineAuthenticationOptions.cs +++ b/src/AspNet.Security.OAuth.Line/LineAuthenticationOptions.cs @@ -23,8 +23,6 @@ public LineAuthenticationOptions() TokenEndpoint = LineAuthenticationDefaults.TokenEndpoint; UserInformationEndpoint = LineAuthenticationDefaults.UserInformationEndpoint; - AdditionalAuthorizationParameters["grant_type"] = "authorization_code"; - Scope.Add("profile"); Scope.Add("openid"); diff --git a/src/AspNet.Security.OAuth.Mixcloud/MixcloudAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Mixcloud/MixcloudAuthenticationHandler.cs index 879727301..a676bbbcb 100644 --- a/src/AspNet.Security.OAuth.Mixcloud/MixcloudAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Mixcloud/MixcloudAuthenticationHandler.cs @@ -35,9 +35,10 @@ protected override string BuildChallengeUrl([NotNull] AuthenticationProperties p { ["client_id"] = Options.ClientId, ["scope"] = scope, + ["response_type"] = "code", }; - if (Options.AdditionalAuthorizationParameters?.Count > 0) + if (Options.AdditionalAuthorizationParameters.Count > 0) { foreach (var parameter in Options.AdditionalAuthorizationParameters) { diff --git a/src/AspNet.Security.OAuth.Mixcloud/MixcloudAuthenticationOptions.cs b/src/AspNet.Security.OAuth.Mixcloud/MixcloudAuthenticationOptions.cs index 649342799..ef19d956f 100644 --- a/src/AspNet.Security.OAuth.Mixcloud/MixcloudAuthenticationOptions.cs +++ b/src/AspNet.Security.OAuth.Mixcloud/MixcloudAuthenticationOptions.cs @@ -23,8 +23,6 @@ public MixcloudAuthenticationOptions() TokenEndpoint = MixcloudAuthenticationDefaults.TokenEndpoint; UserInformationEndpoint = MixcloudAuthenticationDefaults.UserInformationEndpoint; - AdditionalAuthorizationParameters["response_type"] = "code"; - ClaimActions.MapJsonKey(ClaimTypes.NameIdentifier, "key"); ClaimActions.MapJsonKey(ClaimTypes.Name, "username"); ClaimActions.MapJsonKey(Claims.FullName, "name"); diff --git a/src/AspNet.Security.OAuth.Reddit/RedditAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Reddit/RedditAuthenticationHandler.cs index c6bae4d30..033bdba0d 100644 --- a/src/AspNet.Security.OAuth.Reddit/RedditAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Reddit/RedditAuthenticationHandler.cs @@ -58,6 +58,15 @@ protected override async Task CreateTicketAsync( return new AuthenticationTicket(context.Principal!, context.Properties, Scheme.Name); } + protected override string BuildChallengeUrl([NotNull] AuthenticationProperties properties, [NotNull] string redirectUri) + { + var challengeUrl = base.BuildChallengeUrl(properties, redirectUri); + + // Add duration=permanent to the authorization request to get an access token that doesn't expire after 1 hour. + // See https://github.com/reddit/reddit/wiki/OAuth2#authorization for more information. + return QueryHelpers.AddQueryString(challengeUrl, "duration", "permanent"); + } + /// protected override string FormatScope([NotNull] IEnumerable scopes) { diff --git a/src/AspNet.Security.OAuth.Reddit/RedditAuthenticationOptions.cs b/src/AspNet.Security.OAuth.Reddit/RedditAuthenticationOptions.cs index 5d5406c1c..95a25fc0b 100644 --- a/src/AspNet.Security.OAuth.Reddit/RedditAuthenticationOptions.cs +++ b/src/AspNet.Security.OAuth.Reddit/RedditAuthenticationOptions.cs @@ -24,10 +24,6 @@ public RedditAuthenticationOptions() TokenEndpoint = RedditAuthenticationDefaults.TokenEndpoint; UserInformationEndpoint = RedditAuthenticationDefaults.UserInformationEndpoint; - // Add duration=permanent to the authorization request to get an access token that doesn't expire after 1 hour. - // See https://github.com/reddit/reddit/wiki/OAuth2#authorization for more information. - AdditionalAuthorizationParameters["duration"] = "permanent"; - Scope.Add("identity"); ClaimActions.MapJsonKey(ClaimTypes.NameIdentifier, "id"); diff --git a/src/AspNet.Security.OAuth.Shopify/ShopifyAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Shopify/ShopifyAuthenticationHandler.cs index 6ecb08cb2..f2a4c4db1 100644 --- a/src/AspNet.Security.OAuth.Shopify/ShopifyAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Shopify/ShopifyAuthenticationHandler.cs @@ -116,7 +116,7 @@ protected override string BuildChallengeUrl([NotNull] AuthenticationProperties p ["redirect_uri"] = redirectUri, }; - if (Options.AdditionalAuthorizationParameters?.Count > 0) + if (Options.AdditionalAuthorizationParameters.Count > 0) { foreach (var parameter in Options.AdditionalAuthorizationParameters) { diff --git a/src/AspNet.Security.OAuth.Weixin/WeixinAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Weixin/WeixinAuthenticationHandler.cs index e43f3053b..b18986350 100644 --- a/src/AspNet.Security.OAuth.Weixin/WeixinAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Weixin/WeixinAuthenticationHandler.cs @@ -131,9 +131,10 @@ protected override string BuildChallengeUrl([NotNull] AuthenticationProperties p { ["appid"] = Options.ClientId, ["scope"] = scope, + ["response_type"] = "code", }; - if (Options.AdditionalAuthorizationParameters?.Count > 0) + if (Options.AdditionalAuthorizationParameters.Count > 0) { foreach (var parameter in Options.AdditionalAuthorizationParameters) { diff --git a/src/AspNet.Security.OAuth.Weixin/WeixinAuthenticationOptions.cs b/src/AspNet.Security.OAuth.Weixin/WeixinAuthenticationOptions.cs index 7209a71fa..ecb81032d 100644 --- a/src/AspNet.Security.OAuth.Weixin/WeixinAuthenticationOptions.cs +++ b/src/AspNet.Security.OAuth.Weixin/WeixinAuthenticationOptions.cs @@ -23,8 +23,6 @@ public WeixinAuthenticationOptions() TokenEndpoint = WeixinAuthenticationDefaults.TokenEndpoint; UserInformationEndpoint = WeixinAuthenticationDefaults.UserInformationEndpoint; - AdditionalAuthorizationParameters["response_type"] = "code"; - Scope.Add("snsapi_login"); Scope.Add("snsapi_userinfo"); diff --git a/src/AspNet.Security.OAuth.WorkWeixin/WorkWeixinAuthenticationHandler.cs b/src/AspNet.Security.OAuth.WorkWeixin/WorkWeixinAuthenticationHandler.cs index ac777cb03..049760807 100644 --- a/src/AspNet.Security.OAuth.WorkWeixin/WorkWeixinAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.WorkWeixin/WorkWeixinAuthenticationHandler.cs @@ -110,7 +110,7 @@ protected override string BuildChallengeUrl([NotNull] AuthenticationProperties p ["redirect_uri"] = redirectUri, }; - if (Options.AdditionalAuthorizationParameters?.Count > 0) + if (Options.AdditionalAuthorizationParameters.Count > 0) { foreach (var parameter in Options.AdditionalAuthorizationParameters) { From dafdb70783600e5394707ee24f8fc2a326c0e2d0 Mon Sep 17 00:00:00 2001 From: martincostello Date: Fri, 22 Mar 2024 12:41:26 +0000 Subject: [PATCH 3/3] Throw on duplicate parameter Copy the pattern from `OAuthHandler.BuildChallengeUrl()`. See https://github.com/dotnet/aspnetcore/blob/ec293ee75c0c022370951a459b188fa81ec8b7c3/src/Security/Authentication/OAuth/src/OAuthHandler.cs#L331-L334. --- .../AlipayAuthenticationHandler.cs | 7 ++----- .../DeezerAuthenticationHandler.cs | 7 ++----- .../LineAuthenticationHandler.cs | 7 ++----- .../MixcloudAuthenticationHandler.cs | 7 ++----- .../ShopifyAuthenticationHandler.cs | 7 ++----- .../WeixinAuthenticationHandler.cs | 7 ++----- .../WorkWeixinAuthenticationHandler.cs | 7 ++----- 7 files changed, 14 insertions(+), 35 deletions(-) diff --git a/src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationHandler.cs index f8617cffe..fab5cf71b 100644 --- a/src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Alipay/AlipayAuthenticationHandler.cs @@ -214,12 +214,9 @@ protected override string BuildChallengeUrl([NotNull] AuthenticationProperties p ["redirect_uri"] = redirectUri, }; - if (Options.AdditionalAuthorizationParameters.Count > 0) + foreach (var additionalParameter in Options.AdditionalAuthorizationParameters) { - foreach (var parameter in Options.AdditionalAuthorizationParameters) - { - parameters[parameter.Key] = parameter.Value; - } + parameters.Add(additionalParameter.Key, additionalParameter.Value); } if (Options.UsePkce) diff --git a/src/AspNet.Security.OAuth.Deezer/DeezerAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Deezer/DeezerAuthenticationHandler.cs index ba15d9f06..861973944 100644 --- a/src/AspNet.Security.OAuth.Deezer/DeezerAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Deezer/DeezerAuthenticationHandler.cs @@ -98,12 +98,9 @@ protected override string BuildChallengeUrl([NotNull] AuthenticationProperties p ["perms"] = scopes, }; - if (Options.AdditionalAuthorizationParameters.Count > 0) + foreach (var additionalParameter in Options.AdditionalAuthorizationParameters) { - foreach (var parameter in Options.AdditionalAuthorizationParameters) - { - parameters[parameter.Key] = parameter.Value; - } + parameters.Add(additionalParameter.Key, additionalParameter.Value); } if (Options.UsePkce) diff --git a/src/AspNet.Security.OAuth.Line/LineAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Line/LineAuthenticationHandler.cs index 30f97d8da..13519dd50 100644 --- a/src/AspNet.Security.OAuth.Line/LineAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Line/LineAuthenticationHandler.cs @@ -42,12 +42,9 @@ protected override async Task ExchangeCodeAsync([NotNull] OA ["client_secret"] = Options.ClientSecret, }; - if (Options.AdditionalAuthorizationParameters.Count > 0) + foreach (var additionalParameter in Options.AdditionalAuthorizationParameters) { - foreach (var parameter in Options.AdditionalAuthorizationParameters) - { - tokenRequestParameters[parameter.Key] = parameter.Value; - } + tokenRequestParameters.Add(additionalParameter.Key, additionalParameter.Value); } // PKCE https://tools.ietf.org/html/rfc7636#section-4.5, see BuildChallengeUrl diff --git a/src/AspNet.Security.OAuth.Mixcloud/MixcloudAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Mixcloud/MixcloudAuthenticationHandler.cs index a676bbbcb..2f01008a5 100644 --- a/src/AspNet.Security.OAuth.Mixcloud/MixcloudAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Mixcloud/MixcloudAuthenticationHandler.cs @@ -38,12 +38,9 @@ protected override string BuildChallengeUrl([NotNull] AuthenticationProperties p ["response_type"] = "code", }; - if (Options.AdditionalAuthorizationParameters.Count > 0) + foreach (var additionalParameter in Options.AdditionalAuthorizationParameters) { - foreach (var parameter in Options.AdditionalAuthorizationParameters) - { - parameters[parameter.Key] = parameter.Value; - } + parameters.Add(additionalParameter.Key, additionalParameter.Value); } if (Options.UsePkce) diff --git a/src/AspNet.Security.OAuth.Shopify/ShopifyAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Shopify/ShopifyAuthenticationHandler.cs index f2a4c4db1..b14a58c3f 100644 --- a/src/AspNet.Security.OAuth.Shopify/ShopifyAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Shopify/ShopifyAuthenticationHandler.cs @@ -116,12 +116,9 @@ protected override string BuildChallengeUrl([NotNull] AuthenticationProperties p ["redirect_uri"] = redirectUri, }; - if (Options.AdditionalAuthorizationParameters.Count > 0) + foreach (var additionalParameter in Options.AdditionalAuthorizationParameters) { - foreach (var parameter in Options.AdditionalAuthorizationParameters) - { - parameters[parameter.Key] = parameter.Value; - } + parameters.Add(additionalParameter.Key, additionalParameter.Value); } if (Options.UsePkce) diff --git a/src/AspNet.Security.OAuth.Weixin/WeixinAuthenticationHandler.cs b/src/AspNet.Security.OAuth.Weixin/WeixinAuthenticationHandler.cs index b18986350..20c33b242 100644 --- a/src/AspNet.Security.OAuth.Weixin/WeixinAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.Weixin/WeixinAuthenticationHandler.cs @@ -134,12 +134,9 @@ protected override string BuildChallengeUrl([NotNull] AuthenticationProperties p ["response_type"] = "code", }; - if (Options.AdditionalAuthorizationParameters.Count > 0) + foreach (var additionalParameter in Options.AdditionalAuthorizationParameters) { - foreach (var parameter in Options.AdditionalAuthorizationParameters) - { - parameters[parameter.Key] = parameter.Value; - } + parameters.Add(additionalParameter.Key, additionalParameter.Value); } if (Options.UsePkce) diff --git a/src/AspNet.Security.OAuth.WorkWeixin/WorkWeixinAuthenticationHandler.cs b/src/AspNet.Security.OAuth.WorkWeixin/WorkWeixinAuthenticationHandler.cs index 049760807..b3d8beb2d 100644 --- a/src/AspNet.Security.OAuth.WorkWeixin/WorkWeixinAuthenticationHandler.cs +++ b/src/AspNet.Security.OAuth.WorkWeixin/WorkWeixinAuthenticationHandler.cs @@ -110,12 +110,9 @@ protected override string BuildChallengeUrl([NotNull] AuthenticationProperties p ["redirect_uri"] = redirectUri, }; - if (Options.AdditionalAuthorizationParameters.Count > 0) + foreach (var additionalParameter in Options.AdditionalAuthorizationParameters) { - foreach (var parameter in Options.AdditionalAuthorizationParameters) - { - parameters[parameter.Key] = parameter.Value; - } + parameters.Add(additionalParameter.Key, additionalParameter.Value); } if (Options.UsePkce)