Skip to content

Commit 894758f

Browse files
bgavrilMSbgavrilMS
committed
Fix for #4701 - OnBeforeTokenRequest can change the URI
1 parent 7faa358 commit 894758f

File tree

3 files changed

+234
-175
lines changed

3 files changed

+234
-175
lines changed

‎src/client/Microsoft.Identity.Client/OAuth2/OAuth2Client.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ internal async Task<T> ExecuteRequestAsync<T>(
124124
{
125125
var requestData = new OnBeforeTokenRequestData(_bodyParameters, _headers, endpointUri, requestContext.UserCancellationToken);
126126
await onBeforePostRequestData(requestData).ConfigureAwait(false);
127+
endpointUri = requestData.RequestUri;
127128
}
128129

129130
response = await _httpManager.SendPostAsync(

‎tests/Microsoft.Identity.Test.Unit/PublicApiTests/ConfidentialClientApplicationTests.cs‎

Lines changed: 2 additions & 175 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
using System.Threading.Tasks;
1313
using Microsoft.Identity.Client;
1414
using Microsoft.Identity.Client.Cache;
15-
using Microsoft.Identity.Client.Extensibility;
1615
using Microsoft.Identity.Client.Internal;
1716
using Microsoft.Identity.Client.Internal.ClientCredential;
1817
using Microsoft.Identity.Client.OAuth2;
@@ -23,6 +22,7 @@
2322
using Microsoft.Identity.Test.Common.Core.Mocks;
2423
using Microsoft.VisualStudio.TestTools.UnitTesting;
2524
using NSubstitute;
25+
using Microsoft.Identity.Client.Extensibility;
2626

2727
namespace Microsoft.Identity.Test.Unit.PublicApiTests
2828
{
@@ -491,77 +491,6 @@ private enum CredentialType
491491
return (app, handler);
492492
}
493493

494-
private static Task ModifyRequestAsync(OnBeforeTokenRequestData requestData)
495-
{
496-
Assert.AreEqual("https://login.microsoftonline.com/tid/oauth2/v2.0/token", requestData.RequestUri.AbsoluteUri);
497-
requestData.BodyParameters.Add("param1", "val1");
498-
requestData.BodyParameters.Add("param2", "val2");
499-
500-
requestData.Headers.Add("header1", "hval1");
501-
requestData.Headers.Add("header2", "hval2");
502-
503-
return Task.CompletedTask;
504-
}
505-
506-
[TestMethod]
507-
public async Task CertificateOverrideAsync()
508-
{
509-
using (var httpManager = new MockHttpManager())
510-
{
511-
httpManager.AddInstanceDiscoveryMockHandler();
512-
513-
var app = ConfidentialClientApplicationBuilder.Create(TestConstants.ClientId)
514-
.WithAuthority("https://login.microsoftonline.com/tid/")
515-
.WithExperimentalFeatures(true)
516-
.WithHttpManager(httpManager)
517-
.Build();
518-
519-
MockHttpMessageHandler handler = httpManager.AddMockHandlerSuccessfulClientCredentialTokenResponseMessage();
520-
521-
var result = await app.AcquireTokenForClient(TestConstants.s_scope.ToArray())
522-
.WithProofOfPosessionKeyId("key1")
523-
.OnBeforeTokenRequest(ModifyRequestAsync)
524-
.ExecuteAsync()
525-
.ConfigureAwait(false);
526-
527-
Assert.AreEqual("Bearer", result.TokenType);
528-
529-
Assert.AreEqual("val1", handler.ActualRequestPostData["param1"]);
530-
Assert.AreEqual("val2", handler.ActualRequestPostData["param2"]);
531-
Assert.AreEqual("hval1", handler.ActualRequestHeaders.GetValues("header1").Single());
532-
Assert.AreEqual("hval2", handler.ActualRequestHeaders.GetValues("header2").Single());
533-
Assert.IsFalse(handler.ActualRequestPostData.ContainsKey(OAuth2Parameter.ClientAssertion));
534-
Assert.IsFalse(handler.ActualRequestPostData.ContainsKey(OAuth2Parameter.ClientAssertionType));
535-
Assert.AreEqual("key1", (app.AppTokenCache as ITokenCacheInternal).Accessor.GetAllAccessTokens().Single().KeyId);
536-
537-
result = await app.AcquireTokenForClient(TestConstants.s_scope.ToArray())
538-
.WithProofOfPosessionKeyId("key1")
539-
.OnBeforeTokenRequest(ModifyRequestAsync)
540-
.ExecuteAsync()
541-
.ConfigureAwait(false);
542-
543-
Assert.AreEqual("Bearer", result.TokenType);
544-
Assert.AreEqual(TokenSource.Cache, result.AuthenticationResultMetadata.TokenSource);
545-
546-
Assert.AreEqual(
547-
"key1",
548-
(app.AppTokenCache as ITokenCacheInternal).Accessor.GetAllAccessTokens().Single().KeyId);
549-
550-
httpManager.AddMockHandlerSuccessfulClientCredentialTokenResponseMessage();
551-
result = await app.AcquireTokenForClient(TestConstants.s_scope.ToArray())
552-
.OnBeforeTokenRequest(ModifyRequestAsync)
553-
.ExecuteAsync()
554-
.ConfigureAwait(false);
555-
556-
Assert.AreEqual("Bearer", result.TokenType);
557-
Assert.AreEqual(TokenSource.IdentityProvider, result.AuthenticationResultMetadata.TokenSource);
558-
IReadOnlyList<Client.Cache.Items.MsalAccessTokenCacheItem> ats = (app.AppTokenCache as ITokenCacheInternal).Accessor.GetAllAccessTokens();
559-
Assert.AreEqual(2, ats.Count);
560-
Assert.IsTrue(ats.Single(at => at.KeyId == "key1") != null);
561-
Assert.IsTrue(ats.Single(at => at.KeyId == null) != null);
562-
}
563-
}
564-
565494
[TestMethod]
566495
public async Task ConfidentialClientUsingCertificateTestAsync()
567496
{
@@ -1860,108 +1789,6 @@ public async Task ValidateGetAccountAsyncWithNullEmptyAccountIdAsync(string acco
18601789

18611790
Assert.IsNull(acc);
18621791
}
1863-
}
1864-
1865-
[TestMethod]
1866-
public async Task ValidateAppTokenProviderAsync()
1867-
{
1868-
using (var harness = base.CreateTestHarness())
1869-
{
1870-
harness.HttpManager.AddInstanceDiscoveryMockHandler();
1871-
1872-
bool usingClaims = false;
1873-
string differentScopesForAt = string.Empty;
1874-
int callbackInvoked = 0;
1875-
var app = ConfidentialClientApplicationBuilder.Create(TestConstants.ClientId)
1876-
.WithAppTokenProvider((AppTokenProviderParameters parameters) =>
1877-
{
1878-
Assert.IsNotNull(parameters.Scopes);
1879-
Assert.IsNotNull(parameters.CorrelationId);
1880-
Assert.IsNotNull(parameters.TenantId);
1881-
Assert.IsNotNull(parameters.CancellationToken);
1882-
1883-
if (usingClaims)
1884-
{
1885-
Assert.IsNotNull(parameters.Claims);
1886-
}
1887-
1888-
Interlocked.Increment(ref callbackInvoked);
1889-
1890-
return Task.FromResult(GetAppTokenProviderResult(differentScopesForAt));
1891-
})
1892-
.WithHttpManager(harness.HttpManager)
1893-
.BuildConcrete();
1894-
1895-
// AcquireToken from app provider
1896-
AuthenticationResult result = await app.AcquireTokenForClient(TestConstants.s_scope)
1897-
.ExecuteAsync(new CancellationToken()).ConfigureAwait(false);
1898-
1899-
Assert.IsNotNull(result.AccessToken);
1900-
Assert.AreEqual(TestConstants.DefaultAccessToken, result.AccessToken);
1901-
Assert.AreEqual(TokenSource.IdentityProvider, result.AuthenticationResultMetadata.TokenSource);
1902-
Assert.AreEqual(1, callbackInvoked);
1903-
1904-
var tokens = app.AppTokenCacheInternal.Accessor.GetAllAccessTokens();
1905-
1906-
Assert.AreEqual(1, tokens.Count);
1907-
1908-
var token = tokens.FirstOrDefault();
1909-
Assert.IsNotNull(token);
1910-
Assert.AreEqual(TestConstants.DefaultAccessToken, token.Secret);
1911-
1912-
// AcquireToken from cache
1913-
result = await app.AcquireTokenForClient(TestConstants.s_scope)
1914-
.ExecuteAsync(new CancellationToken()).ConfigureAwait(false);
1915-
1916-
Assert.IsNotNull(result.AccessToken);
1917-
Assert.AreEqual(TestConstants.DefaultAccessToken, result.AccessToken);
1918-
Assert.AreEqual(TokenSource.Cache, result.AuthenticationResultMetadata.TokenSource);
1919-
Assert.AreEqual(1, callbackInvoked);
1920-
1921-
// Expire token
1922-
TokenCacheHelper.ExpireAllAccessTokens(app.AppTokenCacheInternal);
1923-
1924-
// Acquire token from app provider with expired token
1925-
result = await app.AcquireTokenForClient(TestConstants.s_scope)
1926-
.ExecuteAsync(new CancellationToken()).ConfigureAwait(false);
1927-
1928-
Assert.IsNotNull(result.AccessToken);
1929-
Assert.AreEqual(TestConstants.DefaultAccessToken, result.AccessToken);
1930-
Assert.AreEqual(TokenSource.IdentityProvider, result.AuthenticationResultMetadata.TokenSource);
1931-
Assert.AreEqual(2, callbackInvoked);
1932-
1933-
differentScopesForAt = "new scope";
1934-
1935-
// Acquire token from app provider with new scopes
1936-
result = await app.AcquireTokenForClient(new[] { differentScopesForAt })
1937-
.ExecuteAsync(new CancellationToken()).ConfigureAwait(false);
1938-
1939-
Assert.IsNotNull(result.AccessToken);
1940-
Assert.AreEqual(TestConstants.DefaultAccessToken + differentScopesForAt, result.AccessToken);
1941-
Assert.AreEqual(TokenSource.IdentityProvider, result.AuthenticationResultMetadata.TokenSource);
1942-
Assert.AreEqual(app.AppTokenCacheInternal.Accessor.GetAllAccessTokens().Count, 2);
1943-
Assert.AreEqual(3, callbackInvoked);
1944-
1945-
// Acquire token from app provider with claims. Should not use cache
1946-
result = await app.AcquireTokenForClient(TestConstants.s_scope)
1947-
.WithClaims(TestConstants.Claims)
1948-
.ExecuteAsync(new CancellationToken()).ConfigureAwait(false);
1949-
1950-
Assert.IsNotNull(result.AccessToken);
1951-
Assert.AreEqual(TestConstants.DefaultAccessToken + differentScopesForAt, result.AccessToken);
1952-
Assert.AreEqual(TokenSource.IdentityProvider, result.AuthenticationResultMetadata.TokenSource);
1953-
Assert.AreEqual(4, callbackInvoked);
1954-
}
1955-
}
1956-
1957-
private AppTokenProviderResult GetAppTokenProviderResult(string differentScopesForAt = "", long? refreshIn = 1000)
1958-
{
1959-
var token = new AppTokenProviderResult();
1960-
token.AccessToken = TestConstants.DefaultAccessToken + differentScopesForAt; //Used to indicate that there is a new access token for a different set of scopes
1961-
token.ExpiresInSeconds = 3600;
1962-
token.RefreshInSeconds = refreshIn;
1963-
1964-
return token;
1965-
}
1792+
}
19661793
}
19671794
}

0 commit comments

Comments
 (0)