diff --git a/.env b/.env new file mode 100644 index 000000000..ff24258b3 --- /dev/null +++ b/.env @@ -0,0 +1,3 @@ +# Environment variables used by Docker (specifically for docker-compose.yml) +DB_NAME=cf_brc_db +COLDFRONT_PORT=8880 \ No newline at end of file diff --git a/.github/workflows/django_testing_ci.yml b/.github/workflows/django_testing_ci.yml index cc2e33cce..b172a6585 100644 --- a/.github/workflows/django_testing_ci.yml +++ b/.github/workflows/django_testing_ci.yml @@ -84,6 +84,7 @@ jobs: - name: Run Tests run: | source ~/venv/bin/activate + export django_secret_key=`openssl rand -base64 64` python manage.py migrate python manage.py test diff --git a/Dockerfile b/Dockerfile index b56ea76b9..be14a05d7 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,34 +1,33 @@ -FROM centos:8 +FROM centos/python-38-centos7 LABEL description="coldfront" -# install dependencies -RUN yum -y install epel-release -RUN yum -y update -RUN yum -y install python36 python36-devel git memcached redis - +USER root WORKDIR /root - -# install coldfront -RUN mkdir /opt/coldfront_app - -WORKDIR /opt/coldfront_app - -RUN cd /opt/coldfront_app -RUN git clone https://github.com/ubccr/coldfront.git -RUN python3.6 -mvenv venv -RUN source venv/bin/activate - -WORKDIR /opt/coldfront_app/coldfront - -RUN cd /opt/coldfront_app/coldfront -RUN pip3 install wheel -RUN pip3 install -r requirements.txt -RUN cp coldfront/config/local_settings.py.sample coldfront/config/local_settings.py -RUN cp coldfront/config/local_strings.py.sample coldfront/config/local_strings.py - -RUN python3 ./manage.py initial_setup -RUN python3 ./manage.py load_test_data - -EXPOSE 8000 +COPY requirements.txt ./ +RUN pip install -r requirements.txt && rm requirements.txt +RUN pip install jinja2 pyyaml + +# mybrc or mylrc +ARG PORTAL="mybrc" +RUN mkdir -p /var/log/user_portals/cf_${PORTAL} \ + && touch /var/log/user_portals/cf_${PORTAL}/cf_${PORTAL}_{portal,api}.log \ + && chmod 775 /var/log/user_portals/cf_${PORTAL} \ + && chmod 664 /var/log/user_portals/cf_${PORTAL}/cf_${PORTAL}_{portal,api}.log + +COPY . /vagrant/coldfront_app/coldfront/ +WORKDIR /vagrant/coldfront_app/coldfront/ + +RUN chmod +x ./manage.py + +CMD ./manage.py initial_setup \ + && ./manage.py add_accounting_defaults \ + && ./manage.py add_allowance_defaults \ + && ./manage.py add_directory_defaults \ + && ./manage.py create_allocation_periods \ + && ./manage.py create_staff_group \ + && ./manage.py collectstatic --noinput \ + && ./manage.py runserver 0.0.0.0:80 + +EXPOSE 80 STOPSIGNAL SIGINT diff --git a/Dockerfile.db b/Dockerfile.db new file mode 100644 index 000000000..5af3ba0ab --- /dev/null +++ b/Dockerfile.db @@ -0,0 +1,5 @@ +FROM postgres:15 + +LABEL description="coldfront db" + +RUN apt update && apt install -y sudo postgresql-client \ No newline at end of file diff --git a/README.md b/README.md index 3433d8423..d7fa75d44 100644 --- a/README.md +++ b/README.md @@ -46,9 +46,15 @@ of variables used by Ansible to configure the system. ``` cp bootstrap/ansible/main.copyme main.yml ``` -7. Customize `main.yml`. In particular, uncomment everything under the `dev_settings` section, and fill in the below variables. Note +7. Generate a key to be used as the `SECRET_KEY` for Django. + ``` + # This produces two lines: condense them into one. + openssl rand -base64 64 + ``` +8. Customize `main.yml`. In particular, uncomment everything under the `dev_settings` section, and fill in the below variables. Note that quotes need not be provided, except in the list variable. ``` + django_secret_key: secret_key_from_previous_step db_admin_passwd: password_here redis_passwd: password_here from_email: you@email.com @@ -56,18 +62,18 @@ that quotes need not be provided, except in the list variable. email_admin_list: ["you@email.com"] request_approval_cc_list: ["you@email.com"] ``` -8. Provision the VM. This should run the Ansible playbook. Expect this to take +9. Provision the VM. This should run the Ansible playbook. Expect this to take a few minutes on the first run. ``` vagrant up ``` -9. SSH into the VM. +10. SSH into the VM. ``` vagrant ssh ``` -10. On the host machine, navigate to `http://localhost:8880`, where the +11. On the host machine, navigate to `http://localhost:8880`, where the application should be served. -11. (Optional) Load data from a database dump file. +12. (Optional) Load data from a database dump file. ``` # Clear the Django database to avoid conflicts. python manage.py sqlflush | python manage.py dbshell @@ -207,6 +213,15 @@ multiple files or directories to omit. - Open `htmlcov/index.html` in a browser to view which lines of code were covered by the tests and which were not. +## Docker - Quick Install (Recommend) +1. Generate configuration (`dev_settings.py`): have Python with the `jinja2` and `pyyaml` libraries installed, and then run `bootstrap/development/gen_config.sh` +2. Build Images: In the base directory, run `docker build . -t coldfront` and `docker build . -f Dockerfile.db -t coldfront_db`. `--build-arg PORTAL=mylrc` can be added to the build command to build for mylrc. +3. If needed, modify `.env` to customize the web server port and the database name (e.g from `cf_mybrc_db` to `cf_mylrc_db`) +4. To run: In the base directory, run `docker compose up` +5. To enter the coldfront container (similar to `vagrant ssh`): run `docker exec -it coldfront-coldfront-1 bash` +6. To load a database backup: run `bootstrap/development/docker_load_database_backup.sh ${DB_NAME} ${PATH_TO_DUMP}` +7. To start from scratch (delete volumes): In the base directory, run `docker compose down --volumes` + ## Local Machine - Quick Install (Not Recommended) 1. ColdFront requires Python 3.6, memcached, and redis. diff --git a/bootstrap/ansible/main.copyme b/bootstrap/ansible/main.copyme index cd0c670e8..c47b1599c 100644 --- a/bootstrap/ansible/main.copyme +++ b/bootstrap/ansible/main.copyme @@ -18,9 +18,14 @@ djangoprojname: coldfront # BRC/LRC Config ############################################################################### +# The secret key used by Django for cryptographic signing. +# TODO: Generate one using: `openssl rand -base64 64`. +django_secret_key: + # The name of the PostgreSQL database. # TODO: For LRC, set this to 'cf_lrc_db'. db_name: cf_brc_db +db_host: localhost # The credentials for the database admin user. # TODO: Replace the username and password. @@ -30,6 +35,7 @@ db_admin_passwd: '' # The password for Redis. # TODO: Replace the password. redis_passwd: '' +redis_host: localhost # Log paths. # TODO: For LRC, use the substring 'cf_mylrc'. @@ -81,9 +87,10 @@ cilogon_app_client_id: "" cilogon_app_secret: "" # Django Flags settings. -# TODO: For LRC, disable basic auth. and enable SSO. -flag_basic_auth_enabled: True -flag_sso_enabled: False +# TODO: For LRC, disable link login. +flag_basic_auth_enabled: False +flag_sso_enabled: True +flag_link_login_enabled: True # TODO: For LRC, disable BRC and enable LRC. flag_brc_enabled: True flag_lrc_enabled: False @@ -91,6 +98,7 @@ flag_lrc_enabled: False # the next allowance year. # TODO: For LRC, set the month number to 9 (September). flag_next_period_renewal_requestable_month: 5 +flag_multiple_email_addresses_allowed: False # Portal settings. # TODO: For LRC, use "MyLRC", "Laboratory Research Computing", "LRC", and diff --git a/bootstrap/ansible/settings_template.tmpl b/bootstrap/ansible/settings_template.tmpl index 21b2ddfb1..279b0b7fc 100644 --- a/bootstrap/ansible/settings_template.tmpl +++ b/bootstrap/ansible/settings_template.tmpl @@ -3,6 +3,8 @@ import os # SECURITY WARNING: don't run with debug turned on in production! DEBUG = {{ debug }} +SECRET_KEY = '{{ django_secret_key }}' + ALLOWED_HOSTS = ['0.0.0.0', '{{ hostname }}', '{{ host_ip }}'] PORTAL_NAME = '{{ portal_name }}' @@ -46,7 +48,7 @@ DATABASES = { 'NAME': '{{ db_name }}', 'USER': '{{ db_admin_user }}', 'PASSWORD': '{{ db_admin_passwd }}', - 'HOST': 'localhost', + 'HOST': '{{ db_host }}', 'PORT': '5432', }, } @@ -108,7 +110,8 @@ if SENTRY_DSN: sentry_sdk.init( dsn=SENTRY_DSN, integrations=[DjangoIntegration()], - traces_sample_rate=0.01, + sample_rate=0.1, + traces_sample_rate=0.001, send_default_pii=True) # Ignore noisy loggers. ignore_logger('coldfront.api') @@ -135,7 +138,7 @@ CONSTANCE_CONFIG = { 'LAUNCH_DATE': (date(1970, 1, 1), 'The date the portal was launched.'), } CONSTANCE_REDIS_CONNECTION = { - 'host': '127.0.0.1', + 'host': '{{ redis_host }}', 'port': 6379, 'db': 0, 'password': '{{ redis_passwd }}', @@ -152,6 +155,12 @@ EXTRA_INTERNAL_IPS = [ {% endfor %} ] +if DEBUG: + # For Docker support + import socket + hostname, _, ips = socket.gethostbyname_ex(socket.gethostname()) + INTERNAL_IPS = [ip[: ip.rfind('.')] + '.1' for ip in ips] + ['10.0.2.2'] + #------------------------------------------------------------------------------ # django-flags settings #------------------------------------------------------------------------------ @@ -162,19 +171,37 @@ FLAGS = { ], 'BASIC_AUTH_ENABLED': [{'condition': 'boolean', 'value': {{ flag_basic_auth_enabled }}}], 'BRC_ONLY': [{'condition': 'boolean', 'value': {{ flag_brc_enabled }}}], + 'LINK_LOGIN_ENABLED': [{'condition': 'boolean', 'value': {{ flag_link_login_enabled }}}], 'LRC_ONLY': [{'condition': 'boolean', 'value': {{ flag_lrc_enabled }}}], + 'MULTIPLE_EMAIL_ADDRESSES_ALLOWED': [{'condition': 'boolean', 'value': {{ flag_multiple_email_addresses_allowed }}}], 'SECURE_DIRS_REQUESTABLE': [{'condition': 'boolean', 'value': {{ flag_brc_enabled }}}], 'SERVICE_UNITS_PURCHASABLE': [{'condition': 'boolean', 'value': {{ flag_brc_enabled }}}], 'SSO_ENABLED': [{'condition': 'boolean', 'value': {{ flag_sso_enabled }}}], } +# Enforce that boolean flags are consistent with each other. +from django.core.exceptions import ImproperlyConfigured +if not (FLAGS['BRC_ONLY'][0]['value'] ^ FLAGS['LRC_ONLY'][0]['value']): + raise ImproperlyConfigured( + 'Exactly one of BRC_ONLY, LRC_ONLY should be enabled.') +if not ( + FLAGS['BASIC_AUTH_ENABLED'][0]['value'] ^ + FLAGS['SSO_ENABLED'][0]['value']): + raise ImproperlyConfigured( + 'Exactly one of BASIC_AUTH_ENABLED, SSO_ENABLED should be enabled.') +if (not FLAGS['SSO_ENABLED'][0]['value'] and + FLAGS['LINK_LOGIN_ENABLED'][0]['value']): + raise ImproperlyConfigured( + 'LINK_LOGIN_ENABLED should only be enabled when SSO_ENABLED is ' + 'enabled.') + #------------------------------------------------------------------------------ # django-q settings #------------------------------------------------------------------------------ Q_CLUSTER = { 'redis': { - 'host': '127.0.0.1', + 'host': '{{ redis_host }}', 'port': 6379, 'db': 0, 'password': '{{ redis_passwd }}', diff --git a/bootstrap/development/docker_load_database_backup.sh b/bootstrap/development/docker_load_database_backup.sh new file mode 100644 index 000000000..6ea06111f --- /dev/null +++ b/bootstrap/development/docker_load_database_backup.sh @@ -0,0 +1,4 @@ +#!/bin/bash +# $1 = database name +# $2 = dump file +docker exec -i coldfront-db-1 pg_restore --verbose --clean -U admin -d $1 < $2 diff --git a/bootstrap/development/gen_config.sh b/bootstrap/development/gen_config.sh new file mode 100644 index 000000000..7382379ed --- /dev/null +++ b/bootstrap/development/gen_config.sh @@ -0,0 +1,14 @@ +#!/usr/bin/env bash +cp coldfront/config/local_settings.py.sample \ + coldfront/config/local_settings.py +cp coldfront/config/local_strings.py.sample \ + coldfront/config/local_strings.py +python -c \ +"from jinja2 import Template, Environment, FileSystemLoader; \ +import yaml; \ +env = Environment(loader=FileSystemLoader('bootstrap/ansible/')); \ +env.filters['bool'] = lambda x: str(x).lower() in ['true', 'yes', 'on', '1']; \ +options = yaml.safe_load(open('main.yml').read()); \ +options.update({'redis_host': 'redis', 'db_host': 'db'}); \ +print(env.get_template('settings_template.tmpl').render(options))" \ + > coldfront/config/dev_settings.py diff --git a/bootstrap/development/load_database_backup.sh b/bootstrap/development/load_database_backup.sh index ef1a43dab..4d6199310 100644 --- a/bootstrap/development/load_database_backup.sh +++ b/bootstrap/development/load_database_backup.sh @@ -1,18 +1,27 @@ -#!/bin/sh +#!/bin/bash # This script clears out the PostgreSQL database with the given name and # loads in the data from the given dump file generated by pg_dump. It # skips loading entries from the Job table. # Store second last and last arguments. -DB_NAME=${@:(-2):1} +if [ "$POSTGRES_DB" != "" ]; then + DB_NAME=$POSTGRES_DB + if [ "$LOAD_DUMP" = true ]; then + DUMP_FILE=/db.dump + else + exit 0 + fi +else + DB_NAME=${@:(-2):1} + DUMP_FILE=${@: -1} +fi DB_OWNER=admin -DUMP_FILE=${@: -1} BIN=/usr/pgsql-15/bin/pg_restore if [[ "$1" == "--help" ]] || [[ "$1" == "-h" ]] ; then - echo "Usage: `basename $0` [OPTION] name-of-database absolute-path-to-dump-file" + echo "Usage: sudo -u postgres `basename $0` [OPTION] name-of-database absolute-path-to-dump-file" echo $' -k, --kill-connections\t\tterminates all connections to the database so it can be dropped' exit 0 elif ! [[ "$DUMP_FILE" = /* ]] ; then @@ -24,18 +33,19 @@ elif ! [[ -f "$DUMP_FILE" ]] ; then fi if [[ "$1" == "-k" || "$1" == "--kill-connections" ]] ; then - echo "TERMINATING DATABASE CONNECTIONS..." - sudo -u postgres psql -c "SELECT pg_terminate_backend(pid) + echo "TERMINATING DATABASE CONNECTIONS..." + psql -U $DB_OWNER -c "SELECT pg_terminate_backend(pid) FROM pg_stat_activity - WHERE pid <> pg_backend_pid() AND datname = '$DB_NAME';" + WHERE pid <> pg_backend_pid() AND datname = '$DB_NAME';" $DB_NAME fi -sudo -u postgres /bin/bash -c "echo DROPPING DATABASE... \ - && psql -c 'DROP DATABASE $DB_NAME;'; \ - \ - echo RECREATING DATABASE... \ - && psql -c 'CREATE DATABASE $DB_NAME OWNER $DB_OWNER;' \ - \ - && echo LOADING DATABASE... \ - && $BIN -d $DB_NAME $DUMP_FILE; \ - psql -c 'ALTER SCHEMA public OWNER TO $DB_OWNER;' $DB_NAME;" +echo DROPPING DATABASE... \ +&& psql -U $DB_OWNER -c "DROP DATABASE $DB_NAME;" $DB_NAME + +echo RECREATING DATABASE... \ +&& psql -U $DB_OWNER -c "CREATE DATABASE $DB_NAME OWNER $DB_OWNER;" $DB_NAME + +echo LOADING DATABASE... \ +&& $BIN -d $DB_NAME $DUMP_FILE + +psql -U $DB_OWNER -c "ALTER SCHEMA public OWNER TO $DB_OWNER;" $DB_NAME diff --git a/coldfront/api/statistics/tests/test_job_view_set.py b/coldfront/api/statistics/tests/test_job_view_set.py index 0f85fcfe7..fbf54694a 100644 --- a/coldfront/api/statistics/tests/test_job_view_set.py +++ b/coldfront/api/statistics/tests/test_job_view_set.py @@ -1154,7 +1154,7 @@ def test_post_allocation_missing_dates(self): self.assertFalse( Job.objects.filter(jobslurmid=data['jobslurmid']).exists()) - @patch('coldfront.api.statistics.views.JobViewSet.validate_job_dates') + @patch('coldfront.api.statistics.views.validate_job_dates') def test_post_handles_exception_when_validating_dates(self, mock_method): """Test that a POST (create) request does not fail to create a job even if an exception is raised when determining whether @@ -1396,7 +1396,7 @@ def test_put_allocation_missing_dates(self): self.assertFalse( Job.objects.filter(jobslurmid=data['jobslurmid']).exists()) - @patch('coldfront.api.statistics.views.JobViewSet.validate_job_dates') + @patch('coldfront.api.statistics.views.validate_job_dates') def test_put_handles_exception_when_validating_dates(self, mock_method): """Test that a PUT (update) request does not fail to create a job even if an exception is raised when determining whether diff --git a/coldfront/api/statistics/utils.py b/coldfront/api/statistics/utils.py index 878ad9363..07568fc47 100644 --- a/coldfront/api/statistics/utils.py +++ b/coldfront/api/statistics/utils.py @@ -225,14 +225,26 @@ def get_accounting_allocation_objects(project, user=None, if not isinstance(user, User): raise TypeError(f'User {user} is not a User object.') - # Check that there is an active association between the user and project. - active_status = ProjectUserStatusChoice.objects.get(name='Active') - ProjectUser.objects.get(user=user, project=project, status=active_status) - - # Check that the user is an active member of the allocation. - active_status = AllocationUserStatusChoice.objects.get(name='Active') - allocation_user = AllocationUser.objects.get( - allocation=allocation, user=user, status=active_status) + project_user_kwargs = { + 'user': user, + 'project': project, + } + if enforce_allocation_active: + # Check that there is an active association between the user and + # project. + project_user_kwargs['status'] = ProjectUserStatusChoice.objects.get( + name='Active') + ProjectUser.objects.get(**project_user_kwargs) + + allocation_user_kwargs = { + 'allocation': allocation, + 'user': user, + } + if enforce_allocation_active: + # Check that the user is an active member of the allocation. + allocation_user_kwargs['status'] = \ + AllocationUserStatusChoice.objects.get(name='Active') + allocation_user = AllocationUser.objects.get(**allocation_user_kwargs) # Check that the allocation user has an attribute for Service Units # and an associated usage. diff --git a/coldfront/api/statistics/views.py b/coldfront/api/statistics/views.py index 9981bd579..abf867a70 100644 --- a/coldfront/api/statistics/views.py +++ b/coldfront/api/statistics/views.py @@ -2,7 +2,6 @@ import pytz from collections import OrderedDict -from datetime import date from datetime import datetime from datetime import timedelta from decimal import Decimal, InvalidOperation @@ -36,6 +35,7 @@ from coldfront.core.resource.utils_.allowance_utils.computing_allowance import ComputingAllowance from coldfront.core.resource.utils_.allowance_utils.interface import ComputingAllowanceInterface from coldfront.core.statistics.models import Job +from coldfront.core.statistics.utils_.accounting_utils import validate_job_dates from coldfront.core.user.models import UserProfile from coldfront.core.utils.common import display_time_zone_date_to_utc_datetime @@ -263,7 +263,7 @@ def create(self, request, *args, **kwargs): logger.warning(f'Job {jobslurmid} has no amount.') try: - job_dates_valid = self.validate_job_dates( + job_dates_valid = validate_job_dates( serializer.validated_data, allocation_objects.allocation, end_date_expected=False) except Exception as e: @@ -374,7 +374,7 @@ def update(self, request, *args, **kwargs): logger.warning(f'Job {jobslurmid} has no amount.') try: - job_dates_valid = self.validate_job_dates( + job_dates_valid = validate_job_dates( serializer.validated_data, allocation_objects.allocation, end_date_expected=True) except Exception as e: @@ -441,91 +441,6 @@ def update(self, request, *args, **kwargs): return Response(serializer.data) - @staticmethod - def validate_job_dates(job_data, allocation, end_date_expected=False): - """Given a dictionary representing a Job, its corresponding - Allocation, and whether the Job is expected to include an end - date, return whether: - (a) The Job has the expected dates, - (b) The Job's corresponding Allocation has the expected - dates, and - (c) The Job started and ended within the Allocation's dates. - - Write errors or warnings to the log if not.""" - logger = logging.getLogger(__name__) - - jobslurmid = job_data['jobslurmid'] - account_name = job_data['accountid'].name - - # The Job should have submit, start, and, if applicable, end dates. - expected_date_keys = ['submitdate', 'startdate'] - if end_date_expected: - expected_date_keys.append('enddate') - expected_dates = { - key: job_data.get(key, None) for key in expected_date_keys} - for key, expected_date in expected_dates.items(): - if not isinstance(expected_date, datetime): - logger.error(f'Job {jobslurmid} does not have a {key}.') - return False - - # The Job's corresponding Allocation should have a start date. - allocation_start_date = allocation.start_date - if not isinstance(allocation_start_date, date): - logger.error( - f'Allocation {allocation.pk} (Project {account_name}) does ' - f'not have a start date.') - return False - - # The Job should not have started before its corresponding Allocation's - # start date. - job_start_dt_utc = expected_dates['startdate'] - allocation_start_dt_utc = display_time_zone_date_to_utc_datetime( - allocation_start_date) - if job_start_dt_utc < allocation_start_dt_utc: - logger.warning( - f'Job {jobslurmid} start date ' - f'({job_start_dt_utc.strftime(DATE_FORMAT)}) is before ' - f'Allocation {allocation.pk} (Project {account_name}) start ' - f'date ({allocation_start_dt_utc.strftime(DATE_FORMAT)}).') - return False - - if not end_date_expected: - return True - - # The Job's corresponding Allocation may have an end date. (Compare - # against the maximum date if not.) - computing_allowance_interface = ComputingAllowanceInterface() - periodic_project_name_prefixes = tuple([ - computing_allowance_interface.code_from_name(allowance.name) - for allowance in computing_allowance_interface.allowances() - if ComputingAllowance(allowance).is_periodic()]) - if account_name.startswith(periodic_project_name_prefixes): - allocation_end_date = allocation.end_date - if not isinstance(allocation_end_date, date): - logger.error( - f'Allocation {allocation.pk} (Project {account_name}) ' - f'does not have an end date.') - return False - allocation_end_dt_utc = ( - display_time_zone_date_to_utc_datetime(allocation_end_date) + - timedelta(hours=24) - - timedelta(microseconds=1)) - else: - allocation_end_dt_utc = datetime.max.replace(tzinfo=pytz.utc) - - # The Job should not have ended after the last microsecond of its - # corresponding Allocation's end date. - job_end_dt_utc = expected_dates['enddate'] - if job_end_dt_utc > allocation_end_dt_utc: - logger.warning( - f'Job {jobslurmid} end date ' - f'({job_end_dt_utc.strftime(DATE_FORMAT)}) is after ' - f'Allocation {allocation.pk} (Project {account_name}) end ' - f'date ({allocation_end_dt_utc.strftime(DATE_FORMAT)}).') - return False - - return True - job_cost_parameter = openapi.Parameter( 'job_cost', diff --git a/coldfront/config/local_settings.py.sample b/coldfront/config/local_settings.py.sample index d5164bebb..92a69c701 100644 --- a/coldfront/config/local_settings.py.sample +++ b/coldfront/config/local_settings.py.sample @@ -8,7 +8,9 @@ configure logging, and ColdFront plugins. # Secret Key -- Generate new key using: https://www.miniwebtool.com/django-secret-key-generator/ # and replace #------------------------------------------------------------------------------ -SECRET_KEY = 'vtri&lztlbinerr4+yg1yzm23ez@+ub6=4*63z1%d!)fg(g4x$' # REPLACE +# Instead of defining this here, it is imported from the deployment-specific +# settings file below. +# SECRET_KEY = 'vtri&lztlbinerr4+yg1yzm23ez@+ub6=4*63z1%d!)fg(g4x$' # REPLACE #------------------------------------------------------------------------------ # Enable debugging. WARNING: These should be set to False in production @@ -16,9 +18,9 @@ SECRET_KEY = 'vtri&lztlbinerr4+yg1yzm23ez@+ub6=4*63z1%d!)fg(g4x$' # REPLACE DEBUG = True DEVELOP = True -if DEBUG is False and SECRET_KEY == 'vtri&lztlbinerr4+yg1yzm23ez@+ub6=4*63z1%d!)fg(g4x$': - from django.core.exceptions import ImproperlyConfigured - raise ImproperlyConfigured("The SECRET_KEY setting is using the preset value. Please update it!") +# if DEBUG is False and SECRET_KEY == 'vtri&lztlbinerr4+yg1yzm23ez@+ub6=4*63z1%d!)fg(g4x$': +# from django.core.exceptions import ImproperlyConfigured +# raise ImproperlyConfigured("The SECRET_KEY setting is using the preset value. Please update it!") #------------------------------------------------------------------------------ # Session settings @@ -438,6 +440,17 @@ SOCIALACCOUNT_PROVIDERS = { # Always request the 'email' scope. SOCIALACCOUNT_QUERY_EMAIL = True +#------------------------------------------------------------------------------ +# Django Sesame settings +#------------------------------------------------------------------------------ + +EXTRA_AUTHENTICATION_BACKENDS += [ + 'coldfront.core.user.auth.SesameBackend', +] + +# The number of seconds a login token is valid for. +SESAME_MAX_AGE = 300 + #------------------------------------------------------------------------------ # Data import settings #------------------------------------------------------------------------------ diff --git a/coldfront/config/test_settings.py.sample b/coldfront/config/test_settings.py.sample index 4d92b5511..52a8dfc88 100644 --- a/coldfront/config/test_settings.py.sample +++ b/coldfront/config/test_settings.py.sample @@ -3,6 +3,8 @@ import os # SECURITY WARNING: don't run with debug turned on in production! DEBUG = True +SECRET_KEY = os.environ.get('django_secret_key', '') + ALLOWED_HOSTS = ['0.0.0.0', 'localhost', 'localhost'] PORTAL_NAME = 'MyBRC' @@ -88,7 +90,9 @@ FLAGS = { ], 'BASIC_AUTH_ENABLED': [{'condition': 'boolean', 'value': True}], 'BRC_ONLY': [{'condition': 'boolean', 'value': True}], + 'LINK_LOGIN_ENABLED': [{'condition': 'boolean', 'value': False}], 'LRC_ONLY': [{'condition': 'boolean', 'value': False}], + 'MULTIPLE_EMAIL_ADDRESSES_ALLOWED': [{'condition': 'boolean', 'value': True}], 'SECURE_DIRS_REQUESTABLE': [{'condition': 'boolean', 'value': True}], 'SERVICE_UNITS_PURCHASABLE': [{'condition': 'boolean', 'value': True}], 'SSO_ENABLED': [{'condition': 'boolean', 'value': False}], diff --git a/coldfront/core/account/admin.py b/coldfront/core/account/admin.py new file mode 100644 index 000000000..c68a52a75 --- /dev/null +++ b/coldfront/core/account/admin.py @@ -0,0 +1,83 @@ +from django.contrib import admin +from django.contrib import messages +from django.core.exceptions import ValidationError + +from allauth.account.models import EmailAddress + +from coldfront.core.account.utils.queries import update_user_primary_email_address + +import logging + + +logger = logging.getLogger(__name__) + + +admin.site.unregister(EmailAddress) + + +@admin.register(EmailAddress) +class EmailAddressAdmin(admin.ModelAdmin): + + actions = ('make_primary', 'make_verified', ) + readonly_fields = ('primary', 'verified', ) + + def delete_model(self, request, obj): + if obj.primary: + raise ValidationError( + 'Cannot delete primary email. Unset primary status in list ' + 'display before deleting.') + else: + super().delete_model(request, obj) + + @admin.action(description='Make selected primary') + def make_primary(self, request, queryset): + """Set the EmailAddresses in the given queryset as the primary + addresses of their respective users. + + Currently, admins are limited to setting at most one address at + a time.""" + if queryset.count() > 1: + raise ValidationError( + 'Cannot set more than one primary email address at a time.') + for email_address in queryset: + user = email_address.user + try: + update_user_primary_email_address(email_address) + except ValueError: + raise ValidationError( + 'Cannot set an unverified email address as primary.') + except Exception as e: + message = ( + f'Encountered unexpected exception when updating User ' + f'{user.pk}\'s primary EmailAddress to ' + f'{email_address.pk}. Details:') + logger.error(message) + logger.exception(e) + raise ValidationError( + f'Failed to set {email_address.pk} as primary. See the ' + f'log for details.') + else: + message = ( + f'Set User {user.pk}\'s primary EmailAddress to ' + f'{email_address.email}.') + messages.success(request, message) + + def delete_queryset(self, request, queryset): + """Delete EmailAddresses in the given queryset, skipping those + that are primary.""" + num_primary, num_non_primary = 0, 0 + for email_address in queryset: + if email_address.primary: + num_primary = num_primary + 1 + else: + email_address.delete() + num_non_primary = num_non_primary + 1 + + success_message = ( + f'Deleted {num_non_primary} non-primary EmailAddresses.') + messages.success(request, success_message) + + if num_primary > 0: + error_message = ( + f'Skipped deleting {num_primary} primary EmailAddresses.') + messages.error(request, error_message) \ No newline at end of file diff --git a/coldfront/core/user/tests/test_models/__init__.py b/coldfront/core/account/tests/__init__.py similarity index 100% rename from coldfront/core/user/tests/test_models/__init__.py rename to coldfront/core/account/tests/__init__.py diff --git a/coldfront/core/account/tests/test_admin/__init__.py b/coldfront/core/account/tests/test_admin/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/coldfront/core/user/tests/test_admin/test_email_address_admin.py b/coldfront/core/account/tests/test_admin/test_email_address_admin.py similarity index 90% rename from coldfront/core/user/tests/test_admin/test_email_address_admin.py rename to coldfront/core/account/tests/test_admin/test_email_address_admin.py index 39ab635f6..1662a7da3 100644 --- a/coldfront/core/user/tests/test_admin/test_email_address_admin.py +++ b/coldfront/core/account/tests/test_admin/test_email_address_admin.py @@ -1,15 +1,17 @@ -from django.core.exceptions import ValidationError -from django.contrib.auth.models import User from django.contrib.admin.sites import AdminSite -from coldfront.core.user.models import EmailAddress -from coldfront.core.user.admin import EmailAddressAdmin -from coldfront.core.user.tests.utils import TestUserBase -from django.http import HttpRequest -from django.contrib.messages.storage import default_storage from django.contrib.messages import get_messages +from django.contrib.messages.storage import default_storage +from django.contrib.auth.models import User +from django.core.exceptions import ValidationError +from django.http import HttpRequest + +from allauth.account.models import EmailAddress + +from coldfront.core.account.admin import EmailAddressAdmin +from coldfront.core.user.tests.utils import TestUserBase -class EmailAddressAdminTest(TestUserBase): +class TestEmailAddressAdmin(TestUserBase): """ Class for testing methods in EmailAddressAdmin """ @@ -35,32 +37,32 @@ def setUp(self): self.email1 = EmailAddress.objects.create( user=self.user1, email='email1@email.com', - is_verified=True, - is_primary=True) + verified=True, + primary=True) self.email2 = EmailAddress.objects.create( user=self.user1, email='email2@email.com', - is_verified=True, - is_primary=False) + verified=True, + primary=False) self.email3 = EmailAddress.objects.create( user=self.user2, email='email3@email.com', - is_verified=True, - is_primary=True) + verified=True, + primary=True) self.email4 = EmailAddress.objects.create( user=self.user2, email='email4@email.com', - is_verified=True, - is_primary=False) + verified=True, + primary=False) self.email5 = EmailAddress.objects.create( user=self.user2, email='email5@email.com', - is_verified=False, - is_primary=False) + verified=False, + primary=False) self.request = HttpRequest() setattr(self.request, 'session', 'session') @@ -121,7 +123,7 @@ def test_make_primary_single_primary_email(self): query_set = EmailAddress.objects.filter(pk=self.email1.pk) self.app_admin.make_primary(self.request, query_set) self.email1.refresh_from_db() - self.assertTrue(self.email1.is_primary) + self.assertTrue(self.email1.primary) storage = list(get_messages(self.request)) self.assertEqual(len(storage), 1) @@ -136,10 +138,10 @@ def test_make_primary_single_non_primary_email(self): query_set = EmailAddress.objects.filter(pk=self.email2.pk) self.app_admin.make_primary(self.request, query_set) self.email1.refresh_from_db() - self.assertFalse(self.email1.is_primary) + self.assertFalse(self.email1.primary) self.email2.refresh_from_db() - self.assertTrue(self.email2.is_primary) + self.assertTrue(self.email2.primary) storage = list(get_messages(self.request)) self.assertEqual(len(storage), 1) @@ -152,7 +154,7 @@ def test_make_delete_queryset_only_primary(self): Testing EmailAddressAdmin delete_queryset method with only primary emails """ - query_set = EmailAddress.objects.filter(is_primary=True) + query_set = EmailAddress.objects.filter(primary=True) self.app_admin.delete_queryset(self.request, query_set) self.assertTrue(EmailAddress.objects.filter(pk=self.email1.pk).exists()) @@ -171,7 +173,7 @@ def test_make_delete_queryset_only_non_primary(self): primary emails """ - query_set = EmailAddress.objects.filter(is_primary=False) + query_set = EmailAddress.objects.filter(primary=False) self.app_admin.delete_queryset(self.request, query_set) self.assertFalse(EmailAddress.objects.filter(pk=self.email2.pk).exists()) diff --git a/coldfront/core/account/tests/test_utils/__init__.py b/coldfront/core/account/tests/test_utils/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/coldfront/core/account/tests/test_utils/test_queries/__init__.py b/coldfront/core/account/tests/test_utils/test_queries/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/coldfront/core/user/tests/test_utils/test_update_user_primary_email_address.py b/coldfront/core/account/tests/test_utils/test_queries/test_update_user_primary_email_address.py similarity index 79% rename from coldfront/core/user/tests/test_utils/test_update_user_primary_email_address.py rename to coldfront/core/account/tests/test_utils/test_queries/test_update_user_primary_email_address.py index 82c35d490..e1a3240ae 100644 --- a/coldfront/core/user/tests/test_utils/test_update_user_primary_email_address.py +++ b/coldfront/core/account/tests/test_utils/test_queries/test_update_user_primary_email_address.py @@ -1,10 +1,12 @@ -from coldfront.core.user.models import EmailAddress -from coldfront.core.user.tests.utils import TestUserBase -from coldfront.core.user.utils import update_user_primary_email_address from django.contrib.auth.models import User +from allauth.account.models import EmailAddress -class TestUpdateUserPrimaryEmailAddress(TestUserBase): +from coldfront.core.account.utils.queries import update_user_primary_email_address +from coldfront.core.utils.tests.test_base import TestBase + + +class TestUpdateUserPrimaryEmailAddress(TestBase): """A class for testing the utility method update_user_primary_email_address.""" @@ -26,8 +28,8 @@ def test_creates_email_address_for_old_user_email_if_nonexistent(self): new_primary = EmailAddress.objects.create( user=self.user, email='new@email.com', - is_verified=True, - is_primary=False) + verified=True, + primary=False) self.assertEqual(EmailAddress.objects.count(), 1) with self.assertLogs('', 'WARNING') as cm: @@ -41,8 +43,8 @@ def test_creates_email_address_for_old_user_email_if_nonexistent(self): f'An EmailAddress for User {self.user} and email {old_email} ' f'should have been created.') else: - self.assertTrue(email_address.is_verified) - self.assertFalse(email_address.is_primary) + self.assertTrue(email_address.verified) + self.assertFalse(email_address.primary) # Assert that a warning was logged. self.assertEqual(len(cm.output), 1) @@ -65,13 +67,13 @@ def test_raises_type_error_for_bad_input(self): self.fail('A TypeError should have been raised.') def test_raises_value_error_for_unverified_input(self): - """Test that, if the input is an EmailAddress with is_verified + """Test that, if the input is an EmailAddress with verified set to False, a ValueError is raised.""" email_address = EmailAddress.objects.create( user=self.user, email=self.user.email, - is_verified=False, - is_primary=False) + verified=False, + primary=False) try: update_user_primary_email_address(email_address) except ValueError as e: @@ -86,14 +88,14 @@ def test_sets_email_address_to_primary(self): new_primary = EmailAddress.objects.create( user=self.user, email='new@email.com', - is_verified=True, - is_primary=False) - self.assertFalse(new_primary.is_primary) + verified=True, + primary=False) + self.assertFalse(new_primary.primary) update_user_primary_email_address(new_primary) new_primary.refresh_from_db() - self.assertTrue(new_primary.is_primary) + self.assertTrue(new_primary.primary) def test_sets_user_email_field(self): """Test that the method sets the User's "email" field to that of @@ -103,8 +105,8 @@ def test_sets_user_email_field(self): new_primary = EmailAddress.objects.create( user=self.user, email='new@email.com', - is_verified=True, - is_primary=False) + verified=True, + primary=False) update_user_primary_email_address(new_primary) @@ -112,32 +114,32 @@ def test_sets_user_email_field(self): self.assertEqual(self.user.email, new_primary.email) def test_unsets_other_primary_email_addresses(self): - """Test that the method unsets the "is_primary" fields of other + """Test that the method unsets the "primary" fields of other EmailAddresses belonging to the User.""" kwargs = { 'user': self.user, - 'is_verified': True, - 'is_primary': False, + 'verified': True, + 'primary': False, } for i in range(3): kwargs['email'] = f'{i}@email.com' EmailAddress.objects.create(**kwargs) # Bypass the "save" method, which prevents multiple primary addresses, # by using the "update" method. - EmailAddress.objects.filter(user=self.user).update(is_primary=True) + EmailAddress.objects.filter(user=self.user).update(primary=True) user_primary_emails = EmailAddress.objects.filter( - user=self.user, is_primary=True) + user=self.user, primary=True) self.assertEqual(user_primary_emails.count(), 3) new_primary = EmailAddress.objects.create( user=self.user, email='new@email.com', - is_verified=True, - is_primary=False) + verified=True, + primary=False) update_user_primary_email_address(new_primary) user_primary_emails = EmailAddress.objects.filter( - user=self.user, is_primary=True) + user=self.user, primary=True) self.assertEqual(user_primary_emails.count(), 1) self.assertEqual(user_primary_emails.first().pk, new_primary.pk) diff --git a/coldfront/core/account/urls.py b/coldfront/core/account/urls.py index d9aa69d70..6474ba91e 100644 --- a/coldfront/core/account/urls.py +++ b/coldfront/core/account/urls.py @@ -1,15 +1,32 @@ from allauth.account.urls import urlpatterns as all_patterns +from flags.urls import flagged_paths + """Include a subset of patterns from allauth.account.""" urlpatterns = [] + + names_to_include = { 'account_inactive', - 'account_email', 'account_email_verification_sent', 'account_confirm_email', } for pattern in all_patterns: if pattern.name in names_to_include: urlpatterns.append(pattern) + + +# Only include the view for managing emails if users are allowed to have +# multiple emails. +names_to_include_if_multiple_emails_allowed = { + 'account_email', +} +with flagged_paths('MULTIPLE_EMAIL_ADDRESSES_ALLOWED') as f_path: + for pattern in all_patterns: + if pattern.name in names_to_include_if_multiple_emails_allowed: + urlpatterns.append( + f_path( + str(pattern.pattern), pattern.callback, + pattern.default_args, pattern.name)) diff --git a/coldfront/core/account/utils/__init__.py b/coldfront/core/account/utils/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/coldfront/core/account/utils/login_activity.py b/coldfront/core/account/utils/login_activity.py new file mode 100644 index 000000000..c33c376a1 --- /dev/null +++ b/coldfront/core/account/utils/login_activity.py @@ -0,0 +1,98 @@ +import pytz + +from django.conf import settings +from django.http.request import HttpRequest +from django.urls import reverse + +from allauth.account.models import EmailAddress +from allauth.account.models import EmailConfirmationHMAC +from user_agents import parse + +from coldfront.core.utils.common import build_absolute_url +from coldfront.core.utils.common import import_from_settings +from coldfront.core.utils.common import utc_now_offset_aware +from coldfront.core.utils.mail import send_email_template + + +class LoginActivityVerifier(object): + """A class that can email an unverified EmailAddress, notifying the + user that a blocked login attempt was made using it and asking the + user to confirm that the attempt was made by them, which will verify + the address.""" + + def __init__(self, request, email_address, request_login_method_str): + """Validate arguments. + + Parameters + - request (HttpRequest): the request object for the login + - email_address (EmailAddress): an unverified EmailAddress + instance + - request_login_method_str (str): A str describing the + method that was used to attempt to log in (e.g., + 'CILogon - Lawrence Berkeley National Laboratory' or + 'Login Link Request') + """ + assert isinstance(request, HttpRequest) + assert isinstance(email_address, EmailAddress) + assert not email_address.verified + assert isinstance(request_login_method_str, str) + + self._request = request + self._email_address = email_address + self._request_time_str = self.get_display_timezone_now_str() + self._request_user_agent_str = self.get_request_user_agent_str() + self._request_login_method_str = request_login_method_str + + @staticmethod + def get_display_timezone_now_str(): + """Return a time string for the current time in the display time + zone (e.g., 'January 1, 1970 at 12:00 PM (UTC)').""" + display_time_zone = pytz.timezone(settings.DISPLAY_TIME_ZONE) + utc_now = utc_now_offset_aware() + display_format = '%B %-d, %Y at %-I:%M %p (%Z)' + return display_time_zone.normalize(utc_now).strftime(display_format) + + def get_request_user_agent_str(self): + """Given an HTTP request, return a str describing the user agent + that made the request (e.g., 'Chrome on Mac OS X').""" + user_agent_header = self._request.headers.get('user-agent', '') + if not user_agent_header: + return 'Unknown Browser and OS' + user_agent = parse(user_agent_header) + return f'{user_agent.browser.family} on {user_agent.os.family}' + + def login_activity_verification_url(self): + """Return an absolute URL to the view for verifying the + EmailAddress. + + This is adapted from allauth.account.adapter. + DefaultAccountAdapter.get_email_confirmation_url.""" + hmac = EmailConfirmationHMAC(self._email_address) + return build_absolute_url( + reverse('account_confirm_email', args=[hmac.key])) + + def send_email(self): + """Email the user, asking them to confirm the login attempt by + clicking on a link, which will verify the address.""" + email_enabled = import_from_settings('EMAIL_ENABLED', False) + if not email_enabled: + return + + subject = 'Verify Login Activity' + template_name = 'email/login/verify_login_activity.txt' + context = { + 'PORTAL_NAME': settings.PORTAL_NAME, + 'email_address': self._email_address.email, + 'request_time_str': self._request_time_str, + 'request_user_agent_str': self._request_user_agent_str, + 'request_login_method_str': self._request_login_method_str, + 'support_email': settings.CENTER_HELP_EMAIL, + 'verification_url': self.login_activity_verification_url(), + 'signature': import_from_settings('EMAIL_SIGNATURE', ''), + } + + sender = settings.EMAIL_SENDER + receiver_list = [self._email_address.email] + + send_email_template( + subject, template_name, context, sender, receiver_list) diff --git a/coldfront/core/account/utils/queries.py b/coldfront/core/account/utils/queries.py new file mode 100644 index 000000000..7fe087e0d --- /dev/null +++ b/coldfront/core/account/utils/queries.py @@ -0,0 +1,64 @@ +from django.db import transaction + +from allauth.account.models import EmailAddress + +import logging + + +logger = logging.getLogger(__name__) + + +def update_user_primary_email_address(email_address): + """Given an EmailAddress, which must be verified, perform the + following: + - If the user's current email field does not have a + corresponding EmailAddress, create one (verified); + - Set the user's email field to it; + - Set it as the primary EmailAddress of the user; and + - Set the user's other EmailAddress objects to be non-primary. + + Perform the updates in a transaction so that they all fail together + or all succeed together. + + Parameters: + - email_address (EmailAddress): the EmailAddress object to set + as the new primary + + Returns: + - None + + Raises: + - TypeError, if the provided address has an invalid type + - ValueError, if the provided address is not verified + """ + if not isinstance(email_address, EmailAddress): + raise TypeError(f'Invalid EmailAddress {email_address}.') + if not email_address.verified: + raise ValueError(f'EmailAddress {email_address} is unverified.') + + user = email_address.user + with transaction.atomic(): + + old_primary, created = EmailAddress.objects.get_or_create( + user=user, email=user.email.lower()) + if created: + message = ( + f'Created EmailAddress {old_primary.pk} for User {user.pk}\'s ' + f'old primary address {old_primary.email}, which unexpectedly ' + f'did not exist.') + logger.warning(message) + old_primary.verified = True + old_primary.primary = False + old_primary.save() + + # TODO: Hide behind feature flag? This seems relevant no matter what. + for ea in EmailAddress.objects.filter( + user=user, primary=True).exclude(pk=email_address.pk): + ea.primary = False + ea.save() + + user.email = email_address.email + user.save() + + email_address.primary = True + email_address.save() diff --git a/coldfront/core/allocation/management/commands/load_allocation_renewal_requests.py b/coldfront/core/allocation/management/commands/load_allocation_renewal_requests.py index cf233f6c1..ecc7598d8 100644 --- a/coldfront/core/allocation/management/commands/load_allocation_renewal_requests.py +++ b/coldfront/core/allocation/management/commands/load_allocation_renewal_requests.py @@ -9,7 +9,8 @@ from django.core.management.base import BaseCommand from django.core.management.base import CommandError from django.db import transaction -from django.utils.module_loading import import_string + +from allauth.account.models import EmailAddress from coldfront.core.allocation.models import AllocationPeriod from coldfront.core.allocation.models import AllocationRenewalRequest @@ -40,10 +41,6 @@ class Command(BaseCommand): logger = logging.getLogger(__name__) - def __init__(self, *args, **kwargs): - super().__init__(*args, *kwargs) - self.email_module_dict = {} - def add_arguments(self, parser): parser.add_argument( 'json', @@ -58,15 +55,6 @@ def add_arguments(self, parser): 'allocation_period_name', help='The name of the AllocationPeriod the renewals are under.', type=str) - # TODO: Remove this once all emails are transitioned to - # TODO: allauth.account.models.EmailAddress. - parser.add_argument( - 'email_module', - choices=['allauth.account.models', 'coldfront.core.user.models'], - help=( - 'There are temporarily two EmailAddress models, until all can ' - 'be transitioned under allauth.account.models.'), - type=str) parser.add_argument( '--process', action='store_true', @@ -92,16 +80,6 @@ def handle(self, *args, **options): raise CommandError( f'Invalid AllocationPeriod {allocation_period_name}.') - email_module = options['email_module'] - if email_module == 'allauth.account.models': - verified_field, primary_field = 'verified', 'primary' - else: - verified_field, primary_field = 'is_verified', 'is_primary' - self.email_module_dict['model'] = import_string( - f'{email_module}.EmailAddress') - self.email_module_dict['verified_field'] = verified_field - self.email_module_dict['primary_field'] = primary_field - valid, already_renewed, invalid = self.parse_input_file( file_path, allocation_period) self.process_valid_objects( @@ -347,10 +325,6 @@ def _update_or_create_user_and_email_address(self, email, first_name='', email. If provided (and not already set), set the given first, middle, and last names. Also update UserProfile.is_pi if requested.""" - EmailAddress = self.email_module_dict['model'] - email_verified_field = self.email_module_dict['verified_field'] - email_primary_field = self.email_module_dict['primary_field'] - user = self._get_user_with_email(email) if isinstance(user, User): user.first_name = user.first_name or first_name @@ -368,8 +342,8 @@ def _update_or_create_user_and_email_address(self, email, first_name='', user=user, email=email, defaults={ - email_verified_field: True, - email_primary_field: True}) + 'verified': True, + 'primary': True}) if created: message = ( f'Created EmailAddress {email_address.pk} for User ' @@ -390,8 +364,8 @@ def _update_or_create_user_and_email_address(self, email, first_name='', kwargs = { 'user': user, 'email': email, - email_verified_field: True, - email_primary_field: True, + 'verified': True, + 'primary': True, } email_address = EmailAddress.objects.create(**kwargs) message = ( @@ -418,7 +392,6 @@ def _get_first_middle_last_names(full_name): def _get_user_with_email(self, email): """Return the User associated with the given email, or None.""" - EmailAddress = self.email_module_dict['model'] try: return User.objects.get(email__iexact=email) except User.DoesNotExist: diff --git a/coldfront/core/allocation/templates/allocation/allocation_cluster_account_request_list_table.html b/coldfront/core/allocation/templates/allocation/allocation_cluster_account_request_list_table.html index de0017a7d..5328157d7 100644 --- a/coldfront/core/allocation/templates/allocation/allocation_cluster_account_request_list_table.html +++ b/coldfront/core/allocation/templates/allocation/allocation_cluster_account_request_list_table.html @@ -4,34 +4,28 @@ # - - + {% include 'common/table_sorter.html' with table_sorter_field='id' %} - {% if request_filter == 'completed' %} - Completion Time - - - {% else %} + {% if request_filter == 'pending' or adj == 'pending' %} Request Time - - + {% include 'common/table_sorter.html' with table_sorter_field='request_time' %} + {% else %} + Completion Time + {% include 'common/table_sorter.html' with table_sorter_field='completion_time' %} {% endif %} User Email - - + {% include 'common/table_sorter.html' with table_sorter_field='allocation_user__user__email' %} Cluster Username - - + {% include 'common/table_sorter.html' with table_sorter_field='allocation_user__user__username' %} Project - - + {% include 'common/table_sorter.html' with table_sorter_field='allocation_user__allocation__project__name' %} Allocation diff --git a/coldfront/core/allocation/templates/secure_dir/secure_dir_manage_user_request_list_table.html b/coldfront/core/allocation/templates/secure_dir/secure_dir_manage_user_request_list_table.html index 366075a51..3e20d1508 100644 --- a/coldfront/core/allocation/templates/secure_dir/secure_dir_manage_user_request_list_table.html +++ b/coldfront/core/allocation/templates/secure_dir/secure_dir_manage_user_request_list_table.html @@ -3,8 +3,7 @@ # - - + {% include 'common/table_sorter.html' with table_sorter_field='id' %} {% if request_filter == 'pending' or adj == 'pending' %} @@ -12,8 +11,7 @@ {% else %} Date Completed {% endif %} - - + {% include 'common/table_sorter.html' with table_sorter_field='created' %} User diff --git a/coldfront/core/allocation/templates/secure_dir/secure_dir_request/secure_dir_request_list_table.html b/coldfront/core/allocation/templates/secure_dir/secure_dir_request/secure_dir_request_list_table.html index 44d7f543a..d6c694152 100644 --- a/coldfront/core/allocation/templates/secure_dir/secure_dir_request/secure_dir_request_list_table.html +++ b/coldfront/core/allocation/templates/secure_dir/secure_dir_request/secure_dir_request_list_table.html @@ -3,23 +3,19 @@ # - - + {% include 'common/table_sorter.html' with table_sorter_field='id' %} Requested - - + {% include 'common/table_sorter.html' with table_sorter_field='request_time' %} Requester - - + {% include 'common/table_sorter.html' with table_sorter_field='requester__email' %} Project - - + {% include 'common/table_sorter.html' with table_sorter_field='project' %} Status diff --git a/coldfront/core/allocation/views_/secure_dir_views.py b/coldfront/core/allocation/views_/secure_dir_views.py index f53e07374..9ee0e7cea 100644 --- a/coldfront/core/allocation/views_/secure_dir_views.py +++ b/coldfront/core/allocation/views_/secure_dir_views.py @@ -353,7 +353,7 @@ def get_queryset(self): direction = '-' order_by = direction + order_by else: - order_by = 'id' + order_by = '-modified' pending_status = self.request_status_obj.objects.filter( Q(name__icontains='Pending') | Q(name__icontains='Processing')) @@ -1091,7 +1091,7 @@ def get_queryset(self): direction = '-' order_by = direction + order_by else: - order_by = 'id' + order_by = '-modified' return SecureDirRequest.objects.order_by(order_by) diff --git a/coldfront/core/portal/templates/portal/nonauthorized_home.html b/coldfront/core/portal/templates/portal/nonauthorized_home.html index dbea8e85a..c0835f63f 100644 --- a/coldfront/core/portal/templates/portal/nonauthorized_home.html +++ b/coldfront/core/portal/templates/portal/nonauthorized_home.html @@ -38,7 +38,12 @@

Welcome to {{ PORTAL_NAME }}

{% if sso_enabled %} + {% flag_enabled 'BRC_ONLY' as brc_only %} + {% if brc_only %} + CalNet: Log In + {% else %} Berkeley Lab: Log In + {% endif %} diff --git a/coldfront/core/project/forms_/new_project_forms/request_forms.py b/coldfront/core/project/forms_/new_project_forms/request_forms.py index c18198347..bf767125f 100644 --- a/coldfront/core/project/forms_/new_project_forms/request_forms.py +++ b/coldfront/core/project/forms_/new_project_forms/request_forms.py @@ -216,9 +216,9 @@ def disable_pi_choices(self): def exclude_pi_choices(self): """Exclude certain Users from being displayed as PI options.""" - # Exclude any user that does not have an email address. + # Exclude any user that does not have an email address or is inactive. self.fields['PI'].queryset = User.objects.exclude( - Q(email__isnull=True) | Q(email__exact='')) + Q(email__isnull=True) | Q(email__exact='') | Q(is_active=False)) class SavioProjectNewPIForm(forms.Form): diff --git a/coldfront/core/project/templates/project/project_allocation_addition/request_list_table.html b/coldfront/core/project/templates/project/project_allocation_addition/request_list_table.html index 3a1fae7d9..b6538d302 100644 --- a/coldfront/core/project/templates/project/project_allocation_addition/request_list_table.html +++ b/coldfront/core/project/templates/project/project_allocation_addition/request_list_table.html @@ -3,39 +3,19 @@ # - - - - - - + {% include 'common/table_sorter.html' with table_sorter_field='id' %} Date Requested/
Last Modified - - - - - - + {% include 'common/table_sorter.html' with table_sorter_field='modified' %} Requester Email - - - - - - + {% include 'common/table_sorter.html' with table_sorter_field='requester__email' %} Project - - - - - - + {% include 'common/table_sorter.html' with table_sorter_field='project' %} Number of Service Units Status diff --git a/coldfront/core/project/templates/project/project_detail.html b/coldfront/core/project/templates/project/project_detail.html index 64c40ba58..80f214a89 100644 --- a/coldfront/core/project/templates/project/project_detail.html +++ b/coldfront/core/project/templates/project/project_detail.html @@ -132,6 +132,15 @@

Requests to Join: User requests to join the project must be approved by a PI.

+ + + {% if survey_answers %} +

+ Survey Responses: + +

+ {% endif %} + @@ -460,4 +469,5 @@

+{% include 'project/project_request/savio/project_request_surveys_modal.html' with survey_answers=survey_answers %} {% endblock %} diff --git a/coldfront/core/project/templates/project/project_join_request_list_table.html b/coldfront/core/project/templates/project/project_join_request_list_table.html index 20fcb94e3..28293a9b3 100644 --- a/coldfront/core/project/templates/project/project_join_request_list_table.html +++ b/coldfront/core/project/templates/project/project_join_request_list_table.html @@ -6,17 +6,19 @@ Username + {% include 'common/table_sorter.html' with table_sorter_field='project_user__user__username' %} User Email + {% include 'common/table_sorter.html' with table_sorter_field='project_user__user__email' %} Project + {% include 'common/table_sorter.html' with table_sorter_field='project_user__project' %} Date Requested - - + {% include 'common/table_sorter.html' with table_sorter_field='created' %} Reason diff --git a/coldfront/core/project/templates/project/project_removal/project_removal_request_list_table.html b/coldfront/core/project/templates/project/project_removal/project_removal_request_list_table.html index 909ed5808..de61baf87 100644 --- a/coldfront/core/project/templates/project/project_removal/project_removal_request_list_table.html +++ b/coldfront/core/project/templates/project/project_removal/project_removal_request_list_table.html @@ -3,37 +3,32 @@ # - - + {% include 'common/table_sorter.html' with table_sorter_field='id' %} {% if request_filter == 'pending' or adj == 'pending' %} Date Requested + {% include 'common/table_sorter.html' with table_sorter_field='request_time' %} {% else %} Date Completed + {% include 'common/table_sorter.html' with table_sorter_field='completion_time' %} {% endif %} - - User Email - - + {% include 'common/table_sorter.html' with table_sorter_field='project_user__user__email' %} User - - + {% include 'common/table_sorter.html' with table_sorter_field='project_user__user__username' %} Requester - - + {% include 'common/table_sorter.html' with table_sorter_field='requester__username' %} Project - - + {% include 'common/table_sorter.html' with table_sorter_field='project_user__project__name' %} Status diff --git a/coldfront/core/project/templates/project/project_renewal/project_renewal_request_list_table.html b/coldfront/core/project/templates/project/project_renewal/project_renewal_request_list_table.html index a13811d24..7045cf82e 100644 --- a/coldfront/core/project/templates/project/project_renewal/project_renewal_request_list_table.html +++ b/coldfront/core/project/templates/project/project_renewal/project_renewal_request_list_table.html @@ -3,48 +3,23 @@ # - - - - - - + {% include 'common/table_sorter.html' with table_sorter_field='id' %} Requested - - - - - - + {% include 'common/table_sorter.html' with table_sorter_field='request_time' %} Requester - - - - - - + {% include 'common/table_sorter.html' with table_sorter_field='requester__email' %} Project - - - - - - + {% include 'common/table_sorter.html' with table_sorter_field='post_project__name' %} PI - - - - - - + {% include 'common/table_sorter.html' with table_sorter_field='pi__email' %} Status diff --git a/coldfront/core/project/templates/project/project_request/savio/project_request_list_table.html b/coldfront/core/project/templates/project/project_request/savio/project_request_list_table.html index 3291afe1b..771816cec 100644 --- a/coldfront/core/project/templates/project/project_request/savio/project_request_list_table.html +++ b/coldfront/core/project/templates/project/project_request/savio/project_request_list_table.html @@ -3,28 +3,23 @@ # - - + {% include 'common/table_sorter.html' with table_sorter_field='id' %} Requested - - + {% include 'common/table_sorter.html' with table_sorter_field='request_time' %} Requester - - + {% include 'common/table_sorter.html' with table_sorter_field='requester__email' %} Project - - + {% include 'common/table_sorter.html' with table_sorter_field='project' %} PI - - + {% include 'common/table_sorter.html' with table_sorter_field='pi__email' %} Status diff --git a/coldfront/core/project/templates/project/project_request/savio/project_request_surveys_modal.html b/coldfront/core/project/templates/project/project_request/savio/project_request_surveys_modal.html new file mode 100644 index 000000000..66d020416 --- /dev/null +++ b/coldfront/core/project/templates/project/project_request/savio/project_request_surveys_modal.html @@ -0,0 +1,77 @@ +{% load crispy_forms_tags %} + + + + diff --git a/coldfront/core/project/templates/project/project_request/vector/project_request_list_table.html b/coldfront/core/project/templates/project/project_request/vector/project_request_list_table.html index 7f9ce2da7..37cf6ca8a 100644 --- a/coldfront/core/project/templates/project/project_request/vector/project_request_list_table.html +++ b/coldfront/core/project/templates/project/project_request/vector/project_request_list_table.html @@ -3,28 +3,23 @@ # - - + {% include 'common/table_sorter.html' with table_sorter_field='id' %} Date Requested/
Last Modified - - + {% include 'common/table_sorter.html' with table_sorter_field='modified' %} Requester Email - - + {% include 'common/table_sorter.html' with table_sorter_field='requester__email' %} Project - - + {% include 'common/table_sorter.html' with table_sorter_field='project' %} PI - - + {% include 'common/table_sorter.html' with table_sorter_field='pi__email' %} Status diff --git a/coldfront/core/project/views.py b/coldfront/core/project/views.py index 09a78d50c..7f0a29852 100644 --- a/coldfront/core/project/views.py +++ b/coldfront/core/project/views.py @@ -34,12 +34,14 @@ ProjectSearchForm, ProjectUpdateForm, ProjectUserUpdateForm) +from coldfront.core.project.forms_.new_project_forms.request_forms import SavioProjectSurveyForm from coldfront.core.project.models import (Project, ProjectReview, ProjectReviewStatusChoice, ProjectStatusChoice, ProjectUser, ProjectUserRoleChoice, ProjectUserStatusChoice, - ProjectUserRemovalRequest) + ProjectUserRemovalRequest, + SavioProjectAllocationRequest) from coldfront.core.project.utils import (annotate_queryset_with_cluster_name, is_primary_cluster_project) from coldfront.core.project.utils_.addition_utils import can_project_purchase_service_units @@ -269,6 +271,24 @@ def get_context_data(self, **kwargs): context['can_request_sec_dir'] = \ pi_eligible_to_request_secure_dir(self.request.user) + # show survey responses if available + allocation_requests = SavioProjectAllocationRequest.objects.filter( + project=self.object, status__name='Approved - Complete').order_by('request_time') + + if allocation_requests.exists(): + survey_answers_list = list(map( + lambda x: SavioProjectSurveyForm( + initial=x.survey_answers, + disable_fields=True + ), + allocation_requests + )) + + context['survey_answers'] = list(zip( + allocation_requests, + survey_answers_list + )) + context['user_agreement_signed'] = \ access_agreement_signed(self.request.user) diff --git a/coldfront/core/project/views_/addition_views/approval_views.py b/coldfront/core/project/views_/addition_views/approval_views.py index 8eaf4da98..8fdaaebc6 100644 --- a/coldfront/core/project/views_/addition_views/approval_views.py +++ b/coldfront/core/project/views_/addition_views/approval_views.py @@ -240,7 +240,7 @@ def get_order_by(self): direction = '-' order_by = direction + order_by else: - order_by = 'id' + order_by = '-modified' return order_by def test_func(self): diff --git a/coldfront/core/project/views_/join_views/approval_views.py b/coldfront/core/project/views_/join_views/approval_views.py index 0e1f25a96..89bd92935 100644 --- a/coldfront/core/project/views_/join_views/approval_views.py +++ b/coldfront/core/project/views_/join_views/approval_views.py @@ -244,9 +244,9 @@ def get_queryset(self): direction = '' else: direction = '-' - order_by = direction + 'created' + order_by = direction + order_by else: - order_by = '-created' + order_by = '-modified' project_join_requests = \ ProjectUserJoinRequest.objects.filter( diff --git a/coldfront/core/project/views_/new_project_views/approval_views.py b/coldfront/core/project/views_/new_project_views/approval_views.py index 9d42339bb..0b5d1b5c0 100644 --- a/coldfront/core/project/views_/new_project_views/approval_views.py +++ b/coldfront/core/project/views_/new_project_views/approval_views.py @@ -69,7 +69,7 @@ def get_queryset(self): direction = '-' order_by = direction + order_by else: - order_by = 'id' + order_by = '-request_time' return annotate_queryset_with_allocation_period_not_started_bool( SavioProjectAllocationRequest.objects.order_by(order_by)) @@ -910,7 +910,7 @@ def get_queryset(self): direction = '-' order_by = direction + order_by else: - order_by = 'id' + order_by = '-modified' return VectorProjectAllocationRequest.objects.order_by(order_by) def get_context_data(self, **kwargs): diff --git a/coldfront/core/project/views_/new_project_views/request_views.py b/coldfront/core/project/views_/new_project_views/request_views.py index d38fdd7bd..e4bfa52d8 100644 --- a/coldfront/core/project/views_/new_project_views/request_views.py +++ b/coldfront/core/project/views_/new_project_views/request_views.py @@ -1,3 +1,5 @@ +from allauth.account.models import EmailAddress + from coldfront.core.allocation.models import Allocation from coldfront.core.allocation.models import AllocationStatusChoice from coldfront.core.billing.forms import BillingIDValidationForm @@ -465,14 +467,14 @@ def __handle_pi_data(self, form_data): # Create a new User object intended to be a new PI. step_number = self.step_numbers_by_form_name['new_pi'] data = form_data[step_number] + email = data['email'] try: - email = data['email'] pi = User.objects.create( username=email, first_name=data['first_name'], last_name=data['last_name'], email=email, - is_active=False) + is_active=True) except IntegrityError as e: self.logger.error(f'User {email} unexpectedly exists.') raise e @@ -488,6 +490,18 @@ def __handle_pi_data(self, form_data): pi_profile.upgrade_request = utc_now_offset_aware() pi_profile.save() + # Create an unverified, primary EmailAddress for the new User object. + try: + EmailAddress.objects.create( + user=pi, + email=email, + verified=False, + primary=True) + except IntegrityError as e: + self.logger.error( + f'EmailAddress {email} unexpectedly already exists.') + raise e + return pi def __handle_recharge_allowance(self, form_data, diff --git a/coldfront/core/project/views_/removal_views.py b/coldfront/core/project/views_/removal_views.py index 79ad39184..2ef834b69 100644 --- a/coldfront/core/project/views_/removal_views.py +++ b/coldfront/core/project/views_/removal_views.py @@ -217,7 +217,7 @@ def get_queryset(self): direction = '-' order_by = direction + order_by else: - order_by = 'id' + order_by = '-modified' project_removal_status_complete, _ = \ ProjectUserRemovalRequestStatusChoice.objects.get_or_create( diff --git a/coldfront/core/project/views_/renewal_views/approval_views.py b/coldfront/core/project/views_/renewal_views/approval_views.py index c58418efc..a1231c008 100644 --- a/coldfront/core/project/views_/renewal_views/approval_views.py +++ b/coldfront/core/project/views_/renewal_views/approval_views.py @@ -55,7 +55,8 @@ def get_queryset(self): direction = '-' order_by = direction + order_by else: - order_by = 'id' + order_by = '-request_time' + return annotate_queryset_with_allocation_period_not_started_bool( AllocationRenewalRequest.objects.order_by(order_by)) diff --git a/coldfront/core/socialaccount/adapter.py b/coldfront/core/socialaccount/adapter.py index 3f1c5c200..5c2345f67 100644 --- a/coldfront/core/socialaccount/adapter.py +++ b/coldfront/core/socialaccount/adapter.py @@ -1,15 +1,21 @@ from allauth.account.models import EmailAddress -from allauth.account.utils import user_email +from allauth.account.utils import user_email as user_email_func from allauth.account.utils import user_field from allauth.account.utils import user_username from allauth.exceptions import ImmediateHttpResponse from allauth.socialaccount.adapter import DefaultSocialAccountAdapter +from allauth.socialaccount.models import SocialAccount +from allauth.socialaccount.providers.base import AuthProcess from allauth.utils import valid_email_or_none +from coldfront.core.account.utils.login_activity import LoginActivityVerifier +from coldfront.core.utils.context_processors import portal_and_program_names from collections import defaultdict from django.conf import settings from django.http import HttpResponseBadRequest from django.http import HttpResponseServerError from django.template.loader import render_to_string +from django.urls import reverse +from flags.state import flag_enabled import logging @@ -19,6 +25,11 @@ class CILogonAccountAdapter(DefaultSocialAccountAdapter): """An adapter that adjusts handling for the CILogon provider.""" + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._flag_multiple_email_addresses_allowed = flag_enabled( + 'MULTIPLE_EMAIL_ADDRESSES_ALLOWED') + def populate_user(self, request, sociallogin, data): """Handle logins using the CILogon provider differently. In particular, use the given email address as the username; raise @@ -45,18 +56,29 @@ def populate_user(self, request, sociallogin, data): f'Provider {provider} did not provide an email address ' f'for User with UID {user_uid}.') logger.error(log_message) - self.raise_server_error(self.get_auth_error_message()) + self._raise_server_error(self._get_auth_error_message()) validated_email = validated_email.lower() user = sociallogin.user user_username(user, validated_email) - user_email(user, validated_email) + user_email_func(user, validated_email) user_field(user, 'first_name', first_name) user_field(user, 'last_name', last_name) return user return super().populate_user(request, sociallogin, data) + def get_connect_redirect_url(self, request, socialaccount): + """ + Returns the default URL to redirect to after successfully + connecting a social account. + """ + if self._flag_multiple_email_addresses_allowed: + url = reverse('socialaccount_connections') + else: + url = reverse('home') + return url + def pre_social_login(self, request, sociallogin): """At this point, the user is authenticated by a provider. If the provider is not CILogon, do nothing. Otherwise, if this @@ -65,6 +87,9 @@ def pre_social_login(self, request, sociallogin): EmailAddress matching one of those given by the provider, connect the two accounts. + Note that login is blocked outside (after) this method if the + user is inactive. + Adapted from: https://github.com/pennersr/django-allauth/issues/418#issuecomment-137259550 """ @@ -86,19 +111,40 @@ def pre_social_login(self, request, sociallogin): if provider != 'cilogon': return + # If users are not allowed to have multiple emails, and the user is + # attempting to connect another SocialAccount to their account (as + # opposed to logging in with an existing one), raise an error. + if not self._flag_multiple_email_addresses_allowed: + if sociallogin.state.get('process', None) == AuthProcess.CONNECT: + message = ( + 'You may not connect more than one third-party account to ' + 'your portal account.') + self._raise_client_error(message) + # If a SocialAccount already exists, meaning the provider account is # connected to a local account, proceed with login. if sociallogin.is_existing: return - # If the provider does not provide any addresses, raise an error. provider_addresses = sociallogin.email_addresses - if not provider_addresses: + num_provider_addresses = len(provider_addresses) + # If the provider does not provide any addresses, raise an error. + if num_provider_addresses == 0: log_message = ( f'Provider {provider} did not provide any email addresses for ' f'User with email {user_email} and UID {user_uid}.') logger.error(log_message) - self.raise_server_error(self.get_auth_error_message()) + self._raise_server_error(self._get_auth_error_message()) + # In general, it is expected that a provider will only give one address. + # If multiple are given, allow all of them to be associated with the + # user (agnostic of whether users are allowed to have multiple), but log + # a warning. + elif num_provider_addresses > 1: + log_message = ( + f'Provider {provider} provided more than one email address for ' + f'User with email {user_email} and UID {user_uid}: ' + f'{", ".join(provider_addresses)}.') + logger.warning(log_message) # SOCIALACCOUNT_PROVIDERS['cilogon']['VERIFIED_EMAIL'] should be True, # so all provider-given addresses should be interpreted as verified. @@ -110,7 +156,7 @@ def pre_social_login(self, request, sociallogin): f'None of the email addresses in ' f'[{", ".join(provider_addresses)}] are verified.') logger.error(log_message) - self.raise_server_error(self.get_auth_error_message()) + self._raise_server_error(self._get_auth_error_message()) # Fetch EmailAddresses matching those given by the provider, divided by # the associated User. @@ -129,64 +175,93 @@ def pre_social_login(self, request, sociallogin): if not matching_addresses_by_user: return elif len(matching_addresses_by_user) == 1: - # If at least one address was verified, connect the two accounts. - # Otherwise, raise an error with instructions on how to proceed. user = next(iter(matching_addresses_by_user)) addresses = matching_addresses_by_user[user] if any([a.verified for a in addresses]): - log_message = ( - f'Attempting to connect data for User with email ' - f'{user_email} and UID {user_uid} from provider ' - f'{provider} to local User {user.pk}.') - logger.info(log_message) - sociallogin.connect(request, user) - log_message = f'Successfully connected data to User {user.pk}.' - logger.info(log_message) + # After this, allauth.account.adapter.pre_login blocks login if + # the user is inactive. Regardless of that, connect the user + # (and trigger signals for creating EmailAddresses). + self._connect_user( + request, sociallogin, provider, user, user_email, user_uid) else: - log_message = ( - f'Found only unverified email addresses associated with ' - f'local User {user.pk} matching those given by provider ' - f'{provider} for User with email {user_email} and UID ' - f'{user_uid}.') - logger.warning(log_message) - message = ( - 'You are attempting to login using an email address ' - 'associated with an existing User, but it is unverified. ' - 'Please login to that account using a different provider, ' - 'verify the email address, and try again.') - self.raise_client_error(message) + self._block_login_for_verification( + request, sociallogin, provider, user, user_email, user_uid, + addresses) else: - user_pks = [user.pk for user in matching_addresses_by_user] + user_pks = sorted([user.pk for user in matching_addresses_by_user]) log_message = ( f'Unexpectedly found multiple Users ([{", ".join(user_pks)}]) ' f'that had email addresses matching those provided by ' f'provider {provider} for User with email {user_email} and ' f'UID {user_uid}.') logger.error(log_message) - self.raise_server_error(self.get_auth_error_message()) + self._raise_server_error(self._get_auth_error_message()) + + def _block_login_for_verification(self, request, sociallogin, provider, + user, user_email, user_uid, + email_addresses): + """Block the login attempt and send verification emails to the + given EmailAddresses.""" + log_message = ( + f'Found only unverified email addresses associated with local User ' + f'{user.pk} matching those given by provider {provider} for User ' + f'with email {user_email} and UID {user_uid}.') + logger.warning(log_message) + + try: + cilogon_idp = sociallogin.serialize()[ + 'account']['extra_data']['idp_name'] + request_login_method_str = f'CILogon - {cilogon_idp}' + except Exception as e: + logger.exception(f'Failed to determine CILogon IDP. Details:\n{e}') + request_login_method_str = 'CILogon' + for email_address in email_addresses: + verifier = LoginActivityVerifier( + request, email_address, request_login_method_str) + verifier.send_email() + + message = ( + 'You are attempting to log in using an email address associated ' + 'with an existing user, but it is unverified. Please check the ' + 'address for a verification email.') + self._raise_client_error(message) + + @staticmethod + def _connect_user(request, sociallogin, provider, user, user_email, + user_uid): + """Connect the provider account to the User's account in the + database.""" + sociallogin.connect(request, user) + log_message = ( + f'Successfully connected data for User with email {user_email} and ' + f'UID {user_uid} from provider {provider} to local User {user.pk}.') + logger.info(log_message) @staticmethod - def get_auth_error_message(): + def _get_auth_error_message(): """Return the generic message the user should receive if authentication-related errors occur.""" return ( f'Unexpected authentication error. Please contact ' f'{settings.CENTER_HELP_EMAIL} for further assistance.') - @staticmethod - def raise_client_error(message): + def _raise_client_error(self, message): """Raise an ImmediateHttpResponse with a client error and the given message.""" + self._raise_error(HttpResponseBadRequest, message) + + @staticmethod + def _raise_error(response_class, message): + """Raise an ImmediateHttpResponse with an error HttpResponse + class (e.g., HttpResponseBadRequest or HttpResponseServerError) + error and the given message.""" template = 'error_with_message.html' - html = render_to_string(template, context={'message': message}) - response = HttpResponseBadRequest(html) + context = {'message': message, **portal_and_program_names(None)} + html = render_to_string(template, context=context) + response = response_class(html) raise ImmediateHttpResponse(response) - @staticmethod - def raise_server_error(message): + def _raise_server_error(self, message): """Raise an ImmediateHttpResponse with a server error and the given message.""" - template = 'error_with_message.html' - html = render_to_string(template, context={'message': message}) - response = HttpResponseServerError(html) - raise ImmediateHttpResponse(response) + self._raise_error(HttpResponseServerError, message) diff --git a/coldfront/core/socialaccount/signals.py b/coldfront/core/socialaccount/signals.py index 25d363c64..662529647 100644 --- a/coldfront/core/socialaccount/signals.py +++ b/coldfront/core/socialaccount/signals.py @@ -6,7 +6,7 @@ from django.db import transaction from django.dispatch import receiver -from allauth.socialaccount.models import EmailAddress +from allauth.account.models import EmailAddress from allauth.socialaccount.models import SocialLogin from allauth.socialaccount.providers.base import AuthProcess from allauth.socialaccount.signals import social_account_added diff --git a/coldfront/core/socialaccount/urls.py b/coldfront/core/socialaccount/urls.py index 0b98ebbd8..06e59322a 100644 --- a/coldfront/core/socialaccount/urls.py +++ b/coldfront/core/socialaccount/urls.py @@ -1,13 +1,40 @@ from allauth.socialaccount.urls import urlpatterns as all_patterns +from flags.urls import flagged_paths + """Include a subset of patterns from allauth.socialaccount.""" urlpatterns = [] -names_to_include = { - 'socialaccount_login_cancelled', - 'socialaccount_connections', -} -for pattern in all_patterns: - if pattern.name in names_to_include: - urlpatterns.append(pattern) + + +# TODO: Come up with a more elegant solution for dealing with views protected by +# multiple flags. +with flagged_paths('SSO_ENABLED') as sso_f_path: + names_to_include = { + 'socialaccount_login_cancelled', + } + for pattern in all_patterns: + if pattern.name in names_to_include: + urlpatterns.append(sso_f_path( + str(pattern.pattern), pattern.callback, + pattern.default_args, pattern.name) + ) + + # Only include the view for connecting additional social accounts if users + # are allowed to have multiple emails. + names_to_include_if_multiple_emails_allowed = { + 'socialaccount_connections', + } + with flagged_paths('MULTIPLE_EMAIL_ADDRESSES_ALLOWED') as multi_email_f_path: + for pattern in all_patterns: + if pattern.name in names_to_include_if_multiple_emails_allowed: + # The URL is not correctly disabled unless passed through both + # context managers. + tmp_pattern = multi_email_f_path( + str(pattern.pattern), pattern.callback, + pattern.default_args, pattern.name) + final_pattern = sso_f_path( + str(tmp_pattern.pattern), tmp_pattern.callback, + tmp_pattern.default_args, tmp_pattern.name) + urlpatterns.append(final_pattern) diff --git a/coldfront/core/statistics/management/__init__.py b/coldfront/core/statistics/management/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/coldfront/core/statistics/management/commands/__init__.py b/coldfront/core/statistics/management/commands/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/coldfront/core/statistics/management/commands/free_qos_jobs.py b/coldfront/core/statistics/management/commands/free_qos_jobs.py new file mode 100644 index 000000000..dc2f0ef18 --- /dev/null +++ b/coldfront/core/statistics/management/commands/free_qos_jobs.py @@ -0,0 +1,252 @@ +import json +import logging + +from decimal import Decimal + +from django.core.management.base import BaseCommand + +from coldfront.core.allocation.utils import get_project_compute_allocation +from coldfront.core.project.models import Project +from coldfront.core.statistics.models import Job +from coldfront.core.statistics.utils_.accounting_utils import set_job_amount +from coldfront.core.statistics.utils_.accounting_utils import validate_job_dates +from coldfront.core.resource.utils_.allowance_utils.computing_allowance import ComputingAllowance +from coldfront.core.resource.utils_.allowance_utils.interface import ComputingAllowanceInterface +from coldfront.core.resource.utils_.allowance_utils.interface import ComputingAllowanceInterfaceError +from coldfront.core.utils.common import add_argparse_dry_run_argument + + +"""An admin command for listing and updating jobs under free QoSes that +have non-zero amounts.""" + + +class Command(BaseCommand): + + help = ( + 'Manage jobs under free QoSes that have non-zero service unit amounts. ' + 'List statistics about them or reset their amounts to zero, updating ' + 'the associated usages.') + + logger = logging.getLogger(__name__) + + def add_arguments(self, parser): + subparsers = parser.add_subparsers( + dest='subcommand', + help='The subcommand to run.', + title='subcommands') + subparsers.required = True + self._add_reset_subparser(subparsers) + self._add_summary_subparser(subparsers) + add_argparse_dry_run_argument(parser) + + def handle(self, *args, **options): + subcommand = options['subcommand'] + if subcommand == 'reset': + self._handle_reset(*args, **options) + elif subcommand == 'summary': + self._handle_summary(*args, **options) + + @staticmethod + def _add_reset_subparser(parsers): + """Add a subparser for the 'reset' subcommand.""" + parser = parsers.add_parser( + 'reset', + help=( + 'Set amounts for relevant jobs to zero, and update associated ' + 'usages.')) + parser.add_argument( + 'qos_names', + help='A space-separated list of free QoS names.', + nargs='+', + type=str) + parser.add_argument( + '--project', + help='The name of a specific project to perform the reset for.', + type=str) + add_argparse_dry_run_argument(parser) + + @staticmethod + def _add_summary_subparser(parsers): + """Add a subparser for the 'summary' subcommand.""" + parser = parsers.add_parser( + 'summary', help='Get a JSON summary of relevant jobs.') + parser.add_argument( + 'qos_names', + help='A space-separated list of free QoS names.', + nargs='+', + type=str) + parser.add_argument( + '--project', + help='The name of a specific project to get a summary for.', + type=str) + + def _handle_reset(self, *args, **options): + """Handle the 'reset' subcommand.""" + if options['project'] is not None: + project = Project.objects.get(name=options['project']) + else: + project = None + self._zero_out_free_qos_jobs( + options['qos_names'], project=project, dry_run=options['dry_run']) + + def _handle_summary(self, *args, **options): + """Handle the 'summary' subcommand.""" + if options['project'] is not None: + project = Project.objects.get(name=options['project']) + else: + project = None + output_json = self._summary_json(options['qos_names'], project=project) + self.stdout.write(json.dumps(output_json, indent=4, sort_keys=True)) + + @staticmethod + def _summary_json(qos_names, project=None): + """Return a dictionary detailing the number of jobs with the + given QoSes that have non-zero amounts, as well as the total + associated usage. Optionally only consider jobs under the given + Project. """ + zero = Decimal('0.00') + + num_jobs = 0 + total_by_project_id = {} + kwargs = { + 'qos__in': qos_names, + 'amount__gt': zero, + } + if project is not None: + kwargs['accountid'] = project + for job in Job.objects.filter(**kwargs).iterator(): + num_jobs += 1 + # Use accountid_id to avoid a foreign key lookup. + project_id = job.accountid_id + if project_id not in total_by_project_id: + total_by_project_id[project_id] = zero + total_by_project_id[project_id] += job.amount + + total_by_project_name = {} + total_by_allowance = {} + for project_id, amount in total_by_project_id.items(): + project = Project.objects.get(id=project_id) + total_by_project_name[project.name] = str(amount) + if '_' in project.name: + allowance_type = project.name.split('_')[0] + else: + allowance_type = project.name + if allowance_type not in total_by_allowance: + total_by_allowance[allowance_type] = zero + total_by_allowance[allowance_type] += amount + + for allowance_name, amount in total_by_allowance.items(): + total_by_allowance[allowance_name] = str(amount) + + return { + 'num_jobs': num_jobs, + 'total_by_allowance': total_by_allowance, + 'total_by_project': total_by_project_name, + } + + def _zero_out_free_qos_jobs(self, qos_names, project=None, dry_run=False): + """For each job with one of the given QoSes, reset the job's + amount to zero, and update the associated usages if + appropriate. Optionally only consider jobs under the given + Project. Optionally display updates instead of performing + them.""" + computing_allowance_interface = ComputingAllowanceInterface() + periodic_project_name_prefixes = tuple([ + computing_allowance_interface.code_from_name(allowance.name) + for allowance in computing_allowance_interface.allowances() + if ComputingAllowance(allowance).is_periodic()]) + + total_by_project_name = {} + project_cache = {} + allocation_cache = {} + + zero = Decimal('0.00') + num_jobs = 0 + kwargs = { + 'qos__in': qos_names, + 'amount__gt': zero, + } + if project is not None: + kwargs['accountid'] = project + for job in Job.objects.filter(**kwargs).iterator(): + num_jobs += 1 + jobslurmid = job.jobslurmid + + project_id = job.accountid_id + if project_id in project_cache: + project = project_cache[project_id] + else: + project = job.accountid + project_cache[project_id] = project + + # Skip updating usages for any job that is outside its allocation's + # allowance period. Some projects don't have meaningful periods; + # avoid expensive lookups for them. + try: + computing_allowance_interface.allowance_from_project(project) + except ComputingAllowanceInterfaceError: + # Non-primary cluster project --> no allowance period + update_usages = True + else: + if project.name.startswith(periodic_project_name_prefixes): + # Has a periodic allowance --> defined allowance period + # Only update usages if the job is within the current + # period. + if project_id in allocation_cache: + allocation = allocation_cache[project_id] + else: + allocation = get_project_compute_allocation(project) + allocation_cache[project_id] = allocation + job_data = job.__dict__ + job_data['accountid'] = project + update_usages = validate_job_dates( + job_data, allocation, end_date_expected=True) + else: + # Does not have a periodic allowance --> no allowance period + update_usages = True + + if project.name not in total_by_project_name: + total_by_project_name[project.name] = { + 'num_jobs': 0, + 'usage': zero, + } + total_by_project_name[project.name]['num_jobs'] += 1 + if update_usages: + total_by_project_name[project.name]['usage'] += job.amount + else: + message = ( + f'Job {jobslurmid} outside of allowance period. Skipping ' + f'usage update.') + self.stdout.write(self.style.WARNING(message)) + + if not dry_run: + try: + set_job_amount( + jobslurmid, zero, update_usages=update_usages) + except Exception as e: + self.logger.exception(e) + message = ( + f'Failed to update amount for Job {jobslurmid}. ' + f'Details:\n{e}') + self.stderr.write(self.style.ERROR(message)) + + for project_name in total_by_project_name: + usage_str = str( + total_by_project_name[project_name]['usage']) + total_by_project_name[project_name]['usage'] = usage_str + result_json = json.dumps( + total_by_project_name, indent=4, sort_keys=True) + + message = ( + f'Corrected amounts for {num_jobs} jobs under free QoSes ' + f'{", ".join(sorted(qos_names))} to zero and associated usages. ' + f'Summary:\n{result_json}') + self.stdout.write(message) + + if not dry_run: + compact_result_json = json.dumps( + total_by_project_name, sort_keys=True) + self.logger.info( + f'Corrected amounts for {num_jobs} jobs under free QoSes ' + f'{", ".join(sorted(qos_names))} to zero and associated ' + f'usages. Summary: {compact_result_json}') diff --git a/coldfront/core/statistics/utils_/accounting_utils.py b/coldfront/core/statistics/utils_/accounting_utils.py new file mode 100644 index 000000000..9f440677c --- /dev/null +++ b/coldfront/core/statistics/utils_/accounting_utils.py @@ -0,0 +1,154 @@ +import logging +import pytz + +from datetime import date +from datetime import datetime +from datetime import timedelta +from decimal import Decimal + +from django.db import transaction + +from coldfront.api.statistics.utils import get_accounting_allocation_objects +from coldfront.core.allocation.models import AllocationAttributeUsage +from coldfront.core.allocation.models import AllocationUserAttributeUsage +from coldfront.core.resource.utils_.allowance_utils.computing_allowance import ComputingAllowance +from coldfront.core.resource.utils_.allowance_utils.interface import ComputingAllowanceInterface +from coldfront.core.statistics.models import Job +from coldfront.core.utils.common import display_time_zone_date_to_utc_datetime + + +def set_job_amount(jobslurmid, amount, update_usages=True): + """Set the number of service units for the Job with the given Slurm + ID to the given amount. Optionally update associated usages. + + Parameters: + - jobslurmid (str) + - amount (Decimal) + + Returns: + - None + + Raises: + + """ + assert isinstance(jobslurmid, str) + assert isinstance(amount, Decimal) + + with transaction.atomic(): + job = Job.objects.select_for_update().get(jobslurmid=jobslurmid) + + if update_usages: + account = job.accountid + user = job.userid + allocation_objects = get_accounting_allocation_objects( + account, user=user, enforce_allocation_active=False) + + account_usage = ( + AllocationAttributeUsage.objects.select_for_update().get( + pk=allocation_objects.allocation_attribute_usage.pk)) + user_account_usage = ( + AllocationUserAttributeUsage.objects.select_for_update().get( + pk=allocation_objects.allocation_user_attribute_usage.pk)) + + difference = amount - job.amount + + new_account_usage = max( + account_usage.value + difference, Decimal('0.00')) + account_usage.value = new_account_usage + account_usage.save() + + new_user_account_usage = max( + user_account_usage.value + difference, Decimal('0.00')) + user_account_usage.value = new_user_account_usage + user_account_usage.save() + + # Do not update the job.amount before calculating the difference. + job.amount = amount + job.save() + + +def validate_job_dates(job_data, allocation, end_date_expected=False): + """Given a dictionary representing a Job, its corresponding + Allocation, and whether the Job is expected to include an end date, + return whether: + (a) The Job has the expected dates, + (b) The Job's corresponding Allocation has the expected dates, + and + (c) The Job started and ended within the Allocation's dates. + + Write errors or warnings to the log if not.""" + logger = logging.getLogger(__name__) + + date_format = '%Y-%m-%d %H:%M:%SZ' + + jobslurmid = job_data['jobslurmid'] + account_name = job_data['accountid'].name + + # The Job should have submit, start, and, if applicable, end dates. + expected_date_keys = ['submitdate', 'startdate'] + if end_date_expected: + expected_date_keys.append('enddate') + expected_dates = { + key: job_data.get(key, None) for key in expected_date_keys} + for key, expected_date in expected_dates.items(): + if not isinstance(expected_date, datetime): + logger.error(f'Job {jobslurmid} does not have a {key}.') + return False + + # The Job's corresponding Allocation should have a start date. + allocation_start_date = allocation.start_date + if not isinstance(allocation_start_date, date): + logger.error( + f'Allocation {allocation.pk} (Project {account_name}) does not ' + f'have a start date.') + return False + + # The Job should not have started before its corresponding Allocation's + # start date. + job_start_dt_utc = expected_dates['startdate'] + allocation_start_dt_utc = display_time_zone_date_to_utc_datetime( + allocation_start_date) + if job_start_dt_utc < allocation_start_dt_utc: + logger.warning( + f'Job {jobslurmid} start date ' + f'({job_start_dt_utc.strftime(date_format)}) is before Allocation ' + f'{allocation.pk} (Project {account_name}) start date ' + f'({allocation_start_dt_utc.strftime(date_format)}).') + return False + + if not end_date_expected: + return True + + # The Job's corresponding Allocation may have an end date. (Compare + # against the maximum date if not.) + computing_allowance_interface = ComputingAllowanceInterface() + periodic_project_name_prefixes = tuple([ + computing_allowance_interface.code_from_name(allowance.name) + for allowance in computing_allowance_interface.allowances() + if ComputingAllowance(allowance).is_periodic()]) + if account_name.startswith(periodic_project_name_prefixes): + allocation_end_date = allocation.end_date + if not isinstance(allocation_end_date, date): + logger.error( + f'Allocation {allocation.pk} (Project {account_name}) does not ' + f'have an end date.') + return False + allocation_end_dt_utc = ( + display_time_zone_date_to_utc_datetime(allocation_end_date) + + timedelta(hours=24) - + timedelta(microseconds=1)) + else: + allocation_end_dt_utc = datetime.max.replace(tzinfo=pytz.utc) + + # The Job should not have ended after the last microsecond of its + # corresponding Allocation's end date. + job_end_dt_utc = expected_dates['enddate'] + if job_end_dt_utc > allocation_end_dt_utc: + logger.warning( + f'Job {jobslurmid} end date ' + f'({job_end_dt_utc.strftime(date_format)}) is after Allocation ' + f'{allocation.pk} (Project {account_name}) end date ' + f'({allocation_end_dt_utc.strftime(date_format)}).') + return False + + return True diff --git a/coldfront/core/user/admin.py b/coldfront/core/user/admin.py index be0ae38a6..40bff30ca 100644 --- a/coldfront/core/user/admin.py +++ b/coldfront/core/user/admin.py @@ -1,14 +1,8 @@ from django.contrib import admin -from django.contrib import messages -from django.core.exceptions import ValidationError -from coldfront.core.user.models import UserProfile, EmailAddress -from coldfront.core.user.utils import update_user_primary_email_address +from allauth.account.models import EmailAddress -import logging - - -logger = logging.getLogger(__name__) +from coldfront.core.user.models import UserProfile @admin.register(UserProfile) @@ -28,79 +22,6 @@ def last_name(self, obj): return obj.user.last_name -@admin.register(EmailAddress) -class EmailAddressAdmin(admin.ModelAdmin): - list_display = ('user', 'email', 'is_primary', 'is_verified', ) - ordering = ('user', '-is_primary', '-is_verified', 'email', ) - list_filter = ('is_primary', 'is_verified', ) - search_fields = [ - 'user__username', 'user__first_name', 'user__last_name', 'email'] - fields = ('email', 'user', 'is_primary', 'is_verified', ) - actions = ('make_primary', ) - readonly_fields = ('is_primary', 'is_verified', ) - - def delete_model(self, request, obj): - if obj.is_primary: - raise ValidationError( - 'Cannot delete primary email. Unset primary status in list ' - 'display before deleting.') - else: - super().delete_model(request, obj) - - @admin.action(description='Make selected primary') - def make_primary(self, request, queryset): - """Set the EmailAddresses in the given queryset as the primary - addresses of their respective users. - - Currently, admins are limited to setting at most one address at - a time.""" - if queryset.count() > 1: - raise ValidationError( - 'Cannot set more than one primary email address at a time.') - for email_address in queryset: - user = email_address.user - try: - update_user_primary_email_address(email_address) - except ValueError: - raise ValidationError( - 'Cannot set an unverified email address as primary.') - except Exception as e: - message = ( - f'Encountered unexpected exception when updating User ' - f'{user.pk}\'s primary EmailAddress to ' - f'{email_address.pk}. Details:') - logger.error(message) - logger.exception(e) - raise ValidationError( - f'Failed to set {email_address.pk} as primary. See the ' - f'log for details.') - else: - message = ( - f'Set User {user.pk}\'s primary EmailAddress to ' - f'{email_address.email}.') - messages.success(request, message) - - def delete_queryset(self, request, queryset): - """Delete EmailAddresses in the given queryset, skipping those - that are primary.""" - num_primary, num_non_primary = 0, 0 - for email_address in queryset: - if email_address.is_primary: - num_primary = num_primary + 1 - else: - email_address.delete() - num_non_primary = num_non_primary + 1 - - success_message = ( - f'Deleted {num_non_primary} non-primary EmailAddresses.') - messages.success(request, success_message) - - if num_primary > 0: - error_message = ( - f'Skipped deleting {num_primary} primary EmailAddresses.') - messages.error(request, error_message) - - class EmailAddressInline(admin.TabularInline): model = EmailAddress extra = 0 diff --git a/coldfront/core/user/auth.py b/coldfront/core/user/auth.py index 4e1022c0d..8616118d9 100644 --- a/coldfront/core/user/auth.py +++ b/coldfront/core/user/auth.py @@ -1,16 +1,24 @@ -from coldfront.core.user.models import EmailAddress -from coldfront.core.user.utils import send_email_verification_email from django.contrib.auth.backends import BaseBackend from django.contrib.auth.models import User + +from allauth.account.models import EmailAddress +from sesame.backends import ModelBackend as BaseSesameBackend + +from coldfront.core.user.utils import send_email_verification_email +from coldfront.core.user.utils_.link_login_utils import UserLoginLinkIneligible +from coldfront.core.user.utils_.link_login_utils import validate_user_eligible_for_login_link + import logging +logger = logging.getLogger(__name__) + + class EmailAddressBackend(BaseBackend): """An authentication backend that allows a user to authenticate using any of their verified EmailAddress objects.""" def authenticate(self, request, username=None, password=None, **kwargs): - logger = logging.getLogger(__name__) if username is None: return None username = username.lower() @@ -24,7 +32,7 @@ def authenticate(self, request, username=None, password=None, **kwargs): # If the EmailAddress exists, but is not verified, send a verification # email to it. Only do this if the user is already active; otherwise, # a separate account activation email will handle email verification. - if user.is_active and not email_address.is_verified: + if user.is_active and not email_address.verified: try: send_email_verification_email(email_address) except Exception as e: @@ -42,3 +50,19 @@ def get_user(self, user_id): return User.objects.get(id=user_id) except User.DoesNotExist: return None + + +class SesameBackend(BaseSesameBackend): + """A subclass of django-sesame's ModelBackend that limits who is + eligible to log in using tokens.""" + + def user_can_authenticate(self, user): + try: + validate_user_eligible_for_login_link(user) + except UserLoginLinkIneligible as e: + message = ( + f'User {user.username} was blocked from Sesame authentication ' + f'because: {str(e)}') + logger.info(message) + return False + return True diff --git a/coldfront/core/user/forms.py b/coldfront/core/user/forms.py index ae742651d..88bc01bff 100644 --- a/coldfront/core/user/forms.py +++ b/coldfront/core/user/forms.py @@ -8,15 +8,16 @@ from django.contrib.auth.models import User from django.contrib.auth.tokens import default_token_generator from django.contrib.sites.shortcuts import get_current_site -from django.core.exceptions import ImproperlyConfigured from django.urls import reverse from django.utils.encoding import force_bytes from django.utils.html import mark_safe from django.utils.http import urlsafe_base64_encode from django.utils.translation import gettext_lazy as _ +from allauth.account.models import EmailAddress + from coldfront.core.user.utils import send_account_activation_email -from coldfront.core.user.models import UserProfile, EmailAddress +from coldfront.core.user.models import UserProfile from coldfront.core.utils.mail import dummy_email_address from phonenumber_field.formfields import PhoneNumberField @@ -240,40 +241,11 @@ def set_acknowledgement_help_text(self): field.help_text = template.format('LBNL', 'LBNL IT Division') -class EmailAddressAddForm(forms.Form): - - email = forms.EmailField(max_length=100, required=True) - - def clean_email(self): - cleaned_data = super().clean() - email = cleaned_data['email'].lower() - if (User.objects.filter(email=email).exists() or - EmailAddress.objects.filter(email=email).exists()): - raise forms.ValidationError( - f'Email address {email} is already in use.') - return email - - class UserReactivateForm(forms.Form): email = forms.EmailField(max_length=100, required=True) -class PrimaryEmailAddressSelectionForm(forms.Form): - - email_address = forms.ModelChoiceField( - label='New Primary Email Address', - queryset=EmailAddress.objects.none(), - required=True, - widget=forms.RadioSelect()) - - def __init__(self, *args, **kwargs): - user = kwargs.pop('user') - super().__init__(*args, **kwargs) - self.fields['email_address'].queryset = EmailAddress.objects.filter( - user=user, is_verified=True, is_primary=False) - - class VerifiedEmailAddressPasswordResetForm(PasswordResetForm): """A subclass of django.contrib.auth.forms.PasswordResetForm that uses EmailAddress.""" @@ -284,7 +256,7 @@ def get_email_address(email): if one exists, else None.""" try: return EmailAddress.objects.select_related('user').get( - email=email, is_verified=True, user__is_active=True) + email=email, verified=True, user__is_active=True) except EmailAddress.DoesNotExist: return None diff --git a/coldfront/core/user/forms_/__init__.py b/coldfront/core/user/forms_/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/coldfront/core/user/forms_/link_login_forms.py b/coldfront/core/user/forms_/link_login_forms.py new file mode 100644 index 000000000..b5367eb62 --- /dev/null +++ b/coldfront/core/user/forms_/link_login_forms.py @@ -0,0 +1,10 @@ +from django import forms + + +class RequestLoginLinkForm(forms.Form): + + email = forms.EmailField() + + def clean_email(self): + email = self.cleaned_data['email'] + return email.lower() diff --git a/coldfront/core/user/management/commands/create_email_addresses.py b/coldfront/core/user/management/commands/create_email_addresses.py index 357217f14..f5a71f065 100644 --- a/coldfront/core/user/management/commands/create_email_addresses.py +++ b/coldfront/core/user/management/commands/create_email_addresses.py @@ -1,5 +1,8 @@ from django.contrib.auth.models import User from django.core.management.base import BaseCommand + +from allauth.account.models import EmailAddress + import logging @@ -15,27 +18,9 @@ class Command(BaseCommand): 'one-time use for existing users loaded in from spreadsheets.') logger = logging.getLogger(__name__) - def add_arguments(self, parser): - # TODO: Remove this once all emails are transitioned to - # TODO: allauth.account.models.EmailAddress. - parser.add_argument( - 'module', - choices=['allauth.account.models', 'coldfront.core.user.models'], - help=( - 'There are temporarily two EmailAddress models, until all can ' - 'be transitioned under allauth.account.models.'), - type=str) - def handle(self, *args, **options): """For each User that has no EmailAddress, create a verified, primary instance using the User's email field.""" - if options['module'] == 'allauth.account.models': - from allauth.account.models import EmailAddress - verified_field, primary_field = 'verified', 'primary' - else: - from coldfront.core.user.models import EmailAddress - verified_field, primary_field = 'is_verified', 'is_primary' - user_pks_with_emails = EmailAddress.objects.values_list( 'user', flat=True) users_without_emails = User.objects.exclude( @@ -46,8 +31,8 @@ def handle(self, *args, **options): kwargs = { 'user': user, 'email': email, - verified_field: True, - primary_field: True, + 'verified': True, + 'primary': True, } if not email: message = f'User {user.pk} email is empty.' diff --git a/coldfront/core/user/management/commands/lower_email_case.py b/coldfront/core/user/management/commands/lower_email_case.py index c467a5291..521079712 100644 --- a/coldfront/core/user/management/commands/lower_email_case.py +++ b/coldfront/core/user/management/commands/lower_email_case.py @@ -1,6 +1,8 @@ -from coldfront.core.user.models import EmailAddress from django.contrib.auth.models import User from django.core.management.base import BaseCommand + +from allauth.account.models import EmailAddress + import logging diff --git a/coldfront/core/user/management/commands/migrate_email_address_model.py b/coldfront/core/user/management/commands/migrate_email_address_model.py new file mode 100644 index 000000000..3593dca8b --- /dev/null +++ b/coldfront/core/user/management/commands/migrate_email_address_model.py @@ -0,0 +1,235 @@ +import logging +import sys +import traceback + +from django.contrib.auth.models import User +from django.core.management.base import BaseCommand + +from allauth.account.models import EmailAddress as NewEmailAddress + +from coldfront.core.user.models import EmailAddress as OldEmailAddress +from coldfront.core.utils.common import add_argparse_dry_run_argument + + +"""An admin command that creates or updates corresponding instances of +allauth.account.models.EmailAddress for existing emails in the User +model and coldfront.core.user.models.EmailAddress model.""" + + +class Command(BaseCommand): + + help = ( + 'Create or update instances of allauth.account.models.EmailAddress ' + 'based on emails stored in the User model and the deprecated ' + 'coldfront.core.users.models.EmailAddress model. This command strictly ' + 'updates whether an email is primary or verified from False to True, ' + 'and never the other way around (i.e., it respects existing values in ' + 'the new model). It is intended for a one-time migration, and may not ' + 'be suited for general reuse.') + + logger = logging.getLogger(__name__) + + def add_arguments(self, parser): + add_argparse_dry_run_argument(parser) + + def handle(self, *args, **options): + dry_run = options['dry_run'] + if not dry_run: + user_confirmation = input( + 'Are you sure you wish to proceed? [Y/y/N/n]: ') + if user_confirmation.strip().lower() != 'y': + self.stdout.write(self.style.WARNING('Migration aborted.')) + sys.exit(0) + for user in User.objects.iterator(): + try: + old_email_data = self._get_old_email_data(user) + self._process_emails_for_user( + user, old_email_data, dry_run=dry_run) + except Exception: + message = ( + f'Failed to process User {user.pk}. Details:\n' + f'{traceback.format_exc()}') + self.stdout.write(self.style.ERROR(message)) + + def _create_address_for_user(self, email, user, primary=False, + verified=False, dry_run=True): + """Create an instance of the new EmailAddress model for the + given email (str) and User, setting its primary and verified + fields. Optionally display the update instead of performing it. + + If the email already belongs to a different user, raise an + error. + + This method makes the following assumptions: + - The User does not already have a corresponding instance. + - If primary is True, the User does not already have a + primary instance. + + It should not be called if the assumptions are not true. + """ + try: + other_user_address = NewEmailAddress.objects.get(email=email) + except NewEmailAddress.DoesNotExist: + if dry_run: + phrase = 'Would create' + pk = 'PK' + style = self.style.WARNING + else: + email_address = NewEmailAddress.objects.create( + user=user, + email=email, + primary=primary, + verified=verified) + phrase = 'Created' + pk = email_address.pk + style = self.style.SUCCESS + + message = ( + f'{phrase} EmailAddress {pk} for User {user.pk} ' + f'({user.username}) with primary={primary} and ' + f'verified={verified}.') + self.stdout.write(style(message)) + if not dry_run: + self.logger.info(message) + else: + message = ( + f'A different User {other_user_address.user.pk} already has ' + f'email "{email}".') + self.stdout.write(self.style.WARNING(message)) + if not dry_run: + self.logger.warning(message) + + @staticmethod + def _get_old_email_data(user): + """Return a dict mapping emails (str) for the given User from + the old model and the User instance to a dict with keys + 'primary' and 'verified', denoting whether the address was + primary and verified. + + The email stored in the User instance is interpreted as being + primary. + + Raise an error if the User does not have exactly one primary + email under the old model. + """ + old_addresses = OldEmailAddress.objects.filter(user=user) + old_emails = {} + + for old_address in old_addresses: + email_str = old_address.email.strip().lower() + old_emails[email_str] = { + 'verified': old_address.is_verified, + 'primary': old_address.is_primary, + } + user_email_str = user.email.strip().lower() + if user_email_str: + if user_email_str in old_emails: + old_emails[user_email_str]['primary'] = True + else: + old_emails[user_email_str] = { + 'verified': False, + 'primary': True, + } + + old_primaries = [] + for email, attributes in old_emails.items(): + if attributes['primary']: + old_primaries.append(email) + + num_primaries = len(old_primaries) + if num_primaries == 0: + raise Exception( + f'Found no old primary emails for User {user.pk} ' + f'({user.username}).') + elif num_primaries == 1: + pass + else: + raise Exception( + f'Found multiple old primary emails for User {user.pk} ' + f'({user.username}): {", ".join(old_primaries)}.') + + return old_emails + + def _process_emails_for_user(self, user, old_email_data, dry_run=True): + """Given a User and a dict containing information about emails + in the old model, create or update emails in the new model. + Optionally display updates instead of performing them. + + old_email_data is assumed to contain exactly one primary + address. + """ + new_addresses = NewEmailAddress.objects.filter(user=user) + new_address_lower_strs = set( + [email.lower() + for email in new_addresses.values_list('email', flat=True)]) + + try: + new_primary = new_addresses.get(primary=True) + except NewEmailAddress.DoesNotExist: + new_primary = None + except NewEmailAddress.MultipleObjectsReturned as e: + raise e + + for old_email, attributes in old_email_data.items(): + # If this was a primary address, but the user already has a + # different primary, do not set this as primary. + if attributes['primary']: + if (new_primary is not None and + new_primary.email.lower() != old_email): + attributes['primary'] = False + if old_email in new_address_lower_strs: + email_address = new_addresses.get(email=old_email) + self._update_address_for_user( + email_address, user, primary=attributes['primary'], + verified=attributes['verified'], dry_run=dry_run) + else: + self._create_address_for_user( + old_email, user, primary=attributes['primary'], + verified=attributes['verified'], dry_run=dry_run) + + def _update_address_for_user(self, email_address, user, primary=False, + verified=False, dry_run=True): + """Update the given EmailAddress instance (new model) for the + given User, setting its primary and verified fields. + + It only sets fields if they would go from False to True. + + This method makes the following assumptions: + - If primary is True, the User does not already have a + primary instance other than the given instance. + + It should not be called if the assumptions are not true. + """ + # Only update "primary" if it would go from False to True, not + # the other way around. + primary_updated = not email_address.primary and primary + if primary_updated: + email_address.primary = True + # Only update "verified" if it would go from False to True, not + # the other way around. + verified_updated = not email_address.verified and verified + if verified_updated: + email_address.verified = True + + if primary_updated or verified_updated: + if dry_run: + phrase = 'Would update' + style = self.style.WARNING + else: + email_address.save() + phrase = 'Updated' + style = self.style.SUCCESS + + updates = [] + if primary_updated: + updates.append('set primary to True') + if verified_updated: + updates.append('set verified to True') + + message = ( + f'{phrase} EmailAddress {email_address.pk} ' + f'({email_address.email}) for User {user.pk} ' + f'({user.username}): {", ".join(updates)}.') + self.stdout.write(style(message)) + if not dry_run: + self.logger.info(message) diff --git a/coldfront/core/user/templates/user/request_login_link.html b/coldfront/core/user/templates/user/request_login_link.html new file mode 100644 index 000000000..8cf24dd0c --- /dev/null +++ b/coldfront/core/user/templates/user/request_login_link.html @@ -0,0 +1,41 @@ +{% extends "common/base.html" %} +{% load static %} +{% load common_tags %} +{% load crispy_forms_tags %} + +{% block title %} +Request Login Link +{% endblock %} + +{% block content %} + +
+ +
+
+ + Request Login Link +
+
+

+ If you are an existing user, but your institution is not listed in + CILogon, you may use this form to request a short-lived link to log in + to the portal. +

+

+ New users must select an institution listed in CILogon. +

+
+ {% csrf_token %} +

+ {{ form|crispy }} + +

+
+
+
+
+{% endblock %} diff --git a/coldfront/core/user/templates/user/sso_login_brc.html b/coldfront/core/user/templates/user/sso_login_brc.html new file mode 100644 index 000000000..7d286f2b6 --- /dev/null +++ b/coldfront/core/user/templates/user/sso_login_brc.html @@ -0,0 +1,152 @@ +{% extends "common/base.html" %} +{% load feature_flags %} +{% load socialaccount %} + +{% load common_tags %} +{% load static %} + +{% block title %} +Log In +{% endblock %} + +{% block content %} + +

Log In: I am a...

+ + +
+
+
+
+ +
+
+ +
+
+
+
+ uc_berkeley_logo_words +
+
+

UC Berkeley User

+

+ I have a CalNet ID. +

+

+

+ + + Log In + +

+
+
+
+
+
+
+
+ berkeley_lab_logo +
+
+

Berkeley Lab Collaborator

+

+ I do not have a CalNet ID, but I do have a Berkeley Lab Identity. +

+

+ + + Log In + +

+
+
+
+
+
+
+
+ cilogon_logo +
+
+

External Collaborator

+

+ I do not have a CalNet ID, and I do not have a Berkeley Lab Identity. +

+

+ + + Log In + +

+
+
+
+
+
+ +
+
+ +
+
+
    +
  • + Authentication is managed by + CILogon. You will be + redirected there, where you will select a provider to authenticate + with (University of California, Berkeley for most users). +
  • +
  • + If you have a CalNet ID, please authenticate using the first option + above. UC Berkeley users who do not use their CalNet ID may + experience delays when requesting cluster access. +
  • +
  • + External collaborators should select their home institution as the + provider. If you are a new user and your institution is not listed, + select the Google provider. +
  • + {% flag_enabled 'LINK_LOGIN_ENABLED' as link_login_enabled %} + {% if link_login_enabled %} +
  • + If you are an existing external collaborator whose institution is + not listed, you may request a short-lived login link + here. +
  • + {% endif %} +
+
+
+
+
+ +{% endblock %} + + +{% block javascript %} +{{ block.super }} + +{% endblock %} diff --git a/coldfront/core/user/templates/user/sso_login.html b/coldfront/core/user/templates/user/sso_login_lrc.html similarity index 86% rename from coldfront/core/user/templates/user/sso_login.html rename to coldfront/core/user/templates/user/sso_login_lrc.html index d1f18fb02..d8d97bd97 100644 --- a/coldfront/core/user/templates/user/sso_login.html +++ b/coldfront/core/user/templates/user/sso_login_lrc.html @@ -100,10 +100,18 @@

External Collaborator

experience delays when requesting cluster access.
  • - External collaborators should select the provider of their home - institution. If your provider is not listed, select the - Google provider. + External collaborators should select their home institution as the + provider. If you are a new user and your institution is not listed, + select the Google provider.
  • + {% flag_enabled 'LINK_LOGIN_ENABLED' as link_login_enabled %} + {% if link_login_enabled %} +
  • + If you are an existing external collaborator whose institution is + not listed, you may request a short-lived login link + here. +
  • + {% endif %} diff --git a/coldfront/core/user/templates/user/user_add_email_address.html b/coldfront/core/user/templates/user/user_add_email_address.html deleted file mode 100644 index aae512d65..000000000 --- a/coldfront/core/user/templates/user/user_add_email_address.html +++ /dev/null @@ -1,31 +0,0 @@ -{% extends "common/base.html" %} -{% load crispy_forms_tags %} -{% load static %} - - -{% block title %} -Add Email Address -{% endblock %} - - -{% block content %} - -

    Add an Email Address

    -
    - -

    Associate another email address with your account. Once verified, you may authenticate to this portal using this address.

    - -
    -
    -
    - {% csrf_token %} - {{ form|crispy }} - - - Cancel - -
    -
    -
    - -{% endblock %} diff --git a/coldfront/core/user/templates/user/user_profile.html b/coldfront/core/user/templates/user/user_profile.html index 7b660ba1c..6904e6961 100644 --- a/coldfront/core/user/templates/user/user_profile.html +++ b/coldfront/core/user/templates/user/user_profile.html @@ -50,12 +50,6 @@

    {{ viewed_user.email }} - {% if primary_address_updatable %} - - - Update - - {% endif %}

    Email notifications are sent to this address.

    @@ -185,77 +179,12 @@


    - {% if core_user_email_addresses_visible %} -
    -
    - - Other Email Addresses{% if not requester_is_viewed_user %}: {{ viewed_user.username }}{% endif %} - {% if core_user_email_addresses_updatable %} - - {% endif %} -
    -
    -

    Below are other email addresses associated with your account. You may authenticate to this portal using any verified addresses.

    -

    You can verify an address by clicking the “Verify” button, which will send a verification email to the address. The address will remain unverified until you have clicked the link provided in that email.

    -
    - - - - - - - - {% for email in other_emails %} - - - - - - {% endfor %} - -
    Email AddressStatusActions
    {{ email.email }} - {% if email.is_verified %} - Verified - {% else %} - Verification Pending - {% endif %} - - {% if core_user_email_addresses_updatable %} - {% if not email.is_verified %} -
    - {% csrf_token %} - -
    - {% endif %} -
    - {% csrf_token %} - -
    - {% else %} - N/A - {% endif %} -
    -
    -
    -
    -
    - {% endif %} - - {% if allauth_email_addresses_visible %} + {% if email_addresses_visible %}
    Other Email Addresses{% if not requester_is_viewed_user %}: {{ viewed_user.username }}{% endif %} - {% if allauth_email_addresses_updatable %} + {% if email_addresses_updatable %} Manage Emails @@ -306,7 +235,7 @@

    Request Linking Email {% else %} -
    + {% csrf_token %}
    {% else %} - {% url 'account_email' as email_url %} + {% flag_enabled 'MULTIPLE_EMAIL_ADDRESSES_ALLOWED' as multiple_email_addresses_allowed %}

    - This email confirmation link expired or is invalid. Please - issue a new email confirmation request. + This email confirmation link expired or is invalid. + {% if multiple_email_addresses_allowed %} + Please issue a new email confirmation request. + {% endif %}

    {% endif %} diff --git a/coldfront/templates/common/table_sorter.html b/coldfront/templates/common/table_sorter.html new file mode 100644 index 000000000..907e21d04 --- /dev/null +++ b/coldfront/templates/common/table_sorter.html @@ -0,0 +1,8 @@ +{% if not hide_table_sorter %} + + + + + + +{% endif %} diff --git a/coldfront/templates/email/login/login_link.txt b/coldfront/templates/email/login/login_link.txt new file mode 100644 index 000000000..f42781534 --- /dev/null +++ b/coldfront/templates/email/login/login_link.txt @@ -0,0 +1,14 @@ +Dear {{ PORTAL_NAME }} user, + +You requested a link to log in to the portal. + +Below is the link, which will expire in {{ login_link_max_age_minutes }} minutes. + +{{ login_url }} + +Do not share this link with anyone else. + +Reminder: If your institution is listed in CILogon, you should use it to log in. + +Thank you, +{{ signature }} \ No newline at end of file diff --git a/coldfront/templates/email/login/login_link_ineligible.txt b/coldfront/templates/email/login/login_link_ineligible.txt new file mode 100644 index 000000000..7908590eb --- /dev/null +++ b/coldfront/templates/email/login/login_link_ineligible.txt @@ -0,0 +1,12 @@ +Dear {{ PORTAL_NAME }} user, + +You requested a link to log in to the portal. + +You are ineligible to receive a link for the following reason: + +{{ reason }} + +Please contact a system administrator if you have any questions. + +Thank you, +{{ signature }} \ No newline at end of file diff --git a/coldfront/templates/email/login/verify_login_activity.txt b/coldfront/templates/email/login/verify_login_activity.txt new file mode 100644 index 000000000..d5011f4c5 --- /dev/null +++ b/coldfront/templates/email/login/verify_login_activity.txt @@ -0,0 +1,18 @@ +Dear {{ PORTAL_NAME }} user, + +You are receiving this email because someone attempted to log in to your {{ PORTAL_NAME }} account using this email address ({{ email_address }}). + +This login attempt was blocked because this email address has not been verified. + +Below are some details about the login attempt: + +When: {{ request_time_str }} +What: {{ request_user_agent_str }} +How: {{ request_login_method_str }} + +If this was you, please confirm by clicking the link below. Doing so will verify this email address, allowing you to log in to the portal using it. If not, please contact us at {{ support_email }}. + +{{ verification_url }} + +Thank you, +{{ signature }} \ No newline at end of file diff --git a/coldfront/templates/error_with_message.html b/coldfront/templates/error_with_message.html index 19d80b5ae..1b7240c00 100644 --- a/coldfront/templates/error_with_message.html +++ b/coldfront/templates/error_with_message.html @@ -2,7 +2,7 @@ {% load static %} {% block title %} -MyBRC - Error +{{ PORTAL_NAME }} - Error {% endblock %} diff --git a/docker-compose.yml b/docker-compose.yml new file mode 100644 index 000000000..60c6d84d8 --- /dev/null +++ b/docker-compose.yml @@ -0,0 +1,60 @@ +services: + db: + image: coldfront_db:latest + build: + context: . + dockerfile: Dockerfile.db + ports: + - 5432:5432 + environment: + POSTGRES_DB: ${DB_NAME} + POSTGRES_USER: admin + POSTGRES_PASSWORD: root + LOAD_DUMP: true + volumes: + - db_data:/var/lib/postgresql/data + healthcheck: + test: ["CMD", "pg_isready", "-U", "admin", "-d", "${DB_NAME}"] + interval: 10s + timeout: 5s + retries: 5 + restart: unless-stopped + + redis: + image: redis:7 + ports: + - 6379:6379 + volumes: + - redis_data:/data + command: --requirepass root + healthcheck: + test: ["CMD", "redis-cli", "ping"] + interval: 10s + timeout: 5s + retries: 5 + restart: unless-stopped + + coldfront: + image: coldfront:latest + build: + context: . + dockerfile: Dockerfile + ports: + - ${COLDFRONT_PORT}:80 + volumes: + - ./:/vagrant/coldfront_app/coldfront/ + extra_hosts: + - "host.docker.internal:host-gateway" + depends_on: + db: + condition: service_healthy + redis: + condition: service_healthy + restart: unless-stopped + +volumes: + db_data: + external: false + redis_data: + external: false + diff --git a/requirements.txt b/requirements.txt index be92d8b90..a7570005c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -17,6 +17,7 @@ django-model-utils==4.1.1 django-phonenumber-field==5.1.0 django-picklefield==2.0 django-q==1.0.1 +django-sesame==3.1 django-settings-export==1.2.1 django-simple-history==2.12.0 django-sslserver==0.20 @@ -52,4 +53,5 @@ sqlparse==0.3.0 text-unidecode==1.3 tqdm==4.62.3 urllib3==1.24.2 +user-agents==2.2.0 wcwidth==0.1.7