From da9318f4313064a019772757fd96d94b65d5a5d9 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Tue, 30 Oct 2018 16:23:55 -0700 Subject: [PATCH] Block enabled 2fa in the UI without cookie consent (#2035) * Block enabled 2fa in the UI without cookie consent * Guard against feature not being there * Set up tweak * Fix --- .../Manage/TwoFactorAuthentication.cshtml | 91 ++++++++++-------- .../Manage/TwoFactorAuthentication.cshtml | 92 +++++++++++-------- .../Infrastructure/DefaultUIContext.cs | 9 ++ .../ManagementTests.cs | 19 +++- .../Pages/Account/Manage/Index.cs | 12 ++- .../Account/Manage/TwoFactorAuthentication.cs | 13 ++- test/Identity.FunctionalTests/UserStories.cs | 19 +++- .../Pages/Privacy.cshtml.cs | 2 + 8 files changed, 170 insertions(+), 87 deletions(-) diff --git a/src/UI/Areas/Identity/Pages/V3/Account/Manage/TwoFactorAuthentication.cshtml b/src/UI/Areas/Identity/Pages/V3/Account/Manage/TwoFactorAuthentication.cshtml index 648add0531..cbfd6850d5 100644 --- a/src/UI/Areas/Identity/Pages/V3/Account/Manage/TwoFactorAuthentication.cshtml +++ b/src/UI/Areas/Identity/Pages/V3/Account/Manage/TwoFactorAuthentication.cshtml @@ -1,4 +1,5 @@ @page +@using Microsoft.AspNetCore.Http.Features @model TwoFactorAuthenticationModel @{ ViewData["Title"] = "Two-factor authentication (2FA)"; @@ -7,50 +8,64 @@

@ViewData["Title"]

-@if (Model.Is2faEnabled) -{ - if (Model.RecoveryCodesLeft == 0) +@{ + var consentFeature = HttpContext.Features.Get(); + @if (consentFeature?.CanTrack ?? true) + { + @if (Model.Is2faEnabled) + { + if (Model.RecoveryCodesLeft == 0) + { +
+ You have no recovery codes left. +

You must generate a new set of recovery codes before you can log in with a recovery code.

+
+ } + else if (Model.RecoveryCodesLeft == 1) + { +
+ You have 1 recovery code left. +

You can generate a new set of recovery codes.

+
+ } + else if (Model.RecoveryCodesLeft <= 3) + { +
+ You have @Model.RecoveryCodesLeft recovery codes left. +

You should generate a new set of recovery codes.

+
+ } + + if (Model.IsMachineRemembered) + { +
+ +
+ } + Disable 2FA + Reset recovery codes + } + +
Authenticator app
+ @if (!Model.HasAuthenticator) + { + Add authenticator app + } + else + { + Set up authenticator app + Reset authenticator app + } + } + else {
- You have no recovery codes left. -

You must generate a new set of recovery codes before you can log in with a recovery code.

+ Privacy and cookie policy have not been accepted. +

You must accept the policy before you can enable two factor authentication.

} - else if (Model.RecoveryCodesLeft == 1) - { -
- You have 1 recovery code left. -

You can generate a new set of recovery codes.

-
- } - else if (Model.RecoveryCodesLeft <= 3) - { -
- You have @Model.RecoveryCodesLeft recovery codes left. -

You should generate a new set of recovery codes.

-
- } - - if (Model.IsMachineRemembered) - { -
- -
- } - Disable 2FA - Reset recovery codes } -
Authenticator app
-@if (!Model.HasAuthenticator) -{ - Add authenticator app -} -else -{ - Setup authenticator app - Reset authenticator app -} @section Scripts { diff --git a/src/UI/Areas/Identity/Pages/V4/Account/Manage/TwoFactorAuthentication.cshtml b/src/UI/Areas/Identity/Pages/V4/Account/Manage/TwoFactorAuthentication.cshtml index 89df307fbd..b07b8c0450 100644 --- a/src/UI/Areas/Identity/Pages/V4/Account/Manage/TwoFactorAuthentication.cshtml +++ b/src/UI/Areas/Identity/Pages/V4/Account/Manage/TwoFactorAuthentication.cshtml @@ -1,4 +1,5 @@ @page +@using Microsoft.AspNetCore.Http.Features @model TwoFactorAuthenticationModel @{ ViewData["Title"] = "Two-factor authentication (2FA)"; @@ -7,49 +8,62 @@

@ViewData["Title"]

-@if (Model.Is2faEnabled) -{ - if (Model.RecoveryCodesLeft == 0) +@{ + var consentFeature = HttpContext.Features.Get(); + @if (consentFeature?.CanTrack ?? true) + { + @if (Model.Is2faEnabled) + { + if (Model.RecoveryCodesLeft == 0) + { +
+ You have no recovery codes left. +

You must generate a new set of recovery codes before you can log in with a recovery code.

+
+ } + else if (Model.RecoveryCodesLeft == 1) + { +
+ You have 1 recovery code left. +

You can generate a new set of recovery codes.

+
+ } + else if (Model.RecoveryCodesLeft <= 3) + { +
+ You have @Model.RecoveryCodesLeft recovery codes left. +

You should generate a new set of recovery codes.

+
+ } + + if (Model.IsMachineRemembered) + { +
+ +
+ } + Disable 2FA + Reset recovery codes + } + +
Authenticator app
+ @if (!Model.HasAuthenticator) + { + Add authenticator app + } + else + { + Set up authenticator app + Reset authenticator app + } + } + else {
- You have no recovery codes left. -

You must generate a new set of recovery codes before you can log in with a recovery code.

+ Privacy and cookie policy have not been accepted. +

You must accept the policy before you can enable two factor authentication.

} - else if (Model.RecoveryCodesLeft == 1) - { -
- You have 1 recovery code left. -

You can generate a new set of recovery codes.

-
- } - else if (Model.RecoveryCodesLeft <= 3) - { -
- You have @Model.RecoveryCodesLeft recovery codes left. -

You should generate a new set of recovery codes.

-
- } - - if (Model.IsMachineRemembered) - { -
- -
- } - Disable 2FA - Reset recovery codes -} - -
Authenticator app
-@if (!Model.HasAuthenticator) -{ - Add authenticator app -} -else -{ - Setup authenticator app - Reset authenticator app } @section Scripts { diff --git a/test/Identity.FunctionalTests/Infrastructure/DefaultUIContext.cs b/test/Identity.FunctionalTests/Infrastructure/DefaultUIContext.cs index abe4eb5a7d..71380d8a86 100644 --- a/test/Identity.FunctionalTests/Infrastructure/DefaultUIContext.cs +++ b/test/Identity.FunctionalTests/Infrastructure/DefaultUIContext.cs @@ -32,6 +32,9 @@ namespace Microsoft.AspNetCore.Identity.FunctionalTests internal DefaultUIContext WithPasswordLogin() => new DefaultUIContext(this) { PasswordLoginEnabled = true }; + internal DefaultUIContext WithCookieConsent() => + new DefaultUIContext(this) { CookiePolicyAccepted = true }; + public string AuthenticatorKey { get => GetValue(nameof(AuthenticatorKey)); @@ -84,5 +87,11 @@ namespace Microsoft.AspNetCore.Identity.FunctionalTests get => GetValue(nameof(PasswordLoginEnabled)); set => SetValue(nameof(PasswordLoginEnabled), value); } + + public bool CookiePolicyAccepted + { + get => GetValue(nameof(CookiePolicyAccepted)); + set => SetValue(nameof(CookiePolicyAccepted), value); + } } } diff --git a/test/Identity.FunctionalTests/ManagementTests.cs b/test/Identity.FunctionalTests/ManagementTests.cs index 1c02bcda75..b2ef750f94 100644 --- a/test/Identity.FunctionalTests/ManagementTests.cs +++ b/test/Identity.FunctionalTests/ManagementTests.cs @@ -43,7 +43,23 @@ namespace Microsoft.AspNetCore.Identity.FunctionalTests var index = await UserStories.RegisterNewUserAsync(client, userName, password); // Act & Assert - await UserStories.EnableTwoFactorAuthentication(index); + Assert.NotNull(await UserStories.EnableTwoFactorAuthentication(index)); + } + + [Fact] + public async Task CannotEnableTwoFactorAuthenticationWithoutCookieConsent() + { + // Arrange + var client = ServerFactory + .CreateClient(); + + var userName = $"{Guid.NewGuid()}@example.com"; + var password = $"!Test.Password1$"; + + var index = await UserStories.RegisterNewUserAsync(client, userName, password); + + // Act & Assert + Assert.Null(await UserStories.EnableTwoFactorAuthentication(index, consent: false)); } [Fact] @@ -241,6 +257,7 @@ namespace Microsoft.AspNetCore.Identity.FunctionalTests var twoFactorKey = showRecoveryCodes.Context.AuthenticatorKey; // Use a new client to simulate a new browser session. + await UserStories.AcceptCookiePolicy(newClient); var index = await UserStories.LoginExistingUser2FaAsync(newClient, userName, password, twoFactorKey); await UserStories.ResetAuthenticator(index); diff --git a/test/Identity.FunctionalTests/Pages/Account/Manage/Index.cs b/test/Identity.FunctionalTests/Pages/Account/Manage/Index.cs index cde616600e..6e3a149713 100644 --- a/test/Identity.FunctionalTests/Pages/Account/Manage/Index.cs +++ b/test/Identity.FunctionalTests/Pages/Account/Manage/Index.cs @@ -47,12 +47,19 @@ namespace Microsoft.AspNetCore.Identity.FunctionalTests.Account.Manage } } - public async Task ClickTwoFactorLinkAsync() + public async Task ClickTwoFactorLinkAsync(bool consent = true) { + // Accept cookie consent if requested + if (consent) + { + await UserStories.AcceptCookiePolicy(Client); + } + var goToTwoFactor = await Client.GetAsync(_twoFactorLink.Href); var twoFactor = await ResponseAssert.IsHtmlDocumentAsync(goToTwoFactor); - return new TwoFactorAuthentication(Client, twoFactor, Context); + var context = consent ? Context.WithCookieConsent() : Context; + return new TwoFactorAuthentication(Client, twoFactor, context); } public async Task ClickTwoFactorEnabledLinkAsync() @@ -60,6 +67,7 @@ namespace Microsoft.AspNetCore.Identity.FunctionalTests.Account.Manage var goToTwoFactor = await Client.GetAsync(_twoFactorLink.Href); var twoFactor = await ResponseAssert.IsHtmlDocumentAsync(goToTwoFactor); Context.TwoFactorEnabled = true; + Context.CookiePolicyAccepted = true; return new TwoFactorAuthentication(Client, twoFactor, Context); } diff --git a/test/Identity.FunctionalTests/Pages/Account/Manage/TwoFactorAuthentication.cs b/test/Identity.FunctionalTests/Pages/Account/Manage/TwoFactorAuthentication.cs index 333f564b9b..3b5573bf63 100644 --- a/test/Identity.FunctionalTests/Pages/Account/Manage/TwoFactorAuthentication.cs +++ b/test/Identity.FunctionalTests/Pages/Account/Manage/TwoFactorAuthentication.cs @@ -16,13 +16,20 @@ namespace Microsoft.AspNetCore.Identity.FunctionalTests.Account.Manage public TwoFactorAuthentication(HttpClient client, IHtmlDocument twoFactor, DefaultUIContext context) : base(client, twoFactor, context) { - if (!Context.TwoFactorEnabled) + if (Context.CookiePolicyAccepted) { - _enableAuthenticatorLink = HtmlAssert.HasLink("#enable-authenticator", twoFactor); + if (!Context.TwoFactorEnabled) + { + _enableAuthenticatorLink = HtmlAssert.HasLink("#enable-authenticator", twoFactor); + } + else + { + _resetAuthenticatorLink = HtmlAssert.HasLink("#reset-authenticator", twoFactor); + } } else { - _resetAuthenticatorLink = HtmlAssert.HasLink("#reset-authenticator", twoFactor); + Assert.Contains("You must accept the policy before you can enable two factor authentication.", twoFactor.DocumentElement.TextContent); } } diff --git a/test/Identity.FunctionalTests/UserStories.cs b/test/Identity.FunctionalTests/UserStories.cs index 92111a8d2d..96c5d496d2 100644 --- a/test/Identity.FunctionalTests/UserStories.cs +++ b/test/Identity.FunctionalTests/UserStories.cs @@ -27,6 +27,7 @@ namespace Microsoft.AspNetCore.Identity.FunctionalTests return await register.SubmitRegisterFormForValidUserAsync(userName, password); } + internal static async Task LoginExistingUserAsync(HttpClient client, string userName, string password) { var index = await Index.CreateAsync(client); @@ -105,12 +106,16 @@ namespace Microsoft.AspNetCore.Identity.FunctionalTests return await login2Fa.Send2FACodeAsync(twoFactorKey); } - internal static async Task EnableTwoFactorAuthentication(Index index) + internal static async Task EnableTwoFactorAuthentication(Index index, bool consent = true) { var manage = await index.ClickManageLinkAsync(); - var twoFactor = await manage.ClickTwoFactorLinkAsync(); - var enableAuthenticator = await twoFactor.ClickEnableAuthenticatorLinkAsync(); - return await enableAuthenticator.SendValidCodeAsync(); + var twoFactor = await manage.ClickTwoFactorLinkAsync(consent); + if (consent) + { + var enableAuthenticator = await twoFactor.ClickEnableAuthenticatorLinkAsync(); + return await enableAuthenticator.SendValidCodeAsync(); + } + return null; } internal static async Task ResetAuthenticator(Index index) @@ -219,5 +224,11 @@ namespace Microsoft.AspNetCore.Identity.FunctionalTests ResponseAssert.IsOK(download); return JsonConvert.DeserializeObject(await download.Content.ReadAsStringAsync()); } + + internal static async Task AcceptCookiePolicy(HttpClient client) + { + var goToPrivacy = await client.GetAsync("/Privacy"); + } + } } diff --git a/test/WebSites/Identity.DefaultUI.WebSite/Pages/Privacy.cshtml.cs b/test/WebSites/Identity.DefaultUI.WebSite/Pages/Privacy.cshtml.cs index fbc931d2a2..62cf49da96 100644 --- a/test/WebSites/Identity.DefaultUI.WebSite/Pages/Privacy.cshtml.cs +++ b/test/WebSites/Identity.DefaultUI.WebSite/Pages/Privacy.cshtml.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Mvc.RazorPages; namespace Identity.DefaultUI.WebSite.Pages @@ -9,6 +10,7 @@ namespace Identity.DefaultUI.WebSite.Pages { public void OnGet() { + HttpContext.Features.Get().GrantConsent(); } } } \ No newline at end of file