Skip to content

ADFS Compatibility with MSAL #834

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

Merged
merged 35 commits into from
May 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
ce00712
Move Reed's initial [PR](https://github.com/AzureAD/azure-activedirec…
MarkZuber Jan 18, 2019
4ad8e79
Merge branch 'dev3x' into rewilli/adfs
Jan 24, 2019
9382823
Public client unit test passing
Jan 29, 2019
78a22db
Fix device code tests
Jan 29, 2019
9ef61a2
Add confidential client test
Jan 29, 2019
ddeaed4
Token cache test
Jan 29, 2019
2b4e8bb
Update paramter name in comment
Jan 29, 2019
e70ddf2
Clean up
Jan 29, 2019
8ad5be0
Make tests consistent with adfs responses
Jan 30, 2019
3c3d657
Minor cr fixes
Jan 30, 2019
b77813b
Adding support for ADFS 2019 lab accounts for testing.
Feb 4, 2019
95aecfb
Adding ADFS 2019 tests to fast run for appcenter
Feb 4, 2019
1443c5b
Slight refactor
Feb 5, 2019
f4efd7f
Add integration tests
Feb 8, 2019
068889f
Merge branch 'dev3x' into rewilli/adfs
Feb 9, 2019
22a94b8
CR feedback
Feb 11, 2019
2d0d8b7
more cr feedback
Feb 11, 2019
fac23e8
Client secret url read from file
Feb 11, 2019
044ef18
Updating client credential directory. Adding comment about running te…
Feb 12, 2019
ef5027a
Device code flow not supported
Feb 12, 2019
536650f
Merge branch 'rewilli/adfs' of https://github.com/AzureAD/microsoft-a…
Feb 12, 2019
939faeb
ignore device code flow tests
Feb 13, 2019
b3be5a3
Merge branch 'dev3x' into rewilli/adfs
Feb 13, 2019
382cb3b
Embedding data file with client secret into project as resource
Feb 14, 2019
28f23fc
Adding integration test for different account types
Feb 20, 2019
ba9f02b
Resolving test failures
Mar 20, 2019
555255d
Merge remote-tracking branch 'origin/dev3x' into rewilli/adfs
May 10, 2019
64ea8ad
Resolving null reference when extracting domain from ADFS UPN.
May 14, 2019
29b229e
Resolving cache lookup errors
May 14, 2019
e42883f
Refactoring/Addressing PR comments
May 15, 2019
288dea6
fixing issue with ADFS direct preferred username.
May 16, 2019
42a3e4d
Merge remote-tracking branch 'origin/master' into rewilli/adfs
May 16, 2019
d96152c
resolving test errors.
May 16, 2019
de446e4
Refactoring
May 16, 2019
88b5dae
Merge remote-tracking branch 'origin/master' into rewilli/adfs
May 16, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/Microsoft.Identity.Client/AccountId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@ public AccountId(string identifier, string objectId, string tenantId)
ValidateId();
}

/// <summary>
/// Constructor of an AccountId meant for Adfs scenarios since Adfs instances lack tenant ids.
/// </summary>
/// <param name="adfsIdentifier">Unique identifier for the account if authority is ADFS</param>
public AccountId(string adfsIdentifier)
:this(adfsIdentifier, adfsIdentifier, null)
{ }


internal static AccountId ParseFromString(string str)
{
if (string.IsNullOrEmpty(str))
Expand All @@ -54,6 +63,11 @@ internal static AccountId ParseFromString(string str)
}
string[] elements = str.Split('.');

if(elements.Length == 1)
{
return new AccountId(str); //Account id is from Adfs; no . in the string
}

return new AccountId(str, elements[0], elements[1]);
}

Expand Down Expand Up @@ -89,7 +103,7 @@ public override string ToString()
[Conditional("DEBUG")]
private void ValidateId()
{
string expectedId = ObjectId + "." + TenantId;
string expectedId = TenantId==null ? ObjectId : ObjectId + "." + TenantId;
if (!string.Equals(expectedId, Identifier, StringComparison.Ordinal))
{
throw new InvalidOperationException(
Expand Down
1,072 changes: 534 additions & 538 deletions src/Microsoft.Identity.Client/AppConfig/AbstractApplicationBuilder.cs

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion src/Microsoft.Identity.Client/AuthenticationResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,10 @@ internal AuthenticationResult(MsalAccessTokenCacheItem msalAccessTokenCacheItem,
{
if (msalAccessTokenCacheItem.HomeAccountId != null)
{
string username = msalAccessTokenCacheItem.IsAdfs ? msalIdTokenCacheItem?.IdToken.Upn : msalIdTokenCacheItem?.IdToken?.PreferredUsername;
Account = new Account(
msalAccessTokenCacheItem.HomeAccountId,
msalIdTokenCacheItem?.IdToken?.PreferredUsername,
username,
msalAccessTokenCacheItem.Environment);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@ internal MsalAccessTokenCacheItem()
CredentialType = StorageJsonValues.CredentialTypeAccessToken;
}


internal MsalAccessTokenCacheItem(
string environment,
string clientId,
MsalTokenResponse response,
string tenantId)
string tenantId,
string userId = null)
: this(
environment,
clientId,
Expand All @@ -31,7 +33,8 @@ internal MsalAccessTokenCacheItem(
response.AccessToken,
response.AccessTokenExpiresOn,
response.AccessTokenExtendedExpiresOn,
response.ClientInfo)
response.ClientInfo,
userId)
{
}

Expand All @@ -43,7 +46,8 @@ internal MsalAccessTokenCacheItem(
string secret,
DateTimeOffset accessTokenExpiresOn,
DateTimeOffset accessTokenExtendedExpiresOn,
string rawClientInfo)
string rawClientInfo,
string userId = null)
: this()
{
Environment = environment;
Expand All @@ -56,10 +60,18 @@ internal MsalAccessTokenCacheItem(
CachedAt = CoreHelpers.CurrDateTimeInUnixTimestamp();
RawClientInfo = rawClientInfo;

//Adfs does not send back client info, so HomeAccountId must be explicitly set
HomeAccountId = userId;
InitUserIdentifier();
}

internal string TenantId { get; set; }
private string _tenantId;

internal string TenantId
{
get => _tenantId;
set => _tenantId = value ?? string.Empty;
}

/// <summary>
/// String comprised of scopes that have been lowercased and ordered.
Expand All @@ -70,9 +82,13 @@ internal MsalAccessTokenCacheItem(
internal string ExpiresOnUnixTimestamp { get; set; }
internal string ExtendedExpiresOnUnixTimestamp { get; set; }
public string UserAssertionHash { get; set; }


internal bool IsAdfs { get; set; }

internal string Authority =>
string.Format(CultureInfo.InvariantCulture, "https://{0}/{1}/", Environment, TenantId ?? "common");
IsAdfs ? string.Format(CultureInfo.InvariantCulture, "https://{0}/{1}/", Environment, "adfs") :
string.Format(CultureInfo.InvariantCulture, "https://{0}/{1}/", Environment, TenantId ?? "common");

internal SortedSet<string> ScopeSet => ScopeHelper.ConvertStringToLowercaseSortedSet(NormalizedScopes);

Expand Down
20 changes: 15 additions & 5 deletions src/Microsoft.Identity.Client/Cache/Items/MsalIdTokenCacheItem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,16 @@ internal MsalIdTokenCacheItem(
string environment,
string clientId,
MsalTokenResponse response,
string tenantId)
string tenantId,
string userId=null
)
: this(
environment,
clientId,
response.IdToken,
response.ClientInfo,
tenantId)
tenantId,
userId)
{
}

Expand All @@ -36,7 +39,9 @@ internal MsalIdTokenCacheItem(
string clientId,
string secret,
string rawClientInfo,
string tenantId)
string tenantId,
string userId=null
)
: this()
{
Environment = environment;
Expand All @@ -45,13 +50,18 @@ internal MsalIdTokenCacheItem(
Secret = secret;
RawClientInfo = rawClientInfo;

//Adfs does not send back client info, so HomeAccountId must be explicitly set
HomeAccountId = userId;
InitUserIdentifier();
}



internal bool IsAdfs { get; set; }
internal string TenantId { get; set; }

internal string Authority =>
string.Format(CultureInfo.InvariantCulture, "https://{0}/{1}/", Environment, TenantId ?? "common");
IsAdfs ? string.Format(CultureInfo.InvariantCulture, "https://{0}/{1}/", Environment, "adfs") :
string.Format(CultureInfo.InvariantCulture, "https://{0}/{1}/", Environment, TenantId ?? "common");

internal IdToken IdToken => IdToken.Parse(Secret);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,20 @@ internal MsalRefreshTokenCacheItem()
internal MsalRefreshTokenCacheItem(
string environment,
string clientId,
MsalTokenResponse response)
: this(environment, clientId, response.RefreshToken, response.ClientInfo, response.FamilyId)
MsalTokenResponse response,
string userId=null)
: this(environment, clientId, response.RefreshToken, response.ClientInfo, response.FamilyId, userId)
{
}


internal MsalRefreshTokenCacheItem(
string environment,
string clientId,
string secret,
string rawClientInfo,
string familyId = null)
string familyId = null,
string userId = null)
: this()
{
ClientId = clientId;
Expand All @@ -38,6 +41,8 @@ internal MsalRefreshTokenCacheItem(
RawClientInfo = rawClientInfo;
FamilyId = familyId;

//Adfs does not send back client info, so HomeAccountId must be explicitly set
HomeAccountId = userId;
InitUserIdentifier();
}

Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.Identity.Client/Core/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public static string FormatAdfsWebFingerUrl(string host, string resource)
{
return string.Format(
CultureInfo.InvariantCulture,
"https://{0}/adfs/.well-known/webfinger?rel={1}&resource={2}",
"https://{0}/.well-known/webfinger?rel={1}&resource={2}",
host,
Constants.DefaultRealm,
resource);
Expand Down
4 changes: 4 additions & 0 deletions src/Microsoft.Identity.Client/Core/IdToken.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ internal class IdTokenClaim
public const string HomeObjectId = "home_oid";
public const string GivenName = "given_name";
public const string FamilyName = "family_name";
public const string Upn = "upn";
}

[DataContract]
Expand Down Expand Up @@ -51,6 +52,9 @@ internal class IdToken
[DataMember(Name = IdTokenClaim.HomeObjectId, IsRequired = false)]
public string HomeObjectId { get; set; }

[DataMember(Name = IdTokenClaim.Upn, IsRequired = false)]
public string Upn { get; set; }

[DataMember(Name = IdTokenClaim.GivenName)]
public string GivenName { get; set; }

Expand Down
36 changes: 36 additions & 0 deletions src/Microsoft.Identity.Client/Instance/AdfsAuthority.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using System.Collections.Generic;
using System.Globalization;
using System.Net.Http;
using System.Threading.Tasks;
using Microsoft.Identity.Client.Core;
using Microsoft.Identity.Client.Http;
using Microsoft.Identity.Client.OAuth2;
using Microsoft.Identity.Client.TelemetryCore;

namespace Microsoft.Identity.Client.Instance
{
internal class AdfsAuthority : Authority
{
private readonly HashSet<string> _validForDomainsList = new HashSet<string>();

public AdfsAuthority(IServiceBundle serviceBundle, AuthorityInfo authorityInfo)
: base(serviceBundle, authorityInfo)
{
}

//ADFS does not have a concept of a tenant ID. This prevents ADFS from supporting multiple tenants
internal override string GetTenantId()
{
return null;
}

internal override void UpdateTenantId(string tenantId)
{
throw new NotImplementedException();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,23 +28,9 @@ public async Task<string> ValidateAuthorityAndGetOpenIdDiscoveryEndpointAsync(
{
if (authorityInfo.ValidateAuthority)
{
DrsMetadataResponse drsResponse = await GetMetadataFromEnrollmentServerAsync(userPrincipalName, requestContext)
.ConfigureAwait(false);

if (drsResponse.IdentityProviderService?.PassiveAuthEndpoint == null)
{
throw new MsalServiceException(
MsalError.MissingPassiveAuthEndpoint,
MsalErrorMessage.CannotFindTheAuthEndpont)
{
OAuth2Response = drsResponse
};
}

string resource = string.Format(CultureInfo.InvariantCulture, authorityInfo.CanonicalAuthority);
string webFingerUrl = Constants.FormatAdfsWebFingerUrl(
drsResponse.IdentityProviderService.PassiveAuthEndpoint.Host,
resource);
string resource = string.Format(CultureInfo.InvariantCulture, "https://{0}", authorityInfo.Host);
string webFingerUrl = Constants.FormatAdfsWebFingerUrl(authorityInfo.Host, resource);


var httpResponse = await _serviceBundle.HttpManager.SendGetAsync(new Uri(webFingerUrl), null, requestContext.Logger)
.ConfigureAwait(false);
Expand All @@ -59,10 +45,10 @@ public async Task<string> ValidateAuthorityAndGetOpenIdDiscoveryEndpointAsync(
};
}

var wfr = OAuth2Client.CreateResponse<AdfsWebFingerResponse>(httpResponse, requestContext, false);
AdfsWebFingerResponse wfr = OAuth2Client.CreateResponse<AdfsWebFingerResponse>(httpResponse, requestContext, false);
if (wfr.Links.FirstOrDefault(
a => a.Rel.Equals(Constants.DefaultRealm, StringComparison.OrdinalIgnoreCase) &&
a.Href.Equals(resource, StringComparison.OrdinalIgnoreCase)) == null)
a.Href.Equals(resource)) == null)
{
throw new MsalClientException(
MsalError.InvalidAuthority,
Expand All @@ -73,35 +59,5 @@ public async Task<string> ValidateAuthorityAndGetOpenIdDiscoveryEndpointAsync(
return authorityInfo.CanonicalAuthority + Constants.WellKnownOpenIdConfigurationPath;
}

private async Task<DrsMetadataResponse> GetMetadataFromEnrollmentServerAsync(
string userPrincipalName,
RequestContext requestContext)
{
try
{
// attempt to connect to on-premise enrollment server first.
return await QueryEnrollmentServerEndpointAsync(
Constants.FormatEnterpriseRegistrationOnPremiseUri(AdfsUpnHelper.GetDomainFromUpn(userPrincipalName)),
requestContext).ConfigureAwait(false);
}
catch (Exception exc)
{
requestContext.Logger.InfoPiiWithPrefix(
exc,
"On-Premise ADFS enrollment server endpoint lookup failed. Error - ");
}

return await QueryEnrollmentServerEndpointAsync(
Constants.FormatEnterpriseRegistrationInternetUri(AdfsUpnHelper.GetDomainFromUpn(userPrincipalName)),
requestContext).ConfigureAwait(false);
}

private async Task<DrsMetadataResponse> QueryEnrollmentServerEndpointAsync(string endpoint, RequestContext requestContext)
{
var client = new OAuth2Client(requestContext.Logger, _serviceBundle.HttpManager, _serviceBundle.TelemetryManager);
client.AddQueryParameter("api-version", "1.0");
return await client.ExecuteRequestAsync<DrsMetadataResponse>(new Uri(endpoint), HttpMethod.Get, requestContext)
.ConfigureAwait(false);
}
}
}
4 changes: 1 addition & 3 deletions src/Microsoft.Identity.Client/Instance/Authority.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,7 @@ public static Authority CreateAuthorityWithOverride(IServiceBundle serviceBundle
switch (authorityInfo.AuthorityType)
{
case AuthorityType.Adfs:
throw new MsalClientException(
MsalError.InvalidAuthorityType,
"ADFS is not a supported authority");
return new AdfsAuthority(serviceBundle, authorityInfo);

case AuthorityType.B2C:
CheckB2CAuthorityHost(serviceBundle, authorityInfo);
Expand Down
Loading