From 55dff81b9f33fa17ed6625e71824e8e2060d532f Mon Sep 17 00:00:00 2001
From: Vincent Salucci <26154748+vincentsalucci@users.noreply.github.com>
Date: Wed, 18 Mar 2020 13:43:20 -0500
Subject: [PATCH] Sanitize Password Length (#783)

* Sanitize Password Length

* Formatting updates

* Else if format

Co-authored-by: Vincent Salucci <vsalucci@bitwarden.com>
---
 .../Services/PasswordGenerationService.cs     | 115 +++++++++++-------
 1 file changed, 69 insertions(+), 46 deletions(-)

diff --git a/src/Core/Services/PasswordGenerationService.cs b/src/Core/Services/PasswordGenerationService.cs
index 34854e6ab..9023265a6 100644
--- a/src/Core/Services/PasswordGenerationService.cs
+++ b/src/Core/Services/PasswordGenerationService.cs
@@ -51,52 +51,7 @@ namespace Bit.Core.Services
             }
 
             // Sanitize
-            if(options.Uppercase.GetValueOrDefault() && options.MinUppercase.GetValueOrDefault() <= 0)
-            {
-                options.MinUppercase = 1;
-            }
-            else if(!options.Uppercase.GetValueOrDefault())
-            {
-                options.MinUppercase = 0;
-            }
-
-            if(options.Lowercase.GetValueOrDefault() && options.MinLowercase.GetValueOrDefault() <= 0)
-            {
-                options.MinLowercase = 1;
-            }
-            else if(!options.Lowercase.GetValueOrDefault())
-            {
-                options.MinLowercase = 0;
-            }
-
-            if(options.Number.GetValueOrDefault() && options.MinNumber.GetValueOrDefault() <= 0)
-            {
-                options.MinNumber = 1;
-            }
-            else if(!options.Number.GetValueOrDefault())
-            {
-                options.MinNumber = 0;
-            }
-
-            if(options.Special.GetValueOrDefault() && options.MinSpecial.GetValueOrDefault() <= 0)
-            {
-                options.MinSpecial = 1;
-            }
-            else if(!options.Special.GetValueOrDefault())
-            {
-                options.MinSpecial = 0;
-            }
-
-            if(options.Length.GetValueOrDefault() < 1)
-            {
-                options.Length = 10;
-            }
-            var minLength = options.MinSpecial.GetValueOrDefault() + options.MinLowercase.GetValueOrDefault() +
-                options.MinNumber.GetValueOrDefault() + options.MinUppercase.GetValueOrDefault();
-            if(options.Length < minLength)
-            {
-                options.Length = minLength;
-            }
+            SanitizePasswordLength(options, true);
 
             var positionsBuilder = new StringBuilder();
             if(options.Lowercase.GetValueOrDefault() && options.MinLowercase.GetValueOrDefault() > 0)
@@ -617,6 +572,8 @@ namespace Bit.Core.Services
             {
                 options.WordSeparator = options.WordSeparator[0].ToString();
             }
+            
+            SanitizePasswordLength(options, false);
         }
 
         // Helpers
@@ -691,5 +648,71 @@ namespace Bit.Core.Services
             var num = await _cryptoService.RandomNumberAsync(0, 9);
             wordList[index] = wordList[index] + num;
         }
+
+        private void SanitizePasswordLength(PasswordGenerationOptions options, bool forGeneration)
+        {
+            var minUppercaseCalc = 0;
+            var minLowercaseCalc = 0;
+            var minNumberCalc = options.MinNumber;
+            var minSpecialCalc = options.MinNumber;
+
+            if(options.Uppercase.GetValueOrDefault() && options.MinUppercase.GetValueOrDefault() <= 0)
+            {
+                minUppercaseCalc = 1;
+            } 
+            else if(!options.Uppercase.GetValueOrDefault())
+            {
+                minUppercaseCalc = 0;
+            }
+
+            if(options.Lowercase.GetValueOrDefault() && options.MinLowercase.GetValueOrDefault() <= 0) 
+            {
+                minLowercaseCalc = 1;
+            }
+            else if(!options.Lowercase.GetValueOrDefault())
+            {
+                minLowercaseCalc = 0;
+            }
+
+            if(options.Number.GetValueOrDefault() && options.MinNumber.GetValueOrDefault() <= 0)
+            {
+                minNumberCalc = 1;
+            } 
+            else if(!options.Number.GetValueOrDefault())
+            {
+                minNumberCalc = 0;
+            }
+
+            if(options.Special.GetValueOrDefault() && options.MinSpecial.GetValueOrDefault() <= 0)
+            {
+                minSpecialCalc = 1;
+            } 
+            else if(!options.Special.GetValueOrDefault())
+            {
+                minSpecialCalc = 0;
+            }
+
+            // This should never happen but is a final safety net
+            if(options.Length.GetValueOrDefault() < 1)
+            {
+                options.Length = 10;
+            }
+
+            var minLength = minUppercaseCalc + minLowercaseCalc + minNumberCalc + minSpecialCalc;
+            // Normalize and Generation both require this modification
+            if(options.Length < minLength)
+            {
+                options.Length = minLength;
+            }
+
+            // Apply other changes if the options object passed in is for generation
+            if(forGeneration)
+            {
+                options.MinUppercase = minUppercaseCalc;
+                options.MinLowercase = minLowercaseCalc;
+                options.MinNumber = minNumberCalc;
+                options.MinSpecial = minSpecialCalc;
+            }
+        }
     }
 }