diff --git a/.gitignore b/.gitignore
index 89723417c..d722ebed6 100644
--- a/.gitignore
+++ b/.gitignore
@@ -160,7 +160,7 @@ use_dev_supervisor.txt
.idea/*
*.unison.tmp
*.#
-/awx/ui/.ui-built
+/ascender/ui/.ui-built
/_build/
/_build_kube_dev/
/Dockerfile
diff --git a/Makefile b/Makefile
index 728680512..c7e0889f3 100644
--- a/Makefile
+++ b/Makefile
@@ -25,8 +25,6 @@ LDAP ?= false
VAULT ?= false
# If set to true docker-compose will also start a hashicorp vault instance with TLS enabled
VAULT_TLS ?= false
-# If set to true docker-compose will also start a tacacs+ instance
-TACACS ?= false
EDITABLE_DEPENDENCIES ?= false
@@ -519,7 +517,6 @@ docker-compose-sources: .git/hooks/pre-commit
-e enable_ldap=$(LDAP) \
-e enable_vault=$(VAULT) \
-e vault_tls=$(VAULT_TLS) \
- -e enable_tacacs=$(TACACS) \
-e install_editable_dependencies=$(EDITABLE_DEPENDENCIES) \
$(EXTRA_SOURCES_ANSIBLE_OPTS)
diff --git a/ascender/api/conf.py b/ascender/api/conf.py
index fea1de262..de133d193 100644
--- a/ascender/api/conf.py
+++ b/ascender/api/conf.py
@@ -78,7 +78,7 @@
label=_('Allow External Users to Create OAuth2 Tokens'),
help_text=_(
'For security reasons, users from external auth providers (LDAP, SAML, '
- 'SSO, Radius, and others) are not allowed to create OAuth2 tokens. '
+ 'SSO, and others) are not allowed to create OAuth2 tokens. '
'To change this behavior, enable this setting. Existing tokens will '
'not be deleted when this setting is toggled off.'
),
diff --git a/ascender/api/serializers/base.py b/ascender/api/serializers/base.py
index 4d0328c93..1e3b800c6 100644
--- a/ascender/api/serializers/base.py
+++ b/ascender/api/serializers/base.py
@@ -28,7 +28,7 @@
from rest_framework import validators
from polymorphic.models import PolymorphicModel
from ascender.dab.lib.utils.models import get_type_for_model
-from ascender.main.access import get_user_capabilities
+from ascender.main.access import UnifiedJobAccess, get_user_capabilities
from ascender.main.models import (
AdHocCommand,
InventorySource,
@@ -458,6 +458,8 @@ def _obj_capability_dict(self, obj):
self.context['capability_map'] = prefetch_page_capabilities(model, qs, prefetch_list, view.request.user)
if obj.id in self.context['capability_map']:
capabilities_cache = self.context['capability_map'][obj.id]
+ if self.parent:
+ capabilities_cache = {**capabilities_cache, **self.get_page_capabilities(obj)}
return get_user_capabilities(
view.request.user, obj, method_list=self.show_capabilities, parent_obj=parent_obj, capabilities_cache=capabilities_cache
)
@@ -465,6 +467,27 @@ def _obj_capability_dict(self, obj):
# Contextual information to produce user_capabilities doesn't exist
return {}
+ def get_page_capabilities(self, obj):
+ """
+ Capabilities answered for a whole page of a list view at once.
+
+ capabilities_prefetch covers what can be said as "the user holds this
+ role on the row or on a related row". A serializer whose capabilities
+ need more than that overrides this to answer them for the page in a
+ batch, on the first row, and hands each row its share. Only called
+ when the serializer is a row of a list, so self.parent.instance is the
+ page. The answers are used exactly as the capabilities_prefetch ones:
+ get_user_capabilities takes them in place of its per row check.
+
+ Args:
+ obj: The row being serialized.
+
+ Returns:
+ dict: Display method to bool, for the capabilities answered in
+ bulk for this row; empty when the per row path should decide.
+ """
+ return {}
+
def get_created(self, obj):
if obj is None:
return None
@@ -713,8 +736,15 @@ def get_summary_fields(self, obj):
return summary_fields
+# Context key under which a page of unified jobs keeps the cancel capability of
+# its rows, computed once for the page by UnifiedJobSerializer.
+CANCEL_ANSWERS = 'unified_job_cancel_answers'
+
+
class UnifiedJobSerializer(BaseSerializer):
- show_capabilities = ['start', 'delete']
+ # cancel is computed with the job's status as well as the user's role, so
+ # the interface can offer Cancel only where the api would accept it.
+ show_capabilities = ['start', 'delete', 'cancel']
event_processing_finished = serializers.BooleanField(
help_text=_('Indicates whether all of the events generated by this unified job have been saved to the database.'), read_only=True
)
@@ -802,6 +832,38 @@ def get_summary_fields(self, obj):
return summary_fields
+ def get_page_capabilities(self, obj):
+ """
+ The cancel capability of this row, answered for the whole page at once.
+
+ For anyone but a superuser, cancel on a running or pending job reads
+ the job's template, project or inventory and asks a role question, a
+ few queries for every such row. The first row asks
+ UnifiedJobAccess.get_cancel_capabilities for the whole page and leaves
+ the answers in the context, shared by the polymorphic row serializers
+ of /api/v2/unified_jobs/ as much as by the rows of /api/v2/jobs/. A row
+ it did not answer is checked on its own, as it would be without this.
+
+ Args:
+ obj: The unified job being serialized.
+
+ Returns:
+ dict: {'cancel': bool} when the page answered this row, else empty.
+ """
+ if 'cancel' not in self.show_capabilities:
+ return {}
+ answers = self.context.get(CANCEL_ANSWERS)
+ if answers is None:
+ request = self.context.get('request', None)
+ page = getattr(self.parent, 'instance', None)
+ if request is None or page is None:
+ return {}
+ answers = UnifiedJobAccess(request.user).get_cancel_capabilities(list(page))
+ self.context[CANCEL_ANSWERS] = answers
+ if obj.pk in answers:
+ return {'cancel': answers[obj.pk]}
+ return {}
+
def get_sub_serializer(self, obj):
from ascender.api.serializers.ad_hoc_command import AdHocCommandSerializer
from ascender.api.serializers.inventory import InventoryUpdateSerializer
@@ -925,6 +987,10 @@ def to_representation(self, obj):
serializer_class = self.get_sub_serializer(obj)
if serializer_class:
serializer = serializer_class(instance=obj, context=self.context)
+ # The row serializer needs the page for the capabilities it answers
+ # in bulk (see get_page_capabilities), as UnifiedJobSerializer does.
+ if self.parent:
+ serializer.parent = self.parent
ret = serializer.to_representation(obj)
else:
ret = super(UnifiedJobListSerializer, self).to_representation(obj)
diff --git a/ascender/api/serializers/label.py b/ascender/api/serializers/label.py
index 1c7f5533f..43bcd80ba 100644
--- a/ascender/api/serializers/label.py
+++ b/ascender/api/serializers/label.py
@@ -33,6 +33,14 @@ def get_summary_fields(self, obj):
class LabelSerializer(BaseSerializer):
+ # edit and delete are both an admin of the label's organization, which the
+ # interface reads to offer the actions. delete does not say whether the
+ # label is in use: that is a state the DELETE answers with 409, and working
+ # it out here would cost a query for every label on a page. The admin check
+ # is prefetched for the page, and delete copies edit.
+ show_capabilities = ['edit', 'delete']
+ capabilities_prefetch = [{'edit': 'organization.admin'}]
+
class Meta:
model = Label
fields = ('*', '-description', 'organization')
diff --git a/ascender/api/serializers/workflow.py b/ascender/api/serializers/workflow.py
index 644d6bb85..0a9596aa8 100644
--- a/ascender/api/serializers/workflow.py
+++ b/ascender/api/serializers/workflow.py
@@ -9,12 +9,16 @@
"""
from datetime import timedelta
+from django.db.models import prefetch_related_objects
+from django.db.models.manager import BaseManager
from django.utils.translation import gettext_lazy as _
from rest_framework import serializers
+from ascender.main.access import WorkflowApprovalAccess, WorkflowJobAccess, get_user_capabilities
from ascender.main.models import (
Inventory,
JobTemplate,
Label,
+ UnifiedJob,
WorkflowApproval,
WorkflowApprovalTemplate,
WorkflowApprovalVote,
@@ -223,20 +227,142 @@ class Meta:
fields = []
+# ----------------------------------------------------------------------------
+# Workflow approvals
+# ----------------------------------------------------------------------------
+
+# Context keys under which a page of approvals carries its permission answers,
+# computed once for the page by WorkflowApprovalPageSerializer.
+APPROVE_OR_DENY_ANSWERS = 'workflow_approval_approve_or_deny_answers'
+CANCEL_WORKFLOW_ANSWERS = 'workflow_approval_cancel_workflow_answers'
+
+
+def waiting_workflow_job(approval):
+ """
+ The workflow job an approval waits in, if it waits in one.
+
+ Only an approval a workflow spawned waits in a workflow job, and it reaches
+ that job through the workflow job node that ran it. Either link can be
+ missing, a node that no longer exists or one with no workflow job, and both
+ read as the approval waiting in nothing, which is what the two callers want.
+
+ Args:
+ approval: The workflow approval to look from.
+
+ Returns:
+ WorkflowJob or None: The workflow job, or None when the approval was
+ not spawned by a workflow or the link to its job no longer exists.
+ """
+ if not approval.spawned_by_workflow:
+ return None
+ try:
+ return approval.unified_job_node.workflow_job
+ except UnifiedJob.unified_job_node.RelatedObjectDoesNotExist:
+ return None
+
+
+class WorkflowApprovalPageSerializer(serializers.ListSerializer):
+ """
+ Serializes a page of approvals with its permission checks done once.
+
+ can_approve_or_deny and can_cancel_workflow each ask role questions about
+ every approval, several queries a row for anyone but a superuser. Before
+ the rows are serialized this asks the access classes for the whole page at
+ once and leaves the answers in the serializer context, where the row
+ methods pick them up. A row the access classes did not answer is checked
+ on its own, as it would be without this.
+
+ BaseSerializer already prefetches user_capabilities for a page through
+ capabilities_prefetch, but that mechanism can only express "the user holds
+ this role on the row or a related row", and neither answer here fits it:
+ approving reads votes and quorum, and canceling asks about another object.
+ """
+
+ # What every row of a page reads besides its own columns: its template for
+ # the related links and summary fields, and the node, workflow job and
+ # workflow template behind can_approve_or_deny and can_cancel_workflow.
+ PAGE_PREFETCH = ('workflow_approval_template', 'unified_job_node__workflow_job__unified_job_template')
+
+ @staticmethod
+ def prefetch_page(approvals):
+ """
+ Load what the rows of a page read, a few queries for the whole page.
+
+ This lives here rather than in WorkflowApprovalAccess.prefetch_related
+ because that queryset also serves the detail, approve and deny views,
+ which read one approval and would pay for every prefetch up front.
+
+ An approval's unified_job_template and workflow_approval_template are
+ two foreign keys to the same row, the second one typed, and the base
+ manager of UnifiedJobTemplate is polymorphic, so reading the first
+ yields the same WorkflowApprovalTemplate the second holds. It is read
+ once, through workflow_approval_template, and handed to the other.
+
+ Args:
+ approvals: The approvals of the page, as a list. They are changed
+ in place: the related objects land in their caches.
+ """
+ prefetch_related_objects(approvals, *WorkflowApprovalPageSerializer.PAGE_PREFETCH)
+ unified_job_template = WorkflowApproval.unified_job_template.field
+ for approval in approvals:
+ if unified_job_template.is_cached(approval) or approval.unified_job_template_id is None:
+ continue
+ template = approval.workflow_approval_template
+ if template is not None and template.pk == approval.unified_job_template_id:
+ unified_job_template.set_cached_value(approval, template)
+
+ def to_representation(self, data):
+ """
+ Answer the page's permission checks, then serialize its rows.
+
+ Args:
+ data: The approvals of the page, as a list, queryset or manager.
+
+ Returns:
+ list: One serialized approval per row, as ListSerializer gives.
+ """
+ iterable = data.all() if isinstance(data, BaseManager) else data
+ approvals = list(iterable)
+ self.prefetch_page(approvals)
+ request = self.context.get('request', None)
+ if request is not None:
+ # Step 1: can_approve_or_deny, keyed by approval.
+ self.context[APPROVE_OR_DENY_ANSWERS] = WorkflowApprovalAccess(request.user).can_approve_or_deny_many(approvals)
+ # Step 2: the cancel capability of the workflow jobs the approvals
+ # wait in, keyed by workflow job. Only approvals a workflow spawned
+ # are asked about, as get_can_cancel_workflow only asks for those.
+ workflow_jobs = {}
+ for approval in approvals:
+ workflow_job = waiting_workflow_job(approval)
+ if workflow_job is not None:
+ workflow_jobs[workflow_job.pk] = workflow_job
+ self.context[CANCEL_WORKFLOW_ANSWERS] = WorkflowJobAccess(request.user).get_cancel_capabilities(workflow_jobs.values())
+ return super().to_representation(approvals)
+
+
class WorkflowApprovalSerializer(UnifiedJobSerializer):
can_approve_or_deny = serializers.SerializerMethodField()
+ can_cancel_workflow = serializers.SerializerMethodField()
approval_expiration = serializers.SerializerMethodField()
timed_out = serializers.ReadOnlyField()
approvals_received = serializers.SerializerMethodField()
user_has_voted = serializers.SerializerMethodField()
+ # An approval is stopped by approving or denying it, or by canceling the
+ # workflow job it waits in (reported as can_cancel_workflow). It has no
+ # cancel endpoint of its own, so the inherited 'cancel' capability would
+ # offer an action no request could carry out.
+ show_capabilities = ['start', 'delete']
+
class Meta:
model = WorkflowApproval
+ list_serializer_class = WorkflowApprovalPageSerializer
fields = (
'*',
'-controller_node',
'-execution_node',
'can_approve_or_deny',
+ 'can_cancel_workflow',
'approval_expiration',
'timed_out',
'context_message',
@@ -252,10 +378,54 @@ def get_approval_expiration(self, obj):
return obj.created + timedelta(seconds=obj.timeout)
def get_can_approve_or_deny(self, obj):
+ """
+ Whether the user may approve or deny this approval now.
+
+ On a page of approvals the access check was answered for the whole
+ page up front (see WorkflowApprovalPageSerializer); an approval it left
+ unanswered, or one serialized on its own, is checked here.
+
+ Args:
+ obj: The workflow approval being serialized.
+
+ Returns:
+ bool: True when the approval is pending and the user may decide it.
+ """
request = self.context.get('request', None)
- allowed = request.user.can_access(WorkflowApproval, 'approve_or_deny', obj)
+ answers = self.context.get(APPROVE_OR_DENY_ANSWERS) or {}
+ if obj.pk in answers:
+ allowed = answers[obj.pk]
+ else:
+ allowed = request.user.can_access(WorkflowApproval, 'approve_or_deny', obj)
return allowed is True and obj.status == 'pending'
+ def get_can_cancel_workflow(self, obj):
+ """
+ Whether the user may cancel the workflow job this approval waits in.
+
+ Canceling the workflow is a right on the workflow job, its creator or an
+ admin of its template, not the approver role, so it is reported apart
+ from can_approve_or_deny.
+
+ Args:
+ obj: The workflow approval being serialized.
+
+ Returns:
+ bool: True when a cancel request for the workflow job would be accepted.
+ """
+ request = self.context.get('request', None)
+ if request is None:
+ return False
+ workflow_job = waiting_workflow_job(obj)
+ if workflow_job is None:
+ return False
+ # On a page of approvals the capability was answered for every
+ # workflow job up front; one left unanswered is checked on its own.
+ answers = self.context.get(CANCEL_WORKFLOW_ANSWERS) or {}
+ if workflow_job.pk in answers:
+ return answers[workflow_job.pk]
+ return bool(get_user_capabilities(request.user, workflow_job, method_list=['cancel']).get('cancel'))
+
def get_approvals_received(self, obj):
return obj.approvals_received()
@@ -291,7 +461,17 @@ class WorkflowApprovalActivityStreamSerializer(WorkflowApprovalSerializer):
class WorkflowApprovalListSerializer(WorkflowApprovalSerializer, UnifiedJobListSerializer):
class Meta:
- fields = ('*', '-controller_node', '-execution_node', 'can_approve_or_deny', 'approval_expiration', 'timed_out', '-context_message')
+ list_serializer_class = WorkflowApprovalPageSerializer
+ fields = (
+ '*',
+ '-controller_node',
+ '-execution_node',
+ 'can_approve_or_deny',
+ 'can_cancel_workflow',
+ 'approval_expiration',
+ 'timed_out',
+ '-context_message',
+ )
def get_field_names(self, declared_fields, info):
field_names = super(WorkflowApprovalListSerializer, self).get_field_names(declared_fields, info)
diff --git a/ascender/api/views/labels.py b/ascender/api/views/labels.py
index 15ada2b26..13b51f6ef 100644
--- a/ascender/api/views/labels.py
+++ b/ascender/api/views/labels.py
@@ -1,5 +1,5 @@
# Ascender
-from ascender.api.generics import SubListCreateAttachDetachAPIView, RetrieveUpdateAPIView, ListCreateAPIView
+from ascender.api.generics import SubListCreateAttachDetachAPIView, RetrieveUpdateDestroyAPIView, ListCreateAPIView
from ascender.main.models import Label
from ascender.api.serializers import LabelSerializer
@@ -8,7 +8,8 @@
# Django REST Framework
from rest_framework.response import Response
-from rest_framework.status import HTTP_400_BAD_REQUEST
+from rest_framework.exceptions import PermissionDenied
+from rest_framework.status import HTTP_204_NO_CONTENT, HTTP_400_BAD_REQUEST, HTTP_409_CONFLICT
class LabelSubListCreateAttachDetachView(SubListCreateAttachDetachAPIView):
@@ -58,10 +59,62 @@ def post(self, request, *args, **kwargs):
return super().post(request, *args, **kwargs)
-class LabelDetail(RetrieveUpdateAPIView):
+class LabelDetail(RetrieveUpdateDestroyAPIView):
+ """
+ Labels used to have no way to be deleted. They were owned by whatever
+ carried them: the sublist view above deletes one when its last attachment
+ goes, so a label that nothing references stopped existing on its own and
+ the only labels reachable were the ones in use.
+
+ A label can now be created on its own, from the Labels screen, and one made
+ that way is attached to nothing, so that cleanup never fires and it would
+ have been permanent. Deleting is how it goes away.
+
+ Only a label attached to nothing can be deleted. A label is shared across
+ everything that carries it, templates, inventories, schedules and workflow
+ nodes of any organization as well as finished jobs, and deleting it would
+ strip it from all of them at once. Detaching never touches job history,
+ since Label.is_detached keeps a label that any job still carries, so a
+ DELETE on a label in use is refused with 409 and the label has to be
+ detached from its holders first.
+ """
+
model = Label
serializer_class = LabelSerializer
+ def destroy(self, request, *args, **kwargs):
+ """
+ Delete the label, provided the user may and nothing carries it.
+
+ The permission check comes first, so a user who may not delete the
+ label learns nothing about where it is used.
+
+ Args:
+ request: The DELETE request.
+ *args: Positional arguments of the URL route.
+ **kwargs: Keyword arguments of the URL route, the label's pk.
+
+ Returns:
+ Response: 204 once deleted, 409 when the label is still attached.
+
+ Raises:
+ PermissionDenied: When the user may not delete the label.
+ """
+ label = self.get_object()
+ if not self.has_delete_permission(label):
+ raise PermissionDenied()
+ if not label.is_detached():
+ return Response(
+ {
+ 'detail': _(
+ 'This label is still attached to templates, inventories, schedules, workflow nodes or jobs. Detach it from them before deleting it.'
+ )
+ },
+ status=HTTP_409_CONFLICT,
+ )
+ self.perform_destroy(label, check_permission=False)
+ return Response(status=HTTP_204_NO_CONTENT)
+
class LabelList(ListCreateAPIView):
name = _("Labels")
diff --git a/ascender/api/views/root.py b/ascender/api/views/root.py
index e4f9a031c..ba0b5b941 100644
--- a/ascender/api/views/root.py
+++ b/ascender/api/views/root.py
@@ -193,6 +193,21 @@ def get(self, request, format=None):
become_methods=PRIVILEGE_ESCALATION_METHODS,
)
+ # The installation's UI defaults are meant for everyone who has not
+ # chosen otherwise, which is mostly the users who cannot read
+ # /api/v2/settings/ui/: that endpoint is for administrators and system
+ # auditors only. The custom theme travels with them because it is one
+ # of the choices DEFAULT_UI_THEME offers, and a default naming a
+ # stylesheet the browser never received would paint nothing. None of
+ # it is sensitive: all of it ends up in every user's browser anyway.
+ data.update(
+ default_ui_theme=settings.DEFAULT_UI_THEME,
+ default_ui_language=settings.DEFAULT_UI_LANGUAGE,
+ max_ui_editor_rows=settings.MAX_UI_EDITOR_ROWS,
+ custom_theme=settings.CUSTOM_THEME,
+ custom_theme_name=settings.CUSTOM_THEME_NAME,
+ )
+
# If LDAP is enabled, user_ldap_fields will return a list of field
# names that are managed by LDAP and should be read-only for users with
# a non-empty ldap_dn attribute.
diff --git a/ascender/dab/VENDORED.md b/ascender/dab/VENDORED.md
index 1e2370253..627add51b 100644
--- a/ascender/dab/VENDORED.md
+++ b/ascender/dab/VENDORED.md
@@ -109,6 +109,20 @@ Do not rename the labels.
unreachable annotation, so such filters always failed with a 400. The Cast now
follows the full related path and the lookup is rewritten to target the top-level
annotation. Covered by `ascender/main/tests/functional/test_dab_rest_filters.py`.
+- `rest_filters/rest_framework/field_lookup_backend.py` — the `__search` loop unpacked
+ its unused third value into `_`, which made `_` a local name for the whole of
+ `filter_queryset` and hid the module's gettext import. A `role_level` filter on a list
+ whose model has no roles, such as `/api/v2/notification_templates/`, then answered 500
+ with an UnboundLocalError instead of the intended 400 ParseError. The variable is now
+ `_distinct`. Covered by `ascender/main/tests/functional/test_dab_rest_filters.py`.
+- `rest_filters/rest_framework/field_lookup_backend.py` — a lookup on a run's type
+ (`type`, which is `polymorphic_ctype__model`) returned right after removing its
+ underscores, so `type__in` never split its list and
+ `/api/v2/unified_jobs/?type__in=job,project_update` compared the whole comma-joined
+ string and matched nothing. The early return now splits an `__in` value first; every
+ other type lookup (`or__type`, `not__type`, `type__icontains`, ...) still keeps its
+ value as a model name, as upstream does. Covered by
+ `ascender/main/tests/functional/test_dab_rest_filters.py`.
Remaining `awx.dab.rbac.*` / `awx.dab.oauth2_provider.*` / `awx.dab.activitystream.*`
imports are lazy and guarded by INSTALLED_APPS checks; those apps are never installed here.
diff --git a/ascender/dab/rest_filters/rest_framework/field_lookup_backend.py b/ascender/dab/rest_filters/rest_framework/field_lookup_backend.py
index d1f8d5bc0..3e4298c65 100644
--- a/ascender/dab/rest_filters/rest_framework/field_lookup_backend.py
+++ b/ascender/dab/rest_filters/rest_framework/field_lookup_backend.py
@@ -106,10 +106,19 @@ def value_to_python(self, model, lookup, value):
# Type names are stored without underscores internally, but are presented and
# and serialized over the API containing underscores so we remove `_`
- # for polymorphic_ctype__model lookups.
+ # for polymorphic_ctype__model lookups. The value stays a model name
+ # whatever the suffix: the field here is the content type key, which
+ # would read it as an id. Only __in goes further, splitting its list so
+ # that type__in matches each listed type rather than one joined string.
if new_lookup.startswith('polymorphic_ctype__model'):
value = value.replace('_', '')
- elif new_lookup.endswith('__isnull'):
+ if new_lookup.endswith('__in'):
+ if not value:
+ raise ValueError(_('cannot provide empty value for __in'))
+ value = value.split(',')
+ return value, new_lookup, needs_distinct
+
+ if new_lookup.endswith('__isnull'):
value = to_python_boolean(value)
elif new_lookup.endswith('__in'):
items = []
@@ -199,7 +208,10 @@ def filter_queryset(self, request, queryset, view):
search_filter_relation = 'AND'
values = reduce(lambda list1, list2: list1 + list2, [i.split(',') for i in values])
for value in values:
- search_value, new_keys, _ = self.value_to_python(queryset.model, key, force_str(value))
+ # Not "_": assigning that anywhere in this method makes it
+ # local throughout, and the role_level ParseError below then
+ # fails on gettext with an UnboundLocalError instead.
+ search_value, new_keys, _distinct = self.value_to_python(queryset.model, key, force_str(value))
assert isinstance(new_keys, list)
search_filters[search_value] = new_keys
# by definition, search *only* joins across relations,
diff --git a/ascender/main/access.py b/ascender/main/access.py
index 4fc440f55..6d19fcb5b 100644
--- a/ascender/main/access.py
+++ b/ascender/main/access.py
@@ -185,6 +185,35 @@ def get_user_capabilities(user, instance, **kwargs):
return access_class(user).get_user_capabilities(instance, **kwargs)
+def user_held_roles(user, role_ids):
+ """
+ Answer `user in role` for many roles with a single query.
+
+ `Role.__contains__` asks whether any ancestor of the role, the role itself
+ included, has the user as a direct member, reading the denormalized role
+ ancestors table. This runs that same lookup over every role at once and
+ keeps the roles it matched, so a page of objects needs one query for its
+ role checks instead of one per object and role.
+
+ Args:
+ user: The user whose memberships are looked up. Anything that is not
+ a saved User is refused, the way `Role.__contains__` refuses it.
+ role_ids: Ids of the roles to test. None entries are ignored.
+
+ Returns:
+ set: The ids among role_ids for which `user in role` would be True.
+
+ Raises:
+ RuntimeError: When user is not a saved User.
+ """
+ if not isinstance(user, User) or user.pk is None:
+ raise RuntimeError(f'Role evaluations only valid for users, received {user}')
+ role_ids = {role_id for role_id in role_ids if role_id is not None}
+ if not role_ids:
+ return set()
+ return set(RoleAncestorEntry.objects.filter(descendent_id__in=role_ids, ancestor__members=user).values_list('descendent_id', flat=True).distinct())
+
+
def check_superuser(func):
"""
check_superuser is a decorator that provides a simple short circuit
@@ -400,7 +429,7 @@ def get_user_capabilities(self, obj, method_list=[], parent_obj=None, capabiliti
user_capabilities = {}
# Custom ordering to loop through methods so we can reuse earlier calcs
- for display_method in ['edit', 'delete', 'start', 'schedule', 'copy', 'adhoc', 'unattach']:
+ for display_method in ['edit', 'delete', 'start', 'schedule', 'copy', 'adhoc', 'unattach', 'cancel']:
if display_method not in method_list:
continue
@@ -470,6 +499,8 @@ def get_method_capability(self, method, obj, parent_obj):
return access_method(obj)
elif method in ['start']:
return self.can_start(obj)
+ elif method in ['cancel']:
+ return self.get_cancel_capability(obj)
elif method in ['attach', 'unattach']: # parent/sub-object call
access_method = getattr(self, "can_%s" % method)
if type(parent_obj) == Team:
@@ -482,6 +513,30 @@ def get_method_capability(self, method, obj, parent_obj):
return False
return False
+ def get_cancel_capability(self, obj):
+ """
+ Whether the user may cancel this unified job right now.
+
+ The cancel views let a superuser through before any access check, and
+ refuse a job whose status has moved past the point of canceling, so
+ this does both too. Access classes without a can_cancel method, such as
+ the one for system jobs, leave canceling to superusers alone.
+
+ Args:
+ obj: The unified job the capability is computed for.
+
+ Returns:
+ bool: True when a cancel request from this user would be accepted.
+ """
+ if not getattr(obj, 'can_cancel', False):
+ return False
+ if self.user.is_superuser:
+ return True
+ access_method = getattr(self, 'can_cancel', None)
+ if access_method is None:
+ return False
+ return bool(access_method(obj))
+
class ReadOnlyAccess:
"""
@@ -2239,6 +2294,88 @@ def can_cancel(self, obj):
return True
return obj.workflow_job_template is not None and self.user in obj.workflow_job_template.admin_role
+ def get_cancel_capabilities(self, workflow_jobs):
+ """
+ The 'cancel' user capability of many workflow jobs, at a fixed query cost.
+
+ Each answer equals what `get_user_capabilities(user, workflow_job,
+ method_list=['cancel'])['cancel']` returns for that workflow job: no for
+ a read-only token, no once the status is past canceling, yes for a
+ superuser, and otherwise can_cancel's creator, organization workflow
+ admin (can_delete) and template admin checks. The templates behind the
+ jobs are read in one query and the role checks in another, where the
+ per job path reads the template, its organization and each role in
+ turn for every job.
+
+ A job is left out of the result, so its caller asks the per job path
+ instead, whenever the batched reading could part from that path: a
+ template reference that no longer resolves, or an organization or
+ template without the role the check reads. Leaving a job out only
+ costs its queries back, never changes an answer.
+
+ Args:
+ workflow_jobs: The workflow jobs to answer for.
+
+ Returns:
+ dict: Workflow job pk to the cancel capability as a bool, for every
+ job answered here.
+ """
+ workflow_jobs = [wj for wj in workflow_jobs if isinstance(wj, WorkflowJob) and wj.pk is not None]
+ # The per job path compares the user to created_by as model instances,
+ # which only a saved User can match; anything else is left to it whole.
+ if not workflow_jobs or not isinstance(self.user, User) or self.user.pk is None:
+ return {}
+
+ # get_user_capabilities refuses every action to a token without write
+ # scope before it looks at the job at all.
+ if 'write' not in getattr(self.user, 'oauth_scopes', ['write']):
+ return {wj.pk: False for wj in workflow_jobs}
+
+ answers = {}
+ undecided = []
+ for wj in workflow_jobs:
+ # get_cancel_capability: the status comes first, then the superuser.
+ if not wj.can_cancel:
+ answers[wj.pk] = False
+ elif self.user.is_superuser:
+ answers[wj.pk] = True
+ # can_cancel: the user who started the job may always cancel it.
+ elif wj.created_by_id is not None and wj.created_by_id == self.user.pk:
+ answers[wj.pk] = True
+ # With no template both can_delete and the admin check are falsy.
+ elif wj.workflow_job_template_id is None:
+ answers[wj.pk] = False
+ else:
+ undecided.append(wj)
+ if not undecided:
+ return answers
+
+ # can_delete reads the template's organization and that organization's
+ # workflow admin role, the last check reads the template's admin role.
+ templates = {
+ row[0]: row[1:]
+ for row in WorkflowJobTemplate.objects.filter(pk__in={wj.workflow_job_template_id for wj in undecided}).values_list(
+ 'pk', 'organization_id', 'organization__workflow_admin_role_id', 'admin_role_id'
+ )
+ }
+ answerable = []
+ for wj in undecided:
+ template = templates.get(wj.workflow_job_template_id)
+ if template is None:
+ continue
+ organization_id, workflow_admin_role_id, admin_role_id = template
+ if admin_role_id is None or (organization_id is not None and workflow_admin_role_id is None):
+ continue
+ answerable.append((wj, organization_id, workflow_admin_role_id, admin_role_id))
+
+ held = user_held_roles(
+ self.user, [role_id for _, _, workflow_admin_role_id, admin_role_id in answerable for role_id in (workflow_admin_role_id, admin_role_id)]
+ )
+ for wj, organization_id, workflow_admin_role_id, admin_role_id in answerable:
+ can_delete = organization_id is not None and workflow_admin_role_id in held
+ answers[wj.pk] = can_delete or admin_role_id in held
+ return answers
+
def annotate_unusable_instance_groups(qs, user):
"""Mark each ad hoc command in qs with whether it was launched on an
@@ -2584,6 +2721,106 @@ def get_queryset(self):
# hands back, so their can_start reads this too
return annotate_unusable_instance_groups(qs, self.user)
+ # For each kind of unified job answered by get_cancel_capabilities: the
+ # model whose admin role can_cancel reads, the job's column pointing at
+ # that model, and the path from the model to the role. The organization
+ # fallback of an orphaned job is handled apart, in get_cancel_capabilities.
+ CANCEL_ROLE_PATHS = {
+ Job: (JobTemplate, 'job_template_id', 'admin_role_id'),
+ ProjectUpdate: (Project, 'project_id', 'admin_role_id'),
+ InventoryUpdate: (InventorySource, 'inventory_source_id', 'inventory__admin_role_id'),
+ AdHocCommand: (Inventory, 'inventory_id', 'admin_role_id'),
+ }
+
+ def get_cancel_capabilities(self, unified_jobs):
+ """
+ The 'cancel' user capability of many unified jobs, at a fixed query cost.
+
+ Each answer equals what `get_user_capabilities(user, job,
+ method_list=['cancel'])['cancel']` returns for that job, whose access
+ class is JobAccess, ProjectUpdateAccess, InventoryUpdateAccess,
+ AdHocCommandAccess or WorkflowJobAccess. The per job path reads the
+ job's template, project or inventory and then asks one role question,
+ so a page of running jobs costs a few queries a row. Here the related
+ rows are read in one query per kind of job and every role question in
+ one more; workflow jobs go to WorkflowJobAccess.get_cancel_capabilities.
+
+ Nothing is answered for a superuser, and nothing for a token without
+ write scope: get_user_capabilities answers both without a query, and a
+ cached False would be turned into True for a superuser. Other kinds of
+ job, a system job or an approval, are left out too, as are jobs whose
+ batched reading could part from the per job path: a related row that
+ no longer resolves, or one without the role the check reads. Leaving a
+ job out only costs its queries back, never changes an answer.
+
+ Args:
+ unified_jobs: The unified jobs to answer for, as the concrete
+ instances a polymorphic queryset yields.
+
+ Returns:
+ dict: Unified job pk to the cancel capability as a bool, for every
+ job answered here.
+ """
+ # The per job path compares the user to created_by as model instances,
+ # which only a saved User can match; anything else is left to it whole.
+ if not isinstance(self.user, User) or self.user.pk is None or self.user.is_superuser:
+ return {}
+ if 'write' not in getattr(self.user, 'oauth_scopes', ['write']):
+ return {}
+
+ unified_jobs = [uj for uj in unified_jobs if uj is not None and uj.pk is not None]
+ answers = WorkflowJobAccess(self.user).get_cancel_capabilities([uj for uj in unified_jobs if type(uj) is WorkflowJob])
+
+ # Step 1: what each can_cancel decides without a query. The status
+ # comes first (get_cancel_capability), then the creator, who may always
+ # cancel their own job, and an ad hoc command whose inventory is gone.
+ undecided = []
+ for uj in unified_jobs:
+ if type(uj) not in self.CANCEL_ROLE_PATHS:
+ continue
+ if not uj.can_cancel:
+ answers[uj.pk] = False
+ elif uj.created_by_id is not None and uj.created_by_id == self.user.pk:
+ answers[uj.pk] = True
+ elif type(uj) is AdHocCommand and uj.inventory_id is None:
+ answers[uj.pk] = False
+ else:
+ undecided.append(uj)
+ if not undecided:
+ return answers
+
+ # Step 2: the role each remaining job is judged by, one query per kind
+ # of related row. A job whose template is gone falls back to its
+ # organization's job template admin role, as JobAccess.can_cancel does.
+ role_of = {}
+ orphans = []
+ for model_class, (related_model, column, role_path) in self.CANCEL_ROLE_PATHS.items():
+ jobs = [uj for uj in undecided if type(uj) is model_class]
+ if model_class is Job:
+ orphans = [uj for uj in jobs if uj.job_template_id is None]
+ jobs = [uj for uj in jobs if uj.job_template_id is not None]
+ if not jobs:
+ continue
+ roles = dict(related_model.objects.filter(pk__in={getattr(uj, column) for uj in jobs}).values_list('pk', role_path))
+ for uj in jobs:
+ role_of[uj.pk] = roles.get(getattr(uj, column))
+ for uj in orphans:
+ if uj.organization_id is None:
+ answers[uj.pk] = False
+ orphans = [uj for uj in orphans if uj.organization_id is not None]
+ if orphans:
+ roles = dict(Organization.objects.filter(pk__in={uj.organization_id for uj in orphans}).values_list('pk', 'job_template_admin_role_id'))
+ for uj in orphans:
+ role_of[uj.pk] = roles.get(uj.organization_id)
+
+ # Step 3: every role question in one query. A job whose role could not
+ # be read stays out of the answers and is checked on its own.
+ role_of = {pk: role_id for pk, role_id in role_of.items() if role_id is not None}
+ held = user_held_roles(self.user, role_of.values())
+ for pk, role_id in role_of.items():
+ answers[pk] = role_id in held
+ return answers
+
class ScheduleAccess(UnifiedCredentialsMixin, BaseAccess):
"""
@@ -2924,11 +3161,17 @@ class WorkflowApprovalAccess(BaseAccess):
"""
model = WorkflowApproval
- prefetch_related = (
+ # The users are forward foreign keys, joined into the same query. The
+ # template, node and workflow job every row of the approvals list reads
+ # are prefetched by WorkflowApprovalPageSerializer for the page instead,
+ # because this queryset also serves the single approval views, which
+ # would otherwise pay for every one of those queries on each request.
+ select_related = (
'created_by',
'modified_by',
- 'votes',
+ 'approved_or_denied_by',
)
+ prefetch_related = ('votes',)
def can_use(self, obj):
return True
@@ -2939,9 +3182,69 @@ def can_start(self, obj):
read_via = (WorkflowJobTemplate, 'unified_job_node__workflow_job__unified_job_template')
def can_approve_or_deny(self, obj):
- if (obj.workflow_job_template and self.user in obj.workflow_job_template.approval_role) or self.user.is_superuser:
+ # The superuser check comes first because it costs nothing, where the
+ # role membership is a query, run once per row on the approvals list.
+ if self.user.is_superuser or (obj.workflow_job_template and self.user in obj.workflow_job_template.approval_role):
return True
+ def can_approve_or_deny_many(self, approvals):
+ """
+ can_approve_or_deny for many approvals, at a fixed query cost.
+
+ Each answer is exactly what can_approve_or_deny returns for that
+ approval, True or None, so a caller wraps it the same way. The
+ approval roles are checked in one query, where the per approval path
+ runs one for each; the templates come from the relations the
+ approvals list already prefetches.
+
+ An approval is left out of the result, so its caller asks the per
+ approval path instead, whenever the batched reading could part from
+ that path: a node without a workflow job, a template that is not a
+ workflow job template, or one without an approval role. Leaving an
+ approval out only costs its queries back, never changes an answer.
+
+ Args:
+ approvals: The workflow approvals to answer for.
+
+ Returns:
+ dict: Approval pk to the can_approve_or_deny result, for every
+ approval answered here.
+ """
+ approvals = [approval for approval in approvals if isinstance(approval, WorkflowApproval) and approval.pk is not None]
+ if not approvals:
+ return {}
+ if self.user.is_superuser:
+ return {approval.pk: True for approval in approvals}
+ # Role.__contains__ refuses anything but a saved user, so such a
+ # reader is left to the per approval path whole.
+ if not isinstance(self.user, User) or self.user.pk is None:
+ return {}
+
+ answers = {}
+ answerable = []
+ for approval in approvals:
+ # The workflow_job_template property swallows a missing node,
+ # workflow job or template and returns None, which
+ # can_approve_or_deny turns into None. A node with no workflow job
+ # at all raises there instead, so that one is left to it.
+ try:
+ workflow_job = approval.unified_job_node.workflow_job
+ if workflow_job is None:
+ continue
+ template = workflow_job.unified_job_template
+ except ObjectDoesNotExist:
+ answers[approval.pk] = None
+ continue
+ if template is None:
+ answers[approval.pk] = None
+ elif isinstance(template, WorkflowJobTemplate) and template.approval_role_id is not None:
+ answerable.append((approval, template.approval_role_id))
+
+ held = user_held_roles(self.user, [role_id for _, role_id in answerable])
+ for approval, role_id in answerable:
+ answers[approval.pk] = True if role_id in held else None
+ return answers
+
class WorkflowApprovalTemplateAccess(BaseAccess):
"""
diff --git a/ascender/main/backends.py b/ascender/main/backends.py
index 62707fbad..ff9bd02a4 100644
--- a/ascender/main/backends.py
+++ b/ascender/main/backends.py
@@ -6,19 +6,40 @@
logger = logging.getLogger('ascender.main.backends')
-class AWXModelBackend(ModelBackend):
- """The local authentication backend.
-
- Keeps the AWX name deliberately. AUTHENTICATION_BACKENDS names this class by
- its dotted path rather than importing it, the path is offered as a choice in
- the authentication settings, and a deployment that has set that setting has
- the old string stored. Renaming the class would leave that string pointing
- at nothing, and the symptom is a login that stops working rather than
- anything that names the cause.
- """
+class AscenderModelBackend(ModelBackend):
+ """The local authentication backend."""
def authenticate(self, request, **kwargs):
if settings.DISABLE_LOCAL_AUTH:
logger.warning(f"User '{kwargs['username']}' attempted login through the disabled local authentication system.")
return
return super().authenticate(request, **kwargs)
+
+
+class AWXModelBackend(AscenderModelBackend):
+ """The name the local backend carried before, kept so old sessions still load.
+
+ Nothing here is referenced by import. Every logged in session records the
+ dotted path of the backend that authenticated it as _auth_user_backend, and
+ django only loads a session whose recorded backend is listed in
+ AUTHENTICATION_BACKENDS. Sessions opened before the rename recorded this
+ path, so dropping it would log out everyone holding one at their next
+ request, with a symptom that names none of that. Both paths are therefore
+ listed.
+
+ Sessions are the only thing that needs this. AUTHENTICATION_BACKENDS is a
+ read only setting computed from the defaults, never stored in the database,
+ so no stored configuration names the old path. Sessions expire after
+ SESSION_COOKIE_AGE, so once a release has been out longer than that, no
+ live session can carry the old path and this alias, its entry in
+ AUTHENTICATION_BACKENDS and the one in AuthenticationBackendsField can be
+ removed in a later release.
+
+ Being listed also means django offers it every login, and as a plain alias it
+ checked the password a second time, so each failed local login was attempted
+ twice. Only get_user is needed to restore a session, so this subclass
+ inherits that and declines to authenticate.
+ """
+
+ def authenticate(self, request, **kwargs):
+ return None
diff --git a/ascender/main/tests/functional/api/test_config.py b/ascender/main/tests/functional/api/test_config.py
new file mode 100644
index 000000000..15ac1bc6d
--- /dev/null
+++ b/ascender/main/tests/functional/api/test_config.py
@@ -0,0 +1,44 @@
+import pytest
+
+from ascender.api.versioning import reverse
+
+
+@pytest.mark.django_db
+def test_ui_defaults_reach_users_who_cannot_read_ui_settings(get, patch, admin, rando):
+ """The UI defaults are for everyone, so /config/ carries them to non-admins.
+
+ /api/v2/settings/ui/ is where they are edited, and an ordinary user is
+ refused there, which used to leave the UI with no defaults at all for the
+ very users they were meant for.
+ """
+ get(reverse('api:setting_singleton_detail', kwargs={'category_slug': 'ui'}), user=rando, expect=403)
+
+ patch(
+ reverse('api:setting_singleton_detail', kwargs={'category_slug': 'ui'}),
+ user=admin,
+ data={
+ 'DEFAULT_UI_THEME': 'custom',
+ 'DEFAULT_UI_LANGUAGE': 'fr',
+ 'MAX_UI_EDITOR_ROWS': 20,
+ 'CUSTOM_THEME': 'html[data-theme="custom"] body { color: red; }',
+ 'CUSTOM_THEME_NAME': 'Red',
+ },
+ expect=200,
+ )
+
+ data = get(reverse('api:api_v2_config_view'), user=rando, expect=200).data
+ assert data['default_ui_theme'] == 'custom'
+ assert data['default_ui_language'] == 'fr'
+ assert data['max_ui_editor_rows'] == 20
+ assert data['custom_theme'] == 'html[data-theme="custom"] body { color: red; }'
+ assert data['custom_theme_name'] == 'Red'
+
+
+@pytest.mark.django_db
+def test_ui_defaults_in_config_before_anything_is_saved(get, rando):
+ data = get(reverse('api:api_v2_config_view'), user=rando, expect=200).data
+ assert data['default_ui_theme'] == 'default'
+ assert data['default_ui_language'] == ''
+ assert data['max_ui_editor_rows'] == 50
+ assert data['custom_theme'] == ''
+ assert data['custom_theme_name'] == ''
diff --git a/ascender/main/tests/functional/api/test_oauth.py b/ascender/main/tests/functional/api/test_oauth.py
index 84d141d27..b1c26aa68 100644
--- a/ascender/main/tests/functional/api/test_oauth.py
+++ b/ascender/main/tests/functional/api/test_oauth.py
@@ -37,10 +37,10 @@ def test_personal_access_token_creation(oauth_application, post, alice):
@pytest.mark.django_db
@pytest.mark.parametrize('allow_oauth, status', [(True, 201), (False, 403)])
def test_token_creation_disabled_for_external_accounts(oauth_application, post, alice, allow_oauth, status):
- UserEnterpriseAuth(user=alice, provider='radius').save()
+ UserEnterpriseAuth(user=alice, provider='saml').save()
url = drf_reverse('api:oauth_authorization_root_view') + 'token/'
- with override_settings(RADIUS_SERVER='example.org', ALLOW_OAUTH2_FOR_EXTERNAL_USERS=allow_oauth):
+ with override_settings(ALLOW_OAUTH2_FOR_EXTERNAL_USERS=allow_oauth):
resp = post(
url,
data='grant_type=password&username=alice&password=alice&scope=read',
@@ -57,9 +57,9 @@ def test_token_creation_disabled_for_external_accounts(oauth_application, post,
@pytest.mark.django_db
def test_existing_token_enabled_for_external_accounts(oauth_application, get, post, admin):
- UserEnterpriseAuth(user=admin, provider='radius').save()
+ UserEnterpriseAuth(user=admin, provider='saml').save()
url = drf_reverse('api:oauth_authorization_root_view') + 'token/'
- with override_settings(RADIUS_SERVER='example.org', ALLOW_OAUTH2_FOR_EXTERNAL_USERS=True):
+ with override_settings(ALLOW_OAUTH2_FOR_EXTERNAL_USERS=True):
resp = post(
url,
data='grant_type=password&username=admin&password=admin&scope=read',
@@ -74,7 +74,7 @@ def test_existing_token_enabled_for_external_accounts(oauth_application, get, po
resp = get(drf_reverse('api:user_me_list', kwargs={'version': 'v2'}), HTTP_AUTHORIZATION='Bearer ' + token, status=200)
assert json.loads(resp.content)['results'][0]['username'] == 'admin'
- with override_settings(RADIUS_SERVER='example.org', ALLOW_OAUTH2_FOR_EXTERNAL_USER=False):
+ with override_settings(ALLOW_OAUTH2_FOR_EXTERNAL_USER=False):
with immediate_on_commit():
resp = get(drf_reverse('api:user_me_list', kwargs={'version': 'v2'}), HTTP_AUTHORIZATION='Bearer ' + token, status=200)
assert json.loads(resp.content)['results'][0]['username'] == 'admin'
@@ -424,8 +424,8 @@ def test_personal_token_for_missing_user(post, admin):
@pytest.mark.django_db
def test_personal_token_for_another_external_user(post, admin, alice):
# the external-account check applies to the owner, not to the superuser issuing it
- UserEnterpriseAuth(user=alice, provider='radius').save()
- with override_settings(RADIUS_SERVER='example.org', ALLOW_OAUTH2_FOR_EXTERNAL_USERS=False):
+ UserEnterpriseAuth(user=alice, provider='saml').save()
+ with override_settings(ALLOW_OAUTH2_FOR_EXTERNAL_USERS=False):
response = post(reverse('api:user_personal_token_list', kwargs={'pk': alice.pk}), {'scope': 'read'}, admin, expect=403)
assert 'external authentication provider' in str(response.data['detail'])
assert AccessToken.objects.count() == 0
diff --git a/ascender/main/tests/functional/api/test_rbac_displays.py b/ascender/main/tests/functional/api/test_rbac_displays.py
index e564e9936..c60b9453d 100644
--- a/ascender/main/tests/functional/api/test_rbac_displays.py
+++ b/ascender/main/tests/functional/api/test_rbac_displays.py
@@ -340,3 +340,48 @@ def test_manual_projects_no_update(manual_project, get, admin_user):
response = get(reverse('api:project_detail', kwargs={'pk': manual_project.pk}), admin_user, expect=200)
assert not response.data['summary_fields']['user_capabilities']['start']
assert not response.data['summary_fields']['user_capabilities']['schedule']
+
+
+@pytest.mark.django_db
+class TestCancelCapability:
+ """
+ The cancel capability mirrors what a cancel request would be told: the
+ run's creator or an admin of what it ran may cancel it, a superuser always
+ may, and nobody may once the run has finished.
+ """
+
+ def cancel_capability(self, get, job, user):
+ response = get(job.get_absolute_url(), user, expect=200)
+ return response.data['summary_fields']['user_capabilities']['cancel']
+
+ def test_creator_and_template_admin_may_cancel(self, get, job_factory, jt_linked, alice, bob, rando):
+ jt_linked.execute_role.members.add(alice, rando)
+ jt_linked.admin_role.members.add(bob)
+ job = job_factory(created_by=alice, initial_state='running')
+
+ assert self.cancel_capability(get, job, alice)
+ assert self.cancel_capability(get, job, bob)
+ assert not self.cancel_capability(get, job, rando)
+
+ def test_superuser_may_cancel(self, get, job_factory, admin):
+ job = job_factory(created_by=None, initial_state='pending')
+ assert self.cancel_capability(get, job, admin)
+
+ def test_finished_run_cannot_be_canceled(self, get, job_factory, admin, alice):
+ job = job_factory(created_by=alice, initial_state='successful')
+ assert not self.cancel_capability(get, job, admin)
+
+ def test_system_job_is_left_to_superusers(self, get, system_job_factory, admin, system_auditor):
+ job = system_job_factory(initial_state='running')
+ assert self.cancel_capability(get, job, admin)
+ assert not self.cancel_capability(get, job, system_auditor)
+
+ def test_unified_job_list_reports_cancel(self, get, job_factory, alice, rando, jt_linked):
+ jt_linked.execute_role.members.add(alice, rando)
+ job = job_factory(created_by=alice, initial_state='running')
+ url = reverse('api:unified_job_list')
+ by_user = {}
+ for user in (alice, rando):
+ results = get(url, user, expect=200).data['results']
+ by_user[user.username] = [r for r in results if r['id'] == job.id][0]['summary_fields']['user_capabilities']['cancel']
+ assert by_user == {'alice': True, 'rando': False}
diff --git a/ascender/main/tests/functional/api/test_settings.py b/ascender/main/tests/functional/api/test_settings.py
index 4ada684dd..02e1c0aa4 100644
--- a/ascender/main/tests/functional/api/test_settings.py
+++ b/ascender/main/tests/functional/api/test_settings.py
@@ -145,35 +145,36 @@ def test_empty_ldap_dn(get, put, patch, delete, admin, setting):
@pytest.mark.django_db
-def test_radius_settings(get, put, patch, delete, admin, settings):
- url = reverse('api:setting_singleton_detail', kwargs={'category_slug': 'radius'})
+def test_encrypted_setting_round_trip(get, put, patch, delete, admin, settings):
+ """An encrypted setting reads back as $encrypted$ and is stored encrypted."""
+ url = reverse('api:setting_singleton_detail', kwargs={'category_slug': 'ldap'})
response = get(url, user=admin, expect=200)
put(url, user=admin, data=response.data, expect=200)
# Set secret via the API.
- patch(url, user=admin, data={'RADIUS_SECRET': 'mysecret'}, expect=200)
+ patch(url, user=admin, data={'AUTH_LDAP_BIND_PASSWORD': 'mysecret'}, expect=200)
response = get(url, user=admin, expect=200)
- assert response.data['RADIUS_SECRET'] == '$encrypted$'
- assert Setting.objects.filter(key='RADIUS_SECRET').first().value.startswith('$encrypted$')
- assert settings.RADIUS_SECRET == 'mysecret'
+ assert response.data['AUTH_LDAP_BIND_PASSWORD'] == '$encrypted$'
+ assert Setting.objects.filter(key='AUTH_LDAP_BIND_PASSWORD').first().value.startswith('$encrypted$')
+ assert settings.AUTH_LDAP_BIND_PASSWORD == 'mysecret'
# Set secret via settings wrapper.
settings_wrapper = settings._ascender_conf_settings
- settings_wrapper.RADIUS_SECRET = 'mysecret2'
+ settings_wrapper.AUTH_LDAP_BIND_PASSWORD = 'mysecret2'
response = get(url, user=admin, expect=200)
- assert response.data['RADIUS_SECRET'] == '$encrypted$'
- assert Setting.objects.filter(key='RADIUS_SECRET').first().value.startswith('$encrypted$')
- assert settings.RADIUS_SECRET == 'mysecret2'
+ assert response.data['AUTH_LDAP_BIND_PASSWORD'] == '$encrypted$'
+ assert Setting.objects.filter(key='AUTH_LDAP_BIND_PASSWORD').first().value.startswith('$encrypted$')
+ assert settings.AUTH_LDAP_BIND_PASSWORD == 'mysecret2'
# If we send back $encrypted$, the setting is not updated.
- patch(url, user=admin, data={'RADIUS_SECRET': '$encrypted$'}, expect=200)
+ patch(url, user=admin, data={'AUTH_LDAP_BIND_PASSWORD': '$encrypted$'}, expect=200)
response = get(url, user=admin, expect=200)
- assert response.data['RADIUS_SECRET'] == '$encrypted$'
- assert Setting.objects.filter(key='RADIUS_SECRET').first().value.startswith('$encrypted$')
- assert settings.RADIUS_SECRET == 'mysecret2'
+ assert response.data['AUTH_LDAP_BIND_PASSWORD'] == '$encrypted$'
+ assert Setting.objects.filter(key='AUTH_LDAP_BIND_PASSWORD').first().value.startswith('$encrypted$')
+ assert settings.AUTH_LDAP_BIND_PASSWORD == 'mysecret2'
# If we send an empty string, the setting is also set to an empty string.
- patch(url, user=admin, data={'RADIUS_SECRET': ''}, expect=200)
+ patch(url, user=admin, data={'AUTH_LDAP_BIND_PASSWORD': ''}, expect=200)
response = get(url, user=admin, expect=200)
- assert response.data['RADIUS_SECRET'] == ''
- assert Setting.objects.filter(key='RADIUS_SECRET').first().value == ''
- assert settings.RADIUS_SECRET == ''
+ assert response.data['AUTH_LDAP_BIND_PASSWORD'] == ''
+ assert Setting.objects.filter(key='AUTH_LDAP_BIND_PASSWORD').first().value == ''
+ assert settings.AUTH_LDAP_BIND_PASSWORD == ''
@pytest.mark.django_db
@@ -192,21 +193,6 @@ def test_login_redirect_override_scheme(get, patch, admin, settings):
patch(url, user=admin, data={'LOGIN_REDIRECT_OVERRIDE': ''}, expect=200)
-@pytest.mark.django_db
-def test_tacacsplus_settings(get, put, patch, admin):
- url = reverse('api:setting_singleton_detail', kwargs={'category_slug': 'tacacsplus'})
- response = get(url, user=admin, expect=200)
- put(url, user=admin, data=response.data, expect=200)
- patch(url, user=admin, data={'TACACSPLUS_SECRET': 'mysecret'}, expect=200)
- patch(url, user=admin, data={'TACACSPLUS_SECRET': ''}, expect=200)
- patch(url, user=admin, data={'TACACSPLUS_HOST': 'localhost'}, expect=400)
- patch(url, user=admin, data={'TACACSPLUS_SECRET': 'mysecret'}, expect=200)
- patch(url, user=admin, data={'TACACSPLUS_HOST': 'localhost'}, expect=200)
- patch(url, user=admin, data={'TACACSPLUS_HOST': '', 'TACACSPLUS_SECRET': ''}, expect=200)
- patch(url, user=admin, data={'TACACSPLUS_HOST': 'localhost', 'TACACSPLUS_SECRET': ''}, expect=400)
- patch(url, user=admin, data={'TACACSPLUS_HOST': 'localhost', 'TACACSPLUS_SECRET': 'mysecret'}, expect=200)
-
-
@pytest.mark.django_db
def test_ui_settings(get, put, patch, delete, admin):
url = reverse('api:setting_singleton_detail', kwargs={'category_slug': 'ui'})
diff --git a/ascender/main/tests/functional/api/test_unified_job_cancel_capability.py b/ascender/main/tests/functional/api/test_unified_job_cancel_capability.py
new file mode 100644
index 000000000..ec11e636e
--- /dev/null
+++ b/ascender/main/tests/functional/api/test_unified_job_cancel_capability.py
@@ -0,0 +1,272 @@
+"""
+The cancel capability of a page of unified jobs, answered once for the page.
+
+The jobs lists report, for every row, whether the reader may cancel it. For
+anyone but a superuser that used to be a role question per running row; now
+UnifiedJobAccess.get_cancel_capabilities answers the page in bulk, and these
+tests hold it to giving exactly the answers the per row path gives.
+"""
+
+import pytest
+
+from django.db import connection
+from django.test.utils import CaptureQueriesContext
+
+from ascender.api.versioning import reverse
+from ascender.main.access import UnifiedJobAccess, get_user_capabilities
+from ascender.main.models import AdHocCommand, InventoryUpdate, Job, JobTemplate, ProjectUpdate, UnifiedJob, User
+
+
+def per_row_cancel(user, pk):
+ """
+ The cancel capability the per row path gives, on a freshly read job.
+
+ Args:
+ user: The user asking.
+ pk: The unified job's primary key.
+
+ Returns:
+ bool: What get_user_capabilities answers for 'cancel'.
+ """
+ return get_user_capabilities(user, UnifiedJob.objects.get(pk=pk), method_list=['cancel'])['cancel']
+
+
+@pytest.fixture
+def cancel_world(
+ admin,
+ user,
+ alice,
+ rando,
+ organization,
+ project,
+ inventory,
+ inventory_source,
+ jt_linked,
+ job_factory,
+ ad_hoc_command_factory,
+ workflow_job_factory,
+ system_job_factory,
+):
+ """
+ Users in every relation to what the jobs ran, and jobs of every kind.
+
+ The users are a superuser, an admin of the job template, of the project
+ and of the inventory, the user who launched the runs, a user who may only
+ execute the template, an organization job template admin and an unrelated
+ user. The jobs are running, pending and finished playbook runs, runs whose
+ template was deleted with and without an organization, and running project
+ updates, inventory updates, ad hoc commands (one whose inventory is gone),
+ workflow jobs and system jobs.
+
+ Args:
+ admin: A superuser, the default launcher of the runs.
+ user: Factory for users.
+ alice: The user who launches some of the runs.
+ rando: A user with no relation to anything.
+ organization: The organization everything belongs to.
+ project: The project the job template and the project updates use.
+ inventory: The inventory the template, source and commands use.
+ inventory_source: The source the inventory updates run.
+ jt_linked: The job template the playbook runs come from.
+ job_factory: Factory for jobs of jt_linked.
+ ad_hoc_command_factory: Factory for ad hoc commands.
+ workflow_job_factory: Factory for workflow jobs.
+ system_job_factory: Factory for system jobs.
+
+ Returns:
+ dict: 'users' maps a role name to its user, 'jobs' maps a name to its
+ unified job.
+ """
+ users = {
+ 'superuser': admin,
+ 'template_admin': user('cancel-template-admin', False),
+ 'project_admin': user('cancel-project-admin', False),
+ 'inventory_admin': user('cancel-inventory-admin', False),
+ 'creator': alice,
+ 'executor': user('cancel-executor', False),
+ 'org_template_admin': user('cancel-org-template-admin', False),
+ 'unrelated': rando,
+ }
+ jt_linked.admin_role.members.add(users['template_admin'])
+ jt_linked.execute_role.members.add(users['creator'], users['executor'])
+ project.admin_role.members.add(users['project_admin'])
+ inventory.admin_role.members.add(users['inventory_admin'])
+ organization.job_template_admin_role.members.add(users['org_template_admin'])
+
+ def orphan(job, org):
+ # A deleted template leaves its jobs with neither template reference.
+ Job.objects.filter(pk=job.pk).update(job_template=None, unified_job_template=None, organization=org)
+ return job
+
+ jobs = {
+ 'job_running_by_creator': job_factory(created_by=users['creator'], initial_state='running'),
+ 'job_running_by_admin': job_factory(initial_state='running'),
+ 'job_pending_by_admin': job_factory(initial_state='pending'),
+ 'job_finished': job_factory(created_by=users['creator'], initial_state='successful'),
+ 'job_orphaned_in_org': orphan(job_factory(initial_state='running'), organization),
+ 'job_orphaned_no_org': orphan(job_factory(initial_state='running'), None),
+ 'project_update_by_admin': ProjectUpdate.objects.create(name='pu', project=project, status='running', created_by=admin),
+ 'project_update_by_creator': ProjectUpdate.objects.create(name='pu', project=project, status='running', created_by=users['creator']),
+ 'inventory_update_by_admin': InventoryUpdate.objects.create(inventory_source=inventory_source, source='ec2', status='running', created_by=admin),
+ 'ad_hoc_by_admin': ad_hoc_command_factory(initial_state='running'),
+ 'ad_hoc_by_creator': ad_hoc_command_factory(initial_state='running', created_by=users['creator']),
+ 'ad_hoc_no_inventory': ad_hoc_command_factory(initial_state='running'),
+ 'workflow_job': workflow_job_factory(initial_state='running'),
+ 'system_job': system_job_factory(initial_state='running'),
+ }
+ AdHocCommand.objects.filter(pk=jobs['ad_hoc_no_inventory'].pk).update(inventory=None)
+ return {'users': users, 'jobs': jobs}
+
+
+@pytest.mark.django_db
+class TestCancelCapabilityMatchesPerRow:
+ """
+ Every cancel answer the page gives is the one the job would get alone.
+ """
+
+ @pytest.mark.parametrize('url_name', ['api:unified_job_list', 'api:job_list'])
+ def test_list_matches_the_per_row_check(self, cancel_world, get, url_name):
+ """
+ Each row of the jobs lists carries the per row cancel answer.
+
+ Args:
+ cancel_world: Users and jobs in every relation and state.
+ get: Fixture issuing GET requests against the API.
+ url_name: The list read, the polymorphic one or the playbook runs.
+ """
+ seen = set()
+ for name, fixture_user in cancel_world['users'].items():
+ reader = User.objects.get(pk=fixture_user.pk)
+ rows = get(reverse(url_name) + '?page_size=200', user=reader, expect=200).data['results']
+ for row in rows:
+ answer = row['summary_fields']['user_capabilities']['cancel']
+ assert answer == per_row_cancel(reader, row['id']), (name, row['id'], row['type'])
+ seen.add(answer)
+ # The comparison only proves something when both answers occur.
+ assert seen == {True, False}
+
+ @pytest.mark.parametrize('scopes', [None, ['read'], ['read', 'write']], ids=['session', 'read-token', 'write-token'])
+ def test_batched_answers_match_the_per_row_checks(self, cancel_world, scopes):
+ """
+ The access class's page answers equal its per job checks.
+
+ Unlike the lists, this covers the jobs a user cannot see as well, and
+ tokens, which may not cancel without write scope.
+
+ Args:
+ cancel_world: Users and jobs in every relation and state.
+ scopes: The token scopes set on the user, or None for a session.
+ """
+ jobs = list(UnifiedJob.objects.filter(pk__in=[job.pk for job in cancel_world['jobs'].values()]))
+ assert len(jobs) == len(cancel_world['jobs'])
+ batched_kinds = {'system_job'}
+ for name, fixture_user in cancel_world['users'].items():
+ reader = User.objects.get(pk=fixture_user.pk)
+ if scopes is not None:
+ reader.oauth_scopes = scopes
+ answers = UnifiedJobAccess(reader).get_cancel_capabilities(jobs)
+ for pk, answer in answers.items():
+ assert answer == per_row_cancel(reader, pk), (name, pk)
+
+ # A write-capable non-superuser has every job but the system job
+ # answered in bulk, which is what keeps the list's queries flat.
+ if name != 'superuser' and scopes != ['read']:
+ expected = {job.pk for key, job in cancel_world['jobs'].items() if key not in batched_kinds}
+ assert set(answers) == expected, name
+
+ # Spot checks that each branch of the can_cancel methods is reached.
+ spots = {
+ 'creator': {'job_running_by_creator': True, 'project_update_by_creator': True, 'job_running_by_admin': False, 'job_finished': False},
+ 'template_admin': {'job_running_by_admin': True, 'job_orphaned_in_org': False, 'project_update_by_admin': False},
+ 'org_template_admin': {'job_orphaned_in_org': True, 'job_orphaned_no_org': False, 'job_running_by_admin': True},
+ 'project_admin': {'project_update_by_admin': True, 'job_running_by_admin': False},
+ 'inventory_admin': {'inventory_update_by_admin': True, 'ad_hoc_by_admin': True, 'ad_hoc_no_inventory': False},
+ 'executor': {'job_running_by_admin': False, 'ad_hoc_by_admin': False},
+ }
+ if scopes != ['read']:
+ for name, expected in spots.items():
+ reader = User.objects.get(pk=cancel_world['users'][name].pk)
+ for key, answer in expected.items():
+ assert per_row_cancel(reader, cancel_world['jobs'][key].pk) is answer, (name, key)
+
+
+@pytest.mark.django_db
+class TestCancelCapabilityQueries:
+ """
+ Running rows cost a non-superuser no more queries than finished ones.
+
+ The start and delete capabilities still ask a role question for every row,
+ so the page is not flat as a whole. What cancel adds is isolated by
+ comparing the growth of a page of running jobs with that of a page of
+ finished ones, for which cancel is answered without a query.
+ """
+
+ @staticmethod
+ def count_queries(get, url, reader):
+ """
+ Read a list and count the queries it took.
+
+ Args:
+ get: Fixture issuing GET requests against the API.
+ url: The list read.
+ reader: The user reading it.
+
+ Returns:
+ int: The number of queries the request ran.
+ """
+ with CaptureQueriesContext(connection) as ctx:
+ get(url, user=reader, expect=200)
+ return len(ctx.captured_queries)
+
+ def growth(self, get, url, reader, make_job):
+ """
+ How many queries three more jobs add to a page that holds one.
+
+ Args:
+ get: Fixture issuing GET requests against the API.
+ url: The list read.
+ reader: The user reading it.
+ make_job: Callable creating one job of the kind measured.
+
+ Returns:
+ int: Queries with four jobs minus queries with one.
+ """
+ make_job()
+ # The first request caches the reader's system auditor lookup, so it
+ # is read once before counting.
+ self.count_queries(get, url, reader)
+ one = self.count_queries(get, url, reader)
+ for _ in range(3):
+ make_job()
+ assert get(url, user=reader, expect=200).data['count'] == 4
+ return self.count_queries(get, url, reader) - one
+
+ @pytest.mark.parametrize('url_name', ['api:unified_job_list', 'api:job_list'])
+ def test_running_rows_cost_what_finished_rows_cost(self, get, user, organization, project, inventory, admin, url_name):
+ """
+ Three more running jobs cost a template admin what three finished do.
+
+ Args:
+ get: Fixture issuing GET requests against the API.
+ user: Factory for users.
+ organization: The organization the templates belong to.
+ project: The project the templates use.
+ inventory: The inventory the templates use.
+ admin: A superuser, who launches every job.
+ url_name: The list read, the polymorphic one or the playbook runs.
+ """
+ growth = {}
+ for state in ('successful', 'running'):
+ # A template of its own per state, and the page filtered to it.
+ template = JobTemplate.objects.create(
+ name='cancel-queries-' + state, project=project, inventory=inventory, playbook='helloworld.yml', organization=organization
+ )
+ url = reverse(url_name) + '?unified_job_template={}'.format(template.pk)
+ reader = user('cancel-queries-' + state, False)
+ template.admin_role.members.add(reader)
+
+ def make_job(template=template, state=state):
+ return template.create_unified_job(_eager_fields={'status': state, 'created_by': admin})
+
+ growth[state] = self.growth(get, url, reader, make_job)
+ assert growth['running'] == growth['successful']
diff --git a/ascender/main/tests/functional/api/test_user.py b/ascender/main/tests/functional/api/test_user.py
index 45e23db91..b5d5a5d90 100644
--- a/ascender/main/tests/functional/api/test_user.py
+++ b/ascender/main/tests/functional/api/test_user.py
@@ -8,7 +8,6 @@
from ascender.main.models import User
from ascender.api.versioning import reverse
-from ascender.sso.backends import RADIUSBackend
#
# user creation
@@ -47,20 +46,6 @@ def test_user_create_without_password_ignores_complexity_settings(post, admin):
assert not User.objects.get(username=user_attrs["username"]).has_usable_password()
-@pytest.mark.django_db
-@override_settings(RADIUS_SERVER='radius.example.org')
-def test_user_created_without_password_is_not_a_radius_user(post, admin):
- """A RADIUS principal with the same username must not take over a password-less local account."""
- user_attrs = {k: v for k, v in EXAMPLE_USER_DATA.items() if k != "password"}
- user_attrs["is_superuser"] = True
- response = post(reverse('api:user_list'), user_attrs, admin, middleware=SessionMiddleware(mock.Mock()))
- assert response.status_code == 201
- with mock.patch.object(RADIUSBackend, '_radius_auth', return_value=([], False, False)):
- assert RADIUSBackend().authenticate(None, user_attrs["username"], "radius-password") is None
- # The stubbed reply does log in a username that has no local account.
- assert RADIUSBackend().authenticate(None, "radius-only", "radius-password").username == "radius-only"
-
-
@pytest.mark.django_db
def test_fail_double_create_user(post, admin):
response = post(reverse('api:user_list'), EXAMPLE_USER_DATA, admin, middleware=SessionMiddleware(mock.Mock()))
diff --git a/ascender/main/tests/functional/api/test_workflow_approval_quorum.py b/ascender/main/tests/functional/api/test_workflow_approval_quorum.py
index 18467c430..f6ff40e53 100644
--- a/ascender/main/tests/functional/api/test_workflow_approval_quorum.py
+++ b/ascender/main/tests/functional/api/test_workflow_approval_quorum.py
@@ -2,14 +2,24 @@
import pytest
+from django.db import connection
from django.http import QueryDict
+from django.test.utils import CaptureQueriesContext
from ascender.api.versioning import reverse
from ascender.api.views.workflow import _approval_vote_comment
-from ascender.main.models import WorkflowJob, WorkflowJobTemplate, WorkflowApproval, WorkflowApprovalVote
+from ascender.main.access import WorkflowApprovalAccess, WorkflowJobAccess, get_user_capabilities
+from ascender.main.models import User, WorkflowJob, WorkflowJobNode, WorkflowJobTemplate, WorkflowApproval, WorkflowApprovalVote
from ascender.main.scheduler import TaskManager, DependencyManager, WorkflowManager
+def run_managers():
+ """Move every launched workflow along until its approval node is waiting."""
+ DependencyManager().schedule()
+ TaskManager().schedule()
+ WorkflowManager().schedule()
+
+
@pytest.fixture
def spawn_approval(post, admin_user, job_template, controlplane_instance_group):
"""Create a WFJT with a single approval node, launch it and hand back the
@@ -24,9 +34,7 @@ def r(**approval_kwargs):
post(url, payload, user=admin_user, expect=201)
post(reverse('api:workflow_job_template_launch', kwargs={'pk': wfjt.pk}), user=admin_user, expect=201)
wf_job = WorkflowJob.objects.order_by('-id').first()
- DependencyManager().schedule()
- TaskManager().schedule()
- WorkflowManager().schedule()
+ run_managers()
approval = wf_job.workflow_nodes.first().job
return wfjt, WorkflowApproval.objects.get(pk=approval.pk)
@@ -214,3 +222,362 @@ def test_votes_hidden_from_unrelated_users(self, spawn_approval, approver, get,
resp = get(reverse('api:workflow_approval_vote_list'), user=rando, expect=200)
assert resp.data['count'] == 1
get(reverse('api:workflow_approval_vote_detail', kwargs={'pk': vote.pk}), user=rando, expect=200)
+
+
+@pytest.mark.django_db
+class TestApprovalCancelWorkflow:
+ """
+ Canceling the workflow an approval waits in is a right on the workflow
+ job, not the approver role, so an approver may vote without being offered
+ the cancel.
+ """
+
+ def test_approver_is_not_offered_the_cancel(self, spawn_approval, approver, get, alice, admin_user):
+ wfjt, approval = spawn_approval()
+ approver(wfjt, alice)
+ url = reverse('api:workflow_approval_detail', kwargs={'pk': approval.pk})
+
+ as_alice = get(url, user=alice, expect=200).data
+ assert as_alice['can_approve_or_deny']
+ assert not as_alice['can_cancel_workflow']
+
+ assert get(url, user=admin_user, expect=200).data['can_cancel_workflow']
+
+ def test_workflow_admin_is_offered_the_cancel_in_the_list(self, spawn_approval, get, bob):
+ wfjt, approval = spawn_approval()
+ wfjt.admin_role.members.add(bob)
+ results = get(reverse('api:workflow_approval_list'), user=bob, expect=200).data['results']
+ row = [r for r in results if r['id'] == approval.pk][0]
+ assert row['can_cancel_workflow']
+
+ def test_approval_does_not_report_a_cancel_capability(self, spawn_approval, get, admin_user):
+ """
+ An approval has no cancel endpoint, so neither its detail nor its list
+ row reports a cancel capability, even to a user who may cancel the
+ workflow it waits in.
+
+ Args:
+ spawn_approval: Fixture launching a workflow with a pending approval.
+ get: Fixture issuing GET requests against the API.
+ admin_user: A superuser, who holds every capability there is.
+ """
+ wfjt, approval = spawn_approval()
+ detail = get(reverse('api:workflow_approval_detail', kwargs={'pk': approval.pk}), user=admin_user, expect=200).data
+ assert 'cancel' not in detail['summary_fields']['user_capabilities']
+ assert detail['summary_fields']['user_capabilities']['delete'] is not None
+ assert detail['can_cancel_workflow']
+
+ results = get(reverse('api:workflow_approval_list'), user=admin_user, expect=200).data['results']
+ row = [r for r in results if r['id'] == approval.pk][0]
+ assert 'cancel' not in row['summary_fields']['user_capabilities']
+
+
+@pytest.mark.django_db
+class TestApprovalListQueries:
+ """
+ The approvals list reads the workflow job each row waits in, to say whether
+ the reader may cancel it. That has to come with the page rather than one
+ row at a time, or the list costs more queries the more approvals it holds.
+ """
+
+ @staticmethod
+ def count_list_queries(get, user, table=None):
+ """
+ Read the approvals list and count the queries it took.
+
+ Args:
+ get: Fixture issuing GET requests against the API.
+ user: The user reading the list.
+ table: When given, only the queries reading from this table count.
+
+ Returns:
+ int: The number of queries the request ran.
+ """
+ with CaptureQueriesContext(connection) as ctx:
+ get(reverse('api:workflow_approval_list'), user=user, expect=200)
+ if table is None:
+ return len(ctx.captured_queries)
+ return len([q for q in ctx.captured_queries if 'FROM "{}"'.format(table) in q['sql']])
+
+ @staticmethod
+ def launch_again(post, wfjt, user):
+ """
+ Launch the workflow once more and hand back the approval it waits on.
+
+ Args:
+ post: Fixture issuing POST requests against the API.
+ wfjt: The workflow job template holding the approval node.
+ user: The user launching it.
+
+ Returns:
+ WorkflowApproval: The pending approval the new workflow job spawned.
+ """
+ post(reverse('api:workflow_job_template_launch', kwargs={'pk': wfjt.pk}), user=user, expect=201)
+ run_managers()
+ return WorkflowApproval.objects.order_by('-id').first()
+
+ @pytest.mark.parametrize('finished', [False, True], ids=['pending', 'finished'])
+ def test_query_count_does_not_grow_with_the_approvals(self, spawn_approval, post, get, admin_user, finished):
+ """
+ Three more approvals on the page cost no more queries than one.
+
+ Args:
+ spawn_approval: Fixture launching a workflow with a pending approval.
+ post: Fixture issuing POST requests against the API.
+ get: Fixture issuing GET requests against the API.
+ admin_user: A superuser, who sees every approval.
+ finished: Whether the approvals are denied before the list is read.
+ """
+ wfjt, first = spawn_approval()
+ wfjt.allow_simultaneous = True
+ wfjt.save(update_fields=['allow_simultaneous'])
+
+ def add_approval(approval=None):
+ # Every further approval comes from launching the same workflow again.
+ if approval is None:
+ approval = self.launch_again(post, wfjt, admin_user)
+ if finished:
+ post(reverse('api:workflow_approval_deny', kwargs={'pk': approval.pk}), user=admin_user, expect=204)
+
+ add_approval(first)
+ baseline = self.count_list_queries(get, admin_user)
+ for _ in range(3):
+ add_approval()
+ assert WorkflowApproval.objects.count() == 4
+ assert self.count_list_queries(get, admin_user) == baseline
+
+
+# ----------------------------------------------------------------------------
+# Page-level permission answers
+# ----------------------------------------------------------------------------
+
+
+@pytest.fixture
+def approval_world(post, admin_user, user, organization, job_template, controlplane_instance_group, system_auditor, rando):
+ """
+ Users in every relation to a workflow and approvals in every state.
+
+ The users are a superuser, an organization admin, an organization workflow
+ admin, an admin of the workflow template, an approver, a user who may only
+ execute it, the user who launched the runs, an unrelated user and a system
+ auditor. The approvals sit in a workflow of the organization, in one with
+ no organization, in one whose template was deleted, and in one whose
+ workflow job was deleted, and are pending, approved, denied or timed out.
+
+ Args:
+ post: Fixture issuing POST requests against the API.
+ admin_user: A superuser, who builds the workflows and decides approvals.
+ user: Factory for users.
+ organization: The organization the main workflow belongs to.
+ job_template: The job template each workflow runs after its approval.
+ controlplane_instance_group: Lets the task manager place the jobs.
+ system_auditor: A system auditor.
+ rando: A user with no relation to anything.
+
+ Returns:
+ dict: 'users' maps a role name to its user, 'approvals' maps a state
+ name to its approval.
+ """
+ users = {
+ 'superuser': admin_user,
+ 'org_admin': user('matrix-org-admin', False),
+ 'org_workflow_admin': user('matrix-org-workflow-admin', False),
+ 'workflow_admin': user('matrix-workflow-admin', False),
+ 'approver': user('matrix-approver', False),
+ 'executor': user('matrix-executor', False),
+ 'creator': user('matrix-creator', False),
+ 'unrelated': rando,
+ 'auditor': system_auditor,
+ }
+ organization.admin_role.members.add(users['org_admin'])
+ organization.workflow_admin_role.members.add(users['org_workflow_admin'])
+
+ def make_workflow(name, org):
+ # One approval node, then the job template, with every template role
+ # handed to the user named after it.
+ wfjt = WorkflowJobTemplate.objects.create(name=name, organization=org, allow_simultaneous=True)
+ node = wfjt.workflow_nodes.create(unified_job_template=job_template)
+ url = reverse('api:workflow_job_template_node_create_approval', kwargs={'pk': node.pk, 'version': 'v2'})
+ post(url, {'name': name + '-approval', 'description': '', 'timeout': 0}, user=admin_user, expect=201)
+ wfjt.admin_role.members.add(users['workflow_admin'])
+ wfjt.approval_role.members.add(users['approver'])
+ wfjt.execute_role.members.add(users['executor'], users['creator'])
+ return wfjt
+
+ def launch(wfjt, by):
+ post(reverse('api:workflow_job_template_launch', kwargs={'pk': wfjt.pk}), user=by, expect=201)
+ # The test client does not record who launched, which is what
+ # can_cancel checks first, so the launcher is set here.
+ WorkflowJob.objects.filter(pk=WorkflowJob.objects.order_by('-id').first().pk).update(created_by=by)
+ run_managers()
+ return WorkflowApproval.objects.order_by('-id').first()
+
+ in_org = make_workflow('matrix-in-org', organization)
+ no_org = make_workflow('matrix-no-org', None)
+ doomed_template = make_workflow('matrix-doomed-template', organization)
+ doomed_job = make_workflow('matrix-doomed-job', organization)
+
+ approvals = {
+ 'pending_by_creator': launch(in_org, users['creator']),
+ 'pending_by_admin': launch(in_org, admin_user),
+ 'approved': launch(in_org, users['creator']),
+ 'denied': launch(in_org, admin_user),
+ 'timed_out': launch(in_org, users['creator']),
+ 'pending_no_org': launch(no_org, users['creator']),
+ 'approved_no_org': launch(no_org, admin_user),
+ 'orphaned_template': launch(doomed_template, users['creator']),
+ 'orphaned_job': launch(doomed_job, users['creator']),
+ }
+ post(reverse('api:workflow_approval_approve', kwargs={'pk': approvals['approved'].pk}), user=admin_user, expect=204)
+ post(reverse('api:workflow_approval_approve', kwargs={'pk': approvals['approved_no_org'].pk}), user=admin_user, expect=204)
+ post(reverse('api:workflow_approval_deny', kwargs={'pk': approvals['denied'].pk}), user=admin_user, expect=204)
+ TaskManager().timeout_approval_node(approvals['timed_out'])
+ run_managers()
+
+ # A deleted template leaves its workflow job with no template at all, a
+ # deleted workflow job takes its nodes along and leaves the approval with
+ # no node.
+ doomed_template.delete()
+ approvals['orphaned_job'].unified_job_node.workflow_job.delete()
+ for approval in approvals.values():
+ approval.refresh_from_db()
+ orphan = approvals['orphaned_template'].unified_job_node.workflow_job
+ assert orphan.unified_job_template_id is None and orphan.workflow_job_template_id is None
+ assert not WorkflowJobNode.objects.filter(job=approvals['orphaned_job']).exists()
+ assert {name: a.status for name, a in approvals.items()} == {
+ 'pending_by_creator': 'pending',
+ 'pending_by_admin': 'pending',
+ 'approved': 'successful',
+ 'denied': 'failed',
+ 'timed_out': 'failed',
+ 'pending_no_org': 'pending',
+ 'approved_no_org': 'successful',
+ 'orphaned_template': 'failed',
+ 'orphaned_job': 'pending',
+ }
+ assert approvals['timed_out'].timed_out is True
+ assert WorkflowJob.objects.get(workflow_job_nodes__job=approvals['orphaned_template']).created_by == users['creator']
+ return {'users': users, 'approvals': approvals}
+
+
+@pytest.mark.django_db
+class TestApprovalListPermissionsMatchPerRow:
+ """
+ The approvals list answers can_approve_or_deny and can_cancel_workflow for
+ its whole page at once. Every answer has to be the one the approval would
+ get on its own, for every kind of user and every state of approval.
+ """
+
+ FIELDS = ('can_approve_or_deny', 'can_cancel_workflow')
+
+ def test_list_matches_detail(self, approval_world, get):
+ """
+ Each row of the list carries the answers its detail view gives.
+
+ Args:
+ approval_world: Users and approvals in every relation and state.
+ get: Fixture issuing GET requests against the API.
+ """
+ seen = {field: set() for field in self.FIELDS}
+ for name, user in approval_world['users'].items():
+ rows = get(reverse('api:workflow_approval_list') + '?page_size=200', user=user, expect=200).data['results']
+ for row in rows:
+ detail = get(reverse('api:workflow_approval_detail', kwargs={'pk': row['id']}), user=user, expect=200).data
+ for field in self.FIELDS:
+ assert row[field] == detail[field], (name, row['id'], field)
+ seen[field].add(row[field])
+ # The matrix is only a proof when both answers actually occur.
+ assert seen == {field: {True, False} for field in self.FIELDS}
+
+ @pytest.mark.parametrize('scopes', [None, ['read'], ['read', 'write']], ids=['session', 'read-token', 'write-token'])
+ def test_batched_answers_match_the_per_row_checks(self, approval_world, scopes):
+ """
+ The access classes' page answers equal their per object checks.
+
+ Unlike the list, this covers the approvals a user cannot see as well,
+ and read-only tokens, which may never cancel.
+
+ Args:
+ approval_world: Users and approvals in every relation and state.
+ scopes: The token scopes set on the user, or None for a session.
+ """
+ approvals = list(
+ WorkflowApprovalAccess(approval_world['users']['superuser']).get_queryset().filter(pk__in=[a.pk for a in approval_world['approvals'].values()])
+ )
+ assert len(approvals) == len(approval_world['approvals'])
+ workflow_jobs = {}
+ for approval in approvals:
+ try:
+ workflow_job = approval.unified_job_node.workflow_job
+ except WorkflowApproval.unified_job_node.RelatedObjectDoesNotExist:
+ continue
+ workflow_jobs[workflow_job.pk] = workflow_job
+
+ for name, fixture_user in approval_world['users'].items():
+ user = User.objects.get(pk=fixture_user.pk)
+ if scopes is not None:
+ user.oauth_scopes = scopes
+
+ approve_answers = WorkflowApprovalAccess(user).can_approve_or_deny_many(approvals)
+ # None of these rows needs the per row fallback.
+ assert set(approve_answers) == {approval.pk for approval in approvals}, name
+ for approval in approvals:
+ fresh = WorkflowApproval.objects.get(pk=approval.pk)
+ assert approve_answers[approval.pk] == user.can_access(WorkflowApproval, 'approve_or_deny', fresh), (name, approval.pk)
+
+ cancel_answers = WorkflowJobAccess(user).get_cancel_capabilities(workflow_jobs.values())
+ assert set(cancel_answers) == set(workflow_jobs), name
+ for pk in workflow_jobs:
+ fresh = WorkflowJob.objects.get(pk=pk)
+ expected = get_user_capabilities(user, fresh, method_list=['cancel'])['cancel']
+ assert cancel_answers[pk] == expected, (name, pk)
+
+ # Spot checks that the matrix reaches each branch of can_cancel,
+ # not only the outcomes: the creator of a run whose template is
+ # gone, the organization workflow admin, and the template admin.
+ if scopes != ['read']:
+ job_of = {
+ key: approval_world['approvals'][key].unified_job_node.workflow_job_id
+ for key in ('orphaned_template', 'pending_by_admin', 'pending_no_org')
+ }
+ expected_spots = {
+ 'creator': {'orphaned_template': True, 'pending_by_admin': False, 'pending_no_org': True},
+ 'org_workflow_admin': {'orphaned_template': False, 'pending_by_admin': True, 'pending_no_org': False},
+ 'workflow_admin': {'orphaned_template': False, 'pending_by_admin': True, 'pending_no_org': True},
+ 'approver': {'orphaned_template': False, 'pending_by_admin': False, 'pending_no_org': False},
+ }
+ for key, answer in expected_spots.get(name, {}).items():
+ assert cancel_answers[job_of[key]] is answer, (name, key)
+
+ @pytest.mark.parametrize('role', ['approver', 'workflow_admin', 'org_workflow_admin', 'executor'])
+ def test_query_count_does_not_grow_for_a_non_superuser(self, spawn_approval, post, get, admin_user, user, organization, role):
+ """
+ Three more approvals on the page cost a non-superuser no more queries.
+
+ Args:
+ spawn_approval: Fixture launching a workflow with a pending approval.
+ post: Fixture issuing POST requests against the API.
+ get: Fixture issuing GET requests against the API.
+ admin_user: A superuser, who launches the workflow.
+ user: Factory for users.
+ organization: The organization the workflow belongs to.
+ role: Which relation the reader has to the workflow.
+ """
+ wfjt, first = spawn_approval()
+ wfjt.allow_simultaneous = True
+ wfjt.organization = organization
+ wfjt.save(update_fields=['allow_simultaneous', 'organization'])
+ reader = user('reader-' + role, False)
+ if role == 'org_workflow_admin':
+ organization.workflow_admin_role.members.add(reader)
+ else:
+ getattr(wfjt, {'approver': 'approval_role', 'workflow_admin': 'admin_role', 'executor': 'execute_role'}[role]).members.add(reader)
+
+ # The first request caches the reader's system auditor lookup, so
+ # it is read once before counting.
+ TestApprovalListQueries.count_list_queries(get, reader)
+ baseline = TestApprovalListQueries.count_list_queries(get, reader)
+ for _ in range(3):
+ TestApprovalListQueries.launch_again(post, wfjt, admin_user)
+ assert get(reverse('api:workflow_approval_list'), user=reader, expect=200).data['count'] == 4
+ assert TestApprovalListQueries.count_list_queries(get, reader) == baseline
diff --git a/ascender/main/tests/functional/test_backends.py b/ascender/main/tests/functional/test_backends.py
new file mode 100644
index 000000000..dc6f3ef6a
--- /dev/null
+++ b/ascender/main/tests/functional/test_backends.py
@@ -0,0 +1,37 @@
+import pytest
+
+from ascender.main.backends import AscenderModelBackend, AWXModelBackend
+
+
+@pytest.mark.django_db
+def test_old_backend_name_loads_sessions_but_does_not_authenticate(user):
+ """Both names are listed in AUTHENTICATION_BACKENDS, and django tries each.
+
+ The old one is there only so sessions recorded under it still load. Were it
+ to authenticate as well, every failed local login would be tried twice.
+ """
+ alice = user('alice', False) # the fixture's password is the username
+
+ assert AscenderModelBackend().authenticate(None, username='alice', password='alice') == alice
+ assert AWXModelBackend().authenticate(None, username='alice', password='alice') is None
+ assert AWXModelBackend().get_user(alice.pk) == alice
+
+
+@pytest.mark.django_db
+def test_session_recorded_under_the_old_backend_name_still_loads(user, rf):
+ """A session opened before the rename records the old path, and still loads.
+
+ django refuses a session whose recorded backend is not listed in
+ AUTHENTICATION_BACKENDS, so this holds only while the alias stays listed.
+ """
+ from django.contrib.auth import BACKEND_SESSION_KEY, HASH_SESSION_KEY, SESSION_KEY, get_user
+ from django.contrib.sessions.backends.db import SessionStore
+
+ alice = user('alice', False)
+ request = rf.get('/')
+ request.session = SessionStore()
+ request.session[SESSION_KEY] = str(alice.pk)
+ request.session[BACKEND_SESSION_KEY] = 'ascender.main.backends.AWXModelBackend'
+ request.session[HASH_SESSION_KEY] = alice.get_session_auth_hash()
+
+ assert get_user(request) == alice
diff --git a/ascender/main/tests/functional/test_dab_rest_filters.py b/ascender/main/tests/functional/test_dab_rest_filters.py
index 28050d465..b32433d27 100644
--- a/ascender/main/tests/functional/test_dab_rest_filters.py
+++ b/ascender/main/tests/functional/test_dab_rest_filters.py
@@ -2,7 +2,8 @@
from django.http import QueryDict
from ascender.api.filters import HostFieldLookupBackend
-from ascender.main.models import Group, Host
+from ascender.api.versioning import reverse
+from ascender.main.models import Group, Host, ProjectUpdate
# Coverage for the vendored ascender.dab.rest_filters JSONField-as-text filtering,
# including lookups that traverse relations (hosts__ansible_facts__...), which
@@ -49,3 +50,61 @@ def test_related_jsonfield_filter(facts_host):
result = _filter(Group.objects.all(), 'hosts__ansible_facts__icontains=rockylinux')
assert list(result) == [group]
assert not _filter(Group.objects.all(), 'hosts__ansible_facts__icontains=does-not-exist').exists()
+
+
+@pytest.mark.django_db
+def test_role_level_on_model_without_roles_is_a_400(get, admin, notification_template):
+ """The ParseError for role_level must reach the client rather than a 500.
+
+ Unpacking a throwaway value into "_" elsewhere in filter_queryset once made
+ gettext unreachable there, so raising this error crashed while building it.
+ """
+ url = reverse('api:notification_template_list')
+ response = get(url + '?role_level=notification_admin_role', admin, expect=400)
+ assert 'role_level' in str(response.data)
+
+
+@pytest.mark.django_db
+def test_type_in_matches_each_listed_type(get, admin, job_factory, project):
+ """type__in names several run types and matches a run of any of them.
+
+ Type names lose their underscores before the lookup, and that step once
+ ended the value handling, so the list was never split and matched nothing.
+ """
+ job = job_factory()
+ update = ProjectUpdate.objects.create(project=project, name=project.name)
+ url = reverse('api:unified_job_list')
+
+ def ids(query):
+ response = get(url + query, admin, expect=200)
+ return {row['id'] for row in response.data['results']}
+
+ assert {job.id, update.id} <= ids('?type__in=job,project_update')
+ assert update.id in ids('?type__in=project_update')
+ assert job.id not in ids('?type__in=project_update')
+ assert job.id in ids('?type=job')
+
+
+@pytest.mark.django_db
+@pytest.mark.parametrize(
+ 'query, matches_job, matches_update',
+ [
+ ('?or__type=job', True, False),
+ ('?not__type=job', False, True),
+ ('?type__exact=project_update', False, True),
+ ('?type__icontains=project_up', False, True),
+ ('?type__startswith=jo', True, False),
+ ],
+)
+def test_type_lookups_keep_the_type_name(get, admin, job_factory, project, query, matches_job, matches_update):
+ """Every lookup on type compares type names, never content type ids.
+
+ Splitting type__in once let the other suffixes reach the generic value
+ conversion, which read the name as an id and answered 400.
+ """
+ job = job_factory()
+ update = ProjectUpdate.objects.create(project=project, name=project.name)
+ response = get(reverse('api:unified_job_list') + query, admin, expect=200)
+ found = {row['id'] for row in response.data['results']}
+ assert (job.id in found) is matches_job
+ assert (update.id in found) is matches_update
diff --git a/ascender/main/tests/functional/test_labels.py b/ascender/main/tests/functional/test_labels.py
index b57b3372f..6fd1646b8 100644
--- a/ascender/main/tests/functional/test_labels.py
+++ b/ascender/main/tests/functional/test_labels.py
@@ -1,7 +1,7 @@
import pytest
# awx
-from ascender.main.models import WorkflowJobTemplate
+from ascender.main.models import Job, Label, WorkflowJobTemplate
from ascender.api.versioning import reverse
@@ -32,3 +32,103 @@ def test_workflow_can_remove_label(org_admin, organization, post, get):
post(url, data, org_admin, expect=204)
results = get(url, org_admin, expect=200)
assert results.data['count'] == 0
+
+
+@pytest.mark.django_db
+class TestLabelDelete:
+ """
+ A label can be deleted from its detail view only while nothing carries it,
+ since deleting one strips it from every template, inventory and job it is
+ on, across organizations and in job history alike.
+ """
+
+ @staticmethod
+ def url(label):
+ """
+ The detail URL of a label.
+
+ Args:
+ label: The label.
+
+ Returns:
+ str: Its /api/v2/labels/N/ URL.
+ """
+ return reverse('api:label_detail', kwargs={'pk': label.pk})
+
+ def test_org_admin_deletes_a_detached_label(self, label, org_admin, delete):
+ """
+ An admin of the label's organization deletes a label nothing carries.
+
+ Args:
+ label: A label of the organization, attached to nothing.
+ org_admin: An admin of that organization.
+ delete: Fixture issuing DELETE requests against the API.
+ """
+ delete(self.url(label), org_admin, expect=204)
+ assert not Label.objects.filter(pk=label.pk).exists()
+
+ @pytest.mark.parametrize('holder', ['job_template', 'inventory', 'finished_job'])
+ @pytest.mark.parametrize('who', ['org_admin', 'superuser'])
+ def test_attached_label_is_refused(self, label, org_admin, admin_user, inventory, job_template, delete, holder, who):
+ """
+ A label anything carries is refused with 409, and nothing changes.
+
+ Args:
+ label: A label of the organization.
+ org_admin: An admin of that organization.
+ admin_user: A superuser, refused the same way.
+ inventory: An inventory of the organization.
+ job_template: A job template.
+ delete: Fixture issuing DELETE requests against the API.
+ holder: What carries the label.
+ who: The user asking for the delete.
+ """
+ if holder == 'job_template':
+ carrier = job_template
+ elif holder == 'inventory':
+ carrier = inventory
+ else:
+ carrier = Job.objects.create(name='finished', status='successful')
+ carrier.labels.add(label)
+ user = {'org_admin': org_admin, 'superuser': admin_user}[who]
+
+ response = delete(self.url(label), user, expect=409)
+
+ assert 'attached' in str(response.data['detail'])
+ assert Label.objects.filter(pk=label.pk).exists()
+ assert list(carrier.labels.values_list('pk', flat=True)) == [label.pk]
+
+ def test_user_who_is_not_org_admin_is_forbidden(self, label, org_member, job_template, delete):
+ """
+ A user who can read the label but is not an admin of its organization
+ gets 403, whether or not the label is in use.
+
+ Args:
+ label: A label of the organization.
+ org_member: A plain member of that organization.
+ job_template: A job template the label is put on halfway.
+ delete: Fixture issuing DELETE requests against the API.
+ """
+ delete(self.url(label), org_member, expect=403)
+ job_template.labels.add(label)
+ delete(self.url(label), org_member, expect=403)
+ assert Label.objects.filter(pk=label.pk).exists()
+
+ def test_capabilities_follow_the_organization_admin(self, label, org_admin, org_member, admin_user, get):
+ """
+ The list reports edit and delete to an organization admin and a
+ superuser only, the same as the label's detail view.
+
+ Args:
+ label: A label of the organization.
+ org_admin: An admin of that organization.
+ org_member: A plain member of that organization.
+ admin_user: A superuser.
+ get: Fixture issuing GET requests against the API.
+ """
+ expected = {org_admin: True, admin_user: True, org_member: False}
+ for user, allowed in expected.items():
+ row = [r for r in get(reverse('api:label_list'), user, expect=200).data['results'] if r['id'] == label.pk][0]
+ detail = get(self.url(label), user, expect=200).data
+ for data in (row, detail):
+ assert data['summary_fields']['user_capabilities'] == {'edit': allowed, 'delete': allowed}, user.username
diff --git a/ascender/main/tests/unit/test_locales.py b/ascender/main/tests/unit/test_locales.py
index 359483d6c..f7f46b773 100644
--- a/ascender/main/tests/unit/test_locales.py
+++ b/ascender/main/tests/unit/test_locales.py
@@ -10,6 +10,11 @@
AWX_ROOT = os.path.dirname(os.path.abspath(ascender.__file__))
BACKEND_CATALOG_DIR = os.path.join(AWX_ROOT, 'locale')
UI_BUNDLE_DIR = os.path.join(AWX_ROOT, 'ui', 'src', 'locales')
+UI_THEME_DIR = os.path.join(AWX_ROOT, 'ui', 'src', 'themes')
+
+# A theme an administrator uploads through CUSTOM_THEME, which is selectable
+# without a stylesheet of its own in the tree.
+THEMES_WITHOUT_A_STYLESHEET = {'custom'}
# 'en' is declared without a backend catalog on purpose: the UI ships its bundle
# under that code, and a request that resolves to 'en' falls through to the
@@ -39,3 +44,25 @@ def test_supported_ui_locales_match_the_ui_bundles():
is a different set from LANGUAGES: the UI names its English bundle 'en' while
the backend catalog is 'en-us'."""
assert SUPPORTED_UI_LOCALES - {''} == _subdirectories(UI_BUNDLE_DIR)
+
+
+def _registered_choices(setting):
+ from ascender.conf import settings_registry
+
+ return set(settings_registry.get_setting_field(setting).choices)
+
+
+@pytest.mark.skipif(not os.path.isdir(UI_THEME_DIR), reason='UI sources are not present in this layout')
+def test_default_ui_theme_choices_match_the_bundled_stylesheets():
+ """DEFAULT_UI_THEME is a hand written list, and the themes it names are
+ stylesheets bundled by the UI build. Nothing connects the two at runtime: a
+ theme added to one and not the other is either missing from the setting or
+ offered and then not found, and neither fails until somebody picks it."""
+ bundled = {name[: -len('.css')] for name in os.listdir(UI_THEME_DIR) if name.endswith('.css')}
+ assert _registered_choices('DEFAULT_UI_THEME') - THEMES_WITHOUT_A_STYLESHEET == bundled
+
+
+def test_default_ui_language_choices_match_the_supported_locales():
+ """The same for DEFAULT_UI_LANGUAGE, against the locales preferred_language
+ is already validated against, which the test above ties to the UI bundles."""
+ assert _registered_choices('DEFAULT_UI_LANGUAGE') == SUPPORTED_UI_LOCALES
diff --git a/ascender/settings/defaults.py b/ascender/settings/defaults.py
index 6615b2d77..c7a53fb1b 100644
--- a/ascender/settings/defaults.py
+++ b/ascender/settings/defaults.py
@@ -282,6 +282,13 @@
# on how many events to display before truncating/hiding
MAX_UI_JOB_EVENTS = 4000
+# The ceiling on how far a read-only variables editor grows to fit its value.
+# The editor renders a line of DOM per line of content, and a host's facts run
+# to thousands, so an uncapped box costs seconds to paint and lags every scroll
+# after. The rest of the value scrolls. A reader can still ask a single editor
+# for more than this; the cap governs the height it takes on its own.
+MAX_UI_EDITOR_ROWS = 50
+
# Returned in index.html, tells the UI if it should make requests
# to update job data in response to status changes websocket events
UI_LIVE_UPDATES_ENABLED = True
@@ -538,8 +545,6 @@
'ascender.sso.backends.LDAPBackend3',
'ascender.sso.backends.LDAPBackend4',
'ascender.sso.backends.LDAPBackend5',
- 'ascender.sso.backends.RADIUSBackend',
- 'ascender.sso.backends.TACACSPlusBackend',
'social_core.backends.google.GoogleOAuth2',
'social_core.backends.github.GithubOAuth2',
'social_core.backends.github.GithubOrganizationOAuth2',
@@ -551,6 +556,10 @@
'social_core.backends.azuread.AzureADOAuth2',
'social_core.backends.azuread_tenant.AzureADTenantOAuth2',
'ascender.sso.backends.SAMLAuth',
+ 'ascender.main.backends.AscenderModelBackend',
+ # The same class under the name it used to have. Sessions record the path
+ # they were created with, and django will not load one that is not listed,
+ # so removing this logs out everyone who signed in before the rename.
'ascender.main.backends.AWXModelBackend',
)
@@ -573,20 +582,6 @@
# Note: This setting may be overridden by database settings.
AUTH_LDAP_CONNECTION_OPTIONS = {ldap.OPT_REFERRALS: 0, ldap.OPT_NETWORK_TIMEOUT: 30}
-# Radius server settings (default to empty string to skip using Radius auth).
-# Note: These settings may be overridden by database settings.
-RADIUS_SERVER = ''
-RADIUS_PORT = 1812
-RADIUS_SECRET = ''
-
-# TACACS+ settings (default host to empty string to skip using TACACS+ auth).
-# Note: These settings may be overridden by database settings.
-TACACSPLUS_HOST = ''
-TACACSPLUS_PORT = 49
-TACACSPLUS_SECRET = ''
-TACACSPLUS_SESSION_TIMEOUT = 5
-TACACSPLUS_AUTH_PROTOCOL = 'ascii'
-TACACSPLUS_REM_ADDR = False
# Enable / Disable HTTP Basic Authentication used in the API browser
# Note: Session limits are not enforced when using HTTP Basic Authentication.
diff --git a/ascender/settings/typed.py b/ascender/settings/typed.py
index 2174312e7..353c1eac8 100644
--- a/ascender/settings/typed.py
+++ b/ascender/settings/typed.py
@@ -237,6 +237,8 @@ class AscenderSettings(Protocol):
CUSTOM_LOGO: str
CUSTOM_THEME: str
CUSTOM_THEME_NAME: str
+ DEFAULT_UI_THEME: str
+ DEFAULT_UI_LANGUAGE: str
CUSTOM_TITLE: str
DATABASES: dict[str, Any]
DATABASE_CONN_MAX_AGE: Any
@@ -387,6 +389,7 @@ class AscenderSettings(Protocol):
MAX_EVENT_RES_DATA: int
MAX_FORKS: int
MAX_PAGE_SIZE: int
+ MAX_UI_EDITOR_ROWS: int
MAX_UI_JOB_EVENTS: int
MAX_WEBSOCKET_EVENT_RATE: int
MEDIA_ROOT: str
@@ -420,9 +423,6 @@ class AscenderSettings(Protocol):
PROJECTS_ROOT: str
PROJECT_UPDATE_VVV: bool
PROXY_IP_ALLOWED_LIST: list[Any]
- RADIUS_PORT: int
- RADIUS_SECRET: str
- RADIUS_SERVER: str
RECEPTOR_LOG_LEVEL: str
RECEPTOR_RELEASE_WORK: bool
RECEPTOR_SERVICE_ADVERTISEMENT_PERIOD: int
@@ -605,12 +605,6 @@ class AscenderSettings(Protocol):
SUBSYSTEM_METRICS_TASK_MANAGER_RECORD_INTERVAL: int
SUBSYSTEM_METRICS_VALKEY_KEY_PREFIX: str
SYSTEM_UUID: str
- TACACSPLUS_AUTH_PROTOCOL: str
- TACACSPLUS_HOST: str
- TACACSPLUS_PORT: int
- TACACSPLUS_REM_ADDR: bool
- TACACSPLUS_SECRET: str
- TACACSPLUS_SESSION_TIMEOUT: int
TASKS: dict[str, Any]
TASK_MANAGER_TIMEOUT: int
TASK_MANAGER_TIMEOUT_GRACE_PERIOD: int
diff --git a/ascender/sso/backends.py b/ascender/sso/backends.py
index 9135a0eca..3c4239911 100644
--- a/ascender/sso/backends.py
+++ b/ascender/sso/backends.py
@@ -13,7 +13,6 @@
from django.contrib.auth.models import User
from django.conf import settings as django_settings
from django.core.signals import setting_changed
-from django.utils.encoding import force_str
from django.http import HttpResponse
# django-auth-ldap
@@ -22,15 +21,6 @@
from django_auth_ldap.backend import populate_user
from django.core.exceptions import ImproperlyConfigured
-# radiusauth
-from radiusauth.backends import RADIUSBackend as BaseRADIUSBackend
-
-# tacacs+ auth
-# tacacs_plus 2.x ships an empty top-level __init__.py, so both names are
-# imported from the modules that define them rather than off the package.
-from tacacs_plus.client import TACACSClient
-from tacacs_plus.flags import TAC_PLUS_AUTHEN_TYPES
-
# social
from social_core.backends.saml import SAMLAuth as BaseSAMLAuth
from social_core.backends.saml import SAMLIdentityProvider as BaseSAMLIdentityProvider
@@ -182,95 +172,6 @@ def _decorate_enterprise_user(user, provider):
return enterprise_auth
-def _get_or_set_enterprise_user(username, password, provider):
- created = False
- try:
- user = User.objects.prefetch_related('enterprise_auth').get(username=username)
- except User.DoesNotExist:
- user = User(username=username)
- enterprise_auth = _decorate_enterprise_user(user, provider)
- logger.debug("Created enterprise user %s via %s backend." % (username, enterprise_auth.get_provider_display()))
- created = True
- if created or user.is_in_enterprise_category(provider):
- return user
- logger.warning("Enterprise user %s already defined in Tower." % username)
-
-
-class RADIUSBackend(BaseRADIUSBackend):
- """RADIUS authentication, which answers only when a server is configured."""
-
- def authenticate(self, request, username, password):
- if not django_settings.RADIUS_SERVER:
- return None
- return super(RADIUSBackend, self).authenticate(request, username, password)
-
- def get_user(self, user_id):
- if not django_settings.RADIUS_SERVER:
- return None
- user = super(RADIUSBackend, self).get_user(user_id)
- if not user.has_usable_password():
- return user
-
- def get_django_user(self, username, password=None, groups=[], is_staff=False, is_superuser=False):
- return _get_or_set_enterprise_user(force_str(username), force_str(password), 'radius')
-
-
-class TACACSPlusBackend(object):
- """
- Custom TACACS+ auth backend for AWX
- """
-
- def authenticate(self, request, username, password):
- if not django_settings.TACACSPLUS_HOST:
- return None
- try:
- # Upstream TACACS+ client does not accept non-string, so convert if needed.
- tacacs_client = TACACSClient(
- django_settings.TACACSPLUS_HOST,
- django_settings.TACACSPLUS_PORT,
- django_settings.TACACSPLUS_SECRET,
- timeout=django_settings.TACACSPLUS_SESSION_TIMEOUT,
- )
-
- # Validate auth protocol before dictionary lookup
- auth_protocol = django_settings.TACACSPLUS_AUTH_PROTOCOL
- if auth_protocol not in TAC_PLUS_AUTHEN_TYPES:
- logger.error(f"Invalid TACACSPLUS_AUTH_PROTOCOL: {auth_protocol}. Valid options: {list(TAC_PLUS_AUTHEN_TYPES.keys())}")
- return None
-
- auth_kwargs = {'authen_type': TAC_PLUS_AUTHEN_TYPES[auth_protocol]}
- if django_settings.TACACSPLUS_REM_ADDR:
- client_ip = self._get_client_ip(request)
- if client_ip:
- auth_kwargs['rem_addr'] = client_ip
- auth = tacacs_client.authenticate(username, password, **auth_kwargs)
- except Exception as e:
- logger.exception("TACACS+ Authentication Error: %s" % str(e))
- return None
- if auth.valid:
- return _get_or_set_enterprise_user(username, password, 'tacacs+')
-
- def get_user(self, user_id):
- if not django_settings.TACACSPLUS_HOST:
- return None
- try:
- return User.objects.get(pk=user_id)
- except User.DoesNotExist:
- return None
-
- def _get_client_ip(self, request):
- if not request or not hasattr(request, 'META'):
- return None
-
- # Check X-Forwarded-For header first (comma-separated list)
- x_forwarded_for = request.META.get('HTTP_X_FORWARDED_FOR')
- if x_forwarded_for:
- return x_forwarded_for.split(',')[0].strip()
-
- # Fall back to REMOTE_ADDR
- return request.META.get('REMOTE_ADDR')
-
-
class AscenderSAMLIdentityProvider(BaseSAMLIdentityProvider):
"""
Custom Identity Provider to make attributes to what we expect.
diff --git a/ascender/sso/common.py b/ascender/sso/common.py
index 81aa1af0b..16a82c42a 100644
--- a/ascender/sso/common.py
+++ b/ascender/sso/common.py
@@ -217,12 +217,10 @@ def get_external_account(user):
def is_remote_auth_enabled():
from ascender.settings.typed import settings
- # Append LDAP, Radius, TACACS+ and SAML options
+ # Append LDAP and SAML options
settings_that_turn_on_remote_auth = [
'AUTH_LDAP_SERVER_URI',
'SOCIAL_AUTH_SAML_ENABLED_IDPS',
- 'RADIUS_SERVER',
- 'TACACSPLUS_HOST',
]
# Also include any SOCAIL_AUTH_*KEY (except SAML)
for social_auth_key in dir(settings):
diff --git a/ascender/sso/conf.py b/ascender/sso/conf.py
index 697362c14..798a85edf 100644
--- a/ascender/sso/conf.py
+++ b/ascender/sso/conf.py
@@ -7,11 +7,8 @@
from django.urls import reverse
from django.utils.translation import gettext_lazy as _
-# Django REST Framework
-from rest_framework import serializers
-
# Ascender
-from ascender.conf import register, register_validate, fields
+from ascender.conf import register, fields
from ascender.sso.fields import (
AuthenticationBackendsField,
LDAPConnectionOptionsField,
@@ -41,7 +38,7 @@
TriggerRuleField,
)
from ascender.main.validators import validate_private_key, validate_certificate
-from ascender.sso.validators import validate_ldap_bind_dn, validate_tacacsplus_disallow_nonascii # noqa
+from ascender.sso.validators import validate_ldap_bind_dn # noqa
class SocialAuthCallbackURL(object):
@@ -459,119 +456,6 @@ def _register_ldap(append=None):
_register_ldap('4')
_register_ldap('5')
-###############################################################################
-# RADIUS AUTHENTICATION SETTINGS
-###############################################################################
-
-register(
- 'RADIUS_SERVER',
- field_class=fields.CharField,
- allow_blank=True,
- default='',
- label=_('RADIUS Server'),
- help_text=_('Hostname/IP of RADIUS server. RADIUS authentication is disabled if this setting is empty.'),
- category=_('RADIUS'),
- category_slug='radius',
- placeholder='radius.example.com',
-)
-
-register(
- 'RADIUS_PORT',
- field_class=fields.IntegerField,
- min_value=1,
- max_value=65535,
- default=1812,
- label=_('RADIUS Port'),
- help_text=_('Port of RADIUS server.'),
- category=_('RADIUS'),
- category_slug='radius',
-)
-
-register(
- 'RADIUS_SECRET',
- field_class=fields.CharField,
- allow_blank=True,
- default='',
- label=_('RADIUS Secret'),
- help_text=_('Shared secret for authenticating to RADIUS server.'),
- category=_('RADIUS'),
- category_slug='radius',
- encrypted=True,
-)
-
-###############################################################################
-# TACACSPLUS AUTHENTICATION SETTINGS
-###############################################################################
-
-register(
- 'TACACSPLUS_HOST',
- field_class=fields.CharField,
- allow_blank=True,
- default='',
- label=_('TACACS+ Server'),
- help_text=_('Hostname of TACACS+ server.'),
- category=_('TACACS+'),
- category_slug='tacacsplus',
-)
-
-register(
- 'TACACSPLUS_PORT',
- field_class=fields.IntegerField,
- min_value=1,
- max_value=65535,
- default=49,
- label=_('TACACS+ Port'),
- help_text=_('Port number of TACACS+ server.'),
- category=_('TACACS+'),
- category_slug='tacacsplus',
-)
-
-register(
- 'TACACSPLUS_SECRET',
- field_class=fields.CharField,
- allow_blank=True,
- default='',
- validators=[validate_tacacsplus_disallow_nonascii],
- label=_('TACACS+ Secret'),
- help_text=_('Shared secret for authenticating to TACACS+ server.'),
- category=_('TACACS+'),
- category_slug='tacacsplus',
- encrypted=True,
-)
-
-register(
- 'TACACSPLUS_SESSION_TIMEOUT',
- field_class=fields.IntegerField,
- min_value=0,
- default=5,
- label=_('TACACS+ Auth Session Timeout'),
- help_text=_('TACACS+ session timeout value in seconds, 0 disables timeout.'),
- category=_('TACACS+'),
- category_slug='tacacsplus',
- unit=_('seconds'),
-)
-
-register(
- 'TACACSPLUS_AUTH_PROTOCOL',
- field_class=fields.ChoiceField,
- choices=['ascii', 'pap'],
- default='ascii',
- label=_('TACACS+ Authentication Protocol'),
- help_text=_('Choose the authentication protocol used by TACACS+ client.'),
- category=_('TACACS+'),
- category_slug='tacacsplus',
-)
-
-register(
- 'TACACSPLUS_REM_ADDR',
- field_class=fields.BooleanField,
- default=True,
- label=_('TACACS+ client address sending enabled'),
- help_text=_('Enable the client address sending by TACACS+ client.'),
- category=_('TACACS+'),
- category_slug='tacacsplus',
-)
-
###############################################################################
# GOOGLE OAUTH2 AUTHENTICATION SETTINGS
###############################################################################
@@ -1921,23 +1805,3 @@ def get_saml_entity_id():
category=_('Authentication'),
category_slug='authentication',
)
-
-
-def tacacs_validate(serializer, attrs):
- if not serializer.instance or not hasattr(serializer.instance, 'TACACSPLUS_HOST') or not hasattr(serializer.instance, 'TACACSPLUS_SECRET'):
- return attrs
- errors = []
- host = serializer.instance.TACACSPLUS_HOST
- if 'TACACSPLUS_HOST' in attrs:
- host = attrs['TACACSPLUS_HOST']
- secret = serializer.instance.TACACSPLUS_SECRET
- if 'TACACSPLUS_SECRET' in attrs:
- secret = attrs['TACACSPLUS_SECRET']
- if host and not secret:
- errors.append('TACACSPLUS_SECRET is required when TACACSPLUS_HOST is provided.')
- if errors:
- raise serializers.ValidationError(_('\n'.join(errors)))
- return attrs
-
-
-register_validate('tacacsplus', tacacs_validate)
diff --git a/ascender/sso/fields.py b/ascender/sso/fields.py
index 3af85d889..581203ed7 100644
--- a/ascender/sso/fields.py
+++ b/ascender/sso/fields.py
@@ -34,7 +34,6 @@
validate_ldap_filter,
validate_ldap_filter_with_user,
validate_trigger_rule,
- validate_tacacsplus_disallow_nonascii,
)
@@ -147,7 +146,6 @@ class AuthenticationBackendsField(fields.StringListField):
('ascender.sso.backends.LDAPBackend3', ['AUTH_LDAP_3_SERVER_URI']),
('ascender.sso.backends.LDAPBackend4', ['AUTH_LDAP_4_SERVER_URI']),
('ascender.sso.backends.LDAPBackend5', ['AUTH_LDAP_5_SERVER_URI']),
- ('ascender.sso.backends.RADIUSBackend', ['RADIUS_SERVER']),
('social_core.backends.google.GoogleOAuth2', ['SOCIAL_AUTH_GOOGLE_OAUTH2_KEY', 'SOCIAL_AUTH_GOOGLE_OAUTH2_SECRET']),
('social_core.backends.github.GithubOAuth2', ['SOCIAL_AUTH_GITHUB_KEY', 'SOCIAL_AUTH_GITHUB_SECRET']),
('social_core.backends.open_id_connect.OpenIdConnectAuth', ['SOCIAL_AUTH_OIDC_KEY', 'SOCIAL_AUTH_OIDC_SECRET', 'SOCIAL_AUTH_OIDC_OIDC_ENDPOINT']),
@@ -203,6 +201,11 @@ class AuthenticationBackendsField(fields.StringListField):
],
),
('django.contrib.auth.backends.ModelBackend', []),
+ ('ascender.main.backends.AscenderModelBackend', []),
+ # The old name of the local backend, which only loads sessions opened
+ # before the rename (see AWXModelBackend). It needs no settings; it is
+ # listed so the fallback in _default_from_required_settings, which
+ # takes the backends from this map, still lets those sessions load.
('ascender.main.backends.AWXModelBackend', []),
]
)
diff --git a/ascender/sso/migrations/0004_drop_radius_and_tacacsplus.py b/ascender/sso/migrations/0004_drop_radius_and_tacacsplus.py
new file mode 100644
index 000000000..497cdebe3
--- /dev/null
+++ b/ascender/sso/migrations/0004_drop_radius_and_tacacsplus.py
@@ -0,0 +1,67 @@
+# Copyright (c) 2026 Ctrl IQ, Inc.
+# All Rights Reserved.
+
+from django.db import migrations, models
+
+#: What the two dropped backends recorded against a user, and the settings
+#: they were configured with.
+DROPPED_PROVIDERS = ['radius', 'tacacs+']
+DROPPED_SETTINGS = [
+ 'RADIUS_SERVER',
+ 'RADIUS_PORT',
+ 'RADIUS_SECRET',
+ 'TACACSPLUS_HOST',
+ 'TACACSPLUS_PORT',
+ 'TACACSPLUS_SECRET',
+ 'TACACSPLUS_SESSION_TIMEOUT',
+ 'TACACSPLUS_AUTH_PROTOCOL',
+ 'TACACSPLUS_REM_ADDR',
+]
+
+
+def forwards(apps, schema_editor):
+ """
+ Clear what the two backends left behind.
+
+ The association rows say a user signs in through a backend that no longer
+ exists; the settings rows configure servers nothing reads any more. The
+ users themselves are left alone: an administrator can give one a password
+ and it becomes an ordinary account.
+
+ This step is irreversible. The reverse is a no-op: migrating back past it
+ does not restore the deleted association rows or settings, which would
+ have to come from a backup.
+
+ Args:
+ apps: The historical app registry of the migration state.
+ schema_editor: The schema editor running the migration, unused.
+ """
+ UserEnterpriseAuth = apps.get_model('sso', 'UserEnterpriseAuth')
+ UserEnterpriseAuth.objects.filter(provider__in=DROPPED_PROVIDERS).delete()
+
+ Setting = apps.get_model('conf', 'Setting')
+ Setting.objects.filter(key__in=DROPPED_SETTINGS).delete()
+
+
+class Migration(migrations.Migration):
+ """
+ Drop the RADIUS and TACACS+ backends' data and their provider choices.
+
+ Migrating backwards runs, but only the provider choices come back. The
+ data step reverses as a no-op, so the RADIUS and TACACS+ associations and
+ settings deleted on the way forward stay deleted.
+ """
+
+ dependencies = [
+ ('sso', '0003_convert_saml_string_to_list'),
+ ('conf', '0001_initial'),
+ ]
+
+ operations = [
+ migrations.RunPython(forwards, migrations.RunPython.noop),
+ migrations.AlterField(
+ model_name='userenterpriseauth',
+ name='provider',
+ field=models.CharField(max_length=32, choices=[('saml', 'SAML')]),
+ ),
+ ]
diff --git a/ascender/sso/models.py b/ascender/sso/models.py
index 28eb23857..2fd3d7ed7 100644
--- a/ascender/sso/models.py
+++ b/ascender/sso/models.py
@@ -10,7 +10,7 @@
class UserEnterpriseAuth(models.Model):
"""Enterprise Auth association model"""
- PROVIDER_CHOICES = (('radius', _('RADIUS')), ('tacacs+', _('TACACS+')), ('saml', _('SAML')))
+ PROVIDER_CHOICES = (('saml', _('SAML')),)
class Meta:
unique_together = ('user', 'provider')
diff --git a/ascender/sso/tests/conftest.py b/ascender/sso/tests/conftest.py
index 64fd64383..cbc7cfa1d 100644
--- a/ascender/sso/tests/conftest.py
+++ b/ascender/sso/tests/conftest.py
@@ -2,14 +2,6 @@
from django.contrib.auth.models import User
-from ascender.sso.backends import TACACSPlusBackend
-from ascender.sso.models import UserEnterpriseAuth
-
-
-@pytest.fixture
-def tacacsplus_backend():
- return TACACSPlusBackend()
-
@pytest.fixture
def existing_normal_user():
@@ -19,16 +11,3 @@ def existing_normal_user():
user = User(username="alice", password="password")
user.save()
return user
-
-
-@pytest.fixture
-def existing_tacacsplus_user():
- try:
- user = User.objects.get(username="foo")
- except User.DoesNotExist:
- user = User(username="foo")
- user.set_unusable_password()
- user.save()
- enterprise_auth = UserEnterpriseAuth(user=user, provider='tacacs+')
- enterprise_auth.save()
- return user
diff --git a/ascender/sso/tests/functional/test_common.py b/ascender/sso/tests/functional/test_common.py
index c96aafae8..71a7ab621 100644
--- a/ascender/sso/tests/functional/test_common.py
+++ b/ascender/sso/tests/functional/test_common.py
@@ -356,7 +356,7 @@ def test_get_external_account(self, enable_ldap, enable_social, enable_enterpris
if enable_enterprise:
from ascender.sso.models import UserEnterpriseAuth
- enterprise_auth = UserEnterpriseAuth(user=user, provider='tacacs+')
+ enterprise_auth = UserEnterpriseAuth(user=user, provider='saml')
enterprise_auth.save()
assert get_external_account(user) == expected_results
@@ -369,8 +369,6 @@ def test_get_external_account(self, enable_ldap, enable_social, enable_enterpris
# Set the hard coded settings
('AUTH_LDAP_SERVER_URI', True),
('SOCIAL_AUTH_SAML_ENABLED_IDPS', True),
- ('RADIUS_SERVER', True),
- ('TACACSPLUS_HOST', True),
# Set some SOCIAL_SOCIAL_AUTH_OIDC_KEYAUTH_*_KEY settings
('SOCIAL_AUTH_AZUREAD_OAUTH2_KEY', True),
('SOCIAL_AUTH_GITHUB_ENTERPRISE_KEY', True),
diff --git a/ascender/sso/tests/functional/test_get_or_set_enterprise_user.py b/ascender/sso/tests/functional/test_get_or_set_enterprise_user.py
deleted file mode 100644
index 5cac7b7b7..000000000
--- a/ascender/sso/tests/functional/test_get_or_set_enterprise_user.py
+++ /dev/null
@@ -1,62 +0,0 @@
-# Python
-import pytest
-from unittest import mock
-
-# Django
-from django.contrib.auth.models import User
-
-# Ascender
-from ascender.sso.backends import _get_or_set_enterprise_user
-from ascender.sso.models import UserEnterpriseAuth
-
-
-@pytest.mark.django_db
-def test_fetch_user_if_exist(existing_tacacsplus_user):
- with mock.patch('ascender.sso.backends.logger') as mocked_logger:
- new_user = _get_or_set_enterprise_user("foo", "password", "tacacs+")
- mocked_logger.debug.assert_not_called()
- mocked_logger.warning.assert_not_called()
- assert new_user == existing_tacacsplus_user
-
-
-@pytest.mark.django_db
-def test_create_user_if_not_exist(existing_tacacsplus_user):
- with mock.patch('ascender.sso.backends.logger') as mocked_logger:
- new_user = _get_or_set_enterprise_user("bar", "password", "tacacs+")
- mocked_logger.debug.assert_called_once_with('Created enterprise user bar via TACACS+ backend.')
- assert new_user != existing_tacacsplus_user
-
-
-@pytest.mark.django_db
-def test_created_user_has_no_usable_password():
- new_user = _get_or_set_enterprise_user("bar", "password", "tacacs+")
- assert not new_user.has_usable_password()
-
-
-@pytest.mark.django_db
-def test_non_enterprise_user_does_not_get_pass(existing_normal_user):
- with mock.patch('ascender.sso.backends.logger') as mocked_logger:
- new_user = _get_or_set_enterprise_user("alice", "password", "tacacs+")
- mocked_logger.warning.assert_called_once_with('Enterprise user alice already defined in Tower.')
- assert new_user is None
-
-
-@pytest.mark.django_db
-def test_radius_does_not_adopt_local_user_without_password():
- """A local user with no usable password, such as a token-only account, is not a RADIUS user."""
- local_user = User.objects.create(username="svc", is_superuser=True)
- local_user.set_unusable_password()
- local_user.save()
- with mock.patch('ascender.sso.backends.logger') as mocked_logger:
- assert _get_or_set_enterprise_user("svc", "password", "radius") is None
- mocked_logger.warning.assert_called_once_with('Enterprise user svc already defined in Tower.')
- assert not UserEnterpriseAuth.objects.filter(user=local_user).exists()
-
-
-@pytest.mark.django_db
-def test_radius_user_with_enterprise_auth_logs_in():
- radius_user = User.objects.create(username="bob")
- radius_user.set_unusable_password()
- radius_user.save()
- UserEnterpriseAuth.objects.create(user=radius_user, provider='radius')
- assert _get_or_set_enterprise_user("bob", "password", "radius") == radius_user
diff --git a/ascender/sso/tests/unit/test_tacacsplus.py b/ascender/sso/tests/unit/test_tacacsplus.py
deleted file mode 100644
index da0446c60..000000000
--- a/ascender/sso/tests/unit/test_tacacsplus.py
+++ /dev/null
@@ -1,171 +0,0 @@
-from unittest import mock
-import pytest
-
-
-def test_empty_host_fails_auth(tacacsplus_backend):
- with mock.patch('ascender.sso.backends.django_settings') as settings:
- settings.TACACSPLUS_HOST = ''
- ret_user = tacacsplus_backend.authenticate(None, "user", "pass")
- assert ret_user is None
-
-
-def test_client_raises_exception(tacacsplus_backend):
- client = mock.MagicMock()
- client.authenticate.side_effect = Exception("foo")
- with (
- mock.patch('ascender.sso.backends.django_settings') as settings,
- mock.patch('ascender.sso.backends.logger') as logger,
- mock.patch('ascender.sso.backends.TACACSClient', return_value=client),
- ):
- settings.TACACSPLUS_HOST = 'localhost'
- settings.TACACSPLUS_AUTH_PROTOCOL = 'ascii'
- ret_user = tacacsplus_backend.authenticate(None, "user", "pass")
- assert ret_user is None
- logger.exception.assert_called_once_with("TACACS+ Authentication Error: foo")
-
-
-def test_client_return_invalid_fails_auth(tacacsplus_backend):
- auth = mock.MagicMock()
- auth.valid = False
- client = mock.MagicMock()
- client.authenticate.return_value = auth
- with mock.patch('ascender.sso.backends.django_settings') as settings, mock.patch('ascender.sso.backends.TACACSClient', return_value=client):
- settings.TACACSPLUS_HOST = 'localhost'
- settings.TACACSPLUS_AUTH_PROTOCOL = 'ascii'
- ret_user = tacacsplus_backend.authenticate(None, "user", "pass")
- assert ret_user is None
-
-
-def test_client_return_valid_passes_auth(tacacsplus_backend):
- auth = mock.MagicMock()
- auth.valid = True
- client = mock.MagicMock()
- client.authenticate.return_value = auth
- user = mock.MagicMock()
- user.has_usable_password = mock.MagicMock(return_value=False)
- with (
- mock.patch('ascender.sso.backends.django_settings') as settings,
- mock.patch('ascender.sso.backends.TACACSClient', return_value=client),
- mock.patch('ascender.sso.backends._get_or_set_enterprise_user', return_value=user),
- ):
- settings.TACACSPLUS_HOST = 'localhost'
- settings.TACACSPLUS_AUTH_PROTOCOL = 'ascii'
- ret_user = tacacsplus_backend.authenticate(None, "user", "pass")
- assert ret_user == user
-
-
-@pytest.mark.parametrize(
- "client_ip_header,client_ip_header_value,expected_client_ip",
- [('HTTP_X_FORWARDED_FOR', '12.34.56.78, 23.45.67.89', '12.34.56.78'), ('REMOTE_ADDR', '12.34.56.78', '12.34.56.78')],
-)
-def test_remote_addr_is_passed_to_client_if_available_and_setting_enabled(tacacsplus_backend, client_ip_header, client_ip_header_value, expected_client_ip):
- auth = mock.MagicMock()
- auth.valid = True
- client = mock.MagicMock()
- client.authenticate.return_value = auth
- user = mock.MagicMock()
- user.has_usable_password = mock.MagicMock(return_value=False)
- request = mock.MagicMock()
- request.META = {
- client_ip_header: client_ip_header_value,
- }
- with (
- mock.patch('ascender.sso.backends.django_settings') as settings,
- mock.patch('ascender.sso.backends.TACACSClient', return_value=client),
- mock.patch('ascender.sso.backends._get_or_set_enterprise_user', return_value=user),
- ):
- settings.TACACSPLUS_HOST = 'localhost'
- settings.TACACSPLUS_AUTH_PROTOCOL = 'ascii'
- settings.TACACSPLUS_REM_ADDR = True
- tacacsplus_backend.authenticate(request, "user", "pass")
-
- client.authenticate.assert_called_once_with('user', 'pass', authen_type=1, rem_addr=expected_client_ip)
-
-
-def test_remote_addr_is_completely_ignored_in_client_call_if_setting_is_disabled(tacacsplus_backend):
- auth = mock.MagicMock()
- auth.valid = True
- client = mock.MagicMock()
- client.authenticate.return_value = auth
- user = mock.MagicMock()
- user.has_usable_password = mock.MagicMock(return_value=False)
- request = mock.MagicMock()
- request.META = {}
- with (
- mock.patch('ascender.sso.backends.django_settings') as settings,
- mock.patch('ascender.sso.backends.TACACSClient', return_value=client),
- mock.patch('ascender.sso.backends._get_or_set_enterprise_user', return_value=user),
- ):
- settings.TACACSPLUS_HOST = 'localhost'
- settings.TACACSPLUS_AUTH_PROTOCOL = 'ascii'
- settings.TACACSPLUS_REM_ADDR = False
- tacacsplus_backend.authenticate(request, "user", "pass")
-
- client.authenticate.assert_called_once_with('user', 'pass', authen_type=1)
-
-
-def test_remote_addr_is_completely_ignored_in_client_call_if_unavailable_and_setting_enabled(tacacsplus_backend):
- auth = mock.MagicMock()
- auth.valid = True
- client = mock.MagicMock()
- client.authenticate.return_value = auth
- user = mock.MagicMock()
- user.has_usable_password = mock.MagicMock(return_value=False)
- request = mock.MagicMock()
- request.META = {}
- with (
- mock.patch('ascender.sso.backends.django_settings') as settings,
- mock.patch('ascender.sso.backends.TACACSClient', return_value=client),
- mock.patch('ascender.sso.backends._get_or_set_enterprise_user', return_value=user),
- ):
- settings.TACACSPLUS_HOST = 'localhost'
- settings.TACACSPLUS_AUTH_PROTOCOL = 'ascii'
- settings.TACACSPLUS_REM_ADDR = True
- tacacsplus_backend.authenticate(request, "user", "pass")
-
- client.authenticate.assert_called_once_with('user', 'pass', authen_type=1)
-
-
-def test_backend_binds_the_names_the_installed_tacacs_plus_actually_exports():
- """Bind the backend's TACACS+ names against the real package, not a mock.
-
- Every other test in this module patches the client, so none of them notices
- when the package moves a name. The backend used to read both names off the
- top-level ``tacacs_plus`` package, which worked on 1.0 and stopped working
- on 2.x, where ``__init__.py`` is empty and the names live in
- ``tacacs_plus.client`` and ``tacacs_plus.flags``. That lookup raises
- ``AttributeError`` inside ``authenticate``, where the broad ``except``
- turns it into a logged error and a ``None`` return, so the only visible
- symptom is that TACACS+ logins stop working.
-
- Asserting against the installed package means a future layout change fails
- here rather than at somebody's login prompt.
- """
- from tacacs_plus.client import TACACSClient as installed_client
- from tacacs_plus.flags import TAC_PLUS_AUTHEN_TYPES as installed_types
-
- from ascender.sso import backends
-
- assert backends.TACACSClient is installed_client
- assert backends.TAC_PLUS_AUTHEN_TYPES is installed_types
-
- # ascender/sso/conf.py offers exactly these two for TACACSPLUS_AUTH_PROTOCOL, and
- # authenticate() indexes the dict with whichever one is set.
- for protocol in ('ascii', 'pap'):
- assert protocol in installed_types
-
-
-def test_installed_tacacs_client_accepts_the_arguments_the_backend_passes():
- """Construct the real client the way the backend does.
-
- The socket is opened lazily, so building the object touches no network. It
- does check that the constructor still takes the host, port, secret and
- timeout that ``authenticate`` hands it positionally and by keyword.
- """
- from tacacs_plus.client import TACACSClient
-
- client = TACACSClient('localhost', 49, 'secret', timeout=5)
-
- assert client.host == 'localhost'
- assert client.port == 49
- assert client.timeout == 5
diff --git a/ascender/sso/validators.py b/ascender/sso/validators.py
index 96fbde2ea..b38abfcaa 100644
--- a/ascender/sso/validators.py
+++ b/ascender/sso/validators.py
@@ -17,7 +17,6 @@
'validate_ldap_bind_dn',
'validate_ldap_filter',
'validate_ldap_filter_with_user',
- 'validate_tacacsplus_disallow_nonascii',
'validate_trigger_rule',
]
@@ -71,13 +70,6 @@ def validate_ldap_filter_with_user(value):
validate_ldap_filter(value, with_user=True)
-def validate_tacacsplus_disallow_nonascii(value):
- try:
- value.encode('ascii')
- except (UnicodeEncodeError, UnicodeDecodeError):
- raise ValidationError(_('TACACS+ secret does not allow non-ascii characters'))
-
-
def validate_trigger_rule(triggers):
"""
The problems with an org/team map trigger rule, keyed by where they are.
diff --git a/ascender/ui/conf.py b/ascender/ui/conf.py
index d6aa77804..ec91035ea 100644
--- a/ascender/ui/conf.py
+++ b/ascender/ui/conf.py
@@ -78,7 +78,7 @@
field_class=CustomThemeField,
allow_blank=True,
default='',
- label=_('Custom Theme'),
+ label=_('Custom Theme CSS'),
help_text=_(
'The contents of a CSS file, offered in the theme list alongside the '
'themes that ship with the product. Scope the rules to '
@@ -102,6 +102,68 @@
category_slug='ui',
)
+# The ids are the stylesheet filenames in ascender/ui/src/themes, and "custom"
+# for the one CUSTOM_THEME supplies. A choice list is what makes this a dropdown
+# rather than a free text box, so adding a theme means adding it here too.
+register(
+ 'DEFAULT_UI_THEME',
+ field_class=fields.ChoiceField,
+ choices=[
+ ('default', _('Default')),
+ ('light', _('Light')),
+ ('dark', _('Dark')),
+ ('classic', _('Classic')),
+ ('custom', _('Custom')),
+ ],
+ default='default',
+ label=_('Default Theme'),
+ help_text=_(
+ 'The theme a user sees before they choose one of their own. There is '
+ 'no blank: a user can always pick a different theme, so an unset value '
+ 'would only mean Default by another name.'
+ ),
+ category=_('UI'),
+ category_slug='ui',
+)
+
+register(
+ 'DEFAULT_UI_LANGUAGE',
+ field_class=fields.ChoiceField,
+ choices=[
+ ('', _('Follow the browser')),
+ ('en', _('English')),
+ ('ar', _('Arabic')),
+ ('zh', _('Chinese')),
+ ('nl', _('Dutch')),
+ ('fr', _('French')),
+ ('hi', _('Hindi')),
+ ('ja', _('Japanese')),
+ ('ko', _('Korean')),
+ ('es', _('Spanish')),
+ ],
+ allow_blank=True,
+ default='',
+ label=_('Default Language'),
+ help_text=_('The language a user sees before they choose one of their own. Leave it blank to follow the language the browser asks for.'),
+ category=_('UI'),
+ category_slug='ui',
+)
+
+register(
+ 'MAX_UI_EDITOR_ROWS',
+ field_class=fields.IntegerField,
+ min_value=1,
+ label=_('Max Editor Rows'),
+ help_text=_(
+ 'How far a read-only variables editor grows to fit its value before the rest scrolls. '
+ 'The editor renders a line of DOM per line of content, so this is what keeps a job with '
+ 'thousands of facts from costing seconds to paint. A reader can still set the height of a '
+ 'single editor past this; the setting governs the height it takes on its own.'
+ ),
+ category=_('UI'),
+ category_slug='ui',
+)
+
register(
'MAX_UI_JOB_EVENTS',
field_class=fields.IntegerField,
diff --git a/ascender/ui/e2e/README.md b/ascender/ui/e2e/README.md
index 7fe11b4ac..9c339a4b2 100644
--- a/ascender/ui/e2e/README.md
+++ b/ascender/ui/e2e/README.md
@@ -46,7 +46,7 @@ and records the job each node produced.
System job templates are used deliberately: every instance has them, they need no
project, inventory, credential or network access, and they finish in seconds. It
-also means the specs navigate under `/jobs/management/`, exercising a url segment
+also means the specs navigate under `/runs/management/`, exercising a url segment
other than the default one.
The workflow job template is reused on later runs. It is safe to delete.
diff --git a/ascender/ui/e2e/accessibility-baseline.json b/ascender/ui/e2e/accessibility-baseline.json
index 0bde38346..c282c89b5 100644
--- a/ascender/ui/e2e/accessibility-baseline.json
+++ b/ascender/ui/e2e/accessibility-baseline.json
@@ -1,6 +1,6 @@
{
"login": [],
- "jobs": [
+ "runs": [
"button-name",
"color-contrast",
"link-name"
diff --git a/ascender/ui/e2e/detailprobe.cjs b/ascender/ui/e2e/detailprobe.cjs
index a6a92355c..6e30c41a4 100644
--- a/ascender/ui/e2e/detailprobe.cjs
+++ b/ascender/ui/e2e/detailprobe.cjs
@@ -13,7 +13,7 @@ const fs = require('fs');
const id = require('./fixtures.json').nodes[0].jobId;
const out = {};
- for (const [name, hash] of [['job detail', `/jobs/system/${id}/details`], ['settings jobs', '/settings/jobs/details']]) {
+ for (const [name, hash] of [['job detail', `/runs/management/${id}/details`], ['settings jobs', '/job_settings/details']]) {
await page.goto(base + '/#' + hash, { waitUntil: 'domcontentloaded' });
await page.waitForLoadState('networkidle');
await page.waitForTimeout(1200);
diff --git a/ascender/ui/e2e/seed.js b/ascender/ui/e2e/seed.js
index 7e524e369..bd3a515b7 100644
--- a/ascender/ui/e2e/seed.js
+++ b/ascender/ui/e2e/seed.js
@@ -7,7 +7,7 @@
// has them, they need no project, inventory, credential or network access, and
// they finish in seconds, so the suite has a real multi-node workflow job to
// navigate without depending on anything that can be slow or absent. They also
-// route under /jobs/management/, so the specs exercise a url segment other than
+// route under /runs/management/, so the specs exercise a url segment other than
// the default one rather than only the common case.
const fs = require('fs');
const path = require('path');
diff --git a/ascender/ui/e2e/tests/accessibility.spec.js b/ascender/ui/e2e/tests/accessibility.spec.js
index 6d507001f..53610a453 100644
--- a/ascender/ui/e2e/tests/accessibility.spec.js
+++ b/ascender/ui/e2e/tests/accessibility.spec.js
@@ -9,7 +9,7 @@ const { login, route, watchConsole } = require('./helpers');
// the list views that most of the application is built from.
const SCREENS = [
{ name: 'login', hash: '/login', authenticated: false },
- { name: 'jobs', hash: '/jobs', authenticated: true },
+ { name: 'runs', hash: '/runs', authenticated: true },
{ name: 'templates', hash: '/templates', authenticated: true },
// The forms, which is where accessibility problems usually live: a label that
// names nothing, a required field that only says so in colour, a control
diff --git a/ascender/ui/e2e/tests/helpers.js b/ascender/ui/e2e/tests/helpers.js
index 2594c3bb8..c99621e77 100644
--- a/ascender/ui/e2e/tests/helpers.js
+++ b/ascender/ui/e2e/tests/helpers.js
@@ -84,7 +84,7 @@ async function watchLayoutLoops(page) {
const workflowToggle = (page) =>
page
.locator('button[class*="menu-toggle"]')
- .filter({ hasText: /Workflow Job|Successful|Failed/ })
+ .filter({ hasText: /Job \d+\/\d+|Successful|Failed/ })
.first();
const menuItem = (page, label) =>
diff --git a/ascender/ui/e2e/tests/job-output.spec.js b/ascender/ui/e2e/tests/job-output.spec.js
index f85a2a82b..1e863ab42 100644
--- a/ascender/ui/e2e/tests/job-output.spec.js
+++ b/ascender/ui/e2e/tests/job-output.spec.js
@@ -24,11 +24,11 @@ test.describe('workflow job selector', () => {
test('shows which of the workflow jobs is on screen', async ({ page }) => {
const { nodes } = fixtures();
for (let i = 0; i < nodes.length; i += 1) {
- await page.goto(route(`/jobs/management/${nodes[i].jobId}/output`), {
+ await page.goto(route(`/runs/management/${nodes[i].jobId}/output`), {
waitUntil: 'domcontentloaded',
});
await expect(workflowToggle(page)).toHaveText(
- `Workflow Job ${i + 1}/${nodes.length}`
+ `Job ${i + 1}/${nodes.length}`
);
}
});
@@ -36,7 +36,7 @@ test.describe('workflow job selector', () => {
test('switches to the job picked from the menu', async ({ page }) => {
const { nodes } = fixtures();
const [first, , third] = nodes;
- await page.goto(route(`/jobs/management/${first.jobId}/output`), {
+ await page.goto(route(`/runs/management/${first.jobId}/output`), {
waitUntil: 'domcontentloaded',
});
const console_ = watchConsole(page);
@@ -44,8 +44,8 @@ test.describe('workflow job selector', () => {
await workflowToggle(page).click();
await menuItem(page, third.identifier).click();
- await expect(page).toHaveURL(new RegExp(`/jobs/management/${third.jobId}/output$`));
- await expect(workflowToggle(page)).toHaveText(`Workflow Job 3/${nodes.length}`);
+ await expect(page).toHaveURL(new RegExp(`/runs/management/${third.jobId}/output$`));
+ await expect(workflowToggle(page)).toHaveText(`Job 3/${nodes.length}`);
console_.expectQuiet();
});
@@ -54,24 +54,24 @@ test.describe('workflow job selector', () => {
// move it still offered the jobs belonging to the page you came from
const { nodes } = fixtures();
const [first, second, third] = nodes;
- await page.goto(route(`/jobs/management/${first.jobId}/output`), {
+ await page.goto(route(`/runs/management/${first.jobId}/output`), {
waitUntil: 'domcontentloaded',
});
await workflowToggle(page).click();
await menuItem(page, third.identifier).click();
- await expect(page).toHaveURL(new RegExp(`/jobs/management/${third.jobId}/output$`));
+ await expect(page).toHaveURL(new RegExp(`/runs/management/${third.jobId}/output$`));
await workflowToggle(page).click();
await menuItem(page, second.identifier).click();
- await expect(page).toHaveURL(new RegExp(`/jobs/management/${second.jobId}/output$`));
- await expect(workflowToggle(page)).toHaveText(`Workflow Job 2/${nodes.length}`);
+ await expect(page).toHaveURL(new RegExp(`/runs/management/${second.jobId}/output$`));
+ await expect(workflowToggle(page)).toHaveText(`Job 2/${nodes.length}`);
});
test('lists every job node, and picking the current one is harmless', async ({ page }) => {
const { nodes } = fixtures();
const current = nodes[1];
- await page.goto(route(`/jobs/management/${current.jobId}/output`), {
+ await page.goto(route(`/runs/management/${current.jobId}/output`), {
waitUntil: 'domcontentloaded',
});
await workflowToggle(page).click();
@@ -79,12 +79,12 @@ test.describe('workflow job selector', () => {
await expect(menuItem(page, node.identifier)).toBeVisible();
}
await menuItem(page, current.identifier).click();
- await expect(page).toHaveURL(new RegExp(`/jobs/management/${current.jobId}/output$`));
+ await expect(page).toHaveURL(new RegExp(`/runs/management/${current.jobId}/output$`));
});
test('filters the list by status', async ({ page }) => {
const { nodes } = fixtures();
- await page.goto(route(`/jobs/management/${nodes[0].jobId}/output`), {
+ await page.goto(route(`/runs/management/${nodes[0].jobId}/output`), {
waitUntil: 'domcontentloaded',
});
await workflowToggle(page).click();
@@ -111,7 +111,7 @@ test.describe('job output layout', () => {
const loops = await watchLayoutLoops(page);
await login(page);
const { nodes } = fixtures();
- await page.goto(route(`/jobs/management/${nodes[0].jobId}/output`), {
+ await page.goto(route(`/runs/management/${nodes[0].jobId}/output`), {
waitUntil: 'domcontentloaded',
});
const scroller = page.locator('.ascender-output-scroll');
diff --git a/ascender/ui/e2e/tests/login.spec.js b/ascender/ui/e2e/tests/login.spec.js
index 70b3424a3..7f6662d9f 100644
--- a/ascender/ui/e2e/tests/login.spec.js
+++ b/ascender/ui/e2e/tests/login.spec.js
@@ -13,7 +13,7 @@ test.describe('signing in', () => {
});
test('an unauthenticated visit is sent to the login page', async ({ page }) => {
- await page.goto(route('/jobs'), { waitUntil: 'domcontentloaded' });
+ await page.goto(route('/runs'), { waitUntil: 'domcontentloaded' });
await page.waitForFunction(() => window.location.hash.includes('/login'), {
timeout: 30_000,
});
diff --git a/ascender/ui/e2e/tests/tabs.spec.js b/ascender/ui/e2e/tests/tabs.spec.js
index a2fe58118..443086f69 100644
--- a/ascender/ui/e2e/tests/tabs.spec.js
+++ b/ascender/ui/e2e/tests/tabs.spec.js
@@ -3,7 +3,7 @@ const { test, expect } = require('@playwright/test');
const { fixtures, login, route } = require('./helpers');
// RoutedTabs is shared by around forty screens, so a break here is broad.
-test.describe('job tabs', () => {
+test.describe('run tabs', () => {
test.beforeEach(async ({ page }) => {
await login(page);
});
@@ -11,30 +11,30 @@ test.describe('job tabs', () => {
test('moves between Details and Output', async ({ page }) => {
const { nodes } = fixtures();
const { jobId } = nodes[0];
- await page.goto(route(`/jobs/management/${jobId}/output`), {
+ await page.goto(route(`/runs/management/${jobId}/output`), {
waitUntil: 'domcontentloaded',
});
await page.getByRole('tab', { name: 'Details' }).click();
- await expect(page).toHaveURL(new RegExp(`/jobs/management/${jobId}/details$`));
+ await expect(page).toHaveURL(new RegExp(`/runs/management/${jobId}/details$`));
await page.getByRole('tab', { name: 'Output' }).click();
- await expect(page).toHaveURL(new RegExp(`/jobs/management/${jobId}/output$`));
+ await expect(page).toHaveURL(new RegExp(`/runs/management/${jobId}/output$`));
});
test('the workflow job has its own tabs', async ({ page }) => {
const { workflowJobId } = fixtures();
- await page.goto(route(`/jobs/workflow/${workflowJobId}/output`), {
+ await page.goto(route(`/runs/workflow/${workflowJobId}/output`), {
waitUntil: 'domcontentloaded',
});
await page.getByRole('tab', { name: 'Details' }).click();
- await expect(page).toHaveURL(new RegExp(`/jobs/workflow/${workflowJobId}/details$`));
+ await expect(page).toHaveURL(new RegExp(`/runs/workflow/${workflowJobId}/details$`));
});
- test('Back to Jobs returns to the job list', async ({ page }) => {
+ test('Back to Runs returns to the runs list', async ({ page }) => {
const { nodes } = fixtures();
- await page.goto(route(`/jobs/management/${nodes[0].jobId}/details`), {
+ await page.goto(route(`/runs/management/${nodes[0].jobId}/details`), {
waitUntil: 'domcontentloaded',
});
- await page.getByRole('tab', { name: 'Back to Jobs' }).click();
- await expect(page).toHaveURL(/#\/jobs/);
+ await page.getByRole('tab', { name: 'Back to Runs' }).click();
+ await expect(page).toHaveURL(/#\/runs/);
});
});
diff --git a/ascender/ui/src/App.test.tsx b/ascender/ui/src/App.test.tsx
index 33ed1ff7c..4f5f9f186 100644
--- a/ascender/ui/src/App.test.tsx
+++ b/ascender/ui/src/App.test.tsx
@@ -5,11 +5,11 @@ import * as SessionContext from 'contexts/Session';
import * as navigation from 'util/navigation';
import * as auth from 'util/auth';
import type { SessionValue } from 'contexts/Session';
+import { createMemoryHistory } from 'history';
import type { ResponseOf } from '../testUtils/responseOf';
import { renderWithContexts } from '../testUtils/rtlContexts';
-import { createMemoryHistory } from '../testUtils/historyShim';
import { SESSION_REDIRECT_URL } from './constants';
-import App, { ProtectedRoute } from './App';
+import App, { AuthorizedRoutes, ProtectedRoute } from './App';
vi.mock('./api');
vi.mock('util/webWorker', () => ({ default: vi.fn() }));
@@ -233,4 +233,62 @@ describe('', () => {
expect(await screen.findByText('foo')).toBeInTheDocument();
});
+
+ describe('old settings addresses', () => {
+ // Two pages standing in for the real screens, so each test is about where
+ // an old address lands rather than what the page there draws.
+ const routeConfig = [
+ {
+ groupTitle: 'Settings',
+ groupId: 'settings',
+ routes: [
+ {
+ title: 'Appearance',
+ path: '/appearance',
+ screen: () =>
+ {/* Disassociating takes the rows out of this list: what they stand
+ for stays where it is, which the reader is told before confirming. */}
+
{t`This disassociates the following. They are not deleted themselves:`}
{itemsToDisassociate.map((item) => (
diff --git a/ascender/ui/src/components/ExecutionEnvironmentDetail/ExecutionEnvironmentDetail.tsx b/ascender/ui/src/components/ExecutionEnvironmentDetail/ExecutionEnvironmentDetail.tsx
index a0eaa422f..e062ea107 100644
--- a/ascender/ui/src/components/ExecutionEnvironmentDetail/ExecutionEnvironmentDetail.tsx
+++ b/ascender/ui/src/components/ExecutionEnvironmentDetail/ExecutionEnvironmentDetail.tsx
@@ -3,12 +3,13 @@ import React from 'react';
import { Link } from 'react-router';
import { Trans, useLingui } from '@lingui/react/macro';
-import { Popover, Tooltip } from '@patternfly/react-core';
+import { Popover } from '@patternfly/react-core';
import { ExclamationTriangleIcon as PFExclamationTriangleIcon } from '@patternfly/react-icons';
import getDocsBaseUrl from 'util/getDocsBaseUrl';
import { useConfig } from 'contexts/Config';
import { Detail } from '../DetailList';
import './ExecutionEnvironmentDetail.css';
+import Tooltip from '../Tooltip';
export interface ExecutionEnvironmentDetailProps {
/**
diff --git a/ascender/ui/src/components/FieldWithPrompt/FieldWithPrompt.test.tsx b/ascender/ui/src/components/FieldWithPrompt/FieldWithPrompt.test.tsx
index 1c55982a7..c05bd34c0 100644
--- a/ascender/ui/src/components/FieldWithPrompt/FieldWithPrompt.test.tsx
+++ b/ascender/ui/src/components/FieldWithPrompt/FieldWithPrompt.test.tsx
@@ -28,7 +28,7 @@ describe('FieldWithPrompt', () => {
);
// the prompt-on-launch checkbox is always rendered
- expect(screen.getByLabelText('Prompt on launch')).toBeInTheDocument();
+ expect(screen.getByLabelText('Prompt on Launch')).toBeInTheDocument();
// no required asterisk
expect(
container.querySelector('.pf-v6-c-form__label-required')
@@ -63,7 +63,7 @@ describe('FieldWithPrompt', () => {
);
- expect(screen.getByLabelText('Prompt on launch')).toBeInTheDocument();
+ expect(screen.getByLabelText('Prompt on Launch')).toBeInTheDocument();
// required asterisk present
expect(
container.querySelector('.pf-v6-c-form__label-required')
diff --git a/ascender/ui/src/components/FieldWithPrompt/FieldWithPrompt.tsx b/ascender/ui/src/components/FieldWithPrompt/FieldWithPrompt.tsx
index e9bb4c21a..9e8f40401 100644
--- a/ascender/ui/src/components/FieldWithPrompt/FieldWithPrompt.tsx
+++ b/ascender/ui/src/components/FieldWithPrompt/FieldWithPrompt.tsx
@@ -46,7 +46,7 @@ function FieldWithPrompt({
className="ascender-field-with-prompt__styled-checkbox-field"
isDisabled={isDisabled}
id={promptId}
- label={t`Prompt on launch`}
+ label={t`Prompt on Launch`}
name={promptName}
ouiaId={`${promptId}-checkbox`}
/>
diff --git a/ascender/ui/src/components/Form/Form.tsx b/ascender/ui/src/components/Form/Form.tsx
index 2e698a227..50645fcfe 100644
--- a/ascender/ui/src/components/Form/Form.tsx
+++ b/ascender/ui/src/components/Form/Form.tsx
@@ -39,8 +39,43 @@ interface ValidatorBox {
current: FieldValidator | undefined;
}
+/*
+ * Every useField call on a name adds its own box, rather than one box per
+ * name. A form commonly reads a field it does not validate: the lookup that
+ * renders the organization carries the required rule, while the form around
+ * it calls a bare useField('organization') to read the value. React runs the
+ * child's effect before the parent's, so with one box per name the parent's
+ * empty registration replaced the lookup's validator and a required field
+ * went out blank. With a set, each registration only ever adds or removes
+ * itself, and every validator for the name runs.
+ */
interface Registry {
- boxes: Map;
+ boxes: Map>;
+}
+
+/**
+ * The first error any of a field's validators reports for a value.
+ *
+ * Args:
+ * boxes: the registrations for one field name.
+ * value: the field's current value.
+ *
+ * Returns:
+ * The first error message, or undefined when every validator passes.
+ */
+function firstError(
+ boxes: Set | undefined,
+ value: unknown
+): string | undefined {
+ if (!boxes) return undefined;
+ let found: string | undefined;
+ boxes.forEach((box) => {
+ if (found !== undefined) return;
+ const validate = box.current as
+ ((value: unknown) => string | undefined) | undefined;
+ found = validate?.(value);
+ });
+ return found;
}
const RegistryContext = createContext(null);
@@ -118,10 +153,8 @@ export function Form({
const runValidators = useCallback((against: Values): Errors => {
let found: Errors = { ...(validateRef.current?.(against as V) ?? {}) };
- registry.current.boxes.forEach((box, name) => {
- const validate = box.current as
- ((value: unknown) => string | undefined) | undefined;
- const error = validate?.(getIn(against, name));
+ registry.current.boxes.forEach((boxes, name) => {
+ const error = firstError(boxes, getIn(against, name));
if (error !== undefined) found = setIn(found, name, error);
});
return found;
@@ -164,10 +197,10 @@ export function Form({
);
const validateField = useCallback((name: string) => {
- const validate = registry.current.boxes.get(name)?.current as
- ((value: unknown) => string | undefined) | undefined;
- if (!validate) return;
- const error = validate(getIn(valuesRef.current, name));
+ const boxes = registry.current.boxes.get(name);
+ // A field nobody validates keeps whatever error it has, as before.
+ if (!boxes || ![...boxes].some((box) => box.current)) return;
+ const error = firstError(boxes, getIn(valuesRef.current, name));
setErrorsState((prev) => setIn(prev, name, error));
}, []);
diff --git a/ascender/ui/src/components/Form/useField.test.tsx b/ascender/ui/src/components/Form/useField.test.tsx
new file mode 100644
index 000000000..90f211389
--- /dev/null
+++ b/ascender/ui/src/components/Form/useField.test.tsx
@@ -0,0 +1,143 @@
+/*
+ * Several useField calls on one name.
+ *
+ * A form commonly reads a field it does not validate while a child component
+ * carries the rule: a lookup registers the required validator for
+ * organization, and the form around it calls a bare useField('organization')
+ * to read the value. React runs the child's effect first, so when the
+ * registry kept one validator per name the parent's empty registration
+ * replaced the child's and a required field was submitted blank.
+ */
+import React, { useState } from 'react';
+import { render, screen, waitFor } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import { FormRoot, useField, useFormContext } from 'components/Form';
+
+const required = (value: unknown) =>
+ value ? undefined : 'This field must not be blank';
+
+/** The child: registers the field with its validator, like a lookup does. */
+function Validated() {
+ const [, meta] = useField({ name: 'organization', validate: required });
+ return {meta.error ?? ''};
+}
+
+/** The parent's bare read of the same field, with no validator. */
+function Bare() {
+ useField('organization');
+ return null;
+}
+
+function Submit() {
+ const { handleSubmit } = useFormContext();
+ return (
+
+ );
+}
+
+describe('useField registrations sharing a name', () => {
+ test('a bare parent useField does not drop the child validator', async () => {
+ const onSubmit = vi.fn();
+ // The bare hook sits in the component that renders the validated child,
+ // so its effect runs after the child's, which is the case that lost the
+ // validator.
+ function Parent() {
+ useField('organization');
+ return (
+ <>
+
+
+ >
+ );
+ }
+ render(
+
+
+
+ );
+
+ await userEvent.click(screen.getByRole('button', { name: 'submit' }));
+
+ await waitFor(() =>
+ expect(screen.getByTestId('error')).toHaveTextContent(
+ 'This field must not be blank'
+ )
+ );
+ expect(onSubmit).not.toHaveBeenCalled();
+ });
+
+ test('unmounting one registration removes only its own validator', async () => {
+ const onSubmit = vi.fn();
+ function Harness() {
+ const [shown, setShown] = useState({ bare: true, validated: true });
+ return (
+ <>
+ {shown.validated && }
+ {shown.bare && }
+
+
+
+ >
+ );
+ }
+ render(
+
+
+
+ );
+
+ // The bare registration going away leaves the validator in place.
+ await userEvent.click(screen.getByRole('button', { name: 'hide bare' }));
+ await userEvent.click(screen.getByRole('button', { name: 'submit' }));
+ await waitFor(() =>
+ expect(screen.getByTestId('error')).toHaveTextContent(
+ 'This field must not be blank'
+ )
+ );
+ expect(onSubmit).not.toHaveBeenCalled();
+
+ // The validated registration going away takes its validator with it, so
+ // the same blank value now submits.
+ await userEvent.click(
+ screen.getByRole('button', { name: 'hide validated' })
+ );
+ await userEvent.click(screen.getByRole('button', { name: 'submit' }));
+ await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1));
+ });
+
+ test('every validator registered for a name runs', async () => {
+ const onSubmit = vi.fn();
+ const short = (value: unknown) =>
+ String(value ?? '').length > 3 ? 'Too long' : undefined;
+ function Second() {
+ useField({ name: 'organization', validate: short });
+ return null;
+ }
+ render(
+
+
+
+
+
+ );
+
+ await userEvent.click(screen.getByRole('button', { name: 'submit' }));
+
+ await waitFor(() =>
+ expect(screen.getByTestId('error')).toHaveTextContent('Too long')
+ );
+ expect(onSubmit).not.toHaveBeenCalled();
+ });
+});
diff --git a/ascender/ui/src/components/Form/useField.ts b/ascender/ui/src/components/Form/useField.ts
index f62ca5186..e676a6da3 100644
--- a/ascender/ui/src/components/Form/useField.ts
+++ b/ascender/ui/src/components/Form/useField.ts
@@ -48,12 +48,23 @@ export default function useField(
});
box.current.current = config.validate;
+ // Each call adds its own box to the name's set and takes only that box out
+ // again, so a bare useField on a name another component validates cannot
+ // replace or remove that component's validator.
useEffect(() => {
const { boxes } = registry;
const mine = box.current;
- boxes.set(name, mine);
+ let forName = boxes.get(name);
+ if (!forName) {
+ forName = new Set();
+ boxes.set(name, forName);
+ }
+ forName.add(mine);
return () => {
- if (boxes.get(name) === mine) boxes.delete(name);
+ const current = boxes.get(name);
+ if (!current) return;
+ current.delete(mine);
+ if (current.size === 0) boxes.delete(name);
};
}, [name, registry]);
diff --git a/ascender/ui/src/components/FormField/PasswordField.tsx b/ascender/ui/src/components/FormField/PasswordField.tsx
index a7cd19a2e..717a2751f 100644
--- a/ascender/ui/src/components/FormField/PasswordField.tsx
+++ b/ascender/ui/src/components/FormField/PasswordField.tsx
@@ -6,8 +6,6 @@ import {
FormHelperText,
HelperText,
HelperTextItem,
- InputGroup,
- InputGroupItem,
} from '@patternfly/react-core';
import Popover from '../Popover';
import PasswordInput from './PasswordInput';
@@ -41,19 +39,15 @@ function PasswordField({
label={label}
labelHelp={helperText ? : undefined}
>
-
-
-
-
-
+
{!isValid && (
diff --git a/ascender/ui/src/components/FormField/PasswordInput.test.tsx b/ascender/ui/src/components/FormField/PasswordInput.test.tsx
index 53f97d6da..eb737d2ca 100644
--- a/ascender/ui/src/components/FormField/PasswordInput.test.tsx
+++ b/ascender/ui/src/components/FormField/PasswordInput.test.tsx
@@ -25,6 +25,16 @@ describe('PasswordInput', () => {
expect(container.querySelector('#test-password')).toBeInTheDocument();
});
+ test('puts the reveal toggle after the input, in an input group', () => {
+ const { container } = renderInput();
+ const input = container.querySelector('#test-password');
+ const toggle = screen.getByRole('button', { name: 'Toggle Password' });
+ const items = container.querySelectorAll('.pf-v6-c-input-group__item');
+ expect(items).toHaveLength(2);
+ expect(items[0]).toContainElement(input as HTMLElement);
+ expect(items[1]).toContainElement(toggle);
+ });
+
test('properly responds to show/hide toggles', async () => {
const { container, user } = renderInput();
const input = container.querySelector('#test-password');
diff --git a/ascender/ui/src/components/FormField/PasswordInput.tsx b/ascender/ui/src/components/FormField/PasswordInput.tsx
index 3333207b1..1d71ecd23 100644
--- a/ascender/ui/src/components/FormField/PasswordInput.tsx
+++ b/ascender/ui/src/components/FormField/PasswordInput.tsx
@@ -6,11 +6,13 @@ import type { FieldValidator } from 'components/Form';
import {
Button,
ButtonVariant,
+ InputGroup,
+ InputGroupItem,
TextInput,
- Tooltip,
} from '@patternfly/react-core';
import { EyeIcon, EyeSlashIcon } from '@patternfly/react-icons';
import type { TextInputProps } from '@patternfly/react-core';
+import Tooltip from '../Tooltip';
export interface PasswordInputProps {
autocomplete?: string;
@@ -42,35 +44,43 @@ function PasswordInput({
setInputType(inputType === 'text' ? 'password' : 'text');
};
+ // The input and its reveal toggle are one input group of their own, input
+ // first and toggle after it, the way PatternFly lays a password field out.
+ // Handing callers two loose siblings left the toggle on the left, butted
+ // against the input with none of the gap an input group puts between items.
return (
- <>
-
-
-
- {
- field.onChange(event);
- }}
- />
- >
+ isRequired={isRequired}
+ validated={isValid || isFieldGroupValid ? 'default' : 'error'}
+ type={inputType as TextInputProps['type']}
+ onChange={(event) => {
+ field.onChange(event);
+ }}
+ />
+
+
+
+
+
+
+
);
}
diff --git a/ascender/ui/src/components/FormLayout/FormLayout.css b/ascender/ui/src/components/FormLayout/FormLayout.css
index c33a2fc73..c918861f3 100644
--- a/ascender/ui/src/components/FormLayout/FormLayout.css
+++ b/ascender/ui/src/components/FormLayout/FormLayout.css
@@ -26,18 +26,21 @@
grid-gap: var(--pf-v6-c-form--GridGap);
}
+/* Spacing between the checkboxes, and nothing hanging off the end.
+
+ This used to give every child a trailing margin and pull the same amount
+ back off the container, which is how gutters were done before gap existed.
+ The pull back is a negative margin, so the container's own box grew by it:
+ on a template form it reached six pixels past the card, and a card set to
+ overflow: auto answered with a horizontal scrollbar and a vertical one to
+ match. Gap spaces the children without putting a margin after the last one,
+ so there is nothing to cancel. */
.ascender-form-checkbox-layout {
display: flex;
justify-content: flex-start;
- flex-wrap: wrap;
- margin-bottom: -10px;
- margin-right: -30px !important;
-}
-
-.ascender-form-checkbox-layout > * {
- margin-bottom: 10px;
- margin-right: 30px;
align-items: center;
+ flex-wrap: wrap;
+ gap: 10px 30px;
}
.ascender-sub-form-layout {
diff --git a/ascender/ui/src/components/HealthCheckButton/HealthCheckButton.test.tsx b/ascender/ui/src/components/HealthCheckButton/HealthCheckButton.test.tsx
new file mode 100644
index 000000000..b83c08c10
--- /dev/null
+++ b/ascender/ui/src/components/HealthCheckButton/HealthCheckButton.test.tsx
@@ -0,0 +1,63 @@
+import React from 'react';
+import { screen } from '@testing-library/react';
+import { renderWithContexts } from '../../../testUtils/rtlContexts';
+import HealthCheckButton from './HealthCheckButton';
+
+function renderButton(selectedItems: unknown[], isDisabled: boolean) {
+ return renderWithContexts(
+ {}}
+ selectedItems={selectedItems}
+ healthCheckPending={false}
+ />
+ );
+}
+
+describe('', () => {
+ test('asks for a selection when nothing is ticked', async () => {
+ const { user } = renderButton([], true);
+ const button = screen.getByRole('button', { name: 'Run Health Check' });
+ expect(button).toBeDisabled();
+ await user.hover(button.closest('div')!);
+ expect(
+ await screen.findByText('Select an instance to run a health check.')
+ ).toBeInTheDocument();
+ });
+
+ test('says why a selection without an execution node cannot be checked', async () => {
+ const { user } = renderButton(
+ [
+ { id: 1, node_type: 'control' },
+ { id: 2, node_type: 'hop' },
+ ],
+ true
+ );
+ await user.hover(
+ screen.getByRole('button', { name: 'Run Health Check' }).closest('div')!
+ );
+ expect(
+ await screen.findByText(
+ 'Health checks can only be run on execution nodes.'
+ )
+ ).toBeInTheDocument();
+ });
+
+ test('invites the check once an execution node is ticked', async () => {
+ const { user } = renderButton(
+ [
+ { id: 1, node_type: 'execution' },
+ { id: 2, node_type: 'control' },
+ ],
+ false
+ );
+ const button = screen.getByRole('button', { name: 'Run Health Check' });
+ expect(button).toBeEnabled();
+ await user.hover(button.closest('div')!);
+ expect(
+ await screen.findByText(
+ 'Click to run a health check on the selected instances.'
+ )
+ ).toBeInTheDocument();
+ });
+});
diff --git a/ascender/ui/src/components/HealthCheckButton/HealthCheckButton.tsx b/ascender/ui/src/components/HealthCheckButton/HealthCheckButton.tsx
index d281c568e..eefc8fc5e 100644
--- a/ascender/ui/src/components/HealthCheckButton/HealthCheckButton.tsx
+++ b/ascender/ui/src/components/HealthCheckButton/HealthCheckButton.tsx
@@ -1,8 +1,9 @@
import React from 'react';
import { Plural, useLingui } from '@lingui/react/macro';
-import { Button, Tooltip, DropdownItem } from '@patternfly/react-core';
+import { Button, DropdownItem } from '@patternfly/react-core';
import { useKebabifiedMenu } from 'contexts/Kebabified';
+import Tooltip from '../Tooltip';
export interface HealthCheckButtonProps {
isDisabled: boolean;
@@ -23,9 +24,18 @@ function HealthCheckButton({
const { isKebabified } = useKebabifiedMenu();
const selectedItemsCount = selectedItems.length;
+ // The api only checks execution nodes and skips the rest, so a selection
+ // holding none of them leaves the button disabled, and the tooltip says why
+ // rather than inviting a click.
+ const hasExecutionNode = selectedItems.some(
+ (item) => (item as { node_type?: string })?.node_type === 'execution'
+ );
- const buildTooltip = () =>
- selectedItemsCount ? (
+ const buildTooltip = () => {
+ if (selectedItemsCount && !hasExecutionNode) {
+ return t`Health checks can only be run on execution nodes.`;
+ }
+ return selectedItemsCount ? (
- {healthCheckPending ? t`Running health check` : t`Run health check`}
+ {healthCheckPending ? t`Running Health Check` : t`Run Health Check`}
);
@@ -60,9 +71,9 @@ function HealthCheckButton({
ouiaId="health-check"
onClick={onClick}
isLoading={healthCheckPending}
- spinnerAriaLabel={t`Running health check`}
+ spinnerAriaLabel={t`Running Health Check`}
>
- {healthCheckPending ? t`Running health check` : t`Run health check`}
+ {healthCheckPending ? t`Running Health Check` : t`Run Health Check`}
diff --git a/ascender/ui/src/components/HostForm/HostForm.tsx b/ascender/ui/src/components/HostForm/HostForm.tsx
index 618943931..42a01584a 100644
--- a/ascender/ui/src/components/HostForm/HostForm.tsx
+++ b/ascender/ui/src/components/HostForm/HostForm.tsx
@@ -8,7 +8,6 @@ import {
FormHelperText,
HelperText,
HelperTextItem,
- Tooltip,
} from '@patternfly/react-core';
import { required } from 'util/validators';
import FormField, { FormSubmitError } from '../FormField';
@@ -17,6 +16,7 @@ import { VariablesField } from '../CodeEditor';
import { InventoryLookup } from '../Lookup';
import { FormColumnLayout, FormFullWidthLayout } from '../FormLayout';
import Popover from '../Popover';
+import Tooltip from '../Tooltip';
export interface InventoryLookupFieldProps {
isDisabled: boolean;
diff --git a/ascender/ui/src/components/HostToggle/HostToggle.test.tsx b/ascender/ui/src/components/HostToggle/HostToggle.test.tsx
index 0bd164c55..cc5f4249f 100644
--- a/ascender/ui/src/components/HostToggle/HostToggle.test.tsx
+++ b/ascender/ui/src/components/HostToggle/HostToggle.test.tsx
@@ -1,5 +1,9 @@
import React from 'react';
-import { screen, waitFor } from '@testing-library/react';
+import { render, screen, waitFor } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import { I18nProvider } from '@lingui/react';
+import { i18n } from '@lingui/core';
+import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import { HostsAPI } from 'api';
import type { Host } from 'types/api';
import { renderWithContexts } from '../../../testUtils/rtlContexts';
@@ -27,7 +31,7 @@ const mockHost = {
} as unknown as Host;
// The PF Switch renders a hidden checkbox input with the aria-label
-const getToggle = () => screen.getByRole('switch', { name: 'Toggle host' });
+const getToggle = () => screen.getByRole('switch', { name: 'Toggle Host' });
describe('', () => {
afterEach(() => {
@@ -47,6 +51,8 @@ describe('', () => {
});
await waitFor(() => expect(getToggle()).not.toBeChecked());
expect(onToggle).toHaveBeenCalledWith(false);
+ // PF6 has no off label, so the label itself has to follow the state.
+ expect(screen.getByText('Off')).toBeInTheDocument();
});
test('should toggle on', async () => {
@@ -70,6 +76,61 @@ describe('', () => {
});
await waitFor(() => expect(getToggle()).toBeChecked());
expect(onToggle).toHaveBeenCalledWith(true);
+ expect(screen.getByText('On')).toBeInTheDocument();
+ });
+
+ test('should follow the host it is handed when that changes', async () => {
+ // A list returning from another screen paints its cached rows and then
+ // the fresh ones into the same row, so the switch has to follow the new
+ // host rather than keep what it was mounted with.
+ const { rerender } = renderWithContexts();
+ expect(getToggle()).toBeChecked();
+
+ rerender(
+
+ );
+
+ await waitFor(() => expect(getToggle()).not.toBeChecked());
+ });
+
+ test('should write the change into the cached host lists', async () => {
+ // Otherwise the list navigated back to would paint the old state until
+ // its own read came back.
+ const queryClient = new QueryClient();
+ const other = { ...mockHost, id: 2 };
+ queryClient.setQueryData(['host-list', ''], {
+ hosts: [mockHost, other],
+ count: 2,
+ });
+ queryClient.setQueryData(['inventory-host-list', 1, ''], {
+ hosts: [mockHost],
+ hostCount: 1,
+ });
+ queryClient.setQueryData(['team-list', ''], { results: [{ id: 1 }] });
+ const user = userEvent.setup();
+ render(
+
+
+
+
+
+ );
+
+ await user.click(getToggle());
+ await waitFor(() => expect(getToggle()).not.toBeChecked());
+
+ interface HostPage {
+ hosts: Host[];
+ }
+ const hostList = queryClient.getQueryData(['host-list', '']);
+ expect(hostList?.hosts.map((h) => h.enabled)).toEqual([false, true]);
+ expect(
+ queryClient.getQueryData(['inventory-host-list', 1, ''])
+ ?.hosts[0]?.enabled
+ ).toBe(false);
+ expect(queryClient.getQueryData(['team-list', ''])).toEqual({
+ results: [{ id: 1 }],
+ });
});
test('should be enabled', async () => {
diff --git a/ascender/ui/src/components/HostToggle/HostToggle.tsx b/ascender/ui/src/components/HostToggle/HostToggle.tsx
index 05bcb6837..777a2fb5e 100644
--- a/ascender/ui/src/components/HostToggle/HostToggle.tsx
+++ b/ascender/ui/src/components/HostToggle/HostToggle.tsx
@@ -1,12 +1,21 @@
import type { Host } from 'types/api';
import React, { useState, useEffect, useCallback } from 'react';
+import { useQueryClient } from '@tanstack/react-query';
import { useLingui } from '@lingui/react/macro';
-import { Switch, Tooltip } from '@patternfly/react-core';
+import { Switch } from '@patternfly/react-core';
import useRequest from 'hooks/useRequest';
import { HostsAPI } from 'api';
import AlertModal from '../AlertModal';
import ErrorDetail from '../ErrorDetail';
+import Tooltip from '../Tooltip';
+
+/** The cached lists whose rows are hosts, named by the first element of their key. */
+const HOST_LISTS = [
+ 'host-list',
+ 'inventory-host-list',
+ 'inventory-group-host-list',
+];
export interface HostToggleProps {
className?: string;
@@ -30,32 +39,58 @@ function HostToggle({
jobs. For hosts that are part of an external inventory, this may be
reset by the inventory sync process.`;
}
+ const queryClient = useQueryClient();
const [isEnabled, setIsEnabled] = useState(host.enabled);
const [showError, setShowError] = useState(false);
+ /*
+ * The switch keeps its own state so that it answers a click at once, but the
+ * host it was handed wins whenever that changes. A list returning from
+ * another screen paints its cached rows first and the fresh ones a moment
+ * later, into the same row, so a state taken only on mount would keep
+ * showing whatever the cache said.
+ */
+ const [lastHostEnabled, setLastHostEnabled] = useState(host.enabled);
+ if (host.enabled !== lastHostEnabled) {
+ setLastHostEnabled(host.enabled);
+ setIsEnabled(host.enabled);
+ }
+
const {
- result,
isLoading,
error,
request: toggleHost,
} = useRequest(
useCallback(async () => {
- await HostsAPI.update(host.id, {
- enabled: !isEnabled,
- });
- return !isEnabled;
- }, [host, isEnabled]),
- host.enabled
- );
-
- useEffect(() => {
- if (result !== isEnabled) {
- setIsEnabled(result);
+ const enabled = !isEnabled;
+ await HostsAPI.update(host.id, { enabled });
+ setIsEnabled(enabled);
+ /*
+ * The cached host lists still hold the old value. They are read again
+ * when they next mount, but until that read lands they would paint this
+ * host the way it was, so the change is written into them as well.
+ */
+ queryClient.setQueriesData<{ hosts?: Host[] }>(
+ {
+ predicate: (query) => HOST_LISTS.includes(String(query.queryKey[0])),
+ },
+ (data) =>
+ data?.hosts
+ ? {
+ ...data,
+ hosts: data.hosts.map((h) =>
+ h.id === host.id ? { ...h, enabled } : h
+ ),
+ }
+ : data
+ );
if (onToggle) {
- onToggle(result);
+ onToggle(enabled);
}
- }
- }, [result, isEnabled, onToggle]);
+ return enabled;
+ }, [host.id, isEnabled, onToggle, queryClient]),
+ host.enabled
+ );
useEffect(() => {
if (error) {
@@ -69,8 +104,7 @@ function HostToggle({
{showError && error && !isLoading && (
diff --git a/ascender/ui/src/components/InstanceGroupLabels/InstanceGroupLabels.tsx b/ascender/ui/src/components/InstanceGroupLabels/InstanceGroupLabels.tsx
index 19bc0a905..8b4772238 100644
--- a/ascender/ui/src/components/InstanceGroupLabels/InstanceGroupLabels.tsx
+++ b/ascender/ui/src/components/InstanceGroupLabels/InstanceGroupLabels.tsx
@@ -16,9 +16,7 @@ function InstanceGroupLabels({
isLinkable = false,
}: InstanceGroupLabelsProps) {
const buildLinkURL = (isContainerGroup: unknown) =>
- isContainerGroup
- ? '/instance_groups/container_group/'
- : '/instance_groups/';
+ isContainerGroup ? '/container_groups/' : '/instance_groups/';
return (
{labels.map(({ id, name, is_container_group }) =>
diff --git a/ascender/ui/src/components/InstanceToggle/InstanceToggle.test.tsx b/ascender/ui/src/components/InstanceToggle/InstanceToggle.test.tsx
index 4e0db933f..758e88809 100644
--- a/ascender/ui/src/components/InstanceToggle/InstanceToggle.test.tsx
+++ b/ascender/ui/src/components/InstanceToggle/InstanceToggle.test.tsx
@@ -36,6 +36,7 @@ const mockInstance = {
// The PF Switch renders a hidden checkbox input with the aria-label
const getToggle = () => screen.getByRole('switch', { name: 'Toggle instance' });
+const switchLabel = () => document.querySelector('.pf-v6-c-switch__label');
describe('', () => {
const onToggle = vi.fn();
@@ -54,12 +55,15 @@ describe('', () => {
/>
);
expect(getToggle()).toBeChecked();
+ expect(switchLabel()).toHaveTextContent('On');
await user.click(getToggle());
expect(InstancesAPI.update).toHaveBeenCalledWith(1, {
enabled: false,
});
await waitFor(() => expect(getToggle()).not.toBeChecked());
+ // The label follows the state rather than always reading On.
+ expect(switchLabel()).toHaveTextContent('Off');
expect(onToggle).toHaveBeenCalledWith(false);
expect(fetchInstances).toHaveBeenCalledTimes(1);
});
diff --git a/ascender/ui/src/components/InstanceToggle/InstanceToggle.tsx b/ascender/ui/src/components/InstanceToggle/InstanceToggle.tsx
index 09f1831a5..c247f393f 100644
--- a/ascender/ui/src/components/InstanceToggle/InstanceToggle.tsx
+++ b/ascender/ui/src/components/InstanceToggle/InstanceToggle.tsx
@@ -2,12 +2,13 @@ import type { Instance } from 'types/api';
import React, { useState, useEffect, useCallback } from 'react';
import { useLingui } from '@lingui/react/macro';
-import { Switch, Tooltip } from '@patternfly/react-core';
+import { Switch } from '@patternfly/react-core';
import useRequest from 'hooks/useRequest';
import { InstancesAPI } from 'api';
import { useConfig } from 'contexts/Config';
import ErrorDetail from '../ErrorDetail';
import AlertModal from '../AlertModal';
+import Tooltip from '../Tooltip';
export interface InstanceToggleProps {
className?: string;
@@ -67,8 +68,9 @@ function InstanceToggle({
', () => {
title="Title"
/>
);
- // default (non-icon) button renders the "Cancel Job" text, no MinusCircleIcon
+ // the default (non-icon) button names the kind of run, no MinusCircleIcon
expect(screen.getByRole('button', { name: 'Title' })).toBeInTheDocument();
- expect(screen.getByText('Cancel Job')).toBeInTheDocument();
+ expect(screen.getByText('Cancel Project Sync')).toBeInTheDocument();
expect(document.querySelector('.pf-v6-c-button svg')).toBeNull();
});
@@ -58,7 +58,7 @@ describe('', () => {
expect(await screen.findByRole('dialog')).toBeInTheDocument();
await user.click(
- screen.getByRole('button', { name: 'Confirm cancel job' })
+ screen.getByRole('button', { name: 'Confirm Cancellation' })
);
await waitFor(() =>
expect(ProjectUpdatesAPI.cancel).toHaveBeenCalledWith(1)
@@ -89,15 +89,15 @@ describe('', () => {
await user.click(screen.getByRole('button', { name: 'Title' }));
expect(await screen.findByRole('dialog')).toBeInTheDocument();
await user.click(
- screen.getByRole('button', { name: 'Confirm cancel job' })
+ screen.getByRole('button', { name: 'Confirm Cancellation' })
);
// error modal (errorTitle="Error", with ErrorDetail's "Details" expandable)
- // replaces the confirm modal whose "Confirm cancel job" button is now gone
+ // replaces the confirm modal whose "Confirm Cancellation" button is now gone
expect(await screen.findByText('Error')).toBeInTheDocument();
expect(screen.getByText('Details')).toBeInTheDocument();
expect(
- screen.queryByRole('button', { name: 'Confirm cancel job' })
+ screen.queryByRole('button', { name: 'Confirm Cancellation' })
).not.toBeInTheDocument();
});
@@ -113,7 +113,7 @@ describe('', () => {
await user.click(screen.getByRole('button', { name: 'Title' }));
expect(await screen.findByRole('dialog')).toBeInTheDocument();
await user.click(
- screen.getByRole('button', { name: 'Confirm cancel job' })
+ screen.getByRole('button', { name: 'Confirm Cancellation' })
);
await waitFor(() =>
expect(AdHocCommandsAPI.cancel).toHaveBeenCalledWith(1)
@@ -132,7 +132,7 @@ describe('', () => {
await user.click(screen.getByRole('button', { name: 'Title' }));
expect(await screen.findByRole('dialog')).toBeInTheDocument();
await user.click(
- screen.getByRole('button', { name: 'Confirm cancel job' })
+ screen.getByRole('button', { name: 'Confirm Cancellation' })
);
await waitFor(() => expect(SystemJobsAPI.cancel).toHaveBeenCalledWith(1));
});
@@ -149,7 +149,7 @@ describe('', () => {
await user.click(screen.getByRole('button', { name: 'Title' }));
expect(await screen.findByRole('dialog')).toBeInTheDocument();
await user.click(
- screen.getByRole('button', { name: 'Confirm cancel job' })
+ screen.getByRole('button', { name: 'Confirm Cancellation' })
);
await waitFor(() => expect(WorkflowJobsAPI.cancel).toHaveBeenCalledWith(1));
});
@@ -166,8 +166,58 @@ describe('', () => {
await user.click(screen.getByRole('button', { name: 'Title' }));
expect(await screen.findByRole('dialog')).toBeInTheDocument();
await user.click(
- screen.getByRole('button', { name: 'Confirm cancel job' })
+ screen.getByRole('button', { name: 'Confirm Cancellation' })
);
await waitFor(() => expect(JobsAPI.cancel).toHaveBeenCalledWith(1));
});
+
+ /*
+ * Without wording of its own from the caller, the button, the question and
+ * the error all name the kind of run, the way Relaunch and Delete do.
+ */
+ test('names the kind of run it cancels', async () => {
+ vi.mocked(ProjectUpdatesAPI.cancel).mockRejectedValue(
+ Object.assign(new Error('An error occurred'), {
+ response: {
+ config: { method: 'post', url: '/api/v2/project_updates/1/cancel' },
+ data: 'An error occurred',
+ status: 403,
+ },
+ })
+ );
+ const { user } = renderWithContexts(
+
+ );
+ expect(screen.getByText('Cancel Project Sync')).toBeInTheDocument();
+
+ await user.click(screen.getByRole('button', { name: 'Title' }));
+ expect(
+ await screen.findByText(
+ 'Are you sure you want to cancel this project sync?'
+ )
+ ).toBeInTheDocument();
+ await user.click(
+ screen.getByRole('button', { name: 'Confirm Cancellation' })
+ );
+
+ expect(
+ await screen.findByText('Project Sync Cancel Error')
+ ).toBeInTheDocument();
+ });
+
+ test('keeps the wording a caller gives it', async () => {
+ const { user } = renderWithContexts(
+
+ );
+ expect(screen.getByText('Cancel Workflow')).toBeInTheDocument();
+ await user.click(screen.getByRole('button', { name: 'Title' }));
+ expect(
+ await screen.findByText('This will cancel all subsequent nodes.')
+ ).toBeInTheDocument();
+ });
});
diff --git a/ascender/ui/src/components/JobCancelButton/JobCancelButton.tsx b/ascender/ui/src/components/JobCancelButton/JobCancelButton.tsx
index a5f046ccf..f1198bd69 100644
--- a/ascender/ui/src/components/JobCancelButton/JobCancelButton.tsx
+++ b/ascender/ui/src/components/JobCancelButton/JobCancelButton.tsx
@@ -2,11 +2,12 @@ import type { AnyJob, DetailedError } from 'types/api';
import React, { useCallback, useState } from 'react';
import { useLingui } from '@lingui/react/macro';
import { MinusCircleIcon } from '@patternfly/react-icons';
-import { Button, Tooltip } from '@patternfly/react-core';
-import { getJobModel } from 'util/jobs';
+import { Button } from '@patternfly/react-core';
+import { getJobModel, getRunActionLabels } from 'util/jobs';
import useRequest, { useDismissableError } from 'hooks/useRequest';
import AlertModal from '../AlertModal';
import ErrorDetail from '../ErrorDetail';
+import Tooltip from '../Tooltip';
export interface JobCancelButtonProps {
errorTitle?: React.ReactNode;
@@ -37,8 +38,12 @@ function JobCancelButton({
cancelationMessage,
onCancelWorkflow,
}: JobCancelButtonProps) {
- const { t } = useLingui();
+ const { t, i18n } = useLingui();
const [isOpen, setIsOpen] = useState(false);
+ // The wording names the kind of run, as Relaunch and Delete do, so a
+ // project sync is never offered as a job. A caller's own text still wins.
+ const actionLabels = getRunActionLabels(job.type);
+ const resolvedErrorTitle = errorTitle ?? i18n._(actionLabels.cancelError);
const { error: cancelError, request: cancelJob } = useRequest(
useCallback(async () => {
setIsOpen(false);
@@ -83,7 +88,7 @@ function JobCancelButton({
onClick={() => setIsOpen(true)}
style={style}
>
- {buttonText || t`Cancel Job`}
+ {buttonText || i18n._(actionLabels.cancel)}
)}
@@ -100,11 +105,11 @@ function JobCancelButton({
id="cancel-job-confirm-button"
key="delete"
variant="danger"
- aria-label={t`Confirm cancel job`}
+ aria-label={t`Confirm Cancellation`}
ouiaId="cancel-job-confirm-button"
onClick={cancelJob}
>
- {t`Confirm cancellation`}
+ {t`Confirm Cancellation`}
,
+ }
+ />
+ );
+ await screen.findByRole('link', { name: 'job 1' });
+
+ await user.click(screen.getByRole('button', { name: 'Run' }));
+ // The tab's own control, not the menu that asks what to run.
+ expect(screen.getByText('Run this one')).toBeInTheDocument();
+ expect(screen.queryByRole('menuitem')).not.toBeInTheDocument();
+ });
+
+ test('leaves the run control out entirely when told to', async () => {
+ const { container } = renderWithContexts();
+ await screen.findByRole('link', { name: 'job 1' });
+
+ expect(
+ screen.queryByRole('button', { name: 'Run' })
+ ).not.toBeInTheDocument();
+ // No toolbar item is left standing empty where the control would be.
+ const emptyItems = [
+ ...container.querySelectorAll('.pf-v6-c-toolbar__item'),
+ ].filter((item) => item.childElementCount === 0);
+ expect(emptyItems).toHaveLength(0);
+ });
+
+ /*
+ * The API ORs every or__ clause together, with each other and with any the
+ * defaults hold, as an inventory's Runs tab does. A Status of Failed has to
+ * be an AND clause on every list, or it widens the list instead: to every
+ * failed run anywhere, or, beside a Type of Job, to every job as well.
+ */
+ test.each([
+ ['a list of its own', undefined, 'job.status=failed'],
+ [
+ 'an inventory Runs tab',
+ { or__job__inventory: 2, or__adhoccommand__inventory: 2 },
+ 'job.status=failed',
+ ],
+ ])(
+ 'filters by status on %s so that it narrows',
+ async (_label, defaultParams, expected) => {
+ const { user, history } = renderWithContexts(
+
+ );
+ await screen.findByRole('link', { name: 'job 1' });
+
+ await user.click(
+ screen.getByRole('button', { name: 'Simple key select' })
+ );
+ await user.click(await screen.findByRole('option', { name: 'Status' }));
+ await user.click(
+ screen.getByRole('button', { name: 'Filter By Status' })
+ );
+ await user.click(await screen.findByRole('checkbox', { name: 'Failed' }));
+
+ await waitFor(() => expect(history.location.search).toContain(expected));
+ }
+ );
+
+ /*
+ * Two ticked statuses mean either of them, so on every list they go as one
+ * __in list: repeated AND clauses would match nothing, and the or__ group
+ * would widen the list.
+ */
+ test.each([
+ [
+ 'a list of its own',
+ undefined,
+ '?job.status=failed&job.status=successful',
+ { status__in: 'failed,successful' },
+ ],
+ [
+ 'an inventory Runs tab',
+ { or__job__inventory: 2, or__adhoccommand__inventory: 2 },
+ '?job.status=failed&job.status=successful',
+ { status__in: 'failed,successful' },
+ ],
+ ])(
+ 'asks for either of two ticked statuses on %s',
+ async (_label, defaultParams, search, expected) => {
+ renderWithContexts(, {
+ context: {
+ router: {
+ history: createMemoryHistory({ initialEntries: [`/${search}`] }),
+ },
+ },
+ });
+
+ await waitFor(() =>
+ expect(UnifiedJobsAPI.read).toHaveBeenCalledWith(
+ expect.objectContaining(expected)
+ )
+ );
+ expect(
+ vi.mocked(UnifiedJobsAPI.read).mock.calls[0]?.[0]
+ ).not.toHaveProperty('status');
+ }
+ );
+
+ /*
+ * Type and Status used to go as or__type and or__status, which the API ORs
+ * together: a Type of Job and a Status of Failed listed every job and every
+ * failed run of any kind, rather than the jobs that failed.
+ */
+ test('asks for runs of the ticked type that also have the ticked status', async () => {
+ const { user, history } = renderWithContexts();
+ await screen.findByRole('link', { name: 'job 1' });
+
+ const tick = async (key: string, option: string) => {
+ await user.click(
+ screen.getByRole('button', { name: 'Simple key select' })
+ );
+ await user.click(await screen.findByRole('option', { name: key }));
+ await user.click(
+ screen.getByRole('button', { name: `Filter By ${key}` })
+ );
+ await user.click(await screen.findByRole('checkbox', { name: option }));
+ };
+ await tick('Type', 'Job');
+ await tick('Status', 'Failed');
+
+ await waitFor(() =>
+ expect(history.location.search).toContain('job.status=failed')
+ );
+ expect(history.location.search).toContain('job.type=job');
+ await waitFor(() =>
+ expect(UnifiedJobsAPI.read).toHaveBeenLastCalledWith(
+ expect.objectContaining({ type: 'job', status: 'failed' })
+ )
+ );
+ const params = vi.mocked(UnifiedJobsAPI.read).mock.lastCall?.[0];
+ expect(params).not.toHaveProperty('or__type');
+ expect(params).not.toHaveProperty('or__status');
+ });
+
+ /* An address saved while the filters were or__ still opens, and asks for
+ what it meant rather than joining the OR group. */
+ test('reads a status saved in the old or__ form', async () => {
+ renderWithContexts(, {
+ context: {
+ router: {
+ history: createMemoryHistory({
+ initialEntries: [
+ '/?job.or__status=failed&job.or__status=successful',
+ ],
+ }),
+ },
+ },
+ });
+
+ await screen.findByRole('link', { name: 'job 1' });
+ const params = vi.mocked(UnifiedJobsAPI.read).mock.calls[0]?.[0];
+ expect(params).toEqual(
+ expect.objectContaining({ status__in: 'failed,successful' })
+ );
+ expect(params).not.toHaveProperty('or__status');
+ });
+
+ /*
+ * The button offers every kind of run the list shows, and each item opens
+ * what that kind is started from.
+ */
+ test('should offer a run of each kind the list shows', async () => {
+ const { user } = renderWithContexts();
+ await screen.findByRole('link', { name: 'job 1' });
+
+ await user.click(screen.getByRole('button', { name: 'Run' }));
+
+ [
+ 'Job',
+ 'Workflow',
+ 'Command',
+ 'Inventory Sync',
+ 'Project Sync',
+ 'Cleanup Job',
+ ].forEach((kind) =>
+ expect(screen.getByRole('menuitem', { name: kind })).toBeVisible()
+ );
+ });
+
+ /* The template is the wizard's first step, then where to run it. */
+ test('should open the launch wizard on the template to run', async () => {
+ vi.mocked(JobTemplatesAPI.read).mockResolvedValue({
+ data: { count: 1, results: [{ id: 7, name: 'a template' }] },
+ } as unknown as Awaited>);
+ const { user } = renderWithContexts();
+ await screen.findByRole('link', { name: 'job 1' });
+
+ await user.click(screen.getByRole('button', { name: 'Run' }));
+ await user.click(screen.getByRole('menuitem', { name: 'Job' }));
+
+ expect(await screen.findByText('a template')).toBeInTheDocument();
+ await waitFor(() =>
+ expect(
+ document.querySelector('.pf-v6-c-wizard__nav-link.pf-m-current')
+ ?.textContent
+ ).toEqual('Template')
+ );
+ });
+
test('should select and un-select items', async () => {
const { user } = renderWithContexts();
await screen.findByRole('link', { name: 'job 1' });
@@ -366,6 +602,75 @@ describe('', () => {
expect(screen.getByRole('button', { name: 'Delete' })).toBeDisabled();
});
+ /*
+ * The selection is a copy of each row taken when it was ticked, while the
+ * websocket moves the rows on. The toolbar must decide on the status a run
+ * has now, so a run that finished after being ticked becomes deletable and
+ * stops being cancelable.
+ */
+ describe('after a ticked run changes status', () => {
+ let mockServer: WS;
+
+ beforeEach(() => {
+ WS.clean();
+ global.document.cookie = 'csrftoken=abc123';
+ mockServer = new WS('ws://localhost/websocket/');
+ vi.mocked(UnifiedJobsAPI.read).mockResolvedValue({
+ data: { count: 1, results: [mockResults[1]] },
+ } as unknown as ResponseOf);
+ });
+
+ afterEach(() => {
+ mockServer.close();
+ WS.clean();
+ });
+
+ async function tickThenFinish() {
+ const rendered = renderWithContexts();
+ await screen.findByRole('link', { name: 'job 2' });
+ await mockServer.connected;
+ await rendered.user.click(
+ screen.getByRole('checkbox', { name: 'Select row 0' })
+ );
+ expect(screen.getByRole('button', { name: 'Delete' })).toBeDisabled();
+ expect(screen.getByRole('button', { name: 'Cancel' })).toBeEnabled();
+
+ await act(async () => {
+ mockServer.send(
+ JSON.stringify({
+ unified_job_id: 2,
+ type: 'job',
+ status: 'successful',
+ })
+ );
+ });
+ return rendered;
+ }
+
+ test('enables Delete once the run has finished', async () => {
+ const destroyJob = mockInherited(JobsAPI, 'destroy');
+ const { user } = await tickThenFinish();
+
+ await waitFor(() =>
+ expect(screen.getByRole('button', { name: 'Delete' })).toBeEnabled()
+ );
+ await user.click(screen.getByRole('button', { name: 'Delete' }));
+ const dialog = await screen.findByRole('dialog');
+ await user.click(
+ within(dialog).getByRole('button', { name: 'confirm delete' })
+ );
+ await waitFor(() => expect(destroyJob).toHaveBeenCalledWith(2));
+ });
+
+ test('disables Cancel once the run has finished', async () => {
+ await tickThenFinish();
+
+ await waitFor(() =>
+ expect(screen.getByRole('button', { name: 'Cancel' })).toBeDisabled()
+ );
+ });
+ });
+
test('error is shown when job not successfully deleted from api', async () => {
vi.mocked(UnifiedJobsAPI.read).mockResolvedValue({
data: { count: 6, results: deletableResults },
@@ -434,7 +739,7 @@ describe('', () => {
await screen.findByRole('link', { name: 'job 1' });
await user.click(screen.getByRole('checkbox', { name: 'Select all' }));
- await user.click(screen.getByRole('button', { name: 'Cancel jobs' }));
+ await user.click(screen.getByRole('button', { name: 'Cancel' }));
// confirm modal -> the danger confirm button
const dialog = await screen.findByRole('dialog');
await user.click(dialog.querySelector('#cancel-job-confirm-button')!);
@@ -469,7 +774,7 @@ describe('', () => {
await user.click(
within(row as unknown as HTMLElement).getByRole('checkbox')
);
- await user.click(screen.getByRole('button', { name: 'Cancel job' }));
+ await user.click(screen.getByRole('button', { name: 'Cancel' }));
const dialog = await screen.findByRole('dialog');
await user.click(dialog.querySelector('#cancel-job-confirm-button')!);
@@ -477,3 +782,39 @@ describe('', () => {
await settleTooltips();
});
});
+
+describe('asChoiceLists', () => {
+ test('sends a status or type with several options as one __in list', () => {
+ expect(
+ asChoiceLists({
+ status: ['failed', 'error'],
+ type: ['job', 'project_update'],
+ page: 2,
+ })
+ ).toEqual({
+ status__in: 'failed,error',
+ type__in: 'job,project_update',
+ page: 2,
+ });
+ });
+
+ test('leaves a single option and other filters as they are', () => {
+ const params = { status: 'failed', name__icontains: ['a', 'b'] };
+ expect(asChoiceLists(params)).toEqual(params);
+ });
+
+ test('folds the old or__ form into the plain one', () => {
+ expect(
+ asChoiceLists({
+ or__status: ['failed', 'error'],
+ status: 'failed',
+ or__type: 'job',
+ or__job__inventory: 2,
+ })
+ ).toEqual({
+ status__in: 'failed,error',
+ type: 'job',
+ or__job__inventory: 2,
+ });
+ });
+});
diff --git a/ascender/ui/src/components/JobList/JobList.tsx b/ascender/ui/src/components/JobList/JobList.tsx
index d001d5d18..8fb0c0b6d 100644
--- a/ascender/ui/src/components/JobList/JobList.tsx
+++ b/ascender/ui/src/components/JobList/JobList.tsx
@@ -1,17 +1,15 @@
import type { UnifiedJob } from 'types/api';
-import React, { useEffect, useCallback, useRef } from 'react';
+import React, { useEffect, useCallback, useMemo, useRef } from 'react';
import { useLocation } from 'react-router';
import { useLingui, Plural } from '@lingui/react/macro';
-import { Card } from '@patternfly/react-core';
import useRequest, {
useDeleteItems,
useDismissableError,
} from 'hooks/useRequest';
-import { useConfig } from 'contexts/Config';
import useSelected from 'hooks/useSelected';
import useExpanded from 'hooks/useExpanded';
-import { isJobRunning, getJobModel } from 'util/jobs';
+import { canDeleteJob, getJobModel, isJobCancelable } from 'util/jobs';
import type { QSParams } from 'util/qs';
import { getQSConfig, parseQueryString } from 'util/qs';
import { UnifiedJobsAPI, InventorySourcesAPI } from 'api';
@@ -26,19 +24,76 @@ import PaginatedTable, {
} from '../PaginatedTable';
import JobListItem from './JobListItem';
import JobListCancelButton from './JobListCancelButton';
+import RunMenu from './RunMenu';
import useWsJobs from './useWsJobs';
export interface JobListProps {
/** Narrows the list to one resource's jobs, merged into the query string. */
defaultParams?: QSParams;
showTypeColumn?: boolean;
+ /**
+ * Keeps the runs another run started for itself, which the list otherwise
+ * leaves out: a project or inventory source's own tab, where the updates a
+ * job launch triggered are most of its history rather than noise.
+ */
+ includeDependencySyncs?: boolean;
+ /**
+ * What starts a run from this list. A resource's runs tab passes its own,
+ * launch this template or sync this project, so Run means this one rather
+ * than a menu of everything; without it the list offers the runs screen's
+ * menu, which asks what to run. False leaves the toolbar without one.
+ */
+ runControl?: React.ReactNode;
additionalRelatedSearchableKeys?: string[];
[key: string]: unknown;
}
+/** The choice filters that may hold several ticked options at once. */
+const CHOICE_FIELDS = ['status', 'type'];
+
+/**
+ * The query with each choice filter that holds several options as one list.
+ *
+ * Repeated plain clauses, status=a&status=b, are ANDed by the API and match
+ * nothing, while status__in=a,b matches either. The API ORs every or__ clause
+ * in a query into a single group, so the or__status and or__type these filters
+ * used to send joined each other and any or__ defaults: Type of Job with Status
+ * of Failed asked for every job and every failed run. An address saved from
+ * then still carries those keys, and they are folded into the plain ones here
+ * so it reads as it was meant to rather than as the API would read it.
+ *
+ * Args:
+ * params: The query as parsed from the address.
+ *
+ * Returns:
+ * A copy of the query, with status and type sent as one plain clause where
+ * one option is ticked and as an __in list wherever more than one is, and
+ * no or__status or or__type left in it.
+ */
+export function asChoiceLists(params: QSParams): QSParams {
+ const next: QSParams = { ...params };
+ CHOICE_FIELDS.forEach((field) => {
+ const legacyKey = `or__${field}`;
+ const values = [next[field], next[legacyKey]]
+ .flat()
+ .filter((value) => value !== undefined && value !== null && value !== '');
+ delete next[legacyKey];
+ const unique = [...new Set(values.map(String))];
+ if (unique.length > 1) {
+ delete next[field];
+ next[`${field}__in`] = unique.join(',');
+ } else if (unique.length === 1) {
+ next[field] = unique[0] as string;
+ }
+ });
+ return next;
+}
+
function JobList({
defaultParams,
showTypeColumn = false,
+ includeDependencySyncs = false,
+ runControl,
additionalRelatedSearchableKeys = [],
}: JobListProps) {
const { t } = useLingui();
@@ -48,14 +103,13 @@ function JobList({
page: 1,
page_size: 20,
order_by: '-finished',
- not__launch_type: 'sync',
+ ...(includeDependencySyncs ? {} : { not__launch_type: 'sync' }),
...defaultParams,
},
['id', 'page', 'page_size'],
['created', 'modified', 'finished']
);
- const { me } = useConfig();
const location = useLocation();
const {
result: {
@@ -71,7 +125,9 @@ function JobList({
} = useRequest(
useCallback(
async () => {
- const params = parseQueryString(qsConfig, location.search);
+ const params = asChoiceLists(
+ parseQueryString(qsConfig, location.search)
+ );
const [response, actionsResponse, { data: inventorySourceOptions }] =
await Promise.all([
UnifiedJobsAPI.read({ ...params }),
@@ -114,7 +170,7 @@ function JobList({
const fetchJobsById = useCallback(
async (ids: (number | string)[]) => {
- const params = parseQueryString(qsConfig, location.search);
+ const params = asChoiceLists(parseQueryString(qsConfig, location.search));
params.id__in = ids.join(',');
try {
const { data } = await UnifiedJobsAPI.read(params);
@@ -129,8 +185,26 @@ function JobList({
const jobs = useWsJobs(results, fetchJobsById, qsConfig);
- const { selected, isAllSelected, handleSelect, selectAll, clearSelected } =
- useSelected(jobs);
+ const {
+ selected: selectedSnapshots,
+ isAllSelected,
+ handleSelect,
+ selectAll,
+ clearSelected,
+ } = useSelected(jobs);
+
+ // useSelected keeps a copy of each row as it was when it was ticked, while
+ // the websocket moves the rows in jobs on. Reading the selection back from
+ // jobs by id keeps the toolbar's Cancel and Delete deciding on the status a
+ // run has now, not the one it had then. A row that has since left the page
+ // keeps its last known copy.
+ const selected = useMemo(
+ () =>
+ selectedSnapshots.map(
+ (row) => jobs.find((job) => job.id === row.id) ?? row
+ ),
+ [selectedSnapshots, jobs]
+ );
const { expanded, isAllExpanded, handleExpand, expandAll } =
useExpanded(jobs);
@@ -144,7 +218,7 @@ function JobList({
async () =>
Promise.all(
selected.map((job) => {
- if (isJobRunning(job.status)) {
+ if (isJobCancelable(job.status)) {
return getJobModel(job.type).cancel(job.id);
}
return Promise.resolve();
@@ -188,146 +262,156 @@ function JobList({
clearSelected();
};
- const cannotDeleteItems = selected.filter((job) => isJobRunning(job.status));
+ // A new run may be deleted as well as a finished one, as the api allows;
+ // the same helper decides it on the details page and the output toolbars.
+ const cannotDeleteItems = selected.filter((job) => !canDeleteJob(job));
return (
<>
-
-
- {t`ID`}
- {t`Name`}
- {t`Status`}
- {showTypeColumn && {t`Type`}}
- {t`Start Time`}
- {t`Finish Time`}
- {t`Actions`}
-
- }
- clearSelected={clearSelected}
- toolbarSearchableKeys={searchableKeys}
- toolbarRelatedSearchableKeys={[
- ...relatedSearchableKeys,
- ...additionalRelatedSearchableKeys,
- ]}
- renderToolbar={(props) => (
- {
- item.name = `${item.id} - ${item.name}`;
- return item;
- })}
- pluralizedItemName={t`Jobs`}
- cannotDelete={(item) =>
- isJobRunning(item.status as string) ||
- !item.summary_fields?.user_capabilities?.delete
- }
- errorMessage={
-
- }
- />,
- ,
- ]}
- />
- )}
- renderRow={(job: UnifiedJob, index: number) => (
- row.id === job.id)}
- onExpand={() => handleExpand(job)}
- isSuperUser={Boolean(me?.is_superuser)}
- showTypeColumn={showTypeColumn}
- onSelect={() => handleSelect(job)}
- isSelected={selected.some((row) => row.id === job.id)}
- rowIndex={index}
- />
- )}
- />
-
+
+ {t`ID`}
+ {t`Name`}
+ {t`Status`}
+ {showTypeColumn && {t`Type`}}
+ {t`Start Time`}
+ {t`Finish Time`}
+ {t`Actions`}
+
+ }
+ clearSelected={clearSelected}
+ toolbarSearchableKeys={searchableKeys}
+ toolbarRelatedSearchableKeys={[
+ ...relatedSearchableKeys,
+ ...additionalRelatedSearchableKeys,
+ ]}
+ renderToolbar={(props) => (
+
+ {runControl ?? }
+ ,
+ ]),
+ ,
+ {
+ item.name = `${item.id} - ${item.name}`;
+ return item;
+ })}
+ pluralizedItemName={t`Jobs`}
+ cannotDelete={(item) => !canDeleteJob(item)}
+ errorMessage={
+
+ }
+ />,
+ ]}
+ />
+ )}
+ renderRow={(job: UnifiedJob, index: number) => (
+ row.id === job.id)}
+ onExpand={() => handleExpand(job)}
+ showTypeColumn={showTypeColumn}
+ onSelect={() => handleSelect(job)}
+ isSelected={selected.some((row) => row.id === job.id)}
+ rowIndex={index}
+ />
+ )}
+ />
{deletionError && (
', () => {
expect(getCancelButton()).toBeDisabled();
// Tooltip content is rendered on hover; asserts the
- // "Select a job to cancel" tooltip text.
+ // "Select a row to cancel" tooltip text.
await user.hover(getCancelButton()!.closest('div')!);
expect(
- await screen.findByText('Select a job to cancel')
+ await screen.findByText('Select a row to cancel')
).toBeInTheDocument();
});
@@ -72,6 +74,31 @@ describe('', () => {
expect(getCancelButton()).toBeDisabled();
});
+ test('reads the cancel capability, not the start one', async () => {
+ const { user } = renderWithContexts(
+
+ );
+ expect(getCancelButton()).toBeDisabled();
+ await user.hover(getCancelButton()!.parentElement!);
+ expect(
+ await screen.findByText('You do not have permission to cancel:')
+ ).toBeInTheDocument();
+ });
+
test('should be enabled when user does have permission to cancel selected job', () => {
renderWithContexts(
', () => {
user_capabilities: {
delete: true,
start: true,
+ cancel: true,
},
},
status: 'running',
@@ -108,6 +136,7 @@ describe('', () => {
user_capabilities: {
delete: true,
start: true,
+ cancel: true,
},
},
status: 'running',
@@ -133,12 +162,29 @@ describe('', () => {
// open modal again, click the confirm (danger) button -> onCancel called
await user.click(getCancelButton()!);
expect(await screen.findByRole('dialog')).toBeInTheDocument();
- // the confirm button shares the "Cancel job" text but lives in the modal
- const dialog = screen.getByRole('dialog');
- const confirmButton = dialog.querySelector(
- '#cancel-job-confirm-button'
- ) as HTMLElement;
- await user.click(confirmButton);
+ // The confirm button says what it does, as the single run's dialog does,
+ // rather than repeating the toolbar's Cancel.
+ await user.click(
+ screen.getByRole('button', { name: 'Confirm Cancellation' })
+ );
expect(onCancel).toHaveBeenCalledTimes(1);
});
+
+ test('the kebab item is named by its own text', () => {
+ renderWithContexts(
+ {} }}
+ >
+
+
+ );
+ const item = screen.getByRole('menuitem', { name: 'Cancel' });
+ expect(item).not.toHaveAttribute('aria-labelledby');
+ });
});
diff --git a/ascender/ui/src/components/JobList/JobListCancelButton.tsx b/ascender/ui/src/components/JobList/JobListCancelButton.tsx
index e602a54ad..462764375 100644
--- a/ascender/ui/src/components/JobList/JobListCancelButton.tsx
+++ b/ascender/ui/src/components/JobList/JobListCancelButton.tsx
@@ -1,20 +1,26 @@
import type { UnifiedJob } from 'types/api';
import React, { useContext, useEffect, useState } from 'react';
-import { Plural, useLingui } from '@lingui/react/macro';
-import { Button, Tooltip, DropdownItem } from '@patternfly/react-core';
+import { useLingui } from '@lingui/react/macro';
+import { Button, DropdownItem } from '@patternfly/react-core';
import { KebabifiedContext } from 'contexts/Kebabified';
-import { isJobRunning } from 'util/jobs';
+import { canCancelJob, isJobCancelable } from 'util/jobs';
import AlertModal from '../AlertModal';
+import Tooltip from '../Tooltip';
+/*
+ * The same rule as a row's own Cancel, the details page and the output
+ * toolbars: the api's cancel capability, which covers the run's creator, an
+ * admin of what it ran and a superuser, and a cleanup job for superusers only.
+ * The start capability it used to read let an executor who may not cancel
+ * pick a cleanup job or someone else's run and be refused.
+ */
function cannotCancelBecausePermissions(job: UnifiedJob) {
- return (
- !job.summary_fields.user_capabilities?.start && isJobRunning(job.status)
- );
+ return isJobCancelable(job.status) && !canCancelJob(job);
}
function cannotCancelBecauseNotRunning(job: UnifiedJob) {
- return !isJobRunning(job.status);
+ return !isJobCancelable(job.status);
}
export interface JobListCancelButtonProps {
@@ -62,11 +68,7 @@ function JobListCancelButton({
{cannotCancelPermissions.length > 0 && (
-
+ {t`You do not have permission to cancel:`}
{cannotCancelPermissions.map((job, i) => (
{' '}
@@ -78,11 +80,7 @@ function JobListCancelButton({
)}
{cannotCancelNotRunning.length > 0 && (
-
+ {t`Not running, so nothing to cancel:`}
{cannotCancelNotRunning.map((job, i) => (
{' '}
@@ -96,24 +94,16 @@ function JobListCancelButton({
);
}
if (numJobsToCancel > 0) {
- return (
-
- );
+ return t`Cancel the selection`;
}
- return t`Select a job to cancel`;
+ return t`Select a row to cancel`;
};
const isDisabled =
jobsToCancel.length === 0 ||
jobsToCancel.some(cannotCancelBecausePermissions) ||
jobsToCancel.some(cannotCancelBecauseNotRunning);
- const cancelJobText = (
-
- );
+ const cancelJobText = t`Cancel`;
return (
<>
@@ -122,7 +112,9 @@ function JobListCancelButton({
key="cancel-job"
isDisabled={isDisabled}
component="button"
- aria-labelledby="jobs-list-cancel-button"
+ // Named by its own text: the toolbar button whose id the label
+ // used to point at is not rendered while the actions sit in a
+ // kebab.
onClick={toggleModal}
ouiaId="cancel-job-dropdown-item"
>
@@ -156,10 +148,9 @@ function JobListCancelButton({
id="cancel-job-confirm-button"
key="delete"
variant="danger"
- aria-labelledby="cancel-job-confirm-button"
onClick={handleCancelJob}
>
- {cancelJobText}
+ {t`Confirm Cancellation`}
,