From 0cf6661ea6ff754bf72b0df10d3de18faf8222f9 Mon Sep 17 00:00:00 2001 From: stvnrlly Date: Mon, 3 Oct 2022 19:51:24 +0000 Subject: [PATCH 1/6] add command for creating users in local dev --- app/commands.py | 77 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 54 insertions(+), 23 deletions(-) diff --git a/app/commands.py b/app/commands.py index ad944178d..de99b45f7 100644 --- a/app/commands.py +++ b/app/commands.py @@ -418,31 +418,31 @@ def update_jobs_archived_flag(start_date, end_date): current_app.logger.info('Total archived jobs = {}'.format(total_updated)) -@notify_command(name='update-emails-to-remove-gsi') -@click.option('-s', '--service_id', required=True, help="service id. Update all user.email_address to remove .gsi") -@statsd(namespace="tasks") -def update_emails_to_remove_gsi(service_id): - users_to_update = """SELECT u.id user_id, u.name, email_address, s.id, s.name - FROM users u - JOIN user_to_service us on (u.id = us.user_id) - JOIN services s on (s.id = us.service_id) - WHERE s.id = :service_id - AND u.email_address ilike ('%.gsi.gov.uk%') - """ - results = db.session.execute(users_to_update, {'service_id': service_id}) - print("Updating {} users.".format(results.rowcount)) +# @notify_command(name='update-emails-to-remove-gsi') +# @click.option('-s', '--service_id', required=True, help="service id. Update all user.email_address to remove .gsi") +# @statsd(namespace="tasks") +# def update_emails_to_remove_gsi(service_id): +# users_to_update = """SELECT u.id user_id, u.name, email_address, s.id, s.name +# FROM users u +# JOIN user_to_service us on (u.id = us.user_id) +# JOIN services s on (s.id = us.service_id) +# WHERE s.id = :service_id +# AND u.email_address ilike ('%.gsi.gov.uk%') +# """ +# results = db.session.execute(users_to_update, {'service_id': service_id}) +# print("Updating {} users.".format(results.rowcount)) - for user in results: - print('User with id {} updated'.format(user.user_id)) +# for user in results: +# print('User with id {} updated'.format(user.user_id)) - update_stmt = """ - UPDATE users - SET email_address = replace(replace(email_address, '.gsi.gov.uk', '.gov.uk'), '.GSI.GOV.UK', '.GOV.UK'), - updated_at = now() - WHERE id = :user_id - """ - db.session.execute(update_stmt, {'user_id': str(user.user_id)}) - db.session.commit() +# update_stmt = """ +# UPDATE users +# SET email_address = replace(replace(email_address, '.gsi.gov.uk', '.gov.uk'), '.GSI.GOV.UK', '.GOV.UK'), +# updated_at = now() +# WHERE id = :user_id +# """ +# db.session.execute(update_stmt, {'user_id': str(user.user_id)}) +# db.session.commit() @notify_command(name='replay-daily-sorted-count-files') @@ -841,3 +841,34 @@ def local_dev_broadcast_permissions(user_id): permission_dao.set_user_service_permission( user, service, permission_list, _commit=True, replace=True ) + +@notify_command(name='create-test-user') +@click.option('-e', '--email', required=True) +@click.option('-m', '--mobile_number', required=True) +@click.option('-p', '--password', required=True) +@click.option('-n', '--name', required=True) +@click.option('-a', '--auth_type', default="sms_auth") +@click.option('-s', '--state', default="active") +@click.option('-d', '--admin', default=False, type=bool) +def create_test_user(name,email, mobile_number, password, auth_type, state, admin): + if os.getenv('NOTIFY_ENVIRONMENT', '') not in ['development', 'test']: + current_app.logger.error('Can only be run in development') + return + + data = { + 'name': name, + 'email_address': email, + 'mobile_number': mobile_number, + 'password': password, + 'auth_type': auth_type, + 'state': state, + 'platform_admin': admin, + } + user = User(**data) + try: + db.session.add(user) + db.session.commit() + except IntegrityError: + print("duplicate user", user.name) + db.session.rollback() + \ No newline at end of file From 50938a79790693b79c281c424949f9df105b9279 Mon Sep 17 00:00:00 2001 From: stvnrlly Date: Mon, 3 Oct 2022 19:53:46 +0000 Subject: [PATCH 2/6] deactivate some deprecated or unnecessary commands --- app/commands.py | 178 ++++++++++++++++++++++++------------------------ 1 file changed, 89 insertions(+), 89 deletions(-) diff --git a/app/commands.py b/app/commands.py index de99b45f7..714540500 100644 --- a/app/commands.py +++ b/app/commands.py @@ -142,84 +142,84 @@ def purge_functional_test_data(user_email_prefix): delete_model_user(usr) -@notify_command() -def backfill_notification_statuses(): - """ - DEPRECATED. Populates notification_status. +# @notify_command() +# def backfill_notification_statuses(): +# """ +# DEPRECATED. Populates notification_status. - This will be used to populate the new `Notification._status_fkey` with the old - `Notification._status_enum` - """ - LIMIT = 250000 - subq = "SELECT id FROM notification_history WHERE notification_status is NULL LIMIT {}".format(LIMIT) # nosec B608 no user-controlled input - update = "UPDATE notification_history SET notification_status = status WHERE id in ({})".format(subq) # nosec B608 no user-controlled input - result = db.session.execute(subq).fetchall() +# This will be used to populate the new `Notification._status_fkey` with the old +# `Notification._status_enum` +# """ +# LIMIT = 250000 +# subq = "SELECT id FROM notification_history WHERE notification_status is NULL LIMIT {}".format(LIMIT) # nosec B608 no user-controlled input +# update = "UPDATE notification_history SET notification_status = status WHERE id in ({})".format(subq) # nosec B608 no user-controlled input +# result = db.session.execute(subq).fetchall() - while len(result) > 0: - db.session.execute(update) - print('commit {} updates at {}'.format(LIMIT, datetime.utcnow())) - db.session.commit() - result = db.session.execute(subq).fetchall() +# while len(result) > 0: +# db.session.execute(update) +# print('commit {} updates at {}'.format(LIMIT, datetime.utcnow())) +# db.session.commit() +# result = db.session.execute(subq).fetchall() -@notify_command() -def update_notification_international_flag(): - """ - DEPRECATED. Set notifications.international=false. - """ - # 250,000 rows takes 30 seconds to update. - subq = "select id from notifications where international is null limit 250000" - update = "update notifications set international = False where id in ({})".format(subq) # nosec B608 no user-controlled input - result = db.session.execute(subq).fetchall() +# @notify_command() +# def update_notification_international_flag(): +# """ +# DEPRECATED. Set notifications.international=false. +# """ +# # 250,000 rows takes 30 seconds to update. +# subq = "select id from notifications where international is null limit 250000" +# update = "update notifications set international = False where id in ({})".format(subq) # nosec B608 no user-controlled input +# result = db.session.execute(subq).fetchall() - while len(result) > 0: - db.session.execute(update) - print('commit 250000 updates at {}'.format(datetime.utcnow())) - db.session.commit() - result = db.session.execute(subq).fetchall() +# while len(result) > 0: +# db.session.execute(update) +# print('commit 250000 updates at {}'.format(datetime.utcnow())) +# db.session.commit() +# result = db.session.execute(subq).fetchall() - # Now update notification_history - subq_history = "select id from notification_history where international is null limit 250000" - update_history = "update notification_history set international = False where id in ({})".format(subq_history) # nosec B608 no user-controlled input - result_history = db.session.execute(subq_history).fetchall() - while len(result_history) > 0: - db.session.execute(update_history) - print('commit 250000 updates at {}'.format(datetime.utcnow())) - db.session.commit() - result_history = db.session.execute(subq_history).fetchall() +# # Now update notification_history +# subq_history = "select id from notification_history where international is null limit 250000" +# update_history = "update notification_history set international = False where id in ({})".format(subq_history) # nosec B608 no user-controlled input +# result_history = db.session.execute(subq_history).fetchall() +# while len(result_history) > 0: +# db.session.execute(update_history) +# print('commit 250000 updates at {}'.format(datetime.utcnow())) +# db.session.commit() +# result_history = db.session.execute(subq_history).fetchall() -@notify_command() -def fix_notification_statuses_not_in_sync(): - """ - DEPRECATED. - This will be used to correct an issue where Notification._status_enum and NotificationHistory._status_fkey - became out of sync. See 979e90a. +# @notify_command() +# def fix_notification_statuses_not_in_sync(): +# """ +# DEPRECATED. +# This will be used to correct an issue where Notification._status_enum and NotificationHistory._status_fkey +# became out of sync. See 979e90a. - Notification._status_enum is the source of truth so NotificationHistory._status_fkey will be updated with - these values. - """ - MAX = 10000 +# Notification._status_enum is the source of truth so NotificationHistory._status_fkey will be updated with +# these values. +# """ +# MAX = 10000 - subq = "SELECT id FROM notifications WHERE cast (status as text) != notification_status LIMIT {}".format(MAX) # nosec B608 no user-controlled input - update = "UPDATE notifications SET notification_status = status WHERE id in ({})".format(subq) # nosec B608 no user-controlled input - result = db.session.execute(subq).fetchall() +# subq = "SELECT id FROM notifications WHERE cast (status as text) != notification_status LIMIT {}".format(MAX) # nosec B608 no user-controlled input +# update = "UPDATE notifications SET notification_status = status WHERE id in ({})".format(subq) # nosec B608 no user-controlled input +# result = db.session.execute(subq).fetchall() - while len(result) > 0: - db.session.execute(update) - print('Committed {} updates at {}'.format(len(result), datetime.utcnow())) - db.session.commit() - result = db.session.execute(subq).fetchall() +# while len(result) > 0: +# db.session.execute(update) +# print('Committed {} updates at {}'.format(len(result), datetime.utcnow())) +# db.session.commit() +# result = db.session.execute(subq).fetchall() - subq_hist = "SELECT id FROM notification_history WHERE cast (status as text) != notification_status LIMIT {}".format(MAX) # nosec B608 - update = "UPDATE notification_history SET notification_status = status WHERE id in ({})".format(subq_hist) # nosec B608 no user-controlled input - result = db.session.execute(subq_hist).fetchall() +# subq_hist = "SELECT id FROM notification_history WHERE cast (status as text) != notification_status LIMIT {}".format(MAX) # nosec B608 +# update = "UPDATE notification_history SET notification_status = status WHERE id in ({})".format(subq_hist) # nosec B608 no user-controlled input +# result = db.session.execute(subq_hist).fetchall() - while len(result) > 0: - db.session.execute(update) - print('Committed {} updates at {}'.format(len(result), datetime.utcnow())) - db.session.commit() - result = db.session.execute(subq_hist).fetchall() +# while len(result) > 0: +# db.session.execute(update) +# print('Committed {} updates at {}'.format(len(result), datetime.utcnow())) +# db.session.commit() +# result = db.session.execute(subq_hist).fetchall() @notify_command(name='insert-inbound-numbers') @@ -813,34 +813,34 @@ def populate_annual_billing_with_defaults(year, missing_services_only): set_default_free_allowance_for_service(service, year) -@click.option('-u', '--user-id', required=True) -@notify_command(name='local-dev-broadcast-permissions') -def local_dev_broadcast_permissions(user_id): - if os.getenv('NOTIFY_ENVIRONMENT', '') not in ['development', 'test']: - current_app.logger.error('Can only be run in development') - return +# @click.option('-u', '--user-id', required=True) +# @notify_command(name='local-dev-broadcast-permissions') +# def local_dev_broadcast_permissions(user_id): +# if os.getenv('NOTIFY_ENVIRONMENT', '') not in ['development', 'test']: +# current_app.logger.error('Can only be run in development') +# return - user = User.query.filter_by(id=user_id).one() +# user = User.query.filter_by(id=user_id).one() - user_broadcast_services = Service.query.filter( - Service.permissions.any(permission='broadcast'), - Service.users.any(id=user_id) - ) +# user_broadcast_services = Service.query.filter( +# Service.permissions.any(permission='broadcast'), +# Service.users.any(id=user_id) +# ) - for service in user_broadcast_services: - permission_list = [ - Permission(service_id=service.id, user_id=user_id, permission=permission) - for permission in [ - 'reject_broadcasts', 'cancel_broadcasts', # required to create / approve - 'create_broadcasts', 'approve_broadcasts', # minimum for testing - 'manage_templates', # unlikely but might be useful - 'view_activity', # normally added on invite / service creation - ] - ] +# for service in user_broadcast_services: +# permission_list = [ +# Permission(service_id=service.id, user_id=user_id, permission=permission) +# for permission in [ +# 'reject_broadcasts', 'cancel_broadcasts', # required to create / approve +# 'create_broadcasts', 'approve_broadcasts', # minimum for testing +# 'manage_templates', # unlikely but might be useful +# 'view_activity', # normally added on invite / service creation +# ] +# ] - permission_dao.set_user_service_permission( - user, service, permission_list, _commit=True, replace=True - ) +# permission_dao.set_user_service_permission( +# user, service, permission_list, _commit=True, replace=True +# ) @notify_command(name='create-test-user') @click.option('-e', '--email', required=True) From 19cdd9b052c5df641a1dc34ce99f600e71214ba2 Mon Sep 17 00:00:00 2001 From: stvnrlly Date: Fri, 28 Oct 2022 14:07:43 -0400 Subject: [PATCH 3/6] tests & prompts for user creation command --- app/commands.py | 10 +++++----- tests/app/test_commands.py | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/app/commands.py b/app/commands.py index 7e319db5f..98afdaaf3 100644 --- a/app/commands.py +++ b/app/commands.py @@ -736,10 +736,10 @@ def populate_annual_billing_with_defaults(year, missing_services_only): @notify_command(name='create-test-user') -@click.option('-e', '--email', required=True) -@click.option('-m', '--mobile_number', required=True) -@click.option('-p', '--password', required=True) -@click.option('-n', '--name', required=True) +@click.option('-n', '--name', required=True, prompt=True) +@click.option('-e', '--email', required=True, prompt=True) +@click.option('-m', '--mobile_number', required=True, prompt=True) +@click.option('-p', '--password', required=True, prompt=True, hide_input=True, confirmation_prompt=True) @click.option('-a', '--auth_type', default="sms_auth") @click.option('-s', '--state', default="active") @click.option('-d', '--admin', default=False, type=bool) @@ -754,7 +754,7 @@ def create_test_user(name,email, mobile_number, password, auth_type, state, admi 'mobile_number': mobile_number, 'password': password, 'auth_type': auth_type, - 'state': state, + 'state': state, # skip the email verification for our test user 'platform_admin': admin, } user = User(**data) diff --git a/tests/app/test_commands.py b/tests/app/test_commands.py index d29949be8..587da4dd8 100644 --- a/tests/app/test_commands.py +++ b/tests/app/test_commands.py @@ -1,14 +1,45 @@ import pytest from app.commands import ( + create_test_user, insert_inbound_numbers_from_file, populate_annual_billing_with_defaults, ) from app.dao.inbound_numbers_dao import dao_get_available_inbound_numbers -from app.models import AnnualBilling +from app.models import AnnualBilling, User from tests.app.db import create_annual_billing, create_service +def test_create_test_user_command(notify_db_session, notify_api): + + # number of users before adding ours + user_count = User.query.count() + + # run the command + notify_api.test_cli_runner().invoke( + create_test_user, [ + '--email', 'somebody@fake.gov', + '--mobile_number', '555-555-5555', + '--password', 'correct horse battery staple', + '--name', 'Fake Personson', + # '--auth_type', 'sms_auth', # this is the default + # '--state', 'active', # this is the default + # '--admin', 'False', # this is the default + ] + ) + + # there should be one more user + assert User.query.count() == user_count + 1 + + # that user should be the one we added + user = User.query.filter_by( + name = 'Fake Personson' + ).first() + assert user.email_address == 'somebody@fake.gov' + assert user.auth_type == 'sms_auth' + assert user.state == 'active' + + def test_insert_inbound_numbers_from_file(notify_db_session, notify_api, tmpdir): numbers_file = tmpdir.join("numbers.txt") numbers_file.write("07700900373\n07700900473\n07700900375\n\n\n\n") From 11d123051ab97573263ff3967827487889134b0c Mon Sep 17 00:00:00 2001 From: stvnrlly Date: Fri, 28 Oct 2022 14:42:25 -0400 Subject: [PATCH 4/6] validate mobile number so that sms auth works --- app/commands.py | 19 ++++++++++++++----- tests/app/test_commands.py | 6 +++--- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/app/commands.py b/app/commands.py index 98afdaaf3..f0c1568b6 100644 --- a/app/commands.py +++ b/app/commands.py @@ -735,19 +735,28 @@ def populate_annual_billing_with_defaults(year, missing_services_only): set_default_free_allowance_for_service(service, year) +def validate_mobile(ctx, param, value): + if (len(''.join(i for i in value if i.isdigit())) != 10): + raise click.BadParameter("mobile number must have 10 digits") + else: + return value + + @notify_command(name='create-test-user') @click.option('-n', '--name', required=True, prompt=True) -@click.option('-e', '--email', required=True, prompt=True) -@click.option('-m', '--mobile_number', required=True, prompt=True) -@click.option('-p', '--password', required=True, prompt=True, hide_input=True, confirmation_prompt=True) +@click.option('-e', '--email', required=True, prompt=True) # TODO: require valid email +@click.option('-m', '--mobile_number', + required=True, prompt=True, callback=validate_mobile) +@click.option('-p', '--password', + required=True, prompt=True, hide_input=True, confirmation_prompt=True) @click.option('-a', '--auth_type', default="sms_auth") @click.option('-s', '--state', default="active") @click.option('-d', '--admin', default=False, type=bool) -def create_test_user(name,email, mobile_number, password, auth_type, state, admin): +def create_test_user(name, email, mobile_number, password, auth_type, state, admin): if os.getenv('NOTIFY_ENVIRONMENT', '') not in ['development', 'test']: current_app.logger.error('Can only be run in development') return - + data = { 'name': name, 'email_address': email, diff --git a/tests/app/test_commands.py b/tests/app/test_commands.py index 587da4dd8..b2b6abd14 100644 --- a/tests/app/test_commands.py +++ b/tests/app/test_commands.py @@ -11,10 +11,10 @@ from tests.app.db import create_annual_billing, create_service def test_create_test_user_command(notify_db_session, notify_api): - + # number of users before adding ours user_count = User.query.count() - + # run the command notify_api.test_cli_runner().invoke( create_test_user, [ @@ -33,7 +33,7 @@ def test_create_test_user_command(notify_db_session, notify_api): # that user should be the one we added user = User.query.filter_by( - name = 'Fake Personson' + name='Fake Personson' ).first() assert user.email_address == 'somebody@fake.gov' assert user.auth_type == 'sms_auth' From 713daee566b3b13760344e343480747f8d445dd0 Mon Sep 17 00:00:00 2001 From: stvnrlly Date: Fri, 28 Oct 2022 14:49:02 -0400 Subject: [PATCH 5/6] flake8 indentation --- app/commands.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/commands.py b/app/commands.py index f0c1568b6..ff95887e6 100644 --- a/app/commands.py +++ b/app/commands.py @@ -746,9 +746,9 @@ def validate_mobile(ctx, param, value): @click.option('-n', '--name', required=True, prompt=True) @click.option('-e', '--email', required=True, prompt=True) # TODO: require valid email @click.option('-m', '--mobile_number', - required=True, prompt=True, callback=validate_mobile) + required=True, prompt=True, callback=validate_mobile) @click.option('-p', '--password', - required=True, prompt=True, hide_input=True, confirmation_prompt=True) + required=True, prompt=True, hide_input=True, confirmation_prompt=True) @click.option('-a', '--auth_type', default="sms_auth") @click.option('-s', '--state', default="active") @click.option('-d', '--admin', default=False, type=bool) From 57adbf3f6bea95409586c3b4790171f66a35b0e2 Mon Sep 17 00:00:00 2001 From: stvnrlly Date: Tue, 1 Nov 2022 12:14:26 -0400 Subject: [PATCH 6/6] document create-test-user command --- README.md | 2 +- app/commands.py | 27 --------------------------- docs/one-off-tasks.md | 14 +++++++++----- docs/testing.md | 8 ++++++++ 4 files changed, 18 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index b8cd421c0..79a376436 100644 --- a/README.md +++ b/README.md @@ -30,7 +30,7 @@ Our other repositories are: ### Common dev work - [Local setup](#local-setup) -- [Testing](./docs/testing.md) +- [Testing](./docs/testing.md), both automated and manual - [Deploying](./docs/deploying.md) - [Running one-off tasks](./docs/one-off-tasks.md) diff --git a/app/commands.py b/app/commands.py index ff95887e6..0f4e94ed3 100644 --- a/app/commands.py +++ b/app/commands.py @@ -340,33 +340,6 @@ def update_jobs_archived_flag(start_date, end_date): current_app.logger.info('Total archived jobs = {}'.format(total_updated)) -# @notify_command(name='update-emails-to-remove-gsi') -# @click.option('-s', '--service_id', required=True, help="service id. Update all user.email_address to remove .gsi") -# @statsd(namespace="tasks") -# def update_emails_to_remove_gsi(service_id): -# users_to_update = """SELECT u.id user_id, u.name, email_address, s.id, s.name -# FROM users u -# JOIN user_to_service us on (u.id = us.user_id) -# JOIN services s on (s.id = us.service_id) -# WHERE s.id = :service_id -# AND u.email_address ilike ('%.gsi.gov.uk%') -# """ -# results = db.session.execute(users_to_update, {'service_id': service_id}) -# print("Updating {} users.".format(results.rowcount)) - -# for user in results: -# print('User with id {} updated'.format(user.user_id)) - -# update_stmt = """ -# UPDATE users -# SET email_address = replace(replace(email_address, '.gsi.gov.uk', '.gov.uk'), '.GSI.GOV.UK', '.GOV.UK'), -# updated_at = now() -# WHERE id = :user_id -# """ -# db.session.execute(update_stmt, {'user_id': str(user.user_id)}) -# db.session.commit() - - @notify_command(name='replay-daily-sorted-count-files') @click.option('-f', '--file_extension', required=False, help="File extension to search for, defaults to rs.txt") @statsd(namespace="tasks") diff --git a/docs/one-off-tasks.md b/docs/one-off-tasks.md index a337eaf01..d107ea849 100644 --- a/docs/one-off-tasks.md +++ b/docs/one-off-tasks.md @@ -2,14 +2,18 @@ For these, we're using Flask commands, which live in [`/app/commands.py`](../app/commands.py). -This includes things that might be one-time operations! Using a command allows the operation to be tested, -both with `pytest` and with trial runs. +This includes things that might be one-time operations! If we're running it on production, it should be a Flask +command Using a command allows the operation to be tested, both with `pytest` and with trial runs in staging. + +To see information about available commands, you can get a list with: + +`pipenv run flask command` + +Appending `--help` to any command will give you more information about parameters. To run a command on cloud.gov, use this format: -``` -cf run-task CLOUD-GOV-APP --commmand "YOUR COMMAND HERE" --name YOUR-COMMAND -``` +`cf run-task CLOUD-GOV-APP --commmand "YOUR COMMAND HERE" --name YOUR-COMMAND` [Here's more documentation](https://docs.cloudfoundry.org/devguide/using-tasks.html) about Cloud Foundry tasks. diff --git a/docs/testing.md b/docs/testing.md index 90c5bdb2d..ad6c7e62e 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -23,6 +23,14 @@ We're using GitHub Actions. See [/.github](../.github/) for the configuration. In addition to commit-triggered scans, the `daily_checks.yml` workflow runs the relevant dependency audits, static scan, and/or dynamic scans at 10am UTC each day. Developers will be notified of failures in daily scans by GitHub notifications. +## Manual testing + +If you're checking out the system locally, you may want to create a user quickly. + +`pipenv run flask command create-test-user` + +This will run an interactive prompt to create a user, and then mark that user as active. *Use a real mobile number* if you want to log in, as the SMS auth code will be sent here. + ## To run a local OWASP scan 1. Run `make run-flask` from within the dev container.