Skip to content

Commit

Permalink
refactor: versioned instance_config, "one configuration option per co…
Browse files Browse the repository at this point in the history
…lumn"
  • Loading branch information
wbaid committed Nov 4, 2019
1 parent 25ba95e commit 745009c
Show file tree
Hide file tree
Showing 9 changed files with 116 additions and 46 deletions.
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
"""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
import sqlalchemy as sa


# revision identifiers, used by Alembic.
revision = 'fef03c1ec779'
revision = '523fff3f969c'
down_revision = '3da3fcab826a'
branch_labels = None
depends_on = None
Expand All @@ -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 ###

Expand Down
Original file line number Diff line number Diff line change
@@ -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 ###
6 changes: 1 addition & 5 deletions securedrop/journalist_app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down
15 changes: 3 additions & 12 deletions securedrop/journalist_app/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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'))
Expand Down
63 changes: 58 additions & 5 deletions securedrop/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from passlib.hash import argon2
from sqlalchemy import ForeignKey
from sqlalchemy.orm import relationship, backref
from sqlalchemy import Column, Integer, String, Boolean, DateTime, LargeBinary, JSON
from sqlalchemy import Column, Integer, String, Boolean, DateTime, LargeBinary
from sqlalchemy.orm.exc import MultipleResultsFound, NoResultFound

from db import db
Expand Down Expand Up @@ -742,12 +742,65 @@ 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 "<InstanceConfig(name='%s', value='%s')>" % (self.name, self.value)
return "<InstanceConfig(version=%s, valid_until=%s)>" % (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() # noqa: E711
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()
6 changes: 1 addition & 5 deletions securedrop/source_app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions securedrop/source_app/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
9 changes: 2 additions & 7 deletions securedrop/source_app/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions securedrop/tests/test_journalist.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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):
Expand Down

0 comments on commit 745009c

Please sign in to comment.