From 6c53bd56d366d1097258857f7fc32f523772b0d2 Mon Sep 17 00:00:00 2001 From: Jonas Betzendahl Date: Tue, 25 Feb 2020 13:52:38 +0100 Subject: [PATCH 1/5] typo --- nativeauthenticator/tests/test_authenticator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nativeauthenticator/tests/test_authenticator.py b/nativeauthenticator/tests/test_authenticator.py index 1fdfcb8..2130a74 100644 --- a/nativeauthenticator/tests/test_authenticator.py +++ b/nativeauthenticator/tests/test_authenticator.py @@ -47,7 +47,7 @@ async def test_create_user(is_admin, open_signup, expected_authorization, assert user_info.is_authorized == expected_authorization -async def test_create_user_bas_characters(tmpcwd, app): +async def test_create_user_bad_characters(tmpcwd, app): '''Test method get_or_create_user with bad characters on username''' auth = NativeAuthenticator(db=app.db) assert not auth.get_or_create_user('john snow', 'password') From d26930825ea0bb379316e108247649a11da05a00 Mon Sep 17 00:00:00 2001 From: Jonas Betzendahl Date: Tue, 25 Feb 2020 14:05:19 +0100 Subject: [PATCH 2/5] add test_create_user_twice for #91 --- nativeauthenticator/tests/test_authenticator.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/nativeauthenticator/tests/test_authenticator.py b/nativeauthenticator/tests/test_authenticator.py index 2130a74..b319784 100644 --- a/nativeauthenticator/tests/test_authenticator.py +++ b/nativeauthenticator/tests/test_authenticator.py @@ -54,6 +54,19 @@ async def test_create_user_bad_characters(tmpcwd, app): assert not auth.get_or_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.get_or_create_user('johnsnow', 'password') + + # Creating the same account again should fail. + assert not auth.get_or_create_user('johnsnow', 'password') + + # Creating a user with same handle but different pw should also fail. + assert not auth.get_or_create_user('johnsnow', 'adifferentpassword') + @pytest.mark.parametrize("password,min_len,expected", [ ("qwerty", 1, False), ("agameofthrones", 1, True), From 72c106b7bd712bed3cc42508b8b70e24455bab13 Mon Sep 17 00:00:00 2001 From: Jonas Betzendahl Date: Tue, 25 Feb 2020 16:02:36 +0100 Subject: [PATCH 3/5] rewrite get_or_create into two methods, adapt test, error on signup with taken username --- nativeauthenticator/handlers.py | 13 ++++-- nativeauthenticator/nativeauthenticator.py | 25 ++++++---- .../tests/test_authenticator.py | 46 +++++++++---------- 3 files changed, 50 insertions(+), 34 deletions(-) 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..de8911c 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,22 @@ 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): - username = self.normalize_username(username) - user = UserInfo.find(self.db, username) + + def get_user(self, username): + user = UserInfo.find(self.db, self.normalize_username(username)) if user: return user + + 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) + + if self.user_exists(username): + return + if not self.is_password_strong(pw) or \ not self.validate_username(username): return @@ -193,7 +202,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 +223,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 +245,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 b319784..8bf23b4 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,31 +41,31 @@ 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_bad_characters(tmpcwd, app): - '''Test method get_or_create_user with bad characters on username''' + '''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.get_or_create_user('johnsnow', 'password') - - # Creating the same account again should fail. - assert not auth.get_or_create_user('johnsnow', 'password') - - # Creating a user with same handle but different pw should also fail. - assert not auth.get_or_create_user('johnsnow', 'adifferentpassword') + '''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), @@ -75,11 +75,11 @@ async def test_create_user_twice(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 @@ -94,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, @@ -126,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 @@ -147,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): @@ -165,7 +165,7 @@ 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') @@ -174,7 +174,7 @@ async def test_change_password(tmpcwd, app): 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) From 4e75ea8a868810d1f41cdf025afef4b3509a1c0f Mon Sep 17 00:00:00 2001 From: Jonas Betzendahl Date: Fri, 27 Mar 2020 17:02:00 +0100 Subject: [PATCH 4/5] incorporate requested changes --- nativeauthenticator/nativeauthenticator.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/nativeauthenticator/nativeauthenticator.py b/nativeauthenticator/nativeauthenticator.py index de8911c..9660935 100644 --- a/nativeauthenticator/nativeauthenticator.py +++ b/nativeauthenticator/nativeauthenticator.py @@ -168,15 +168,12 @@ def is_password_strong(self, password): return all(checks) def get_user(self, username): - user = UserInfo.find(self.db, self.normalize_username(username)) - if user: - return user + 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) if self.user_exists(username): From 4aea615e93313ac4aeb672ad8b94eba5c3b980ba Mon Sep 17 00:00:00 2001 From: Jonas Betzendahl Date: Fri, 27 Mar 2020 17:02:28 +0100 Subject: [PATCH 5/5] add test for getting (non-)existing users --- nativeauthenticator/tests/test_authenticator.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/nativeauthenticator/tests/test_authenticator.py b/nativeauthenticator/tests/test_authenticator.py index 8bf23b4..722242c 100644 --- a/nativeauthenticator/tests/test_authenticator.py +++ b/nativeauthenticator/tests/test_authenticator.py @@ -57,13 +57,13 @@ async def test_create_user_bad_characters(tmpcwd, app): 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') @@ -172,6 +172,16 @@ async def test_change_password(tmpcwd, app): 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.create_user('johnsnow', 'password')