From 91465520a08985c6e0aa16571669d6e0410cd8b9 Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Thu, 28 Jan 2016 12:31:24 +0000 Subject: [PATCH 1/7] Call to client for password check incorrectly passed user instead of user.id --- app/main/views/sign_in.py | 2 +- tests/app/main/views/test_sign_in.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index af96db8e9..3aed7a5ae 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -39,7 +39,7 @@ def _get_and_verify_user(email_address, password): return None elif not user.is_active(): return None - elif not users_dao.verify_password(user, password): + elif not users_dao.verify_password(user.id, password): return None else: return user diff --git a/tests/app/main/views/test_sign_in.py b/tests/app/main/views/test_sign_in.py index a1eeb3f1f..35420f7dc 100644 --- a/tests/app/main/views/test_sign_in.py +++ b/tests/app/main/views/test_sign_in.py @@ -31,6 +31,7 @@ def test_logged_in_user_redirects_to_choose_service(app_, def test_process_sign_in_return_2fa_template(app_, + api_user_active, mock_send_verify_code, mock_get_user, mock_get_user_by_email, @@ -43,6 +44,7 @@ def test_process_sign_in_return_2fa_template(app_, 'password': 'val1dPassw0rd!'}) assert response.status_code == 302 assert response.location == 'http://localhost/two-factor' + mock_verify_password.assert_called_with(api_user_active.id, 'val1dPassw0rd!') def test_should_return_locked_out_true_when_user_is_locked(app_, From ab20eaa4919f7dd9e061479fb3ade39339ab4958 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Thu, 28 Jan 2016 12:38:43 +0000 Subject: [PATCH 2/7] Fix for service settings. --- app/main/views/service_settings.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 81e218227..3e2897d98 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -64,7 +64,7 @@ def service_name_change_confirm(service_id): # Validate password for form def _check_password(pwd): - return verify_password(current_user, pwd) + return verify_password(current_user.id, pwd) form = ConfirmPasswordForm(_check_password) if form.validate_on_submit(): @@ -134,7 +134,7 @@ def service_status_change_confirm(service_id): # Validate password for form def _check_password(pwd): - return verify_password(current_user, pwd) + return verify_password(current_user.id, pwd) form = ConfirmPasswordForm(_check_password) if form.validate_on_submit(): @@ -183,7 +183,7 @@ def service_delete_confirm(service_id): # Validate password for form def _check_password(pwd): - return verify_password(current_user, pwd) + return verify_password(current_user.id, pwd) form = ConfirmPasswordForm(_check_password) if form.validate_on_submit(): From 414e468cb03f8077e5515f06a83ac46c149ff61a Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Thu, 28 Jan 2016 14:17:13 +0000 Subject: [PATCH 3/7] Link to send email broken in nav --- app/main/views/index.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/main/views/index.py b/app/main/views/index.py index 8d9e55390..daa532fc8 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -27,7 +27,7 @@ def verify_mobile(): @main.route("/services//send-email") @login_required def send_email(service_id): - return render_template('views/send-email.html') + return render_template('views/send-email.html', service_id=service_id) @main.route("/services//check-email") From 5a17bba97e6d7955b4231344ea643c7b7ac22dcd Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 28 Jan 2016 15:01:44 +0000 Subject: [PATCH 4/7] Set SESSION_COOKIE_SECURE=True for live. --- app/its_dangerous_session.py | 2 +- config.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/its_dangerous_session.py b/app/its_dangerous_session.py index d281b32d9..a5e6f6699 100644 --- a/app/its_dangerous_session.py +++ b/app/its_dangerous_session.py @@ -47,4 +47,4 @@ class ItsdangerousSessionInterface(SessionInterface): val = self.get_serializer(app).dumps(dict(session)) response.set_cookie(app.session_cookie_name, val, expires=expires, httponly=True, - domain=domain) + domain=domain, secure=app.config.get('SESSION_COOKIE_SECURE')) diff --git a/config.py b/config.py index b810d9229..81b40b715 100644 --- a/config.py +++ b/config.py @@ -56,6 +56,8 @@ class Test(Development): class Live(Config): DEBUG = False HTTP_PROTOCOL = 'https' + SESSION_COOKIE_SECURE = True + configs = { 'live': Live, From 54a61ac9286b7956374a4236d3d6ff494a211057 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 28 Jan 2016 15:31:32 +0000 Subject: [PATCH 5/7] Update the cookie to secure on LIVE Set the expiration of the cookie. --- app/its_dangerous_session.py | 5 ++++- config.py | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/its_dangerous_session.py b/app/its_dangerous_session.py index a5e6f6699..27b4890a6 100644 --- a/app/its_dangerous_session.py +++ b/app/its_dangerous_session.py @@ -1,3 +1,5 @@ +from datetime import timedelta, datetime + from werkzeug.datastructures import CallbackDict from flask.sessions import SessionInterface, SessionMixin from itsdangerous import URLSafeTimedSerializer, BadSignature @@ -43,7 +45,8 @@ class ItsdangerousSessionInterface(SessionInterface): response.delete_cookie(app.session_cookie_name, domain=domain) return - expires = self.get_expiration_time(app, session) + session.permanent=True + expires= datetime.utcnow() + timedelta(app.config.get('PERMANENT_SESSION_LIFETIME')) val = self.get_serializer(app).dumps(dict(session)) response.set_cookie(app.session_cookie_name, val, expires=expires, httponly=True, diff --git a/config.py b/config.py index 81b40b715..e8d51e092 100644 --- a/config.py +++ b/config.py @@ -20,7 +20,7 @@ class Config(object): SESSION_COOKIE_NAME = 'notify_admin_session' SESSION_COOKIE_PATH = '/admin' SESSION_COOKIE_HTTPONLY = True - SESSION_COOKIE_SECURE = True + SESSION_COOKIE_SECURE = False PERMANENT_SESSION_LIFETIME = 3600 # seconds API_HOST_NAME = os.getenv('API_HOST_NAME') From 5f52c0d3d692bb882450cfb995dc8a527c06ded7 Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Thu, 28 Jan 2016 15:34:02 +0000 Subject: [PATCH 6/7] Inherit from flask usermixin for default implementation of is_authenticated and is_anonymous --- app/notify_client/user_api_client.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index a083310d3..7e318c20a 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -4,6 +4,8 @@ from client.errors import ( InvalidResponse ) +from flask.ext.login import UserMixin + class UserApiClient(BaseAPIClient): def __init__(self, base_url=None, client_id=None, secret=None): @@ -84,7 +86,7 @@ class UserApiClient(BaseAPIClient): raise e -class User(object): +class User(UserMixin): def __init__(self, fields, max_failed_login_count=3): self._id = fields.get('id') self._name = fields.get('name') @@ -98,9 +100,6 @@ class User(object): def get_id(self): return self.id - def is_authenticated(self): - return True - def is_active(self): return self.state == 'active' @@ -160,9 +159,6 @@ class User(object): def failed_login_count(self, num): self._failed_login_count += num - def is_anonymous(self): - return False - def is_locked(self): return self.failed_login_count >= self.max_failed_login_count From 595a17b780373168a32d1bf11826ec305a6b06cf Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 28 Jan 2016 15:36:18 +0000 Subject: [PATCH 7/7] Fix checkstyle --- app/its_dangerous_session.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/its_dangerous_session.py b/app/its_dangerous_session.py index 27b4890a6..25f7aef20 100644 --- a/app/its_dangerous_session.py +++ b/app/its_dangerous_session.py @@ -6,10 +6,10 @@ from itsdangerous import URLSafeTimedSerializer, BadSignature class ItsdangerousSession(CallbackDict, SessionMixin): - def __init__(self, initial=None): def on_update(self): self.modified = True + CallbackDict.__init__(self, initial, on_update) self.modified = False @@ -45,8 +45,8 @@ class ItsdangerousSessionInterface(SessionInterface): response.delete_cookie(app.session_cookie_name, domain=domain) return - session.permanent=True - expires= datetime.utcnow() + timedelta(app.config.get('PERMANENT_SESSION_LIFETIME')) + session.permanent = True + expires = datetime.utcnow() + timedelta(app.config.get('PERMANENT_SESSION_LIFETIME')) val = self.get_serializer(app).dumps(dict(session)) response.set_cookie(app.session_cookie_name, val, expires=expires, httponly=True,