Skip to content

Commit

Permalink
azrepos: use organizations authority for MSA accounts
Browse files Browse the repository at this point in the history
Use the /organizations authority for MSA accounts with Azure
DevOps/Repos. This is because we're using MSA pass-through, an internal
Microsoft mechanism to support both MSA and 'work' (AAD) accounts with
the same auth stacks.

You should be able to use /common, but this doesn't work. At the same
time we're using ADAL Obj-C on macOS rather than MSAL.NET like we do on
Windows, and ADAL speaks to the "v1" AAD endpoints, which don't know the
/organizations tenant :(

For macOS we need to fudge the authority _back_ to /common for MSA
accounts.
  • Loading branch information
mjcheetham committed Jul 17, 2020
1 parent 5c23c97 commit f80007b
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 4 deletions.
17 changes: 17 additions & 0 deletions src/osx/Microsoft.Authentication.Helper.Mac/Source/main.m
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,23 @@ int main(int argc, const char * argv[]) {
NSString* redirectUri = [configs objectForKey:@"redirectUri"];
NSString* interactive = [configs objectForKey:@"interactive"];

// Because ADAL only supports the v1 endpoints we need to transform any request
// for the /organizations or /consumers authority to the /common one or else
// we get errors back from the server.
NSString *lowerAuthority = [authority lowercaseString];
if ([lowerAuthority hasSuffix:@"/organizations"] || [lowerAuthority hasSuffix:@"/consumers"])
{
NSError *error = nil;
NSRegularExpression *regex = [NSRegularExpression regularExpressionWithPattern:@"/(organizations|consumers)$"
options:NSRegularExpressionCaseInsensitive
error:&error];
NSString* newAuthority = [regex stringByReplacingMatchesInString:authority
options:0
range:NSMakeRange(0, authority.length)
withTemplate:@"/common"];
authority = newAuthority;
}

// We only perform interactive flows
if (isTruthy(interactive))
{
Expand Down
7 changes: 5 additions & 2 deletions src/shared/Microsoft.AzureRepos.Tests/AzureDevOpsApiTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ public class AzureDevOpsApiTests
private const string ExpectedLocationServicePath = "_apis/ServiceDefinitions/LocationService2/951917AC-A960-4999-8464-E3F0AA25B381?api-version=1.0";
private const string ExpectedIdentityServicePath = "_apis/token/sessiontokens?api-version=1.0&tokentype=compact";
private const string CommonAuthority = "https://login.microsoftonline.com/common";
private const string OrganizationsAuthority = "https://login.microsoftonline.com/organizations";

[Fact]
public async Task AzureDevOpsRestApi_GetAuthorityAsync_NullUri_ThrowsException()
Expand Down Expand Up @@ -169,13 +170,15 @@ public async Task AzureDevOpsRestApi_GetAuthorityAsync_VssResourceTenantMultiple
}

[Fact]
public async Task AzureDevOpsRestApi_GetAuthorityAsync_VssResourceTenantMsa_ReturnsCommonAuthority()
public async Task AzureDevOpsRestApi_GetAuthorityAsync_VssResourceTenantMsa_ReturnsOrganizationsAuthority()
{
var context = new TestCommandContext();
var uri = new Uri("https://example.com");
var msaTenantId = Guid.Empty;

const string expectedAuthority = CommonAuthority;
// This is only the case because we're using MSA pass-through.. in the future, if and when we
// move away from MSA pass-through, this should be the common authority.
const string expectedAuthority = OrganizationsAuthority;

var httpResponse = new HttpResponseMessage(HttpStatusCode.Unauthorized)
{
Expand Down
10 changes: 8 additions & 2 deletions src/shared/Microsoft.AzureRepos/AzureDevOpsRestApi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ public async Task<string> GetAuthorityAsync(Uri organizationUri)
const string authorityBase = "https://login.microsoftonline.com/";
const string commonAuthority = authorityBase + "common";

// We should be using "/common" or "/consumer" as the authority for MSA but since
// Azure DevOps uses MSA pass-through (an internal hack to support MSA and AAD
// accounts in the same auth stack), which actually need to consult the "/organizations"
// authority instead.
const string msaAuthority = authorityBase + "organizations";

_context.Trace.WriteLine($"HTTP: HEAD {organizationUri}");
using (HttpResponseMessage response = await HttpClient.HeadAsync(organizationUri))
{
Expand Down Expand Up @@ -74,14 +80,14 @@ public async Task<string> GetAuthorityAsync(Uri organizationUri)
if (tenantIds.Length == 1 && Guid.TryParse(tenantIds[0], out guid) && guid == Guid.Empty)
{
_context.Trace.WriteLine($"Found {AzureDevOpsConstants.VssResourceTenantHeader} header with MSA tenant ID (empty GUID).");
return commonAuthority;
return msaAuthority;
}
}
}
}

// Use the common authority if we can't determine a specific one
_context.Trace.WriteLine("Falling back to common authority.");
_context.Trace.WriteLine($"Unable to determine AAD/MSA tenant - falling back to common authority");
return commonAuthority;
}

Expand Down

0 comments on commit f80007b

Please sign in to comment.