Merge pull request #22 from cul-it/accounts-0.2

Pre-release merge for accounts-0.2

Author: erickpeirson

Pipfile.lock

@@ -18,11 +18,13 @@ sqlalchemy = "*"
 uwsgi = "*"
 wtforms = "*"
 arxiv-base = "==0.10.1rc2"
-arxiv-auth = "==0.1.0rc11"
+authlib = "*"
+redis-py-cluster = "*"
+arxiv-auth = "==0.1.0rc12"
 
 [dev-packages]
 mimesis = "*"
-mypy = "==0.610"
+mypy = "*"
 pydocstyle = "*"
 pylint = "*"
 sphinx = "*"
@@ -31,6 +33,7 @@ coverage = "*"
 coveralls = "*"
 pytest = "*"
 pytest-cov = "*"
+"nose2" = "*"
 
 [requires]
 python_version = "3.6"

accounts/Pipfile

@@ -15,7 +15,7 @@
 
 from werkzeug import MultiDict, ImmutableMultiDict
 from werkzeug.exceptions import BadRequest, InternalServerError
-from flask import url_for
+from flask import url_for, Markup
 
 from wtforms import StringField, PasswordField, SelectField, \
     SelectMultipleField, BooleanField, Form, HiddenField
@@ -88,6 +88,16 @@ def login(method: str, form_data: MultiDict, ip: str,
         data.update({'error': 'Invalid username or password.'})
         return data, status.HTTP_400_BAD_REQUEST, {}
 
+    if not userdata.verified:
+        data.update({
+            'error': Markup(
+                'Your account has not yet been verified. Please contact '
+                '<a href="mailto:help@arxiv.org">help@arxiv.org</a> if '
+                 'you believe this to be in error.'
+            )
+        })
+        return data, status.HTTP_400_BAD_REQUEST, {}
+
     # Create a session in the distributed session store.
     try:
         session = sessions.create(auths, ip, ip, track, user=userdata)

accounts/Pipfile.lock

@@ -119,7 +119,8 @@ def test_post_great(self, mock_legacy, mock_sessions, mock_users):
         user = domain.User(
             user_id=42,
             username='foouser',
-            email='user@ema.il'
+            email='user@ema.il',
+            verified=True
         )
         auths = domain.Authorizations(
             classic=6,
@@ -156,3 +157,54 @@ def test_post_great(self, mock_legacy, mock_sessions, mock_users):
                          "Session cookie is returned")
         self.assertEqual(data['cookies']['classic_cookie'], (c_cookie, None),
                          "Classic session cookie is returned")
+
+    @mock.patch('accounts.controllers.authentication.users')
+    @mock.patch('accounts.controllers.authentication.sessions')
+    @mock.patch('accounts.controllers.authentication.legacy')
+    def test_post_not_verified(self, mock_legacy, mock_sessions, mock_users):
+        """Form data are valid and check out."""
+        mock_users.exceptions.AuthenticationFailed = \
+            users.exceptions.AuthenticationFailed
+        mock_sessions.exceptions.SessionCreationFailed = \
+            sessions.exceptions.SessionCreationFailed
+        mock_legacy.exceptions.SessionCreationFailed = \
+            legacy.exceptions.SessionCreationFailed
+        form_data = MultiDict({'username': 'foouser', 'password': 'bazpass'})
+        ip = '123.45.67.89'
+        next_page = '/foo'
+        start_time = datetime.now(tz=EASTERN)
+        user = domain.User(
+            user_id=42,
+            username='foouser',
+            email='user@ema.il',
+            verified=False
+        )
+        auths = domain.Authorizations(
+            classic=6,
+            scopes=['public:read', 'submission:create']
+        )
+        mock_users.authenticate.return_value = user, auths
+        c_session = domain.Session(
+            session_id='barsession',
+            user=user,
+            start_time=start_time,
+            authorizations=auths
+        )
+        c_cookie = 'bardata'
+        mock_legacy.create.return_value = c_session
+        mock_legacy.generate_cookie.return_value = c_cookie
+        session = domain.Session(
+            session_id='foosession',
+            user=user,
+            start_time=start_time,
+            authorizations=domain.Authorizations(
+                scopes=['public:read', 'submission:create']
+            )
+        )
+        cookie = 'foodata'
+        mock_sessions.create.return_value = session
+        mock_sessions.generate_cookie.return_value = cookie
+
+        data, status_code, header = login('POST', form_data, ip, next_page)
+        self.assertEqual(status_code, status.HTTP_400_BAD_REQUEST,
+                         "Bad request error is returned")

accounts/accounts/controllers/authentication.py

@@ -67,6 +67,22 @@ def set_cookies(response: Response, data: dict) -> None:
                             **params)
 
 
+# This is unlikely to be useful once the classic submission UI is disabled.
+def unset_submission_cookie(response: Response) -> None:
+    """
+    Unset the legacy Catalyst submission cookie.
+
+    In addition to the authenticated session (which was originally from the
+    Tapir auth system), Catalyst also tracks a session used specifically for
+    the submission process. The legacy Catalyst controller sets this
+    automatically, so we don't need to do anything on login. But on logout,
+    if this cookie is not cleared, Catalyst may attempt to use the same
+    submission session upon subsequent logins. This can lead to weird
+    inconsistencies.
+    """
+    response.set_cookie('submit_session', '', max_age=0, httponly=True)
+
+
 # @blueprint.route('/register', methods=['GET', 'POST'])
 @anonymous_only
 def register() -> Response:
@@ -133,6 +149,7 @@ def login() -> Response:
         # Set the session cookie.
         response = make_response(redirect(headers.get('Location'), code=code))
         set_cookies(response, data)
+        unset_submission_cookie(response)    # Fix for ARXIVNG-1149.
         return response
 
     # Form is invalid, or login failed.
@@ -156,6 +173,7 @@ def logout() -> Response:
         logger.debug('Redirecting to %s: %i', headers.get('Location'), code)
         response = make_response(redirect(headers.get('Location'), code=code))
         set_cookies(response, data)
+        unset_submission_cookie(response)    # Fix for ARXIVNG-1149.
         return response
     return redirect(url_for('get_login'), code=status.HTTP_302_FOUND)
 

accounts/accounts/controllers/tests/test_authentication.py

@@ -447,3 +447,133 @@ def test_login_logout(self):
             '0',
             'Classic session cookie is expired'
         )
+
+
+class TestLogoutLegacySubmitCookie(TestCase):
+    """The legacy system has a submission session cookie that must be unset."""
+
+    @classmethod
+    def setUpClass(self):
+        """Spin up redis."""
+        # self.redis = subprocess.run(
+        #     "docker run -d -p 7000:7000 -p 7001:7001 -p 7002:7002 -p 7003:7003"
+        #     " -p 7004:7004 -p 7005:7005 -p 7006:7006 -e \"IP=0.0.0.0\""
+        #     " --hostname=server grokzen/redis-cluster:4.0.9",
+        #     stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True
+        # )
+        # time.sleep(10)    # In case it takes a moment to start.
+        # if self.redis.returncode > 0:
+        #     raise RuntimeError('Could not start redis. Is Docker running?')
+        #
+        # self.container = self.redis.stdout.decode('ascii').strip()
+        self.secret = 'bazsecret'
+        self.db = 'db.sqlite'
+        try:
+            self.app = create_web_app()
+            self.app.config['CLASSIC_COOKIE_NAME'] = 'foo_tapir_session'
+            self.app.config['SESSION_COOKIE_NAME'] = 'baz_session'
+            self.app.config['SESSION_COOKIE_SECURE'] = '0'
+            self.app.config['JWT_SECRET'] = self.secret
+            self.app.config['CLASSIC_DATABASE_URI'] = f'sqlite:///{self.db}'
+            self.app.config['REDIS_HOST'] = 'localhost'
+            self.app.config['REDIS_PORT'] = '7000'
+
+            with self.app.app_context():
+                from accounts.services import legacy, users
+                legacy.create_all()
+                users.create_all()
+
+                with users.transaction() as session:
+                    # We have a good old-fashioned user.
+                    db_user = users.models.DBUser(
+                        user_id=1,
+                        first_name='first',
+                        last_name='last',
+                        suffix_name='iv',
+                        email='first@last.iv',
+                        policy_class=2,
+                        flag_edit_users=1,
+                        flag_email_verified=1,
+                        flag_edit_system=0,
+                        flag_approved=1,
+                        flag_deleted=0,
+                        flag_banned=0,
+                        tracking_cookie='foocookie',
+                    )
+                    db_nick = users.models.DBUserNickname(
+                        nick_id=1,
+                        nickname='foouser',
+                        user_id=1,
+                        user_seq=1,
+                        flag_valid=1,
+                        role=0,
+                        policy=0,
+                        flag_primary=1
+                    )
+                    salt = b'fdoo'
+                    password = b'thepassword'
+                    hashed = hashlib.sha1(salt + b'-' + password).digest()
+                    encrypted = b64encode(salt + hashed)
+                    db_password = users.models.DBUserPassword(
+                        user_id=1,
+                        password_storage=2,
+                        password_enc=encrypted
+                    )
+                    session.add(db_user)
+                    session.add(db_password)
+                    session.add(db_nick)
+
+        except Exception as e:
+            stop_container(self.container)
+            raise
+
+    @classmethod
+    def tearDownClass(self):
+        """Tear down redis and the test DB."""
+        # stop_container(self.container)
+        os.remove(self.db)
+
+    def test_logout_clears_legacy_submit_cookie(self):
+        """When the user logs out, the legacy submit cookie is unset."""
+        client = self.app.test_client()
+        # client.set_cookie('localhost')
+        form_data = {'username': 'foouser', 'password': 'thepassword'}
+
+        # Werkzeug should keep the cookies around for the next request.
+        response = client.post('/login', data=form_data)
+        cookies = _parse_cookies(response.headers.getlist('Set-Cookie'))
+
+        client.set_cookie('', 'submit_session', '12345678')
+        self.assertIn(self.app.config['SESSION_COOKIE_NAME'], cookies,
+                      "Sets cookie for authn session.")
+        self.assertIn(self.app.config['CLASSIC_COOKIE_NAME'], cookies,
+                      "Sets cookie for classic sessions.")
+
+        response = client.get('/logout')
+        logout_cookies = _parse_cookies(response.headers.getlist('Set-Cookie'))
+
+        self.assertEqual(
+            logout_cookies[self.app.config['SESSION_COOKIE_NAME']]['value'],
+            '',
+            'Session cookie is unset'
+        )
+        self.assertEqual(
+            logout_cookies[self.app.config['SESSION_COOKIE_NAME']]['Max-Age'],
+            '0',
+            'Session cookie is expired'
+        )
+        self.assertEqual(
+            logout_cookies[self.app.config['CLASSIC_COOKIE_NAME']]['value'],
+            '',
+            'Classic cookie is unset'
+        )
+        self.assertEqual(
+            logout_cookies[self.app.config['CLASSIC_COOKIE_NAME']]['Max-Age'],
+            '0',
+            'Classic session cookie is expired'
+        )
+        self.assertEqual(
+            logout_cookies['submit_session']['Max-Age'],
+            '0',
+            'Legacy submission cookie is expired'
+        )

accounts/accounts/routes/ui.py

@@ -240,6 +240,21 @@ class User(NamedTuple):
     profile: Optional[UserProfile] = None
     """The user's account profile (if available)."""
 
+    verified: bool = False
+    """Whether or not the users' e-mail address has been verified."""
+
+    # TODO: consider whether this information is relevant beyond the
+    # ``arxiv.users.legacy.authenticate`` module.
+    #
+    # approved: bool = True
+    # """Whether or not the users' account is approved."""
+    #
+    # banned: bool = False
+    # """Whether or not the user has been banned."""
+    #
+    # deleted: bool = False
+    # """Whether or not the user has been deleted."""
+
 
 class Client(NamedTuple):
     """API client."""

accounts/accounts/tests/test_end_to_end.py

@@ -73,7 +73,8 @@ def authenticate(username_or_email: Optional[str]=None,
             forename=db_user.first_name,
             surname=db_user.last_name,
             suffix=db_user.suffix_name
-        )
+        ),
+        verified=bool(db_user.flag_email_verified)
     )
     auths = domain.Authorizations(
         classic=util.compute_capabilities(db_user),

users/arxiv/users/domain.py

@@ -97,7 +97,8 @@ def load(cookie: str) -> domain.Session:
                 forename=db_user.first_name,
                 surname=db_user.last_name,
                 suffix=db_user.suffix_name
-            )
+            ),
+            verified=bool(db_user.flag_email_verified)
         )
 
         # We should get one row per endorsement.