Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@jennyf19
jennyf19 committed Apr 12, 2023
1 parent 51b539a commit 5950222
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 40 deletions.
64 changes: 26 additions & 38 deletions src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,16 +357,11 @@ public Task<AuthenticationResult> GetAuthenticationResultForAppAsync(

if (tokenAcquisitionOptions != null)
{
if (tokenAcquisitionOptions.ExtraQueryParameters != null)
var dict = MergeExtraQueryParameters(mergedOptions, tokenAcquisitionOptions);

if (dict != null)
{
if (mergedOptions.ExtraQueryParameters != null)
{
builder.WithExtraQueryParameters((Dictionary<string, string>)(MergeExtraQueryParameters(mergedOptions, tokenAcquisitionOptions)));
}
else
{
builder.WithExtraQueryParameters((Dictionary<string, string>)(tokenAcquisitionOptions.ExtraQueryParameters));
}
builder.WithExtraQueryParameters(dict);
}
if (tokenAcquisitionOptions.ExtraHeadersParameters != null)
{
Expand Down Expand Up @@ -719,16 +714,11 @@ private IConfidentialClientApplication BuildConfidentialClientApplication(Merged
}
if (tokenAcquisitionOptions != null)
{
if (tokenAcquisitionOptions.ExtraQueryParameters != null)
var dict = MergeExtraQueryParameters(mergedOptions, tokenAcquisitionOptions);

if (dict != null)
{
if (mergedOptions.ExtraQueryParameters != null)
{
builder.WithExtraQueryParameters((Dictionary<string, string>)(MergeExtraQueryParameters(mergedOptions, tokenAcquisitionOptions)));
}
else
{
builder.WithExtraQueryParameters((Dictionary<string, string>)(tokenAcquisitionOptions.ExtraQueryParameters));
}
builder.WithExtraQueryParameters(dict);
}
if (tokenAcquisitionOptions.ExtraHeadersParameters != null)
{
Expand Down Expand Up @@ -861,20 +851,11 @@ private Task<AuthenticationResult> GetAuthenticationResultForWebAppWithAccountFr

if (tokenAcquisitionOptions != null)
{
if (tokenAcquisitionOptions.ExtraQueryParameters != null)
{
if (mergedOptions.ExtraQueryParameters != null)
{
builder.WithExtraQueryParameters((Dictionary<string, string>)(MergeExtraQueryParameters(mergedOptions, tokenAcquisitionOptions)));
}
else
{
builder.WithExtraQueryParameters((Dictionary<string, string>)(tokenAcquisitionOptions.ExtraQueryParameters));
}
}
else if (mergedOptions.ExtraQueryParameters != null)
var dict = MergeExtraQueryParameters(mergedOptions, tokenAcquisitionOptions);

if (dict != null)
{
builder.WithExtraQueryParameters((Dictionary<string, string>)mergedOptions.ExtraQueryParameters);
builder.WithExtraQueryParameters(dict);
}
if (tokenAcquisitionOptions.ExtraHeadersParameters != null)
{
Expand Down Expand Up @@ -914,18 +895,25 @@ private Task<AuthenticationResult> GetAuthenticationResultForWebAppWithAccountFr
return builder.ExecuteAsync(tokenAcquisitionOptions != null ? tokenAcquisitionOptions.CancellationToken : CancellationToken.None);
}

internal static IDictionary<string, string> MergeExtraQueryParameters(
internal static Dictionary<string, string>? MergeExtraQueryParameters(
MergedOptions mergedOptions,
TokenAcquisitionOptions tokenAcquisitionOptions)
{
var mergedDict = tokenAcquisitionOptions!.ExtraQueryParameters;
foreach (var pair in mergedOptions!.ExtraQueryParameters!)
if (tokenAcquisitionOptions.ExtraQueryParameters != null)
{
if (!mergedDict!.ContainsKey(pair.Key))
mergedDict.Add(pair.Key, pair.Value);
var mergedDict = new Dictionary<string, string>(tokenAcquisitionOptions.ExtraQueryParameters);
if (mergedOptions.ExtraQueryParameters != null)
{
foreach (var pair in mergedOptions!.ExtraQueryParameters)
{
if (!mergedDict!.ContainsKey(pair.Key))
mergedDict.Add(pair.Key, pair.Value);
}
}
return mergedDict;
}

return mergedDict!;
return (Dictionary<string, string>?)mergedOptions.ExtraQueryParameters;
}

protected static bool AcceptedTokenVersionMismatch(MsalUiRequiredException msalServiceException)
Expand Down
1 change: 0 additions & 1 deletion tests/DevApps/WebAppCallsMicrosoftGraph/WebAppCallsMicrosoftGraph.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,5 @@
<ProjectReference Include="..\..\..\src\Microsoft.Identity.Web.MicrosoftGraph\Microsoft.Identity.Web.MicrosoftGraph.csproj" />
<ProjectReference Include="..\..\..\src\Microsoft.Identity.Web.UI\Microsoft.Identity.Web.UI.csproj" />
<ProjectReference Include="..\..\..\src\Microsoft.Identity.Web\Microsoft.Identity.Web.csproj" />
<PackageReference Include="Microsoft.Identity.Client" Version="4.53.0-preview" />
</ItemGroup>
</Project>
51 changes: 50 additions & 1 deletion tests/Microsoft.Identity.Web.Test/TokenAcquisitionAuthorityTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ public void TestParseAuthorityIfNecessary()
[Fact]
public void MergeExtraQueryParametersTest()
{
// Arrange
var mergedOptions = new MergedOptions
{
ExtraQueryParameters = new Dictionary<string, string>
Expand All @@ -253,12 +254,60 @@ public void MergeExtraQueryParametersTest()
}
};

// Act
var mergedDict = TokenAcquisition.MergeExtraQueryParameters(mergedOptions, tokenAcquisitionOptions);

Assert.Equal(3, mergedDict.Count);

// Assert
Assert.Equal(3, mergedDict!.Count);
Assert.Equal("newvalue1", mergedDict["key1"]);
Assert.Equal("value2", mergedDict["key2"]);
Assert.Equal("value3", mergedDict["key3"]);
}

[Fact]
public void MergeExtraQueryParameters_TokenAcquisitionOptionsNull_Test()
{
// Arrange
var mergedOptions = new MergedOptions
{
ExtraQueryParameters = new Dictionary<string, string>
{
{ "key1", "value1" },
{ "key2", "value2" }
}
};
var tokenAcquisitionOptions = new TokenAcquisitionOptions
{
ExtraQueryParameters = null,
};

// Act
var mergedDict = TokenAcquisition.MergeExtraQueryParameters(mergedOptions, tokenAcquisitionOptions);

// Assert
Assert.Equal("value1", mergedDict!["key1"]);
Assert.Equal("value2", mergedDict["key2"]);
}

[Fact]
public void MergeExtraQueryParameters_MergedOptionsNull_Test()
{
// Arrange
var mergedOptions = new MergedOptions
{
ExtraQueryParameters = null,
};
var tokenAcquisitionOptions = new TokenAcquisitionOptions
{
ExtraQueryParameters = null,
};

// Act
var mergedDict = TokenAcquisition.MergeExtraQueryParameters(mergedOptions, tokenAcquisitionOptions);

// Assert
Assert.Null(mergedDict);
}
}
}

0 comments on commit 5950222

Please sign in to comment.