Skip to content

Fix Azure Repos sign-in with Microsoft Accounts #148

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 1 commit into from
Jul 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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