diff --git a/nativeauthenticator/handlers.py b/nativeauthenticator/handlers.py index 4a57dc5..ca31e5f 100644 --- a/nativeauthenticator/handlers.py +++ b/nativeauthenticator/handlers.py @@ -40,7 +40,7 @@ async def get(self): ) self.finish(html) - def get_result_message(self, user): + def get_result_message(self, user, username): alert = 'alert-info' message = 'Your information have been sent to the admin' @@ -51,11 +51,17 @@ def get_result_message(self, user): if not user: alert = 'alert-danger' pw_len = self.authenticator.minimum_password_length + taken = self.authenticator.user_exists(username) if pw_len: message = ("Something went wrong. Be sure your password has " "at least {} characters, doesn't have spaces or " "commas and is not too common.").format(pw_len) + + elif taken: + message = ("Something went wrong. It appears that this " + "username is already in use. Please try again " + "with a different username.") else: message = ("Something went wrong. Be sure your password " @@ -71,9 +77,10 @@ async def post(self): 'email': self.get_body_argument('email', '', strip=False), 'has_2fa': bool(self.get_body_argument('2fa', '', strip=False)), } - user = self.authenticator.get_or_create_user(**user_info) + user = self.authenticator.create_user(**user_info) + name = self.authenticator.user_exists(user_info['username']) - alert, message = self.get_result_message(user) + alert, message = self.get_result_message(user, name) otp_secret, user_2fa = '', '' if user: diff --git a/nativeauthenticator/nativeauthenticator.py b/nativeauthenticator/nativeauthenticator.py index c13a043..9660935 100644 --- a/nativeauthenticator/nativeauthenticator.py +++ b/nativeauthenticator/nativeauthenticator.py @@ -128,7 +128,7 @@ def authenticate(self, handler, data): username = self.normalize_username(data['username']) password = data['password'] - user = UserInfo.find(self.db, username) + user = self.get_user(username) if not user: return @@ -166,13 +166,19 @@ def is_password_strong(self, password): checks.append(not self.is_password_common(password)) return all(checks) - - def get_or_create_user(self, username, pw, **kwargs): + + def get_user(self, username): + return UserInfo.find(self.db, self.normalize_username(username)) + + def user_exists(self, username): + return self.get_user(username) is not None + + def create_user(self, username, pw, **kwargs): username = self.normalize_username(username) - user = UserInfo.find(self.db, username) - if user: - return user - + + if self.user_exists(username): + return + if not self.is_password_strong(pw) or \ not self.validate_username(username): return @@ -193,7 +199,7 @@ def get_or_create_user(self, username, pw, **kwargs): return user_info def change_password(self, username, new_password): - user = UserInfo.find(self.db, username) + user = self.get_user(username) user.password = bcrypt.hashpw(new_password.encode(), bcrypt.gensalt()) self.db.commit() @@ -214,7 +220,7 @@ def get_handlers(self, app): return native_handlers def delete_user(self, user): - user_info = UserInfo.find(self.db, user.name) + user_info = self.get_user(user.name) if user_info is not None: self.db.delete(user_info) self.db.commit() @@ -236,7 +242,7 @@ def add_data_from_firstuse(self): with dbm.open(self.firstuse_db_path, 'c', 0o600) as db: for user in db.keys(): password = db[user].decode() - new_user = self.get_or_create_user(user.decode(), password) + new_user = self.create_user(user.decode(), password) if not new_user: error = '''User {} was not created. Check password restrictions or username problems before trying diff --git a/nativeauthenticator/tests/test_authenticator.py b/nativeauthenticator/tests/test_authenticator.py index 1fdfcb8..722242c 100644 --- a/nativeauthenticator/tests/test_authenticator.py +++ b/nativeauthenticator/tests/test_authenticator.py @@ -33,7 +33,7 @@ def app(): ]) async def test_create_user(is_admin, open_signup, expected_authorization, tmpcwd, app): - '''Test method get_or_create_user for new user and authorization ''' + '''Test method create_user for new user and authorization ''' auth = NativeAuthenticator(db=app.db) if is_admin: @@ -41,19 +41,32 @@ async def test_create_user(is_admin, open_signup, expected_authorization, if open_signup: auth.open_signup = True - auth.get_or_create_user('johnsnow', 'password') + auth.create_user('johnsnow', 'password') user_info = UserInfo.find(app.db, 'johnsnow') assert user_info.username == 'johnsnow' assert user_info.is_authorized == expected_authorization -async def test_create_user_bas_characters(tmpcwd, app): - '''Test method get_or_create_user with bad characters on username''' +async def test_create_user_bad_characters(tmpcwd, app): + '''Test method create_user with bad characters on username''' auth = NativeAuthenticator(db=app.db) - assert not auth.get_or_create_user('john snow', 'password') - assert not auth.get_or_create_user('john,snow', 'password') + assert not auth.create_user('john snow', 'password') + assert not auth.create_user('john,snow', 'password') +async def test_create_user_twice(tmpcwd, app): + '''Test if creating users with an existing handle errors.''' + auth = NativeAuthenticator(db=app.db) + + # First creation should succeed. + assert auth.create_user('johnsnow', 'password') + + # Creating the same account again should fail. + assert not auth.create_user('johnsnow', 'password') + + # Creating a user with same handle but different pw should also fail. + assert not auth.create_user('johnsnow', 'adifferentpassword') + @pytest.mark.parametrize("password,min_len,expected", [ ("qwerty", 1, False), ("agameofthrones", 1, True), @@ -62,11 +75,11 @@ async def test_create_user_bas_characters(tmpcwd, app): ]) async def test_create_user_with_strong_passwords(password, min_len, expected, tmpcwd, app): - '''Test if method get_or_create_user and strong passwords''' + '''Test if method create_user and strong passwords mesh''' auth = NativeAuthenticator(db=app.db) auth.check_common_password = True auth.minimum_password_length = min_len - user = auth.get_or_create_user('johnsnow', password) + user = auth.create_user('johnsnow', password) assert bool(user) == expected @@ -81,7 +94,7 @@ async def test_authentication(username, password, authorized, expected, tmpcwd, app): '''Test if authentication fails with a unexistent user''' auth = NativeAuthenticator(db=app.db) - auth.get_or_create_user('johnsnow', 'password') + auth.create_user('johnsnow', 'password') if authorized: UserInfo.change_authorization(app.db, 'johnsnow') response = await auth.authenticate(app, {'username': username, @@ -113,7 +126,7 @@ async def test_authentication_login_count(tmpcwd, app): auth = NativeAuthenticator(db=app.db) infos = {'username': 'johnsnow', 'password': 'password'} wrong_infos = {'username': 'johnsnow', 'password': 'wrong_password'} - auth.get_or_create_user(infos['username'], infos['password']) + auth.create_user(infos['username'], infos['password']) UserInfo.change_authorization(app.db, 'johnsnow') assert not auth.login_attempts @@ -134,7 +147,7 @@ async def test_authentication_with_exceed_atempts_of_login(tmpcwd, app): auth.secs_before_next_try = 10 infos = {'username': 'johnsnow', 'password': 'wrongpassword'} - auth.get_or_create_user(infos['username'], 'password') + auth.create_user(infos['username'], 'password') UserInfo.change_authorization(app.db, 'johnsnow') for i in range(3): @@ -152,16 +165,26 @@ async def test_authentication_with_exceed_atempts_of_login(tmpcwd, app): async def test_change_password(tmpcwd, app): auth = NativeAuthenticator(db=app.db) - user = auth.get_or_create_user('johnsnow', 'password') + user = auth.create_user('johnsnow', 'password') assert user.is_valid_password('password') auth.change_password('johnsnow', 'newpassword') assert not user.is_valid_password('password') assert user.is_valid_password('newpassword') +async def test_get_user(tmpcwd, app): + auth = NativeAuthenticator(db=app.db) + auth.create_user('johnsnow', 'password') + + # Getting existing user is successful. + assert auth.get_user('johnsnow') != None + + # Getting non-existing user fails. + assert auth.get_user('samwelltarly') == None + async def test_delete_user(tmpcwd, app): auth = NativeAuthenticator(db=app.db) - auth.get_or_create_user('johnsnow', 'password') + auth.create_user('johnsnow', 'password') user = type('User', (), {'name': 'johnsnow'}) auth.delete_user(user)