Add EULA enforcement #671

jrlagrone · 2025-04-02T14:27:36Z

Merged and resolved conflicts between main and #609, rebased into a single commit and signed.

cecilialau6776 · 2025-04-09T16:11:12Z

@aebruno Just wanted to make sure this PR is on the radar. Thank you!

aebruno

@jrlagrone Thanks again for this PR. Here's a first pass review. Generally, adding a feature like this we prefer to default to having the feature disabled and only if a site wants to enable it should the code modify ColdFront behavior. So we should be able to merge your PR and run ColdFront and nothing should change. Then we enable it and test the new feature.

Also, it would be helpful if you could provide a summary of what this feature does and how we can go about testing it. For example, explain exactly where and what screens are effected when this feature is enabled and what data is required to make it work etc. What a user can expect to see when this is enabled and how to use it. Does this tick all the boxes in #542 ? or a subset? does it add other functionality we should know about? That will make testing much easier.

Finally, please add any new config options to our documentation here.

aebruno · 2025-04-19T13:51:30Z

coldfront/config/core.py

@@ -46,6 +50,7 @@
 SETTINGS_EXPORT += [
    'ALLOCATION_ACCOUNT_ENABLED',
    'CENTER_HELP_URL',
+    'EULA_AGREEMENT'


Missing a comma here.. should be:

'ALLOCATION_EULA_ENABLE',

This has been changed as requested.

aebruno · 2025-04-19T14:03:46Z

coldfront/config/core.py

+#------------------------------------------------------------------------------
+# Enable EULA force agreement
+#------------------------------------------------------------------------------
+EULA_AGREEMENT = ENV.bool('EULA_AGREEMENT', default=True)


Let's rename this to ALLOCATION_EULA_ENABLE and default to False here

aebruno · 2025-04-19T14:04:13Z

coldfront/config/email.py

@@ -22,3 +22,4 @@
 EMAIL_ALLOCATION_EXPIRING_NOTIFICATION_DAYS = ENV.list('EMAIL_ALLOCATION_EXPIRING_NOTIFICATION_DAYS', cast=int, default=[7, 14, 30])
 EMAIL_SIGNATURE = ENV.str('EMAIL_SIGNATURE', default='', multiline=True)
 EMAIL_ADMINS_ON_ALLOCATION_EXPIRE = ENV.bool('EMAIL_ADMINS_ON_ALLOCATION_EXPIRE', default=False)
+EMAIL_EULA_REMINDERS = ENV.bool('EMAIL_EULA_REMINDERS', default=True)


Same here, rename to EMAIL_ALLOCATION_EULA_REMINDERS and let's default to False

aebruno · 2025-04-19T14:07:24Z

coldfront/core/allocation/urls.py

@@ -8,6 +8,8 @@
         allocation_views.AllocationCreateView.as_view(), name='allocation-create'),
    path('<int:pk>/', allocation_views.AllocationDetailView.as_view(),
         name='allocation-detail'),
+     path('<int:pk>/review-eula', allocation_views.AllocationEULAView.as_view(),


This URL should only be added if ALLOCATION_EULA_ENABLE is True.

aebruno · 2025-04-19T14:08:29Z

coldfront/core/allocation/views.py

@@ -104,7 +105,18 @@ def get_context_data(self, **kwargs):
        pk = self.kwargs.get('pk')
        allocation_obj = get_object_or_404(Allocation, pk=pk)
        allocation_users = allocation_obj.allocationuser_set.exclude(
-            status__name__in=['Removed']).order_by('user__username')
+            status__name__in=['Removed',]).order_by('user__username')
+        user_in_allocation = allocation_users.filter(user=self.request.user).exists()


This block should also probably be guarded by if ALLOCATION_EULA_ENABLE:

aebruno · 2025-04-19T14:19:03Z

coldfront/core/project/forms.py

    allocation = forms.MultipleChoiceField(
-        widget=forms.CheckboxSelectMultiple(attrs={'checked': 'checked'}), required=False)
+        widget=forms.CheckboxSelectMultiple(), required=False)


Curious why was this removed?

Not sure, but I added it back

aebruno · 2025-04-19T14:20:59Z

coldfront/core/project/views.py

@@ -604,6 +612,24 @@ def post(self, request, *args, **kwargs):
                    users_already_in_project.append(ele)
            context['users_already_in_project'] = users_already_in_project

+        allocations_with_eula = {}


Should we guard here with if ALLOCATION_EULA_ENABLE:? it's likely not needed if this is not enabled.

Added guard and removed code that did not appear to be used anywhere

aebruno · 2025-04-19T14:21:45Z

coldfront/core/utils/mail.py

@@ -125,7 +125,7 @@ def send_allocation_customer_email(allocation_obj, subject, template_name, url_p
    email_receiver_list = []
    for allocation_user in allocation_users:
        if allocation_user.allocation.project.projectuser_set.get(
-                                user=allocation_user.user).enable_notifications:
+                                user=allocation_user.user):


Why was this removed? I believe we'll want to keep this?

Re-added. This was an oversight.

However, it is worth noting that EULA emails should maybe follow a different opt-in/opt-out model than general notifications. It may be worth adding, for instance a ALLOW_ALLOCATION_EULA_EMAIL_OPT_OUT option and associated logic.

jrlagrone · 2025-04-21T16:53:00Z

I think I've made the requested changes to the code (doc changes still outstanding), but I haven't tested much. We're at the end of the semester, so we're anticipating the usual chaos associated with that and not having a lot of time to work on things like this.

Instead of having to jump to various other PRs. Here's a summary of the changes:

From #542:

changing the user's allocation status to something other than 'active' as they move through this process: if the allocation has a EULA attribute on it, new users that get added to it are in the 'pending' status
sending the user an email notifying them that they need to complete this step.
sending the user an email reminding them after a given time if they don't complete it (should be configurable like the other email features)
-- Emails reminders are sent weekly. Can be turned on or off with an environment variable. The actual times can be configured in the admin panel by changing when the task(s) run
sending the user an email when they complete the process so they have proof it was done, cc'ing the managers on the project
-- ~~Emails are not currently sent on acceptance.~~ Should be implemented now
changing the user's allocation status to 'active' once they've completed the EULA review
change the user's allocation status to 'DeclinedEULA' if they decline to agree to the EULA
ensure that this does not affect any other allocations that a user might be added to at the same time. This should be a configuration option for centers that want an advanced EULA process.
-- I think this is done with the caveat of it is on a per allocation basis (eulas are a resource property.) If a center, for instance, wanted a single EULA acceptance that could be applied to all future allocations that option is not presently available.

From #609: (main changes to test)

Users added to an allocation must agree to the EULA for the associated resource, if it exists.

This adds PendingEULA and DeclinedEULA statuses to allocation users. An allocation is available to an user if the status of the allocation and the users allocation status are both Active. An allocation may be active for some users added to it, but not all added users based on whether or not they accepted the EULA (if applicable). PI's must accept the EULA to submit an allocation request, where enabled.

This attaches EULA agreements to individual allocations, but EULAs are associated with resources. This means an user has to agree to the EULA for each individual allocation on a given resource.
PIs agree to the EULA when submitting an allocation request.
users can only accept a EULA for themselves
users choice is reflected in displayed and available allocations
- check accounts are generated with appropriate outputs, e.g. try DEBUG=True PLUGIN_SLURM=True coldfront slurm_dump and verify that output matches expectations
- Note, there are subtleties -- Users with permissions to see all allocations (like admin or director views) will typically see the status of the allocation. This may not reflect their ability to use the allocation. I.e. some pages must be viewed as user and not a superuser to display relevant information.
users who reject a eula can be removed and re-added to an allocation by a PI/Manager. They will have a new opportunity to accept. Admins can manually reset the check by changing DeclinedEULA -> PendingEULA status.
resources without a EULA should not require agreements
setting EULA_AGREEMENT = True should enable eula enforcement. Switching that on/off may result in some unexpected behaviors though.
Email reminders work.

jrlagrone · 2025-04-21T16:55:09Z

I'll try to get to the doc updates and adding an option to send verification of acceptance emails as I have time (hopefully by the end of the week)

aebruno · 2025-04-22T00:01:43Z

I'll try to get to the doc updates and adding an option to send verification of acceptance emails as I have time (hopefully by the end of the week)

@jrlagrone This is great. Thanks so much for the extra details. We'll start reviewing this and get you any additional feedback. Thanks again.

commit ede2d714d20d58750bd50ca75732a6ac71cb54a8 Merge: 2fa1ee2 8e555c1 Author: John LaGrone <jlagrone@gmail.com> Date: Thu Apr 24 10:44:57 2025 -0500 Merge branch 'main' into add_eula_enforcement Signed-off-by: John LaGrone <jlagrone@gmail.com> commit 2fa1ee2 Author: John LaGrone <jlagrone@gmail.com> Date: Thu Apr 24 10:12:22 2025 -0500 add option to include eula in email. fix link Signed-off-by: John LaGrone <jlagrone@gmail.com> commit 9a50502 Author: John LaGrone <jlagrone@gmail.com> Date: Thu Apr 24 10:02:39 2025 -0500 add cc option to send email template Signed-off-by: John LaGrone <jlagrone@gmail.com> commit 123ec0f Author: John LaGrone <jlagrone@gmail.com> Date: Thu Apr 24 09:55:35 2025 -0500 put appropriate blocks in for getting user status needed commit 4743341 Author: John LaGrone <jlagrone@gmail.com> Date: Thu Apr 24 09:52:07 2025 -0500 add user allocation status to portal home commit 61d4f25 Author: John LaGrone <jlagrone@gmail.com> Date: Thu Apr 24 09:44:59 2025 -0500 show pending eula in allocation bar on portal home commit 7c62ff0 Author: John LaGrone <jlagrone@gmail.com> Date: Thu Apr 24 09:36:23 2025 -0500 fix default values Signed-off-by: John LaGrone <jlagrone@gmail.com> commit c231c39 Author: John LaGrone <jlagrone@gmail.com> Date: Wed Apr 23 14:24:18 2025 -0500 add docs for new config variables commit 2c62474 Author: John LaGrone <jlagrone@gmail.com> Date: Wed Apr 23 11:30:42 2025 -0500 add templates for eula acceptance / decline commit 2481a4d Author: John LaGrone <jlagrone@gmail.com> Date: Wed Apr 23 11:22:00 2025 -0500 add logic to send eula accepted / declined messages commit 63a3965 Author: John LaGrone <jlagrone@gmail.com> Date: Wed Apr 23 10:51:21 2025 -0500 update allocation reminders to to allow for email notification settings to be ignored and only send to active project users commit 7565fe2 Author: John LaGrone <jlagrone@gmail.com> Date: Mon Apr 21 11:02:52 2025 -0500 add back checked default Signed-off-by: John LaGrone <jlagrone@gmail.com> commit 9d8861e Author: John LaGrone <jlagrone@gmail.com> Date: Mon Apr 21 10:57:46 2025 -0500 set allocation enable to false by default commit b9d74fb Author: John LaGrone <jlagrone@gmail.com> Date: Mon Apr 21 10:57:02 2025 -0500 set email reminders to false by default commit f017b29 Author: John LaGrone <jlagrone@gmail.com> Date: Mon Apr 21 10:20:54 2025 -0500 remove unused code Signed-off-by: John LaGrone <jlagrone@gmail.com> commit 4eb9375 Author: John LaGrone <jlagrone@gmail.com> Date: Mon Apr 21 09:22:15 2025 -0500 conditionally include eula url Signed-off-by: John LaGrone <jlagrone@gmail.com> commit 32c43bc Author: John LaGrone <jlagrone@gmail.com> Date: Mon Apr 21 09:19:57 2025 -0500 add missing , Signed-off-by: John LaGrone <jlagrone@gmail.com> commit 8a95483 Author: John LaGrone <jlagrone@gmail.com> Date: Mon Apr 21 09:19:01 2025 -0500 re-add email notification check Signed-off-by: John LaGrone <jlagrone@gmail.com> commit c2f27e4 Author: John LaGrone <jlagrone@gmail.com> Date: Mon Apr 21 09:15:00 2025 -0500 Only check some eula stuff if enabled Signed-off-by: John LaGrone <jlagrone@gmail.com> commit bed8aa7 Author: John LaGrone <jlagrone@gmail.com> Date: Mon Apr 21 08:58:17 2025 -0500 Change config names for EULAs Signed-off-by: John LaGrone <jlagrone@gmail.com> commit db0283e Author: John LaGrone <jlagrone@gmail.com> Date: Wed Apr 2 08:59:54 2025 -0500 activate user when they accept eula commit 4e1ddad Author: John LaGrone <jlagrone@gmail.com> Date: Wed Apr 2 08:58:11 2025 -0500 only send activate if eula is active commit 731d52d Merge: a4d5ac5 7aeded9 Author: John LaGrone <jlagrone@gmail.com> Date: Wed Apr 2 08:53:21 2025 -0500 Merge branch 'main' into add_eula_enforcement commit a4d5ac5 Author: John LaGrone <jlagrone@gmail.com> Date: Fri May 10 13:12:19 2024 -0500 remove site specific .gitignore commit 5545cd3 Author: John LaGrone <jlagrone@gmail.com> Date: Fri May 10 12:50:01 2024 -0500 remove white space commit 8b458f9 Author: John LaGrone <jlagrone@gmail.com> Date: Tue Apr 30 10:40:47 2024 -0500 only get user status if user is in allocation commit b248fed Author: John <jlagrone@gmail.com> Date: Fri Apr 12 10:57:16 2024 -0500 remove import of unused form commit 7980662 Author: John <jlagrone@gmail.com> Date: Fri Apr 12 10:42:28 2024 -0500 fix some links and remove unused form commit 0a43c0d Author: John <jlagrone@gmail.com> Date: Fri Apr 12 10:32:53 2024 -0500 finish moving EULA acceptance /review to it's own page commit 3546808 Author: John <jlagrone@gmail.com> Date: Fri Apr 12 08:49:54 2024 -0500 start breaking out some eula logic to it's own page commit 8556e5a Author: John <jlagrone@gmail.com> Date: Fri Apr 12 07:27:17 2024 -0500 remove debugging / handle reject eula better commit 1d55c0b Author: John <jlagrone@gmail.com> Date: Thu Apr 11 17:53:09 2024 -0500 remove debug prints commit 0b363b1 Author: John <jlagrone@gmail.com> Date: Thu Apr 11 17:50:39 2024 -0500 add more helpful messaging and links for pending allocations commit 3d44d76 Author: John <jlagrone@gmail.com> Date: Thu Apr 11 16:24:46 2024 -0500 remove another instance of get_eula def commit bf9ee30 Author: John <jlagrone@gmail.com> Date: Thu Apr 11 15:53:22 2024 -0500 default new users on allocations to pending eula not active commit a6334f0 Author: John <jlagrone@gmail.com> Date: Thu Apr 11 14:59:01 2024 -0500 remove unused pending allocations commit dca5baa Author: John <jlagrone@gmail.com> Date: Thu Apr 11 14:45:33 2024 -0500 refactor to not redfine the same function many times commit ca141eb Author: John <jlagrone@gmail.com> Date: Thu Apr 11 14:07:45 2024 -0500 don't default to users being active. They may not have agreed to the EULA commit da0d7f5 Author: John <jlagrone@gmail.com> Date: Thu Apr 11 13:40:22 2024 -0500 remove more PI prompts to accept user EULA commit 6a6ed57 Author: John <jlagrone@gmail.com> Date: Thu Apr 11 13:36:27 2024 -0500 don't ask a PI to accept EULA for user commit c7caf60 Author: John <jlagrone@gmail.com> Date: Thu Apr 11 13:21:35 2024 -0500 fix user in allocation check commit 7be2046 Author: John <jlagrone@gmail.com> Date: Thu Apr 11 10:13:43 2024 -0500 handled pending eula differently than other cases commit d35f8f3 Author: John <jlagrone@gmail.com> Date: Wed Apr 10 13:06:20 2024 -0500 only show eula approval to current user commit a79ba7f Author: John <jlagrone@gmail.com> Date: Wed Apr 10 10:01:42 2024 -0500 change denied -> deniedeula for clarity commit 679e261 Author: John <jlagrone@gmail.com> Date: Wed Apr 10 09:58:13 2024 -0500 changing pending -> pendingeula for clarity commit db0f320 Author: John <jlagrone@gmail.com> Date: Wed Apr 10 09:10:55 2024 -0500 add EULA_AGREEMENT to resource view commit 76d1d98 Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Wed Jul 5 14:53:14 2023 -0400 fixed html side of bug commit 6627fe8 Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Wed Jul 5 14:52:24 2023 -0400 fixed bug in eula display commit 709e8c1 Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Tue Jun 27 11:55:26 2023 -0400 code cleanup commit 67f62a6 Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Mon Jun 26 11:48:59 2023 -0400 fixed checkbox functionality commit 131b361 Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Sun Jun 18 14:21:20 2023 -0400 fixed display for allocations without eula commit a78cb0f Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Sun Jun 18 14:11:58 2023 -0400 fixed functionality when adding 0 users to allocation commit ae2cb8f Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Sun Jun 18 14:01:09 2023 -0400 fixed formatting and removed comments commit 3f47066 Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Sun Jun 18 13:58:51 2023 -0400 done commit d23c4be Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Sun Jun 18 12:47:03 2023 -0400 commit with comments commit 6ed7dd4 Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Wed Jun 14 18:01:44 2023 -0400 added ability to prompt users when adding to allocation and have eula show up on allocation detail page commit 8aec097 Author: Ria Gupta <74742605+rg663@users.noreply.github.com> Date: Thu Jul 27 14:13:20 2023 -0400 Update add_allocation_defaults.py commit 40beb99 Author: Ria Gupta <74742605+rg663@users.noreply.github.com> Date: Thu Jul 27 14:12:46 2023 -0400 Update add_allocation_defaults.py commit fcc451c Author: Ria Gupta <74742605+rg663@users.noreply.github.com> Date: Tue Jul 25 15:41:53 2023 -0400 Update .gitignore commit d314048 Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Thu Jul 13 15:18:41 2023 -0400 refactored eula reminder emails commit 5a32cf4 Author: Ria Gupta <74742605+rg663@users.noreply.github.com> Date: Fri Jul 7 20:28:56 2023 -0400 Update allocation_agree_to_eula.txt commit 2c1b66e Author: Ria Gupta <74742605+rg663@users.noreply.github.com> Date: Fri Jul 7 20:15:03 2023 -0400 Update add_scheduled_tasks.py commit 6d40263 Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Fri Jul 7 20:03:15 2023 -0400 small fixes commit 850a223 Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Fri Jul 7 14:41:01 2023 -0400 config fix commit e023f7e Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Fri Jul 7 14:32:51 2023 -0400 email updates commit 99825f4 Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Fri Jul 7 14:21:46 2023 -0400 emails work commit a8c1667 Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Thu Jul 6 14:03:07 2023 -0400 bug fix commit 9a3dd2d Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Thu Jul 6 13:58:27 2023 -0400 fixed functionality for when config is false commit 4a8a14f Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Wed Jul 5 20:47:53 2023 -0400 email functionality almost done commit edd4d13 Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Wed Jul 5 19:03:51 2023 -0400 functionality near complete commit 2701732 Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Wed Jul 5 18:56:25 2023 -0400 email functionality close to working commit 913594b Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Wed Jul 5 17:20:46 2023 -0400 code cleanup commit 2e9c468 Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Wed Jul 5 16:25:23 2023 -0400 fixed email commit 019d7a5 Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Wed Jul 5 16:05:27 2023 -0400 small change commit e21f12a Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Wed Jul 5 15:23:09 2023 -0400 users can now accept/decline eulas commit ba9ccf0 Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Wed Jul 5 14:51:32 2023 -0400 working on agree button commit a7f6c7c Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Fri Jun 30 22:04:32 2023 -0400 started selection functionality commit 1ca566d Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Thu Jun 29 15:34:18 2023 -0400 enabling checkbox functionality commit 6f26dcf Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Thu Jun 29 13:54:19 2023 -0400 working through user statuses commit 0c361d2 Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Tue Jun 27 17:33:06 2023 -0400 add config variable check to each part commit 47f2f3f Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Tue Jun 27 17:18:17 2023 -0400 line deletion commit d9f6bf3 Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Tue Jun 27 17:17:24 2023 -0400 added email functionality commit 90d9b5e Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Tue Jun 27 14:59:23 2023 -0400 made it so user status is pending when allocation has eula commit cb60d7c Author: riathetechie@gmail.com <74742605+rg663@users.noreply.github.com> Date: Tue Jun 27 12:34:14 2023 -0400 started eula user functionality Signed-off-by: John LaGrone <jlagrone@gmail.com>

jrlagrone · 2025-04-24T16:05:56Z

@aebruno The docs are updated and added some confirmation email options.

Specifically:

Emails can optionally be sent to users when they accept or decline a EULA by setting EMAIL_ALLOCATION_EULA_CONFIRMATIONS
- Project managers can optionally be cc'd by enabling EMAIL_ALLOCATION_EULA_CONFIRMATIONS_CC_MANAGERS
- User email opt out settings can be ignored for EULA related emails by setting EMAIL_ALLOCATION_EULA_IGNORE_OPT_OUT
- The EULA can optionally be included in the acceptance confirmation emails (if enabled) with EMAIL_ALLOCATION_EULA_INCLUDE_ACCEPTED_EULA

Some other notes / potential TODOs:

rerunning add_allocation_defaults would be necessary if this is turned on for an existing deployment. I believe this is safe because everything is get_or_create based at least in that call (whereas rerunning initial_setup would likely not be safe.)
We've been using this (with other site-specific things attached) and some of are observations are:
- We have disabled the ability to decline EULAs (they can be accepted or left unanswered). The main reason for this was support burdens -- as it is implemented manual intervention from either a manager (remove and re-add user to allocation) or admin (change user allocation status) is needed to give someone a second opportunity to accept. That would proably be a good config option.
- There is a desire to conditionally rename "EULA" in some places, e.g. use "Terms and Conditions" or "Usage Guidelines", etc. where appropriate. We've essentially hard-coded these as needed, but it would probably make sense to add a resource attribute like eula-name or something to that end for display purposes.
I'm unsure how this will behave on allocations with multiple attached resources.

dsajdak · 2025-05-05T17:44:34Z

@jrlagrone THANK YOU for picking up this feature and running with it! We really appreciate the development and improvements you've made. I think it will be extremely valuable to many centers. I was going to wait until I had tested everything to provide feedback but as I'm working my way through testing I'm struggling on a few things and thought I should ask about them now rather than waiting. The first issue I'm having is that I'm not seeing initial emails go out to users that need to sign the EULA. How I'm testing is, I create an allocation request for a resource with a EULA attribute. I add a few users to the allocation and see that their allocation status is "PendingEULA" as expected. However, those users aren't getting an email to tell them to go review the EULA. Once they login and agree or decline the EULA, those confirmation emails are coming through and all the different env settings you have defined work for me. I'm just not getting that initial email. I see that you have a setting to send reminder emails every 7 days and a suggestion to manually force those emails to go out using the django admin interface. The two scheduled tasks in the django queue are coldfront.core.allocation.tasks.update_statuses and coldfront.core.allocation.tasks.send_expiry_emails but running those doesn't send out the reminder emails. Should there be a new task for this feature? Are the reminder emails separate from the email they should get on allocation creation?

Another thing I wanted to bring to your attention is your logic (or maybe this was ours from our original PR?) that the emails going out shouldn't go to a user that is a PI. Unfortunately, this may cause problems for collaborative projects where the "PI" of the project requests an allocation for a resource with a EULA and then adds other users to the allocation, some of which may also be PIs. Is there a different check we can do to get around this? Something like: don't send the EULA email to the user requesting the allocation. You can probably tell I'm not a programmer - this may not be possible.

jrlagrone · 2025-05-05T20:19:40Z

@dsajdak I think I see all the issues you pointed out.

I see that you have a setting to send reminder emails every 7 days and a suggestion to manually force those emails to go out using the django admin interface. The two scheduled tasks in the django queue are ...

That probably needs to be implemented as some sort of migration. It is added here:

coldfront/coldfront/core/utils/management/commands/add_scheduled_tasks.py

Lines 29 to 32 in 527d1cd

    
           if ALLOCATION_EULA_ENABLE and EMAIL_ALLOCATION_EULA_REMINDERS: 
        
               schedule('coldfront.core.allocation.tasks.send_eula_reminders', 
        
                       schedule_type=Schedule.WEEKLY, 
        
                       next_run=date)

but that would only get called with the initial setup stuff. That's probably mostly documentation, but it is not reasonable (or safe) to rerun the initial setup on a live system.

For the rest, adding users to an allocation (from an allocation page) should send emails with this:

coldfront/coldfront/core/allocation/views.py

Lines 811 to 821 in 527d1cd

    
               if ALLOCATION_EULA_ENABLE and not user_obj.userprofile.is_pi and allocation_obj.get_eula(): 
        
                   allocation_user_obj.status = allocation_user_pending_status_choice 
        
                   send_email_template(f'Agree to EULA for {allocation_obj.get_parent_resource.__str__()}', 'email/allocation_agree_to_eula.txt', {"resource": allocation_obj.get_parent_resource, "url": build_link(reverse('allocation-review-eula', kwargs={'pk': allocation_obj.pk}), domain_url=get_domain_url(self.request))}, self.request.user.email, [user_obj]) 
        
               else: 
        
                   allocation_user_obj.status = allocation_user_active_status_choice 
        
               allocation_user_obj.save() 
        
           else: 
        
               if ALLOCATION_EULA_ENABLE and not user_obj.userprofile.is_pi and allocation_obj.get_eula(): 
        
                   allocation_user_obj = AllocationUser.objects.create( 
        
                   allocation=allocation_obj, user=user_obj, status=allocation_user_pending_status_choice) 
        
                   send_email_template(f'Agree to EULA for {allocation_obj.get_parent_resource.__str__()}', 'email/allocation_agree_to_eula.txt', {"resource": allocation_obj.get_parent_resource, "url": build_link(reverse('allocation-review-eula', kwargs={'pk': allocation_obj.pk}), domain_url=get_domain_url(self.request))}, self.request.user.email, [user_obj])

It looks like it's (potentially) missing from adding users via project pages. I see in our live instance I appear to have just grouped it together with the existing allocation activated and similar emails, so I'll try to untangle that. Maybe the flow should go like (assuming EULAs are enabled):

Someone with PI/Manager status submits an allocations request.
- This user has accepted the EULA in the request
- (optional) email is sent to requestor confirming acceptance of EULA (templates exist, but this email is not sent now)
- PI / project owner auto-added to the allocation even if they are not the requestor, so they may not have accepted the EULA.
PI / Manager adds other users to the allocation.
- These users need to agree to EULA before they can be 'Active'.
- Emails about accepting the EULA
  - On being added the the allocation (may be before allocation is reviewed) (optional? -- seems partially implemented -- only catching users from allocation add user form)
  - On being added to the allocation and the allocation is approved / active (not implemented -- suggest conditionally adding to the existing allocation activated email?)
Reminders sent
- Weekly / configurable reminders (** implemented, but needs instructions / method to enable on existing installs **)

I'll try to make those changes soon.

dsajdak · 2025-05-05T20:41:49Z

@jrlagrone thanks for confirming these things. I just tried and if I add a new user to an allocation, then I get the email. If I create a new allocation request and add project users to it, that's when I'm not getting the email. Good to know.

When I was testing, I was thinking I might not get emails until the allocation is activated. That would make sense from our use case. I wouldn't want our users having to login to review the EULA if their allocation wasn't going to get activated. But I guess other centers might not agree with that.

For allocation renewals, is there a mechanism to force the re-review of the EULA? It didn't look like it to me so I was going to list that as a future enhancement, but just wanted to make sure I wasn't missing something. I think our users would hate us if we did that but I think some centers want to force that.

jrlagrone · 2025-05-06T14:21:38Z

@dsajdak That shouldn't be difficult to add re-review for EULAs.

Locally, we just removed the "Decline Eula" button so users can either accept it or ignore it. Our thought was it doesn't really make sense to let people change, but they still have to accept it. Something like that should be easy to add as an option.

Alternatively, the status could just stay in "PendingEULA" until it is agreed to. That's maybe a little more subtle, but I think it could function essentially the same way.

I'll look at options for when the EULA emails are sent to.

jrlagrone mentioned this pull request Apr 2, 2025

Add eula enforcement #609

Closed

8 tasks

aebruno requested changes Apr 19, 2025

View reviewed changes

aebruno changed the title ~~Rebased add EULAs~~ Add EULA enforcement Apr 19, 2025

aebruno requested a review from dsajdak April 22, 2025 00:30

jrlagrone force-pushed the rebased_add_eulas branch from 69ca5b8 to 8e965aa Compare April 24, 2025 15:40

jrlagrone force-pushed the rebased_add_eulas branch from 8e965aa to 527d1cd Compare April 24, 2025 15:46

jrlagrone requested a review from aebruno April 24, 2025 19:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add EULA enforcement #671

Add EULA enforcement #671

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Add EULA enforcement #671

Are you sure you want to change the base?

Add EULA enforcement #671

Uh oh!

Conversation

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!