brockallen / BrockAllen.MembershipReboot

MembershipReboot is a user identity management and authentication library.
Other
742 stars 239 forks source link

Open question for email/mobile verification #231

Closed brockallen closed 10 years ago

brockallen commented 10 years ago

Should there be APIs to circumvent the email and mobile phone verification process and just set them as verified/confirmed?

Presumably these would be used if the app has its own external verification process or if accounts are being migrated from other systems where email/mobile have already been verified.

My concern is that an app will misuse these APIs (accidentally or otherwise) and open themselves to security holes.

Thoughts?

JellyMaster commented 10 years ago

Well I am kind of doing this within my website (still building this at the moment)

I have setup an admin user section that is able to create new accounts. To ensure the admin user doesn't know the user's password I have created a complex password method that will generate a password that has varying levels of complexity from simple all the way to ultra complex. (I'll list my password generating code at the end with sample output)

Once the account is created a verification email is then sent out and as part of the verification process the user will be forced to enter a new password so thus keeping any system admin's blind of the end user's password (again enforcing complex password checking here).

Personally I think that the system admin's shouldn't know the end users password as that has the potential for misuse. As for importing external accounts as part of a migration process I think the user should be forced to change the password as part of this process. Again we can enforce password complexity. Maybe if we are importing an external account and need to verify it then the password expired flag should be set to true to again force the user to change their password.

Happy to discuss any possible solutions you may have thought about. (feel free to rip my code to pieces if you see any obvious flaws :) )

        /// <summary>
        /// This is the auto generated password process
        /// </summary>
        /// <param name="charSet">A character set used to generate passwords</param>
        /// <param name="minpasswordlength">minimum length the password should be </param>
        /// <param name="complexity">enforce complexity checking from the generated password process</param>
        /// <param name="lowercase">enforce checking to ensure password has a lowercase letter in the password</param>
        /// <param name="uppercase">enforce checking to ensure password has an uppercase letter in the password</param>
        /// <param name="numeric">enforce checking to ensure numeric digit is in the password</param>
        /// <param name="specialCharacter">enforce checking to ensure a special character is in the password</param>
        /// <returns>returns the created password. </returns>
        public string GeneratePassword(string charSet = "" ,  int minpasswordlength = 16, bool complexity = true, bool lowercase = true, bool uppercase = true, bool numeric = true, bool specialCharacter = true)
        {
            //lowercase, uppercase, numeric and special checks are only processed when complexity checking is set to true. 
            //default character set is: "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ01234567890!\"£$%^&*()_+=-[]{};:'@#~<,>.?/|\\"
            //need to decide if we should enforce some checking should the character set be changed and not contain a value that is required. 
            //e.g. lowercase checking is enabled but no lowercase values in the character set. 

            //set a random value based on number of ticks (helps when multiple passwords need to be created in a single go) 
            Random rand = new Random((int)DateTime.Now.Ticks);

            string password = string.Empty; 
            bool meetsRequirements = false; 

            //if the character set is passed in as null or empty string used default character set listed above.
            if(string.IsNullOrEmpty(charSet))
            {
                charSet = passwordCharSet;
            }

            //if complexity is swicthed on then we enforce the password checking other just generate any random password. 
            if (complexity)
            {
                //we will be enforcing complexity here. 

                //keep looping through the password generator until a valid password is created. 
                while( !meetsRequirements)
                {
                    //presume the password meets requirements by default and set to false if a check that is switched on fails. 
                    meetsRequirements = true; 

                    //create a list of char's (bit of linq here) that make up the password
                    List<char> passwordList =  Enumerable.Repeat(charSet, minpasswordlength).Select(s => s[rand.Next(s.Length)]).ToList(); 

                    //if lowercase letter checking is turned on
                    if(lowercase)
                    {
                        //check the count to see if we have at least one lowercase letter in the password.
                        if(passwordList.Count(s => char.IsLower(s)) <= 0)
                        {
                            meetsRequirements = false; 
                        }
                    }

                    //if we are still valid then check if uppercase letter checking is on. 
                    if (uppercase && meetsRequirements)
                    {
                        //check the count to see if we have at least one uppercase letter in the password.
                        if (passwordList.Count(s => char.IsUpper(s)) <= 0)
                        {
                            meetsRequirements = false;
                        }
                    }

                    //if we are still valid then check if numeric checking is on.
                    if (numeric && meetsRequirements)
                    {
                        //check the count to see if we have at least one numeric in the password.
                        if (passwordList.Count(s => char.IsNumber(s)) <= 0)
                        {
                            meetsRequirements = false;
                        }
                    }

                    //if we are still valid then check if special character checking is on.
                    if (specialCharacter && meetsRequirements)
                    {
                        //check the count to see if we have at least one special character in the password.
                        if (passwordList.Count(s => !char.IsLetterOrDigit(s)) <= 0)
                        {
                            meetsRequirements = false;
                        }
                    }

                    //if the passwordList has passed all checking set it to the returned password variable
                    if(meetsRequirements)
                    {
                        password = new string(passwordList.ToArray());
                    }

                }

            }
            else
            {
                //no password complexity checking but we will probably get a pretty complex password using the default length and character set. 
                password = new string(Enumerable.Repeat(charSet, minpasswordlength).Select(s => s[rand.Next(s.Length)]).ToArray()); 
            }

            //return the generated password. 
            return password; 
        }

Sample output using default settings:

Password generated = pD9£"W&SC5;>5.$+
Password generated = #6kl_1uspc?0O8$X
Password generated = @6/[4Y}$e/CqF_u6
Password generated = '7{XOUTs,,9"w<&%
Password generated = ;7$uxRk£['\snfy-
Password generated = }81~gO*r$]G$es_#
Password generated = ]8O5@LJ"3+"u.FC\
Password generated = [9BD^IaqS&c^;T[i
Password generated = =9oaZF8!H£Jw)6Gr
Password generated = +0b$ICzpw8^*!(;A
Password generated = )!'Lrz#0l3fy2@KJ
Password generated = U[4^/;\l>pNsF7I=
Password generated = &"5={tq0#U(AKqO1
Password generated = ^"SU0q-o+Pj+BD>0
Password generated = $£FsTnP9!LPCsQR(
Password generated = ££s@Dkgn0G+-j3|}
Password generated = "$f3mh$8PBmDa&V>
Password generated = 0$~A.eFmExT]~:cd
Password generated = 9%_|_b?7ts[F[aZm
Password generated = hR|>Wii4^rjxo<%H
Password generated = 6^VJO.v6/jWH7B3E
Password generated = 4^Igx<;k;e;:YOkN
Password generated = 3&v_g@U6*\sJP17W
Password generated = 1*iS#;lk7.Z@G$o5
Password generated = 0*.p^](5X#@Lx}!$
Password generated = Xsc;!bNRCsJAb8Sc
Password generated = X("1I)b4B-2Nfl%@
Password generated = V)Zys&9iq(<,?yv|
Password generated = U)M.b$A3f$zP:M(g
Password generated = S_z0{!~h>05._Zzp
Password generated = R_mH08Z2]5.R""=y
Password generated = Q+\eU5rh%1C/3[DH
Password generated = Y'*<W4m6TuHF}9Wn
Password generated = j{NhtxW,hugF],-3
Password generated = L=3n?Wh1INFVCw'8
Password generated = x*)K30B0zvUG+VLS
Password generated = I[DZ5QG0nDcXkW~]
Password generated = H[qwON/eczIcb0P<
Password generated = F]d,yK5Z~u%Z<=.b
Password generated = E]#7hHwd=pfe],Tk
Password generated = C{)F#E:Y"lM1&g\t
Password generated = B{7c^BVd1g*g8uXC
Password generated = z}U^0ymYQbi3ZHdL
Password generated = y}HNJv)cF/PiQU1U
Password generated = ^OV74KcJFXRtIfZS
Password generated = $PIENH0:uS-^zsf1
Password generated = £PvbwEBIjOovqF30
Password generated = \ZOyD3D?[inGjWWF
Password generated = 0R>N@y0H:E{x|67}
Password generated = 8R-k%vr}(Ar)@(n>
Password generated = 7S![Ys[G8vYz=@0d
Password generated = 5SYWIpQ{Xq'+$crm
Password generated = 4TLtrmhFNmuB5q$v
Password generated = 3Ty#aj%]Ch1=WDvE
Password generated = 1Ul5]gGFrc~DNQ*N
Password generated = 0U|C0d/]g|y[E3zW
Password generated = YV}\Ta5E.,4Fv^+5
Password generated = AHfZ~cx.>s0u];(_
Password generated = VW2Ll>'D^{BHda]=
Password generated = UWPi.~W-5+8;>nH@
Password generated = R.rxDiG\2mLO=wEl
Password generated = RYpU4}_=J£E'(OKh
Password generated = e]j6Jr4I)qQKS?b<
Password generated = w}\!8FL(rI~lM^b}
Password generated = 2]9rK:n9~\&rlq0)
Password generated = <O?7WFPE)0p;)-?)
Password generated = ^&\U)q~zkaosh*n9
Password generated = aX'q0}Gd8EWa$A)P
Password generated = :9G=oT£'Ca3udE&Z
Password generated = /Y51C+5cNvtcW1-7
Password generated = )"V|5P{feQ>YEL'4
Password generated = *£I"OMS0,LAdvYL£
Password generated = &£vJxJje[H70m"<+
Password generated = H8'T\GkW0=-^;ozV
Password generated = $$._#DId3xE2>.?/
Password generated = "%[S^A\YSt0h}iTg
Password generated = !%"pZx7cHoa3(wap
Password generated = 9^Z;IuyYwjHj0JXy
Password generated = >xtmK[7xtH$@UEd9
Password generated = ,xg{t+y*iCdMLR0*
Password generated = ~y,Yc(@w/xK~C4h{
Password generated = #y=v;^X&;t&Ot&4,
Password generated = CPX:b0rBWyrN;DXJ
Password generated = :zX7V0+^8jNQbb8l
Password generated = ;AKEE7NuXf).<opu
Password generated = {Axbo4e^MakS]B"D
Password generated = ]Bk%/1"uB.R/&OtM
Password generated = -B/N=YD%q~=U82^V
Password generated = =C{k6V>tf;n\Z%x4
Password generated = Emw,;m#Q£ApR£E2U
Password generated = )D1WzPts])]bH|A+
Password generated = X=a}Ly;da4uQRsv%
Password generated = W-:ZvvUY@Z1?IG*-
Password generated = 710|SI!Vv)Xra:#4
Password generated = T[4>:p(X0Qx|q6+\
Password generated = wCdAa|LI{A8NA]^<
Password generated = Y]£0b4)i-X|uk_wD
Password generated = W]0x{1L3£SF^b~(M
Password generated = V{N>!Ych2N!w<eAV
Password generated = T{A9UV02RJb*]s=4

This is still code being tested but hopefully gives you an idea of what I am trying to do to secure my system.

girtsl commented 10 years ago

I'm very much in favor of such APIs. Recently our customer requested that the administrator users should be able to create other accounts and because the verification would have been done already using different means it would be very inconvenient to require it once again.

It is true that app developers could misuse these APIs and shoot themselves in the foot. If well documented, perhaps that wouldn't be an issue?

kabongsteve commented 10 years ago

I agree that such APIs are required.

My scenario is that we will have both internal users and external users (different tenants for each).

The internal useraccounts will be created by the administrator and they'll know and have verified these users and email addresses.

Some external user types will also be created by the administrator, who can "vouch" for the validity of the user.

Any external user who "registers" through the system should have to verify email/mobile, but those created by an internal admin should not.

An alternate scenario for me would to be able to set the requireAccountVerification on a tenant by tenant basis (other settings like allowAccountDeletion would also be useful on a tenant by tenant basis)

brockallen commented 10 years ago

Yea, ok, I guess I'm convinced. I'll work on it.

brockallen commented 10 years ago

Ok, added: https://github.com/brockallen/BrockAllen.MembershipReboot/commit/bc1686bdfb7c53e2d40132901333a28d26ce752f

Please take and look and provide any feedback. Thanks.