Merge pull request #4349 from freedomofpress/revoke-tokens

Logout to revoke tokens

docs/development/journalist_api.rst

@@ -56,11 +56,22 @@ HTTP Authorization header:
   Authorization: Token eyJhbGciOiJIUzI1NiIsImV4cCI6MTUzMDU4NjU4MiwifWF0IjoxNTMwNTc5MzgyfQ.eyJpZCI6MX0.P_PfcLMk1Dq5VCIANo-lJbu0ZyCL2VcT8qf9fIZsTCM
 
 This header will be checked with each API request to see if it is valid and
-not yet expired. Tokens currently expire after 8 hours, but note that clients
-should use the expiration time provided in the response to determine when
-the token will expire. After the token expires point, users must
-login again. Clients implementing logout functionality should delete tokens
-locally upon logout.
+not yet expired. Tokens currently expire after 8 hours. 
+
+Logout
+------
+
+Clients should use the logout endpoint to invalidate their token:
+
+``POST /api/v1/logout`` with the token in the HTTP Authorization header
+and you will get the following response upon successful invalidation of the
+API token:
+
+.. code:: sh
+
+  {
+  "message": "Your token has been revoked."
+  }
 
 Errors
 ~~~~~~

securedrop/alembic/versions/b58139cfdc8c_add_checksum_columns.py → securedrop/alembic/versions/b58139cfdc8c_add_checksum_columns_revoke_table.py

@@ -1,4 +1,4 @@
-"""add checksum columns
+"""add checksum columns and revoke token table
 
 Revision ID: b58139cfdc8c
 Revises: f2833ac34bb6
@@ -36,6 +36,16 @@ def upgrade():
     with op.batch_alter_table('submissions', schema=None) as batch_op:
         batch_op.add_column(sa.Column('checksum', sa.String(length=255), nullable=True))
 
+    op.create_table(
+        'revoked_tokens',
+        sa.Column('id', sa.Integer(), nullable=False),
+        sa.Column('journalist_id', sa.Integer(), nullable=True),
+        sa.Column('token', sa.Text(), nullable=False),
+        sa.ForeignKeyConstraint(['journalist_id'], ['journalists.id'], ),
+        sa.PrimaryKeyConstraint('id'),
+        sa.UniqueConstraint('token')
+    )
+
     try:
         app = create_app(config)
 
@@ -77,6 +87,8 @@ def upgrade():
 
 
 def downgrade():
+    op.drop_table('revoked_tokens')
+
     with op.batch_alter_table('submissions', schema=None) as batch_op:
         batch_op.drop_column('checksum')
 

securedrop/journalist_app/__init__.py

@@ -18,7 +18,8 @@
 from db import db
 from journalist_app import account, admin, api, main, col
 from journalist_app.utils import (get_source, logged_in,
-                                  JournalistInterfaceSessionInterface)
+                                  JournalistInterfaceSessionInterface,
+                                  cleanup_expired_revoked_tokens)
 from models import Journalist
 from store import Storage
 from worker import rq_worker_queue
@@ -124,6 +125,10 @@ def _handle_http_exception(error):
         template_filters.rel_datetime_format
     app.jinja_env.filters['filesizeformat'] = template_filters.filesizeformat
 
+    @app.before_first_request
+    def expire_blacklisted_tokens():
+        return cleanup_expired_revoked_tokens()
+
     @app.before_request
     def setup_g():
         """Store commonly used values in Flask's special g object"""

securedrop/journalist_app/api.py

@@ -10,7 +10,7 @@
 
 from db import db
 from journalist_app import utils
-from models import (Journalist, Reply, Source, Submission,
+from models import (Journalist, Reply, Source, Submission, RevokedToken,
                     LoginThrottledException, InvalidUsernameException,
                     BadTokenException, WrongPasswordException)
 from store import NotEncrypted
@@ -75,10 +75,18 @@ def get_endpoints():
     @api.before_request
     def validate_data():
         if request.method == 'POST':
-            # flag and star can have empty payloads
+            # flag, star, and logout can have empty payloads
             if not request.data:
-                if ('flag' not in request.path and 'star' not in request.path):
-                    return abort(400, 'malformed request')
+                dataless_endpoints = [
+                    'add_star',
+                    'remove_star',
+                    'flag',
+                    'logout',
+                ]
+                for endpoint in dataless_endpoints:
+                    if request.endpoint == 'api.' + endpoint:
+                        return
+                return abort(400, 'malformed request')
             # other requests must have valid JSON payload
             else:
                 try:
@@ -309,6 +317,16 @@ def get_current_user():
         user = get_user_object(request)
         return jsonify(user.to_json()), 200
 
+    @api.route('/logout', methods=['POST'])
+    @token_required
+    def logout():
+        user = get_user_object(request)
+        auth_token = request.headers.get('Authorization').split(" ")[1]
+        revoked_token = RevokedToken(token=auth_token, journalist_id=user.id)
+        db.session.add(revoked_token)
+        db.session.commit()
+        return jsonify({'message': 'Your token has been revoked.'}), 200
+
     def _handle_api_http_exception(error):
         # Workaround for no blueprint-level 404/5 error handlers, see:
         # https://github.com/pallets/flask/issues/503#issuecomment-71383286

securedrop/journalist_app/utils.py

@@ -13,7 +13,7 @@
 from models import (get_one_or_else, Source, Journalist,
                     InvalidUsernameException, WrongPasswordException,
                     LoginThrottledException, BadTokenException, SourceStar,
-                    PasswordError, Submission)
+                    PasswordError, Submission, RevokedToken)
 from rm import srm
 from store import add_checksum_for_file
 from worker import rq_worker_queue
@@ -353,3 +353,18 @@ def save_session(self, app, session, response):
         else:
             super(JournalistInterfaceSessionInterface, self).save_session(
                 app, session, response)
+
+
+def cleanup_expired_revoked_tokens():
+    """Remove tokens that have now expired from the revoked token table."""
+
+    revoked_tokens = db.session.query(RevokedToken).all()
+
+    for revoked_token in revoked_tokens:
+        if Journalist.validate_token_is_not_expired_or_invalid(revoked_token.token):
+            pass  # The token has not expired, we must keep in the revoked token table.
+        else:
+            # The token is no longer valid, remove from the revoked token table.
+            db.session.delete(revoked_token)
+
+    db.session.commit()

securedrop/models.py

@@ -567,13 +567,28 @@ def generate_api_token(self, expiration):
             current_app.config['SECRET_KEY'], expires_in=expiration)
         return s.dumps({'id': self.id}).decode('ascii')
 
+    @staticmethod
+    def validate_token_is_not_expired_or_invalid(token):
+        s = TimedJSONWebSignatureSerializer(current_app.config['SECRET_KEY'])
+        try:
+            s.loads(token)
+        except BadData:
+            return None
+
+        return True
+
     @staticmethod
     def validate_api_token_and_get_user(token):
         s = TimedJSONWebSignatureSerializer(current_app.config['SECRET_KEY'])
         try:
             data = s.loads(token)
         except BadData:
             return None
+
+        revoked_token = RevokedToken.query.filter_by(token=token).one_or_none()
+        if revoked_token is not None:
+            return None
+
         return Journalist.query.get(data['id'])
 
     def to_json(self):
@@ -598,3 +613,16 @@ class JournalistLoginAttempt(db.Model):
 
     def __init__(self, journalist):
         self.journalist_id = journalist.id
+
+
+class RevokedToken(db.Model):
+
+    """
+    API tokens that have been revoked either through a logout or other revocation mechanism.
+    """
+
+    __tablename__ = 'revoked_tokens'
+
+    id = Column(Integer, primary_key=True)
+    journalist_id = Column(Integer, ForeignKey('journalists.id'))
+    token = db.Column(db.Text, nullable=False, unique=True)

securedrop/tests/migrations/migration_b58139cfdc8c.py

@@ -151,6 +151,8 @@ def check_upgrade(self):
         and without being able to inject this config, the checksum function won't succeed. The above
         `load_data` function provides data that can be manually verified by checking the `rqworker`
         log file in `/tmp/`.
+
+        The other part of the migration, creating a table, cannot be tested regardless.
         '''
         pass
 
@@ -175,9 +177,15 @@ def load_data(self):
             self.create_submission(checksum=True)
             self.create_reply(checksum=True)
 
+            # add a revoked token for enable a foreign key connection
+            self.add_revoked_token()
+
     def check_downgrade(self):
         '''
         Verify that the checksum column is now gone.
+
+        The dropping of the revoked_tokens table cannot be checked. If the migration completes,
+        then it wokred correctly.
         '''
         with self.app.app_context():
             sql = "SELECT * FROM submissions"
@@ -197,3 +205,13 @@ def check_downgrade(self):
                     submission['checksum']
                 except NoSuchColumnError:
                     pass
+
+    def add_revoked_token(self):
+        params = {
+            'journalist_id': self.journalist_id,
+            'token': 'abc123',
+        }
+        sql = '''INSERT INTO revoked_tokens (journalist_id, token)
+                 VALUES (:journalist_id, :token)
+              '''
+        db.engine.execute(text(sql), **params)

securedrop/tests/test_i18n.py

@@ -25,6 +25,7 @@
 
 os.environ['SECUREDROP_ENV'] = 'test'  # noqa
 from sdconfig import SDConfig
+from db import db
 import i18n
 import i18n_tool
 import journalist_app as journalist_app_module
@@ -217,6 +218,8 @@ def test_i18n(journalist_app, config):
     # grabs values at init time and we can't inject them later.
     for app in (journalist_app_module.create_app(fake_config),
                 source_app.create_app(fake_config)):
+        with app.app_context():
+            db.create_all()
         assert i18n.LOCALES == fake_config.SUPPORTED_LOCALES
         verify_i18n(app)
 

securedrop/tests/test_journalist_api.py

@@ -10,7 +10,7 @@
 from itsdangerous import TimedJSONWebSignatureSerializer
 
 from db import db
-from models import Journalist, Reply, Source, SourceStar, Submission
+from models import Journalist, Reply, Source, SourceStar, Submission, RevokedToken
 
 os.environ['SECUREDROP_ENV'] = 'test'  # noqa
 from .utils.api_helper import get_api_headers
@@ -933,3 +933,20 @@ def test_reply_download_generates_checksum(journalist_app,
     assert fetched_reply.checksum
     # we don't want to recalculat this value
     assert not mock_add_checksum.called
+
+
+def test_revoke_token(journalist_app, test_journo, journalist_api_token):
+    with journalist_app.test_client() as app:
+        # without token 403's
+        resp = app.post(url_for('api.logout'))
+        assert resp.status_code == 403
+
+        resp = app.post(url_for('api.logout'), headers=get_api_headers(journalist_api_token))
+        assert resp.status_code == 200
+
+        revoked_token = RevokedToken.query.filter_by(token=journalist_api_token).one()
+        assert revoked_token.journalist_id == test_journo['id']
+
+        resp = app.get(url_for('api.get_all_sources'),
+                       headers=get_api_headers(journalist_api_token))
+        assert resp.status_code == 403

securedrop/tests/test_journalist_utils.py

@@ -0,0 +1,42 @@
+# -*- coding: utf-8 -*-
+from flask import url_for
+import os
+import pytest
+import random
+
+from models import RevokedToken
+from sqlalchemy.orm.exc import NoResultFound
+
+from journalist_app.utils import cleanup_expired_revoked_tokens
+
+os.environ['SECUREDROP_ENV'] = 'test'  # noqa
+from .utils.api_helper import get_api_headers
+
+random.seed('◔ ⌣ ◔')
+
+
+def test_revoke_token_cleanup_does_not_delete_tokens_if_not_expired(journalist_app, test_journo,
+                                                                    journalist_api_token):
+    with journalist_app.test_client() as app:
+        resp = app.post(url_for('api.logout'), headers=get_api_headers(journalist_api_token))
+        assert resp.status_code == 200
+
+        cleanup_expired_revoked_tokens()
+
+        revoked_token = RevokedToken.query.filter_by(token=journalist_api_token).one()
+        assert revoked_token.journalist_id == test_journo['id']
+
+
+def test_revoke_token_cleanup_does_deletes_tokens_that_are_expired(journalist_app, test_journo,
+                                                                   journalist_api_token, mocker):
+    with journalist_app.test_client() as app:
+        resp = app.post(url_for('api.logout'), headers=get_api_headers(journalist_api_token))
+        assert resp.status_code == 200
+
+        # Mock response from expired token method when token is expired
+        mocker.patch('journalist_app.admin.Journalist.validate_token_is_not_expired_or_invalid',
+                     return_value=None)
+        cleanup_expired_revoked_tokens()
+
+        with pytest.raises(NoResultFound):
+            RevokedToken.query.filter_by(token=journalist_api_token).one()

securedrop/tests/test_template_filters.py

@@ -5,6 +5,7 @@
 from flask import session
 
 os.environ['SECUREDROP_ENV'] = 'test'  # noqa
+from db import db
 import i18n
 import i18n_tool
 import journalist_app
@@ -110,6 +111,8 @@ def do_test(config, create_app):
         pybabel('init', '-i', pot, '-d', config.TEMP_DIR, '-l', l)
 
     app = create_app(config)
+    with app.app_context():
+        db.create_all()
 
     assert i18n.LOCALES == config.SUPPORTED_LOCALES
     verify_filesizeformat(app)