diff --git a/docs/api/tom_observations/facilities.rst b/docs/api/tom_observations/facilities.rst index d98415027..a73ee6139 100644 --- a/docs/api/tom_observations/facilities.rst +++ b/docs/api/tom_observations/facilities.rst @@ -37,4 +37,16 @@ Blanco ****** .. automodule:: tom_observations.facilities.blanco - :members: \ No newline at end of file + :members: + + +******************************** +Facilities as ``INSTALLED_APPS`` +******************************** + +The following Facilities are implemented as ``INSTALLED_APPS``. As such, +each resides in its own git repository: + +* `Neil Gehrels Swift Observatory (tom_swift) `__ +* `European Southern Observatory (tom_eso) `__ +* `Liverpool Telescope (tom_lt) `__ \ No newline at end of file diff --git a/docs/observing/observation_module.rst b/docs/observing/observation_module.rst index ab2a1eaee..85b3fcf15 100644 --- a/docs/observing/observation_module.rst +++ b/docs/observing/observation_module.rst @@ -349,6 +349,39 @@ Even if the facilities you observe at are not API-accessible, you can still add them to your TOM’s airmass plots to judge what targets to observe when. +How to add User-specific credentials to your Facility +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Observation facilities typically require credentials (username and password or API key) +to interact and submit observation requests. That is, you have to "log in" to use the Facility's +API or client library. + +A TOM can be configured to provide common Facility credentials for every User of the TOM. +These are specified in the ``FACILITIES`` dictionary of ``settings.py``. In this case, the TOM +administrator would be resposible for maintaining the appropriate credentials. All observation +requests from the TOM would use those same credentials. + +While you may write your Facility to use that mechanism, a few additional steps allow your +Facility to provide for individual, User-specific credentials. Each TOM user can thus manage +their own credentials and use them to submit their own observation requests. + +In outline, your Facility must: + + 1. Create a FaciltyProfile model in your Facility's ``models.py``. (Your Facility is typically + implmemented as an ``INSTALLED_APP`` and as such has a ``models.py`` module). This is described + and demonstrated in the `tom_demoapp `__ and documented + `here `__. + + 2. The fields of your Facility's Profile model would store the required credentials (for example, + a username and password or an API-key). Sensitive data such as passwords and API-keys should be + saved in an encrypted fashion and that is described + :doc:`here <../customization/encrypted_model_fields>`. + + 3. Finally, your Facility class must access these credentials to intereact with your Facility. + An example of this can be found in the TOMToolkit + `ESOFacility `__. + + Happy developing! diff --git a/tom_common/models.py b/tom_common/models.py index b2c2f2af3..8091ed2f4 100644 --- a/tom_common/models.py +++ b/tom_common/models.py @@ -65,7 +65,8 @@ def __get__(self, instance, owner): if not isinstance(cipher, Fernet): raise AttributeError( f"A Fernet cipher must be set on the '{owner.__name__}' instance " - f"as '_cipher' to access property '{self.property_name}'." + f"as '_cipher' to access property '{self.property_name}'. " + f"Please use session_utils.get_encrypted_field() instead of direct access." ) encrypted_value = getattr(instance, self.db_field_name) diff --git a/tom_observations/facility.py b/tom_observations/facility.py index aa13dbfb0..1091d5575 100644 --- a/tom_observations/facility.py +++ b/tom_observations/facility.py @@ -1,5 +1,6 @@ from abc import ABC, abstractmethod import copy +from enum import Enum import logging import requests @@ -8,6 +9,7 @@ from django import forms from django.conf import settings from django.contrib.auth.models import Group +from django.core.exceptions import ImproperlyConfigured from django.core.files.base import ContentFile from django.utils.module_loading import import_string @@ -15,6 +17,24 @@ logger = logging.getLogger(__name__) + +class CredentialStatus(Enum): + """ + Enum representing the status of facility credentials. + + This enum is used to track the state of credentials throughout the facility lifecycle, + providing clear information about whether credentials are available, where they came from, + and any validation issues. + """ + NOT_INITIALIZED = "not_initialized" # set_user() hasn't been called yet + NO_PROFILE = "no_profile" # User has no Profile (raises ImproperlyConfigured) + PROFILE_EMPTY = "profile_empty" # Profile exists but credentials empty + USING_DEFAULTS = "using_defaults" # Using settings.FACILITIES defaults + USING_USER_CREDS = "using_user_creds" # Using user's Profile credentials + VALIDATION_FAILED_AUTH = "validation_failed_auth" # Credentials failed (401/403) + VALIDATION_FAILED_NETWORK = "validation_failed_network" # Network/server issue + + DEFAULT_FACILITY_CLASSES = [ 'tom_observations.facilities.lco.LCOFacility', 'tom_observations.facilities.gemini.GEMFacility', @@ -67,8 +87,17 @@ class BaseObservationForm(forms.Form): observation_type = forms.CharField(required=False, max_length=50, widget=forms.HiddenInput()) def __init__(self, *args, **kwargs): + # DEBUG: Log what parameters are being passed + logger.debug(f'BaseObservationForm.__init__ kwargs: {kwargs}') + + # Accept facility parameter but don't require it (for backward compatibility) + facility = kwargs.pop('facility', None) self.validation_message = 'This observation is valid.' super().__init__(*args, **kwargs) + + # Store facility reference if provided + if facility is not None: + self.facility = facility self.helper = FormHelper() if settings.TARGET_PERMISSIONS_ONLY: self.common_layout = Layout('facility', 'target_id', 'observation_type') @@ -195,10 +224,91 @@ class BaseObservationFacility(ABC): def __init__(self): self.user = None + self.credential_status = CredentialStatus.NOT_INITIALIZED def set_user(self, user): self.user = user + def _is_credential_empty(self, credential): + """ + Check if a credential is empty (None, empty string, or whitespace only). + + Args: + credential: The credential value to check + + Returns: + bool: True if credential is empty, False otherwise + """ + return credential is None or (isinstance(credential, str) and not credential.strip()) + + def _get_setting_credentials(self, facility_name, credential_keys): + """ + Safely get credentials from settings.FACILITIES. + + Args: + facility_name (str): Name of the facility in settings.FACILITIES + credential_keys (list): List of credential key names to retrieve + + Returns: + dict: Dictionary mapping credential keys to their values + + Raises: + ImproperlyConfigured: If facility or required keys are missing from settings + """ + if not hasattr(settings, 'FACILITIES') or facility_name not in settings.FACILITIES: + raise ImproperlyConfigured( + f"No configuration found for '{facility_name}' in settings.FACILITIES. " + f"Please add default credentials to settings.FACILITIES['{facility_name}']." + ) + + facility_settings = settings.FACILITIES[facility_name] + credentials = {} + + for key in credential_keys: + if key not in facility_settings: + raise ImproperlyConfigured( + f"Required credential key '{key}' not found in " + f"settings.FACILITIES['{facility_name}']. " + f"Please add '{key}' to the facility configuration." + ) + credentials[key] = facility_settings[key] + + return credentials + + def _raise_no_profile_error(self, user, facility_name): + """ + Raise ImproperlyConfigured for missing user profile. + + Args: + user: Django User instance + facility_name (str): Name of the facility + + Raises: + ImproperlyConfigured: Always raises with informative message + """ + raise ImproperlyConfigured( + f"User '{user.username}' has no {facility_name}Profile configured. " + f"Please create a {facility_name}Profile for this user in the admin interface." + ) + + def _raise_no_defaults_error(self, user, facility_name): + """ + Raise ImproperlyConfigured when default credentials are needed but missing. + + Args: + user: Django User instance + facility_name (str): Name of the facility + + Raises: + ImproperlyConfigured: Always raises with informative message + """ + raise ImproperlyConfigured( + f"User '{user.username}' has no credentials configured and no default credentials " + f"found in settings.FACILITIES['{facility_name}']. " + f"Please configure either user credentials in {facility_name}Profile or " + f"default credentials in settings." + ) + def all_data_products(self, observation_record): from tom_dataproducts.models import DataProduct products = {'saved': [], 'unsaved': []} @@ -227,7 +337,27 @@ def all_data_products(self, observation_record): def get_form(self, observation_type): """ This method takes in an observation type and returns the form type that matches it. + + Note: This method returns form classes, not instances, to support composite form creation + in ObservationCreateView. Use create_form_instance() for direct form instantiation. + """ + + def create_form_instance(self, observation_type, **kwargs): + """ + Create a form instance with facility context injected. + + The ObservationCreateView handles setting the user context on the facility instance + via set_user() in its dispatch() method. Forms receive the facility instance and + can query it for user-specific data (credentials, API clients, etc.) rather than + handling business logic themselves. + + :param observation_type: The type of observation form to create + :param kwargs: Additional keyword arguments for form instantiation + :return: Form instance configured for the observation type """ + form_class = self.get_form(observation_type) + kwargs['facility'] = self + return form_class(**kwargs) def get_form_classes_for_display(self, **kwargs): """ diff --git a/tom_observations/tests/test_credentials.py b/tom_observations/tests/test_credentials.py new file mode 100644 index 000000000..552a62fa8 --- /dev/null +++ b/tom_observations/tests/test_credentials.py @@ -0,0 +1,164 @@ +""" +Tests for credential management functionality in BaseObservationFacility. + +This test module covers: +- CredentialStatus enum +- Credential helper methods in BaseObservationFacility +- Credential validation and fallback logic +""" +from django.test import TestCase, override_settings +from django.contrib.auth.models import User +from django.core.exceptions import ImproperlyConfigured + +from tom_observations.facility import BaseObservationFacility, CredentialStatus + + +class TestFacility(BaseObservationFacility): + """Concrete test facility for testing credential management.""" + name = 'TestFacility' + + def get_form(self, observation_type): + return None + + def submit_observation(self, observation_payload): + return [1] + + def validate_observation(self, observation_payload): + return True + + def get_observation_url(self, observation_id): + return '' + + def get_terminal_observing_states(self): + return ['COMPLETED', 'FAILED'] + + def get_observing_sites(self): + return {} + + +class CredentialStatusEnumTests(TestCase): + """Test the CredentialStatus enum values and behavior.""" + + def test_enum_values_exist(self): + """Test that all expected enum values are defined.""" + self.assertEqual(CredentialStatus.NOT_INITIALIZED.value, "not_initialized") + self.assertEqual(CredentialStatus.NO_PROFILE.value, "no_profile") + self.assertEqual(CredentialStatus.PROFILE_EMPTY.value, "profile_empty") + self.assertEqual(CredentialStatus.USING_DEFAULTS.value, "using_defaults") + self.assertEqual(CredentialStatus.USING_USER_CREDS.value, "using_user_creds") + self.assertEqual(CredentialStatus.VALIDATION_FAILED_AUTH.value, "validation_failed_auth") + self.assertEqual(CredentialStatus.VALIDATION_FAILED_NETWORK.value, "validation_failed_network") + + def test_enum_membership(self): + """Test that we can check enum membership.""" + self.assertIn(CredentialStatus.NOT_INITIALIZED, CredentialStatus) + self.assertIn(CredentialStatus.USING_USER_CREDS, CredentialStatus) + + def test_enum_comparison(self): + """Test that enum values can be compared.""" + status1 = CredentialStatus.USING_USER_CREDS + status2 = CredentialStatus.USING_USER_CREDS + status3 = CredentialStatus.USING_DEFAULTS + + self.assertEqual(status1, status2) + self.assertNotEqual(status1, status3) + + def test_enum_string_representation(self): + """Test that enum has useful string representation.""" + status = CredentialStatus.USING_USER_CREDS + self.assertIn("using_user_creds", str(status.value)) + + +class BaseObservationFacilityCredentialTests(TestCase): + """Test credential-related methods in BaseObservationFacility.""" + + def setUp(self): + """Set up test fixtures.""" + self.facility = TestFacility() + self.user = User.objects.create_user(username='testuser', password='testpass') + + def test_initial_credential_status(self): + """Test that facility starts with NOT_INITIALIZED status.""" + self.assertEqual(self.facility.credential_status, CredentialStatus.NOT_INITIALIZED) + + def test_is_credential_empty_with_none(self): + """Test that None is recognized as empty credential.""" + self.assertTrue(self.facility._is_credential_empty(None)) + + def test_is_credential_empty_with_empty_string(self): + """Test that empty string is recognized as empty credential.""" + self.assertTrue(self.facility._is_credential_empty('')) + self.assertTrue(self.facility._is_credential_empty("")) + + def test_is_credential_empty_with_whitespace(self): + """Test that whitespace-only strings are recognized as empty.""" + self.assertTrue(self.facility._is_credential_empty(' ')) + self.assertTrue(self.facility._is_credential_empty('\t\n')) + + def test_is_credential_empty_with_valid_credential(self): + """Test that valid credentials are not recognized as empty.""" + self.assertFalse(self.facility._is_credential_empty('valid_username')) + self.assertFalse(self.facility._is_credential_empty('p@ssw0rd')) + + @override_settings(FACILITIES={ + 'TEST_FACILITY': { + 'username': 'default_user', + 'password': 'default_pass' + } + }) + def test_get_setting_credentials_success(self): + """Test successfully getting credentials from settings.""" + creds = self.facility._get_setting_credentials( + 'TEST_FACILITY', + ['username', 'password'] + ) + + self.assertEqual(creds['username'], 'default_user') + self.assertEqual(creds['password'], 'default_pass') + + @override_settings(FACILITIES={}) + def test_get_setting_credentials_no_facility(self): + """Test that missing facility in settings raises ImproperlyConfigured.""" + with self.assertRaises(ImproperlyConfigured) as cm: + self.facility._get_setting_credentials('MISSING_FACILITY', ['username']) + + self.assertIn('MISSING_FACILITY', str(cm.exception)) + self.assertIn('settings.FACILITIES', str(cm.exception)) + + @override_settings(FACILITIES={ + 'TEST_FACILITY': { + 'username': 'default_user' + # 'password' is missing + } + }) + def test_get_setting_credentials_missing_key(self): + """Test that missing credential key raises ImproperlyConfigured.""" + with self.assertRaises(ImproperlyConfigured) as cm: + self.facility._get_setting_credentials( + 'TEST_FACILITY', + ['username', 'password'] + ) + + self.assertIn('password', str(cm.exception)) + self.assertIn('TEST_FACILITY', str(cm.exception)) + + def test_raise_no_profile_error(self): + """Test that _raise_no_profile_error raises with proper message.""" + with self.assertRaises(ImproperlyConfigured) as cm: + self.facility._raise_no_profile_error(self.user, 'TestFacility') + + exception_message = str(cm.exception) + self.assertIn('testuser', exception_message) + self.assertIn('TestFacility', exception_message) + self.assertIn('Profile', exception_message) + + def test_raise_no_defaults_error(self): + """Test that _raise_no_defaults_error raises with proper message.""" + with self.assertRaises(ImproperlyConfigured) as cm: + self.facility._raise_no_defaults_error(self.user, 'TestFacility') + + exception_message = str(cm.exception) + self.assertIn('testuser', exception_message) + self.assertIn('TestFacility', exception_message) + self.assertIn('default credentials', exception_message) + self.assertIn('settings.FACILITIES', exception_message) diff --git a/tom_observations/views.py b/tom_observations/views.py index 140b03cdd..29c06e18d 100644 --- a/tom_observations/views.py +++ b/tom_observations/views.py @@ -1,7 +1,7 @@ from io import StringIO from urllib.parse import urlencode import logging -from typing import List +from typing import Any, List from crispy_forms.bootstrap import FormActions from crispy_forms.layout import HTML, Layout, Submit @@ -144,6 +144,22 @@ class ObservationCreateView(LoginRequiredMixin, FormView): """ template_name = 'tom_observations/observation_form.html' + def dispatch(self, request, *args, **kwargs): + """Figure out what HTTP method (GET, POST, etc) should be called + to handle this request. + + Here, we extend the method to attach the Facility class to the View so + we don't have to create more than one instances of it. So, + instantiate the facility class once and store it on the view instance + for the duration of the request-response cycle. + """ + self.facility_instance = self.get_facility_class()() + # attach the user the Faciliy after instantiation -- not in __init__() + self.facility_instance.set_user(request.user) + + # now go find the HTTP method to use... + return super().dispatch(request, *args, **kwargs) + def get_template_names(self) -> List[str]: """Override the base class method to ask the Facility if it has specified a Facility-specific template to use. If so, put it at the @@ -152,11 +168,10 @@ def get_template_names(self) -> List[str]: template_names = super().get_template_names() # get the facility_class and its template_name, if defined - facility_class = self.get_facility_class() try: - if facility_class.template_name: + if self.facility_instance.template_name: # add to front of list b/c first template will be tried first - template_names.insert(0, facility_class.template_name) + template_names.insert(0, self.facility_instance.template_name) except AttributeError: # some Facilities won't have a custom template_name defined and so # we will just use the one defined above. @@ -223,9 +238,7 @@ def get_context_data(self, **kwargs): # reloaded due to form errors, only repopulate the form that was submitted. observation_type_choices = [] initial = self.get_initial() - facility = self.get_facility_class()() - facility.set_user(self.request.user) - observation_form_classes = facility.get_form_classes_for_display(**kwargs) + observation_form_classes = self.facility_instance.get_form_classes_for_display(**kwargs) for observation_type, observation_form_class in observation_form_classes.items(): form_data = {**initial, **{'observation_type': observation_type}} # Repopulate the appropriate form with form data if the original submission was invalid @@ -233,7 +246,10 @@ def get_context_data(self, **kwargs): form_data.update(**self.request.POST.dict()) observation_form_class = type(f'Composite{observation_type}Form', (self.get_cadence_strategy_form(), observation_form_class), {}) - observation_type_choices.append((observation_type, observation_form_class(initial=form_data))) + # Pass facility parameter to form instantiation for user context + form_kwargs = {'initial': form_data, 'facility': self.facility_instance} + form_instance = observation_form_class(**form_kwargs) + observation_type_choices.append((observation_type, form_instance)) context['observation_type_choices'] = observation_type_choices # Ensure correct tab is active if submission is unsuccessful @@ -243,34 +259,33 @@ def get_context_data(self, **kwargs): context['target'] = target # allow the Facility class to add data to the context - facility_context = facility.get_facility_context_data(target=target) + facility_context = self.facility_instance.get_facility_context_data(target=target) context.update(facility_context) - context['facility_link'] = getattr(facility, 'link', '') + context['facility_link'] = getattr(self.facility_instance, 'link', '') try: - context['missing_configurations'] = ", ".join(facility.facility_settings.get_unconfigured_settings()) + missing_settings = self.facility_instance.facility_settings.get_unconfigured_settings() + context['missing_configurations'] = ", ".join(missing_settings) except AttributeError: context['missing_configurations'] = '' return context - def get_form_class(self): + def get_form_class(self, observation_type=None): """ Gets the observation form class for the facility and selected observation type in the query parameters. :returns: observation form :rtype: subclass of GenericObservationForm """ - observation_type = None - if self.request.method == 'GET': - observation_type = self.request.GET.get('observation_type') - elif self.request.method == 'POST': - observation_type = self.request.POST.get('observation_type') - facility = self.get_facility_class()() - facility.set_user(self.request.user) + if not observation_type: + if self.request.method == 'GET': + observation_type = self.request.GET.get('observation_type') + elif self.request.method == 'POST': + observation_type = self.request.POST.get('observation_type') form_class = type(f'Composite{observation_type}Form', - (facility.get_form(observation_type), self.get_cadence_strategy_form()), + (self.facility_instance.get_form(observation_type), self.get_cadence_strategy_form()), {}) return form_class @@ -281,8 +296,12 @@ def get_form(self, form_class=None): :returns: observation form :rtype: subclass of GenericObservationForm """ + if form_class is None: + form_class = self.get_form_class() + + form_kwargs = self.get_form_kwargs() try: - form = super().get_form() + form = form_class(**form_kwargs) except Exception as ex: logger.error(f"Error loading {self.get_facility()} form: {repr(ex)}") raise BadRequest(f"Error loading {self.get_facility()} form: {repr(ex)}") @@ -312,8 +331,37 @@ def get_initial(self): initial.update(self.request.GET.dict()) return initial + def get_form_kwargs(self) -> dict[str, Any]: + """Return the keyword arguments for instantiating the form. + + Here, we extend the super-class method to add the facility instance: + call the super() and then add the facility to the form kwargs. + The facility already has user context set via set_user() in dispatch(). + """ + kwargs = super().get_form_kwargs() + kwargs['facility'] = self.facility_instance + return kwargs + def post(self, request, *args, **kwargs): - form = self.get_form() + """ + Handles the POST request to the view. + + This method is responsible for processing the form submission. It + instantiates the form with the POST data and files, and then + checks if the form is valid. If the form is valid, it calls + form_valid(); otherwise, it calls form_invalid(). + + We override this method to handle a TypeError that may occur + when instantiating the form. Some forms may not accept the 'user' + keyword argument. In this case, we catch the TypeError, remove + the 'user' from the keyword arguments, and try to instantiate + the form again. + """ + form_class = self.get_form_class() + form_kwargs = self.get_form_kwargs() + + form = form_class(**form_kwargs) + if form.is_valid(): if 'validate' in request.POST: return self.form_validation_valid(form) @@ -338,10 +386,8 @@ def form_valid(self, form): :type form: subclass of GenericObservationForm """ # Submit the observation - facility = self.get_facility_class()() - facility.set_user(self.request.user) target = self.get_target() - observation_ids = facility.submit_observation(form.observation_payload()) + observation_ids = self.facility_instance.submit_observation(form.observation_payload()) records = [] for observation_id in observation_ids: @@ -349,7 +395,7 @@ def form_valid(self, form): record = ObservationRecord.objects.create( target=target, user=self.request.user, - facility=facility.name, + facility=self.facility_instance.name, parameters=form.serialize_parameters(), observation_id=observation_id )