auth: add tfa check to remove tfa

darakeon · Feb 11, 2025 · 091aa18 · 091aa18
1 parent a40d2b2
commit 091aa18
Show file tree

Hide file tree

Showing 12 changed files with 92 additions and 60 deletions.
diff --git a/core/BusinessLogic/Response/TFACheck.cs b/core/BusinessLogic/Response/TFACheck.cs
@@ -0,0 +1,9 @@
+using System;
+
+namespace DFM.BusinessLogic.Response;
+
+public class TFACheck
+{
+	public String Code { get; set; }
+	public String Password { get; set; }
+}
diff --git a/core/BusinessLogic/Response/TFAInfo.cs b/core/BusinessLogic/Response/TFAInfo.cs
@@ -2,10 +2,8 @@
 
 namespace DFM.BusinessLogic.Response
 {
-	public class TFAInfo
+	public class TFAInfo : TFACheck
 	{
 		public String Secret { get; set; }
-		public String Code { get; set; }
-		public String Password { get; set; }
 	}
 }
diff --git a/core/BusinessLogic/Services/AuthService.cs b/core/BusinessLogic/Services/AuthService.cs
@@ -224,16 +224,17 @@ public void UpdateTFA(TFAInfo info)
 			});
 		}
 
-		public void RemoveTFA(String currentPassword)
+		public void RemoveTFA(TFACheck info)
 		{
 			var user = GetCurrent();
 
 			valids.User.CheckUserDeletion(user);
 			parent.Law.CheckContractAccepted(user);
 
-			valids.User.CheckPassword(user, currentPassword);
+			valids.User.CheckPassword(user, info.Password);
 
 			valids.User.CheckTFAConfigured(user);
+			valids.User.CheckTFA(user, info.Code);
 
 			inTransaction(
 				"RemoveTFA",

diff --git a/core/Language/Language/Site/general.json b/core/Language/Language/Site/general.json
@@ -149,6 +149,7 @@
 
 		"TFATitle": "These Settings keeps your data safer",
 		"TFAExplanation": "Each time one try to login, after typing e-mail and password, a code will be asked. This code is generated by cellphone apps. So, if someone break your password, it will still need to have your device to login.{0}To activate it, install an two-factor authentication app (we recommend using {1} or {2}), scan the QRCode below, and type the code generated here.",
+		"LostTFA": "I don't have the code",
 
 		"AndroidRobotLicenseText": "The Android robot is reproduced or modified from work created and shared by Google and used according to terms described in the Creative Commons 3.0 Attribution License.",
 
@@ -366,6 +367,7 @@
 
 		"TFATitle": "Esta configuração mantém seus dados mais seguros",
 		"TFAExplanation": "A cada vez que for se logar, depois de digitar e-mail e senha, um código será pedido. Esse código é gerado por aplicativos de celular. Então, se alguém descobrir sua senha, ainda vai precisar ter acesso ao seu dispositivo para entrar.{0}Para ativar, instale um aplicativo de autenticação em duas etapas (recomendamos {1} ou {2}), aponte para o QRCode, e digite aqui o código que aparecer no aplicativo.",
+		"LostTFA": "não sei o código",
 
 		"AndroidRobotLicenseText": "O robô Android é reproduzido ou modificado a partir de trabalhos criados e compartilhados pela Google e usado de acordo com os termos descritos na Licença de atribuição Creative Commons 3.0.",
 

diff --git a/core/Language/Language/Site/settings.json b/core/Language/Language/Site/settings.json
@@ -45,9 +45,10 @@
 		"TFAWhy": "Why to use this?",
 		"TFAExplanation": "Each time one try to login, after typing e-mail and password, a code will be asked. This code is generated by cellphone apps. So, if someone break your password, it will still need to have your device to login.{0}To activate it, install an two-factor authentication app (we recommend using {1} or {2}), scan the QRCode below, and type the code generated here.",
 		"TFARemove": "Remove safer login",
-		"TFAPasswordEnable": "Use safer login\ncode as password",
-		"TFAPasswordDisable": "Stop using safer login\ncode as password",
-		"TFAAuthenticated": "Safer login set!",
+		"TFAPasswordEnable": "Use safer login code as password",
+		"TFAPasswordDisable": "Stop using safer login code as password",
+		"TFASet": "Safer login set!",
+		"TFAUnset": "Safer login removed.",
 
 		"Wipe_Title": "Watch out, there is no way back!",
 		"Wipe_What": "Here you can ask the removal of your data from the system.",
@@ -112,9 +113,10 @@
 		"TFAWhy": "Por que usar?",
 		"TFAExplanation": "A cada vez que for se logar, depois de digitar e-mail e senha, um código será pedido. Esse código é gerado por aplicativos de celular. Então, se alguém descobrir sua senha, ainda vai precisar ter acesso ao seu dispositivo para entrar.{0}Para ativar, instale um aplicativo de autenticação em duas etapas (recomendamos {1} ou {2}), aponte para o QRCode, e digite aqui o código que aparecer no aplicativo.",
 		"TFARemove": "Remover login mais seguro",
-		"TFAPasswordEnable": "Usar o código de login\nmais seguro como senha",
-		"TFAPasswordDisable": "Deixar de usar o código de\nlogin mais seguro como senha",
-		"TFAAuthenticated": "Login mais seguro configurado!",
+		"TFAPasswordEnable": "Usar o código de login mais seguro como senha",
+		"TFAPasswordDisable": "Deixar de usar o código de login mais seguro como senha",
+		"TFASet": "Login mais seguro configurado!",
+		"TFAUnset": "Login mais seguro removido.",
 
 		"Wipe_Title": "Cuidado, esse passo não tem volta!",
 		"Wipe_What": "Aqui você pode pedir a remoção de seus dados do sistema.",

diff --git a/core/Language/Language/Site/users.json b/core/Language/Language/Site/users.json
@@ -15,7 +15,6 @@
 		"ContractBeginDate": "Effective Date",
 		"CookiesWarning": "Cookies are used to control logon and security of forms of this website",
 		"TFAVerification": "Use the code of the app registered before",
-		"LostTFA": "I don't have the code",
 
 		"UserVerificationSent": "Code sent",
 
@@ -44,7 +43,6 @@
 		"ContractBeginDate": "Início de Vigência",
 		"CookiesWarning": "Cookies são usados para controlar seu acesso a informações e a segurança dos formulários deste site",
 		"TFAVerification": "Use o código gerado pelo aplicativo cadastrado anteriormente",
-		"LostTFA": "não sei o código",
 
 		"UserVerificationSent": "Código enviado",
 

diff --git a/core/Tests/BusinessLogic/A.Auth/i.RemoveTFA.feature b/core/Tests/BusinessLogic/A.Auth/i.RemoveTFA.feature
@@ -44,31 +44,55 @@ Scenario: Ai04. With null password
 	Then I will receive this core error: WrongPassword
 		And the two-factor will be [123]
 
-Scenario: Ai05. Not remove if user is marked for deletion
+Scenario: Ai05. With wrong code
+	Given I have this two-factor data
+			| Code   | Password  |
+			| 150124 | pass_word |
+	When I try to remove two-factor
+	Then I will receive this core error: TFAWrongCode
+		And the two-factor will be [123]
+
+Scenario: Ai06. With empty code
+	Given I have this two-factor data
+			| Code | Password  |
+			|      | pass_word |
+	When I try to remove two-factor
+	Then I will receive this core error: TFAWrongCode
+		And the two-factor will be [123]
+
+Scenario: Ai07. With null code
+	Given I have this two-factor data
+			| Code   | Password  |
+			| {null} | pass_word |
+	When I try to remove two-factor
+	Then I will receive this core error: TFAWrongCode
+		And the two-factor will be [123]
+
+Scenario: Ai08. Not remove if user is marked for deletion
 	Given I have this two-factor data
 			| Code        | Password |
 			| {generated} | password |
 		But the user is marked for deletion
 	When I try to remove two-factor
 	Then I will receive this core error: UserDeleted
 
-Scenario: Ai06. Not remove if user requested wipe
+Scenario: Ai09. Not remove if user requested wipe
 	Given I have this two-factor data
 			| Code        | Password |
 			| {generated} | password |
 		But the user asked data wipe
 	When I try to remove two-factor
 	Then I will receive this core error: UserAskedWipe
 
-Scenario: Ai07. Not remove if not signed last contract
+Scenario: Ai10. Not remove if not signed last contract
 	Given I have this two-factor data
 			| Code        | Password  |
 			| {generated} | pass_word |
 		But there is a new contract
 	When I try to remove two-factor
 	Then I will receive this core error: NotSignedLastContract
 
-Scenario: Ai08. Remove if not configured
+Scenario: Ai11. Remove if not configured
 	Given I have this two-factor data
 			| Code        | Password  |
 			| {generated} | pass_word |
@@ -77,7 +101,7 @@ Scenario: Ai08. Remove if not configured
 	Then I will receive this core error: TFANotConfigured
 		And the two-factor will be empty
 
-Scenario: Ai09. Remove if set as password
+Scenario: Ai12. Remove if set as password
 	Given I have this two-factor data
 			| Code        | Password  |
 			| {generated} | pass_word |

diff --git a/core/Tests/BusinessLogic/A.Auth/n.AskRemoveTFA.feature b/core/Tests/BusinessLogic/A.Auth/n.AskRemoveTFA.feature
@@ -85,9 +85,12 @@ Scenario: An07. Not remove if not signed last contract
 
 Scenario: An08. Ask remove with no TFA
 	Given I have this two-factor data
+			| Code        | Password  |
+			| {generated} | pass_word |
+		And I remove two-factor
+		But I have this two-factor data
 			| Password  |
 			| pass_word |
-		But I remove two-factor
 	When I ask to remove two-factor
 	Then I will receive this core error: TFANotConfigured
 		And the two-factor will be empty

diff --git a/core/Tests/BusinessLogic/Steps/_A.SafeStep.cs b/core/Tests/BusinessLogic/Steps/_A.SafeStep.cs
@@ -1347,10 +1347,12 @@ public void GivenILoginThisUser(Table table)
 		public void GivenIHaveThisTwoFactorData(Table table)
 		{
 			tfa = table.CreateInstance<TFAInfo>();
+			var secret = tfa.Secret
+			    ?? repos.User.GetByEmail(userEmail).TFASecret;
 
 			if (tfa.Code == "{generated}")
 			{
-				tfa.Code = CodeGenerator.Generate(tfa.Secret);
+				tfa.Code = CodeGenerator.Generate(secret);
 			}
 
 			if (tfa.Password == "{null}")
@@ -1388,7 +1390,7 @@ public void WhenITryToRemoveTwoFactor()
 		{
 			try
 			{
-				service.Auth.RemoveTFA(tfa.Password);
+				service.Auth.RemoveTFA(tfa);
 			}
 			catch (CoreError e)
 			{

diff --git a/docs/RELEASES.md b/docs/RELEASES.md
@@ -24,7 +24,7 @@ This is the list of project releases, past and current. To see tasks that are st
 - [ ] Disable account with too much MFA retries
 - [ ] Add MFA to change email
 - [ ] Add MFA to change password
-- [ ] `250209>......` Add MFA to disable MFA
+- [x] `250209>250211` Add MFA to disable MFA
 - [x] `250202>250208` Allow disable MFA by sending link to email in the MFA require screen, asking for password
 - [x] `250126>250129` Create measure of recovering after lost authy
 

diff --git a/site/MVC/Models/SettingsTFAModel.cs b/site/MVC/Models/SettingsTFAModel.cs
@@ -33,11 +33,11 @@ public IList<String> Save()
 			try
 			{
 				if (IsActive)
-					auth.RemoveTFA(TFA.Password);
+					auth.RemoveTFA(TFA);
 				else
 					auth.UpdateTFA(TFA);
 
-				errorAlert.Add("TFAAuthenticated");
+				errorAlert.Add(IsActive ? "TFAUnset" : "TFASet");
 			}
 			catch (CoreError e)
 			{

diff --git a/site/MVC/Views/Settings/TFA.cshtml b/site/MVC/Views/Settings/TFA.cshtml
@@ -25,16 +25,7 @@
 	<script type="text/javascript" src="~/Assets/scripts/[email protected]"></script>
 }
 
-@Html.HiddenFor(m => m.IsActive)
-
-@if (Model.IsActive)
-{
-	<div class="form-group">
-		@Html.LabelFor(m => m.TFA.Password, Context.Translate("CurrentPassword"), new { @class = "control-label" })
-		@Html.PasswordFor(m => m.TFA.Password, new { @class = "form-control" })
-	</div>
-}
-else
+@if (!Model.IsActive)
 {
 	<div class="text-center">
 		<span id="qrcode" data-url="@Model.UrlPath">
@@ -49,43 +40,45 @@ else
 			</a>
 		</small>
 	</div>
+}
 
-	<div class="form-group">
-		@Html.LabelFor(m => m.TFA.Code, Context.Translate("Code"), new { @class = "control-label" })
-		@Html.TextBoxFor(m => m.TFA.Code, new { @class = "form-control" })
-	</div>
+<div class="form-group">
+	@Html.LabelFor(m => m.TFA.Code, Context.Translate("Code"), new { @class = "control-label" })
+	@Html.TextBoxFor(m => m.TFA.Code, new { @class = "form-control" })
+</div>
 
-	<div class="form-group">
-		@Html.LabelFor(m => m.TFA.Password, Context.Translate("CurrentPassword"), new { @class = "control-label" })
-		@Html.PasswordFor(m => m.TFA.Password, new { @class = "form-control" })
-	</div>
+<div class="form-group">
+	@Html.LabelFor(m => m.TFA.Password, Context.Translate("CurrentPassword"), new { @class = "control-label" })
+	@Html.PasswordFor(m => m.TFA.Password, new { @class = "form-control" })
+</div>
+
+@if (Model.IsActive)
+{
+	var action = Model.TFAPassword
+		? "TFAPasswordDisable"
+		: "TFAPasswordEnable";
+
+	@Html.ActionLink(
+		Context.Translate(action),
+		action, "Settings", null,
+		new { @class = "text-warning pull-left" }
+	)
 }
 
 @await Html.PartialAsync("Modals/TFA")
 
 @section Footer {
-	@{
-		var button = "Save";
-	}
-
 	@if (Model.IsActive)
 	{
-		button = "TFARemove";
-
-		var action = Model.TFAPassword
-			? "TFAPasswordDisable"
-			: "TFAPasswordEnable";
-
-		<small>
-			<a class="pull-left text-left"
-					href="@Url.Action(action, "Settings")">
-				@Html.Raw(Context.Translate(action).Replace("\n", "<br />"))
-			</a>
-		</small>
+		@Html.ActionLink(
+			Context.Translate("LostTFA"),
+			"AskRemoveTFA", null, null,
+			new { @class = "btn btn-sm btn-info pull-left" }
+		)
 	}
 
 	<button type="submit"
-			class="btn btn-success tab-submit">
-		@Context.Translate(button)
+			class="btn btn-sm btn-@(Model.IsActive ? "warning" : "success") tab-submit">
+		@Context.Translate(Model.IsActive ? "TFARemove" : "Save")
 	</button>
 }