Skip to content

Commit 515ada5

Browse files
Fix duplicated query parameters for VSO/AzDo (#814)
Fix query string parameters being duplicated if `AuthorizationEndpoint` contains any user-added query string parameters.
1 parent e46b08b commit 515ada5

File tree

2 files changed

+21
-5
lines changed

2 files changed

+21
-5
lines changed

src/AspNet.Security.OAuth.VisualStudio/VisualStudioAuthenticationHandler.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using System.Security.Claims;
1010
using System.Text.Encodings.Web;
1111
using System.Text.Json;
12+
using Microsoft.AspNetCore.Http;
1213
using Microsoft.AspNetCore.WebUtilities;
1314
using Microsoft.Extensions.Logging;
1415
using Microsoft.Extensions.Options;
@@ -91,12 +92,16 @@ protected override string BuildChallengeUrl([NotNull] AuthenticationProperties p
9192
var challengeUrl = base.BuildChallengeUrl(properties, redirectUri);
9293

9394
// Visual Studio Online/Azure DevOps uses "Assertion" instead of "code"
94-
var challengeUri = new Uri(challengeUrl, UriKind.Absolute);
95+
var challengeUri = new UriBuilder(challengeUrl);
9596
var query = QueryHelpers.ParseQuery(challengeUri.Query);
9697

9798
query["response_type"] = "Assertion";
9899

99-
return QueryHelpers.AddQueryString(Options.AuthorizationEndpoint, query);
100+
// Replace the query with the edit so that the parameters are not duplicated.
101+
// See https://github.com/dotnet/aspnetcore/issues/47054 for more context.
102+
challengeUri.Query = QueryString.Create(query).Value;
103+
104+
return challengeUri.Uri.AbsoluteUri;
100105
}
101106

102107
private static partial class Log

test/AspNet.Security.OAuth.Providers.Tests/VisualStudio/VisualStudioTests.cs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,22 @@ public async Task Can_Sign_In_Using_Visual_Studio(string claimType, string claim
2626
=> await AuthenticateUserAndAssertClaimValue(claimType, claimValue);
2727

2828
[Theory]
29-
[InlineData(false)]
30-
[InlineData(true)]
31-
public async Task BuildChallengeUrl_Generates_Correct_Url(bool usePkce)
29+
[InlineData(false, "")]
30+
[InlineData(true, "")]
31+
[InlineData(false, "?foo=bar")]
32+
[InlineData(true, "?foo=bar")]
33+
public async Task BuildChallengeUrl_Generates_Correct_Url(
34+
bool usePkce,
35+
string authorizationEndpointSuffix)
3236
{
3337
// Arrange
3438
var options = new VisualStudioAuthenticationOptions()
3539
{
3640
UsePkce = usePkce,
3741
};
3842

43+
options.AuthorizationEndpoint += authorizationEndpointSuffix;
44+
3945
var redirectUrl = "https://my-site.local/signin-visualstudio";
4046

4147
// Act
@@ -66,5 +72,10 @@ public async Task BuildChallengeUrl_Generates_Correct_Url(bool usePkce)
6672
query.ShouldNotContainKey(OAuthConstants.CodeChallengeKey);
6773
query.ShouldNotContainKey(OAuthConstants.CodeChallengeMethodKey);
6874
}
75+
76+
foreach (var parameter in query)
77+
{
78+
parameter.Value.Count.ShouldBe(1, $"Query string parameter {parameter.Key} appears more than once: {parameter.Value}.");
79+
}
6980
}
7081
}

0 commit comments

Comments
 (0)