fix: Remove secret from DB and enforce env

.travis.yml

@@ -21,7 +21,7 @@ install:
   - pip3 install -r requirements/tests.txt
 
 env:
-  - APP_CONFIG="config.TestingConfig" PATH=$PATH:${HOME}/google-cloud-sdk/bin
+  - APP_CONFIG="config.TestingConfig" SECRET_KEY="super secret key" PATH=$PATH:${HOME}/google-cloud-sdk/bin
 
 before_script:
   - psql -c 'create database test;' -U postgres

app.json

@@ -12,6 +12,9 @@
         "APP_SECRET_TOKEN": {
             "generator": "secret"
         },
+        "SECRET_KEY": {
+            "generator": "secret"
+        },
         "ON_HEROKU": "true",
         "FORCE_SSL": "true",
         "INVITATION_CODE": {

app/api/helpers/storage.py

@@ -285,9 +285,11 @@ def upload_to_gs(bucket_name, client_id, client_secret, file, key, acl='public-r
 # ########
 
 
-def generate_hash(key):
+def generate_hash(key, salt=None):
     """
     Generate hash for key
     """
-    phash = generate_password_hash(key, get_settings()['secret'])
+    if not salt:
+        salt = app.secret_key
+    phash = generate_password_hash(key, salt)
     return str(b64encode(phash), 'utf-8')[:10]  # limit len to 10, is sufficient

app/api/schema/settings.py

@@ -111,8 +111,6 @@ class Meta:
 
     app_environment = fields.Str(default=Environment.PRODUCTION)
 
-    # App secret
-    secret = fields.Str(allow_none=True)
     # Static domain
     static_domain = fields.Str(allow_none=True)
 

app/factories/setting.py

@@ -15,8 +15,6 @@ class Meta:
     app_name = common.string_
     # Tagline for the application. (Eg. Event Management and Ticketing, Home)
     tagline = common.string_
-    # App secret
-    secret = common.secret_
     # Static domain
     static_domain = common.url_
     # Order Expiry Time

app/instance.py

@@ -1,6 +1,7 @@
 from celery.signals import after_task_publish
 import logging
 import os.path
+import secrets
 from envparse import env
 
 import sys
@@ -86,6 +87,17 @@ def create_app():
     Migrate(app, db)
 
     app.config.from_object(env('APP_CONFIG', default='config.ProductionConfig'))
+
+    if not app.config['SECRET_KEY']:
+        if app.config['PRODUCTION']:
+            app.logger.error('SECRET_KEY must be set in .env or environment variables in production')
+            exit(1)
+        else:
+            random_secret = secrets.token_hex()
+            app.logger.warning(f'Using random secret "{ random_secret }" for development server. '
+                               'This is NOT recommended. Set proper SECRET_KEY in .env or environment variables')
+            app.config['SECRET_KEY'] = random_secret
+
     db.init_app(app)
 
     if app.config['CACHING']:
@@ -94,7 +106,6 @@ def create_app():
         cache.init_app(app, config={'CACHE_TYPE': 'null'})
 
     stripe.api_key = 'SomeStripeKey'
-    app.secret_key = 'super secret key'
     app.config['JSONIFY_PRETTYPRINT_REGULAR'] = False
     app.config['FILE_SYSTEM_STORAGE_FILE_VIEW'] = 'static'
 

app/models/setting.py

@@ -26,8 +26,6 @@ class Setting(db.Model):
     app_name = db.Column(db.String)
     # Tagline for the application. (Eg. Event Management and Ticketing, Home)
     tagline = db.Column(db.String)
-    # App secret
-    secret = db.Column(db.String)
     # Static domain
     static_domain = db.Column(db.String)
     # Order Expiry Time in Minutes
@@ -200,7 +198,7 @@ def __init__(self,
                  stripe_test_secret_key=None, stripe_test_publishable_key=None,
                  in_client_id=None, in_client_secret=None,
                  tw_consumer_secret=None, sendgrid_key=None,
-                 secret=None, storage_place=None,
+                 storage_place=None,
                  app_name=None,
                  static_domain=None,
                  tagline=None,
@@ -283,7 +281,6 @@ def __init__(self,
         self.app_name = app_name
         self.static_domain = static_domain
         self.tagline = tagline
-        self.secret = secret
         self.storage_place = storage_place
         self.google_url = google_url
         self.github_url = github_url

app/settings/__init__.py

@@ -15,11 +15,11 @@ def get_settings(from_db=False):
     s = Setting.query.order_by(desc(Setting.id)).first()
     app_environment = current_app.config.get('ENV', 'production')
     if s is None:
-        set_settings(secret='super secret key', app_name='Open Event', app_environment=app_environment)
+        set_settings(app_name='Open Event', app_environment=app_environment)
     else:
         current_app.config['custom_settings'] = make_dict(s)
-        if not current_app.config['custom_settings'].get('secret'):
-            set_settings(secret='super secret key', app_name='Open Event', app_environment=app_environment)
+        if not current_app.config['custom_settings'].get('app_environment'):
+            set_settings(app_name='Open Event', app_environment=app_environment)
     return current_app.config['custom_settings']
 
 
@@ -71,7 +71,6 @@ def set_settings(**kwargs):
                 setattr(setting, key, value)
         from app.api.helpers.db import save_to_db
         save_to_db(setting, 'Setting saved')
-        current_app.secret_key = setting.secret
         stripe.api_key = setting.stripe_secret_key
 
         if setting.app_environment == Environment.DEVELOPMENT and not current_app.config['DEVELOPMENT']:

config.py

@@ -40,6 +40,8 @@ class Config:
     PRODUCTION = False
     TESTING = False
 
+    SECRET_KEY = env.str('SECRET_KEY', default=None)
+
     CACHING = False
     PROFILE = False
     SQLALCHEMY_RECORD_QUERIES = False
@@ -100,8 +102,6 @@ class ProductionConfig(Config):
     PRODUCTION = True
     CACHING = True
 
-    # if force on
-
 
 class StagingConfig(ProductionConfig):
     """

docs/api/blueprint/settings.apib

@@ -167,7 +167,6 @@ To update or get any attribute of this data layer, you will need admin access. H
               "app-environment": "testing",
               "app-name": "Event Yay!",
               "tagline": "Event Management and Ticketing",
-              "secret": "example1234",
               "order-expiry-time": "15",
               "storage-place": "s3",
               "aws-key": "example1234",
@@ -253,7 +252,6 @@ To update or get any attribute of this data layer, you will need admin access. H
                   "app-environment": "testing",
                   "app-name": "Event Yay!",
                   "tagline": "Event Management and Ticketing",
-                  "secret": "example1234",
                   "storage-place": "s3",
                   "aws-key": "example1234",
                   "aws-secret": "example1234",
@@ -320,7 +318,6 @@ To update or get any attribute of this data layer, you will need admin access. H
               "app-environment": "testing",
               "app-name": "Event Yay!",
               "tagline": "Event Management and Ticketing",
-              "secret": "example1234",
               "order-expiry-time": "15",
               "storage-place": "s3",
               "aws-key": "example1234",

docs/installation/basic.md

@@ -45,6 +45,9 @@ CREATE DATABASE oevent WITH OWNER open_event_user;
 cp .env.example .env
 ```
 
+Add `SECRET_KEY={{something random}}` in .env file for cryptographic usage. Note that server will not run in production mode if you don't supply a secret.
+To get a good secret value, run `python -c 'import secrets;print(secrets.token_hex())'` in a terminal and replace `{{something random}}` with its output in the line above in `.env` file
+
 
 * **Step 4** - Start the postgres service.
 

docs/installation/local.md

@@ -122,7 +122,8 @@ CREATE DATABASE opev_test WITH OWNER open_event_user;
 cp .env.example .env
 ```
 
-The URL is short, thanks to the resemble of Postgres user and OS user.
+Add `SECRET_KEY={{something random}}` in .env file for cryptographic usage. Note that server will not run in production mode if you don't supply a secret.
+To get a good secret value, run `python -c 'import secrets;print(secrets.token_hex())'` in a terminal and replace `{{something random}}` with its output in the line above and paste it in `.env` file
 
 
 * **Step 4** - Start the postgres service.

migrations/versions/rev-2020-01-17-01:09:54-30a490ad1609_remove_secret_from_settings.py

@@ -0,0 +1,28 @@
+"""Remove secret from settings
+
+Revision ID: 30a490ad1609
+Revises: eef7c9bc83a12
+Create Date: 2020-01-17 01:09:54.175788
+
+"""
+
+from alembic import op
+import sqlalchemy as sa
+import sqlalchemy_utils
+
+
+# revision identifiers, used by Alembic.
+revision = '30a490ad1609'
+down_revision = 'eef7c9bc83a12'
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.drop_column('settings', 'secret')
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.add_column('settings', sa.Column('secret', sa.VARCHAR(), autoincrement=False, nullable=True))
+    # ### end Alembic commands ###

tests/all/integration/setup_database.py

@@ -15,12 +15,11 @@ class Setup(object):
     @staticmethod
     def create_app():
         app.config.from_object('config.TestingConfig')
-        app.secret_key = 'super secret key'
         app.logger.addHandler(logging.StreamHandler(sys.stdout))
         app.logger.setLevel(logging.ERROR)
         with app.test_request_context():
             db.create_all()
-            set_settings(secret='super secret key', app_name='Open Event', app_environment=Environment.TESTING)
+            set_settings(app_name='Open Event', app_environment=Environment.TESTING)
 
         return app
 

tests/all/unit/api/helpers/test_storage.py

@@ -47,24 +47,18 @@ def test_create_url(self):
     def test_generate_hash(self):
         """Test generation of hash for a key."""
 
-        def patch_settings(settings):
-            settings.return_value = {
-                'secret': 'secret_key'
-            }
-
-        with patch('app.api.helpers.storage.get_settings') as get_settings:
-            patch_settings(get_settings)
-            test_input = 'case1'
-            exepected_output = 'WUFCV0xHVk'
-            actual_output = generate_hash(test_input)
-            self.assertEqual(exepected_output, actual_output)
-            self.assertEqual(len(actual_output), 10)
-
-            test_input = '/static/uploads/pdf/temp/'
-            exepected_output = 'MzRueVhjY0'
-            actual_output = generate_hash(test_input)
-            self.assertEqual(exepected_output, actual_output)
-            self.assertEqual(len(actual_output), 10)
+        secret_key = 'secret_key'
+        test_input = 'case1'
+        exepected_output = 'WUFCV0xHVk'
+        actual_output = generate_hash(test_input, secret_key)
+        self.assertEqual(exepected_output, actual_output)
+        self.assertEqual(len(actual_output), 10)
+
+        test_input = '/static/uploads/pdf/temp/'
+        exepected_output = 'MzRueVhjY0'
+        actual_output = generate_hash(test_input, secret_key)
+        self.assertEqual(exepected_output, actual_output)
+        self.assertEqual(len(actual_output), 10)
 
 
 class TestUploadedFile(unittest.TestCase):