Skip to content
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

[BugBash] Prevent AAD to MSA account transformation #5492

Merged
merged 9 commits into from
Feb 16, 2018
10 changes: 9 additions & 1 deletion src/NuGetGallery/Controllers/AuthenticationController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,15 @@ public virtual ActionResult AuthenticatePost(string returnUrl, string provider)
[HttpGet]
public virtual ActionResult AuthenticateExternal(string returnUrl)
{
string externalAuthProvider = GetExternalProvider();
var user = GetCurrentUser();
var aadCredential = user?.Credentials.GetAzureActiveDirectoryCredential();
if (aadCredential != null)
{
TempData["WarningMessage"] = Strings.ChangeCredential_NotAllowed;
return Redirect(returnUrl);
}

var externalAuthProvider = GetExternalProvider();
if (externalAuthProvider == null)
{
TempData["Message"] = Strings.ChangeCredential_ProviderNotFound;
Expand Down
9 changes: 0 additions & 9 deletions src/NuGetGallery/Controllers/UsersController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -610,15 +610,6 @@ public virtual async Task<ActionResult> RemoveCredential(string credentialType,
[ValidateAntiForgeryToken]
public virtual ActionResult LinkOrChangeExternalCredential()
{
var user = GetCurrentUser();
var userHasAADCredential = user.Credentials.Any(c => CredentialTypes.IsAzureActiveDirectoryAccount(c.Type));

if (userHasAADCredential)
{
TempData["WarningMessage"] = Strings.ChangeCredential_NotAllowed;
return RedirectToAction("Account");
}

return Redirect(Url.AuthenticateExternal(Url.AccountSettings()));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -994,6 +994,77 @@ public async Task GivenNewCredential_ItSuccessfullyReplacesExternalCredentialsAn
}
}

public class TheAuthenticateExternalAction : TestContainer
{
[Fact]
public void ForAADLinkedAccount_ErrorIsReturned()
{
var fakes = Get<Fakes>();
var aadCred = new CredentialBuilder().CreateExternalCredential("AzureActiveDirectory", "blorg", "bloog");
var passwordCred = new Credential("password.v3", "random");
var msftCred = new CredentialBuilder().CreateExternalCredential("MicrosoftAccount", "bloom", "filter");
var user = fakes.CreateUser("test", aadCred, passwordCred, msftCred);
var controller = GetController<AuthenticationController>();
controller.SetCurrentUser(user);

// Act
var result = controller.AuthenticateExternal("theReturnUrl");

// Assert
ResultAssert.IsRedirectTo(result, "theReturnUrl");
Assert.Equal(Strings.ChangeCredential_NotAllowed, controller.TempData["WarningMessage"]);
}

[Fact]
public void ForMissingExternalProvider_ErrorIsReturned()
{
var controller = GetController<AuthenticationController>();

// Act
var result = controller.AuthenticateExternal("theReturnUrl");

// Assert
ResultAssert.IsRedirectTo(result, "theReturnUrl");
Assert.Equal(Strings.ChangeCredential_ProviderNotFound, controller.TempData["Message"]);
}

[Fact]
public void WillCallChallengeAuthenticationForAADv2ProviderForUserWithNoAADCredential()
{
// Arrange
const string returnUrl = "/theReturnUrl";
EnableAllAuthenticators(Get<AuthenticationService>());

var fakes = Get<Fakes>();
var passwordCred = new Credential("password.v3", "random");
var msftCred = new CredentialBuilder().CreateExternalCredential("MicrosoftAccount", "bloom", "filter");
var user = fakes.CreateUser("test", passwordCred, msftCred);
var controller = GetController<AuthenticationController>();
controller.SetCurrentUser(user);

// Act
var result = controller.AuthenticateExternal(returnUrl);

// Assert
ResultAssert.IsChallengeResult(result, "AzureActiveDirectoryV2", controller.Url.LinkOrChangeExternalCredential(returnUrl));
}

[Fact]
public void WillCallChallengeAuthenticationForAADv2Provider()
{
// Arrange
const string returnUrl = "/theReturnUrl";
EnableAllAuthenticators(Get<AuthenticationService>());
var controller = GetController<AuthenticationController>();

// Act
var result = controller.AuthenticateExternal(returnUrl);

// Assert
ResultAssert.IsChallengeResult(result, "AzureActiveDirectoryV2", controller.Url.LinkOrChangeExternalCredential(returnUrl));
}
}

public class TheLinkExternalAccountAction : TestContainer
{
[Fact]
Expand Down
20 changes: 0 additions & 20 deletions tests/NuGetGallery.Facts/Controllers/UsersControllerFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1603,26 +1603,6 @@ public async Task GivenValidRequest_CanDeleteMicrosoftAccountWithMultipleMicroso

public class TheLinkOrChangeCredentialAction : TestContainer
{
[Fact]
public void ForAADLinkedAccount_ErrorIsReturned()
{
// Arrange
var fakes = Get<Fakes>();
var aadCred = new CredentialBuilder().CreateExternalCredential("AzureActiveDirectory", "blorg", "bloog");
var passwordCred = new Credential("password.v3", "random");
var msftCred = new CredentialBuilder().CreateExternalCredential("MicrosoftAccount", "bloom", "filter");
var user = fakes.CreateUser("test", aadCred, passwordCred, msftCred);
var controller = GetController<UsersController>();
controller.SetCurrentUser(user);

// Act
var result = controller.LinkOrChangeExternalCredential();

// Assert
ResultAssert.IsRedirectToRoute(result, new { action = "Account" });
Assert.Equal(Strings.ChangeCredential_NotAllowed, controller.TempData["WarningMessage"]);
}

[Fact]
public void ForNonAADLinkedAccount_RedirectsToAuthenticateExternalLogin()
{
Expand Down