diff --git a/.travis.yml b/.travis.yml index e0b5186faa..02deda4e52 100644 --- a/.travis.yml +++ b/.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 diff --git a/app.json b/app.json index dcaa201a22..e07ac1f833 100644 --- a/app.json +++ b/app.json @@ -12,6 +12,9 @@ "APP_SECRET_TOKEN": { "generator": "secret" }, + "SECRET_KEY": { + "generator": "secret" + }, "ON_HEROKU": "true", "FORCE_SSL": "true", "INVITATION_CODE": { diff --git a/app/api/helpers/storage.py b/app/api/helpers/storage.py index b5a38731e8..79272665c7 100644 --- a/app/api/helpers/storage.py +++ b/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 diff --git a/app/api/schema/settings.py b/app/api/schema/settings.py index 20b5fc226f..7ad1b08f35 100644 --- a/app/api/schema/settings.py +++ b/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) diff --git a/app/factories/setting.py b/app/factories/setting.py index fdbaea5b03..0801b36f1e 100644 --- a/app/factories/setting.py +++ b/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 diff --git a/app/instance.py b/app/instance.py index 7fc41aef54..ad6572d889 100644 --- a/app/instance.py +++ b/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' diff --git a/app/models/setting.py b/app/models/setting.py index a99b2420a1..3f7115b987 100644 --- a/app/models/setting.py +++ b/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 diff --git a/app/settings/__init__.py b/app/settings/__init__.py index 09913aecea..e5e7a1eb04 100644 --- a/app/settings/__init__.py +++ b/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']: diff --git a/config.py b/config.py index a174d46df2..e4fad61cf8 100644 --- a/config.py +++ b/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): """ diff --git a/docs/api/blueprint/settings.apib b/docs/api/blueprint/settings.apib index 7d1e160a38..3fc4ffdc38 100644 --- a/docs/api/blueprint/settings.apib +++ b/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", diff --git a/docs/installation/basic.md b/docs/installation/basic.md index 6486595d2e..b57c740dbf 100644 --- a/docs/installation/basic.md +++ b/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. diff --git a/docs/installation/local.md b/docs/installation/local.md index 3099c20753..962a54e365 100644 --- a/docs/installation/local.md +++ b/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. diff --git a/migrations/versions/rev-2020-01-17-01:09:54-30a490ad1609_remove_secret_from_settings.py b/migrations/versions/rev-2020-01-17-01:09:54-30a490ad1609_remove_secret_from_settings.py new file mode 100644 index 0000000000..1c03315233 --- /dev/null +++ b/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 ### diff --git a/tests/all/integration/setup_database.py b/tests/all/integration/setup_database.py index 324c5fa671..bb0b12f608 100644 --- a/tests/all/integration/setup_database.py +++ b/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 diff --git a/tests/all/unit/api/helpers/test_storage.py b/tests/all/unit/api/helpers/test_storage.py index 7fc7fd1319..0c501afab0 100644 --- a/tests/all/unit/api/helpers/test_storage.py +++ b/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):