diff --git a/securedrop/alembic/versions/fef03c1ec779_add_instance_config_key_value_store.py b/securedrop/alembic/versions/523fff3f969c_add_versioned_instance_config.py similarity index 61% rename from securedrop/alembic/versions/fef03c1ec779_add_instance_config_key_value_store.py rename to securedrop/alembic/versions/523fff3f969c_add_versioned_instance_config.py index 2fd42c03d08..cd108acdf40 100644 --- a/securedrop/alembic/versions/fef03c1ec779_add_instance_config_key_value_store.py +++ b/securedrop/alembic/versions/523fff3f969c_add_versioned_instance_config.py @@ -1,8 +1,8 @@ -"""add instance_config key-value store +"""add versioned instance config -Revision ID: fef03c1ec779 +Revision ID: 523fff3f969c Revises: 3da3fcab826a -Create Date: 2019-10-21 00:43:19.477835 +Create Date: 2019-11-02 23:06:12.161868 """ from alembic import op @@ -10,7 +10,7 @@ # revision identifiers, used by Alembic. -revision = 'fef03c1ec779' +revision = '523fff3f969c' down_revision = '3da3fcab826a' branch_labels = None depends_on = None @@ -19,9 +19,10 @@ def upgrade(): # ### commands auto generated by Alembic - please adjust! ### op.create_table('instance_config', - sa.Column('name', sa.String(), nullable=False), - sa.Column('value', sa.JSON(), nullable=True), - sa.PrimaryKeyConstraint('name') + sa.Column('version', sa.Integer(), nullable=False), + sa.Column('valid_until', sa.DateTime(), nullable=True), + sa.PrimaryKeyConstraint('version'), + sa.UniqueConstraint('valid_until') ) # ### end Alembic commands ### diff --git a/securedrop/alembic/versions/e0719fbc39a0_add_instance_config_allow_document_.py b/securedrop/alembic/versions/e0719fbc39a0_add_instance_config_allow_document_.py new file mode 100644 index 00000000000..91c1fa4fad4 --- /dev/null +++ b/securedrop/alembic/versions/e0719fbc39a0_add_instance_config_allow_document_.py @@ -0,0 +1,40 @@ +"""add instance_config.allow_document_uploads + +Revision ID: e0719fbc39a0 +Revises: 523fff3f969c +Create Date: 2019-11-02 23:06:52.999672 + +""" +from alembic import op +import sqlalchemy as sa + +import datetime + + +# revision identifiers, used by Alembic. +revision = 'e0719fbc39a0' +down_revision = '523fff3f969c' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table('instance_config', schema=None) as batch_op: + batch_op.add_column(sa.Column('allow_document_uploads', sa.Boolean(), nullable=True)) + + # ### end Alembic commands ### + + # Data migration: Since allow_document_uploads is the first + # instance_config setting (column), all we have to do is insert a + # row with its default value. + conn = op.get_bind() + conn.execute("""INSERT INTO instance_config (allow_document_uploads) VALUES (1)""") + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table('instance_config', schema=None) as batch_op: + batch_op.drop_column('allow_document_uploads') + + # ### end Alembic commands ### diff --git a/securedrop/journalist_app/__init__.py b/securedrop/journalist_app/__init__.py index 21d7aeecd01..959dd46bc3c 100644 --- a/securedrop/journalist_app/__init__.py +++ b/securedrop/journalist_app/__init__.py @@ -126,11 +126,7 @@ def expire_blacklisted_tokens(): @app.before_request def load_instance_config(): - """Update app.config from the InstanceConfig table.""" - - instance_config = InstanceConfig.query.all() - settings = dict(map(lambda x: (x.name, x.value), instance_config)) - app.config.from_mapping(settings) + app.instance_config = InstanceConfig.get_current() @app.before_request def setup_g(): diff --git a/securedrop/journalist_app/admin.py b/securedrop/journalist_app/admin.py index 6737b758c1d..8132ca18d5d 100644 --- a/securedrop/journalist_app/admin.py +++ b/securedrop/journalist_app/admin.py @@ -31,9 +31,7 @@ def index(): def manage_config(): # The UI prompt ("prevent") is the opposite of the setting ("allow"): submission_preferences_form = SubmissionPreferencesForm( - prevent_document_uploads=not current_app.config.get( - 'ALLOW_DOCUMENT_UPLOADS', - True)) + prevent_document_uploads=not current_app.instance_config.allow_document_uploads) logo_form = LogoForm() if logo_form.validate_on_submit(): f = logo_form.logo.data @@ -60,16 +58,9 @@ def manage_config(): def update_submission_preferences(): form = SubmissionPreferencesForm() if form.validate_on_submit(): - # Upsert ALLOW_DOCUMENT_UPLOADS: - setting = InstanceConfig.query.get('ALLOW_DOCUMENT_UPLOADS') - if not setting: - setting = InstanceConfig(name='ALLOW_DOCUMENT_UPLOADS') - # The UI prompt ("prevent") is the opposite of the setting ("allow"): - setting.value = not bool(request.form.get('prevent_document_uploads')) - - db.session.add(setting) - db.session.commit() + value = not bool(request.form.get('prevent_document_uploads')) + InstanceConfig.set('allow_document_uploads', value) return redirect(url_for('admin.manage_config')) @view.route('/add', methods=('GET', 'POST')) diff --git a/securedrop/models.py b/securedrop/models.py index 1f09970fbd1..e946bc68567 100644 --- a/securedrop/models.py +++ b/securedrop/models.py @@ -742,12 +742,64 @@ class RevokedToken(db.Model): class InstanceConfig(db.Model): - '''Key-value store of settings configurable from the journalist interface. + '''Versioned key-value store of settings configurable from the journalist + interface. The current version has valid_until=None. ''' __tablename__ = 'instance_config' - name = Column(String, primary_key=True) - value = Column(JSON) + version = Column(Integer, primary_key=True) + valid_until = Column(DateTime, default=None, unique=True) + + allow_document_uploads = Column(Boolean, default=True) + + # Columns not listed here will be included by InstanceConfig.copy() when + # updating the configuration. + metadata_cols = ['version', 'valid_until'] def __repr__(self): - return "" % (self.name, self.value) + return "" % (self.version, self.valid_until) + + def copy(self): + '''Make a copy of only the configuration columns of the given + InstanceConfig object: i.e., excluding metadata_cols. + ''' + + new = type(self)() + for col in self.__table__.columns: + if col.name in self.metadata_cols: continue + + setattr(new, col.name, getattr(self, col.name)) + + return new + + @classmethod + def get_current(cls): + '''If the database was created via db.create_all(), data migrations + weren't run, and the "instance_config" table is empty. In this case, + save and return a base configuration derived from each setting's + column-level default. + ''' + + try: + return cls.query.filter(cls.valid_until==None).one() + except NoResultFound: + current = cls() + db.session.add(current) + db.session.commit() + return current + + @classmethod + def set(cls, name, value): + '''Invalidate the current configuration and append a new one with the + requested change. + ''' + + old = cls.get_current() + old.valid_until = datetime.datetime.utcnow() + db.session.add(old) + + new = old.copy() + setattr(new, name, value) + db.session.add(new) + + db.session.commit() diff --git a/securedrop/source_app/__init__.py b/securedrop/source_app/__init__.py index 550e407a938..b52d76849a5 100644 --- a/securedrop/source_app/__init__.py +++ b/securedrop/source_app/__init__.py @@ -133,11 +133,7 @@ def check_tor2web(): @app.before_request @ignore_static def load_instance_config(): - """Update app.config from the InstanceConfig table.""" - - instance_config = InstanceConfig.query.all() - settings = dict(map(lambda x: (x.name, x.value), instance_config)) - app.config.from_mapping(settings) + app.instance_config = InstanceConfig.get_current() @app.before_request @ignore_static diff --git a/securedrop/source_app/api.py b/securedrop/source_app/api.py index 4bc0aba2022..e6171211c74 100644 --- a/securedrop/source_app/api.py +++ b/securedrop/source_app/api.py @@ -12,9 +12,7 @@ def make_blueprint(config): @view.route('/metadata') def metadata(): meta = { - 'allow_document_uploads': current_app.config.get( - 'ALLOW_DOCUMENT_UPLOADS', - True), + 'allow_document_uploads': current_app.instance_config.allow_document_uploads, 'gpg_fpr': config.JOURNALIST_KEY, 'sd_version': version.__version__, 'server_os': platform.linux_distribution()[1], diff --git a/securedrop/source_app/main.py b/securedrop/source_app/main.py index a3d3b6fdc4d..d6c90f0faef 100644 --- a/securedrop/source_app/main.py +++ b/securedrop/source_app/main.py @@ -83,9 +83,6 @@ def create(): @view.route('/lookup', methods=('GET',)) @login_required def lookup(): - allow_document_uploads = current_app.config.get( - 'ALLOW_DOCUMENT_UPLOADS', - True) replies = [] source_inbox = Reply.query.filter(Reply.source_id == g.source.id) \ .filter(Reply.deleted_by_source == False).all() # noqa @@ -124,7 +121,7 @@ def lookup(): return render_template( 'lookup.html', - allow_document_uploads=allow_document_uploads, + allow_document_uploads=current_app.instance_config.allow_document_uploads, codename=g.codename, replies=replies, flagged=g.source.flagged, @@ -135,9 +132,7 @@ def lookup(): @view.route('/submit', methods=('POST',)) @login_required def submit(): - allow_document_uploads = current_app.config.get( - 'ALLOW_DOCUMENT_UPLOADS', - True) + allow_document_uploads = current_app.instance_config.allow_document_uploads msg = request.form['msg'] fh = None if allow_document_uploads and 'fh' in request.files: diff --git a/securedrop/tests/test_journalist.py b/securedrop/tests/test_journalist.py index 82d8519d8f5..886b2a71891 100644 --- a/securedrop/tests/test_journalist.py +++ b/securedrop/tests/test_journalist.py @@ -1296,7 +1296,7 @@ def test_prevent_document_uploads(journalist_app, test_admin): app.post(url_for('admin.update_submission_preferences'), data=form.data, follow_redirects=True) - assert InstanceConfig.query.get('ALLOW_DOCUMENT_UPLOADS').value is False + assert InstanceConfig.get_current().allow_document_uploads is False def test_no_prevent_document_uploads(journalist_app, test_admin): @@ -1305,7 +1305,7 @@ def test_no_prevent_document_uploads(journalist_app, test_admin): test_admin['otp_secret']) app.post(url_for('admin.update_submission_preferences'), follow_redirects=True) - assert InstanceConfig.query.get('ALLOW_DOCUMENT_UPLOADS').value is True + assert InstanceConfig.get_current().allow_document_uploads is True def test_logo_upload_with_valid_image_succeeds(journalist_app, test_admin):