From 4e812ec20fed2893a208d21d79c9a937e48702b7 Mon Sep 17 00:00:00 2001 From: Jord Date: Tue, 5 May 2026 21:28:49 +0100 Subject: [PATCH 1/3] Adding new backfill option --- back/admin/integrations/models.py | 29 +++++++++ back/admin/integrations/serializers.py | 1 + back/admin/integrations/tasks.py | 51 ++++++++++++++++ back/admin/integrations/urls.py | 5 ++ back/admin/integrations/utils.py | 59 ++++++++++++++++--- back/admin/integrations/views.py | 23 ++++++++ .../templates/settings_integrations.html | 9 +++ 7 files changed, 170 insertions(+), 7 deletions(-) diff --git a/back/admin/integrations/models.py b/back/admin/integrations/models.py index a14489411..421deff74 100644 --- a/back/admin/integrations/models.py +++ b/back/admin/integrations/models.py @@ -220,6 +220,13 @@ def is_sync_users_integration(self): def can_revoke_access(self): return len(self.manifest.get("revoke", [])) + @property + def can_backfill_ids(self): + # True when the manifest's `exists` block declares `store_data`, + # meaning we can extract IDs from the lookup response and backfill + # them into existing users' extra_fields. + return bool(self.manifest.get("exists", {}).get("store_data")) + @property def update_url(self): return reverse("integrations:update", args=[self.id]) @@ -530,6 +537,28 @@ def user_exists(self, new_hire, save_result=True): user_exists = self.tracker.steps.last().found_expected + # If the user was found and the manifest declares store_data on its + # exists block, capture those values into extra_fields. Lets a single + # lookup populate IDs (e.g. ATLASSIAN_USER_ID, bitwarden_id) for users + # that pre-existed in the upstream system. + store_data = self.manifest["exists"].get("store_data", {}) + if user_exists and store_data: + try: + json_response = response.json() + except (ValueError, AttributeError): + json_response = {} + for new_hire_prop, notation in store_data.items(): + try: + value = get_value_from_notation( + self._replace_vars(notation), json_response + ) + except KeyError: + continue + if value is None: + continue + new_hire.extra_fields[new_hire_prop] = value + new_hire.save() + if save_result: IntegrationUser.objects.update_or_create( integration=self, user=new_hire, defaults={"revoked": not user_exists} diff --git a/back/admin/integrations/serializers.py b/back/admin/integrations/serializers.py index 12130b926..89e0d0d84 100644 --- a/back/admin/integrations/serializers.py +++ b/back/admin/integrations/serializers.py @@ -51,6 +51,7 @@ class ManifestPollingSerializer(ValidateMixin, serializers.Serializer): class ManifestExistSerializer(ValidateMixin, serializers.Serializer): url = serializers.CharField() expected = serializers.CharField() + store_data = serializers.DictField(child=serializers.CharField(), default=dict) status_code = serializers.ListField( child=serializers.IntegerField(), required=False ) diff --git a/back/admin/integrations/tasks.py b/back/admin/integrations/tasks.py index bcb9776ba..4de43fa32 100644 --- a/back/admin/integrations/tasks.py +++ b/back/admin/integrations/tasks.py @@ -1,8 +1,12 @@ +import logging + from django.contrib.auth import get_user_model from admin.integrations.models import Integration from admin.integrations.sync_userinfo import SyncUsers +logger = logging.getLogger(__name__) + def retry_integration(new_hire_id, integration_id, params): integration = Integration.objects.get(id=integration_id) @@ -15,3 +19,50 @@ def sync_user_info(integration_id): # users or we will add new users. This is done in the background. integration = Integration.objects.get(id=integration_id) SyncUsers(integration).run() + + +def backfill_integration_ids(integration_id): + # Run the integration's `exists` lookup against every user. Any + # store_data fields declared on the exists block get written to the + # user's extra_fields. Used to populate IDs for users who were + # provisioned in the external system before this integration existed. + integration = Integration.objects.get(id=integration_id) + store_keys = list( + integration.manifest.get("exists", {}).get("store_data", {}).keys() + ) + + users = get_user_model().objects.exclude(email="").order_by("id") + matched = skipped = not_found = errored = 0 + + for user in users: + # skip users who already have all backfill keys set + if store_keys and all(k in user.extra_fields for k in store_keys): + skipped += 1 + continue + try: + result = integration.user_exists(user, save_result=False) + except Exception as e: + logger.warning( + "Backfill error for integration %s, user %s: %s", + integration_id, user.email, e, + ) + errored += 1 + continue + if result is True: + matched += 1 + elif result is False: + not_found += 1 + else: + errored += 1 + + logger.info( + "Backfill complete for integration %s: " + "%s matched, %s skipped, %s not found, %s errored", + integration_id, matched, skipped, not_found, errored, + ) + return { + "matched": matched, + "skipped": skipped, + "not_found": not_found, + "errored": errored, + } diff --git a/back/admin/integrations/urls.py b/back/admin/integrations/urls.py index 8f182daee..b2117cb0c 100644 --- a/back/admin/integrations/urls.py +++ b/back/admin/integrations/urls.py @@ -33,6 +33,11 @@ views.IntegrationDeleteExtraArgsView.as_view(), name="delete-creds", ), + path( + "backfill_ids//", + views.IntegrationBackfillIDsView.as_view(), + name="backfill-ids", + ), path( "tracker/", views.IntegrationTrackerListView.as_view(), diff --git a/back/admin/integrations/utils.py b/back/admin/integrations/utils.py index c2e4daf68..89a661859 100644 --- a/back/admin/integrations/utils.py +++ b/back/admin/integrations/utils.py @@ -1,27 +1,72 @@ +def _tokenize_notation(notation): + # split on '.' but keep [...] groups intact, so values inside a filter + # expression (which may themselves contain '.', e.g. emails) aren't split + tokens = [] + buf = "" + depth = 0 + for ch in notation: + if ch == "[": + depth += 1 + buf += ch + elif ch == "]": + depth -= 1 + buf += ch + elif ch == "." and depth == 0: + if buf: + tokens.append(buf) + buf = "" + else: + buf += ch + if buf: + tokens.append(buf) + return tokens + + def get_value_from_notation(notation, value): # if we don't need to go into props, then just return the value if notation == "": return value - notations = notation.split(".") - for notation in notations: + for token in _tokenize_notation(notation): + # filter form: optional_key[field=expected] - pick first list entry + # whose `field` equals `expected`. Useful when the upstream API returns + # an unfiltered list (e.g. Bitwarden /public/members). + if "[" in token and token.endswith("]"): + list_key, _, filter_expr = token.partition("[") + filter_expr = filter_expr[:-1] + + if list_key: + try: + value = value[list_key] + except (KeyError, TypeError): + raise KeyError + + if "=" not in filter_expr or not isinstance(value, list): + raise KeyError + + field, _, expected = filter_expr.partition("=") + for item in value: + if isinstance(item, dict) and str(item.get(field, "")) == expected: + value = item + break + else: + raise KeyError + continue + try: - value = value[notation] + value = value[token] except TypeError: - # check if array if not isinstance(value, list): raise KeyError try: - index = int(notation) + index = int(token) except (TypeError, ValueError): - # keep errors consistent, we are only expecting a KeyError raise KeyError try: value = value[index] except (TypeError, ValueError, IndexError): - # keep errors consistent, we are only expecting a KeyError raise KeyError return value diff --git a/back/admin/integrations/views.py b/back/admin/integrations/views.py index 832bfc033..bfe625fac 100644 --- a/back/admin/integrations/views.py +++ b/back/admin/integrations/views.py @@ -15,6 +15,7 @@ from django.views.generic.detail import DetailView from django.views.generic.edit import CreateView, DeleteView, UpdateView from django.views.generic.list import ListView +from django_q.tasks import async_task from users.mixins import AdminOrManagerPermMixin, AdminPermMixin @@ -244,3 +245,25 @@ def get_context_data(self, **kwargs): } context["subtitle"] = _("integrations") return context + + +class IntegrationBackfillIDsView(AdminPermMixin, View): + def post(self, request, pk): + integration = get_object_or_404(Integration, pk=pk) + if not integration.can_backfill_ids: + messages.error( + request, + _("This integration has no store_data declared on its exists block."), + ) + return redirect("settings:integrations") + async_task( + "admin.integrations.tasks.backfill_integration_ids", + integration.id, + task_name=f"Backfill IDs: {integration.name}", + ) + messages.success( + request, + _("Backfill started for %(name)s. Users' extra fields will populate " + "as the lookup runs in the background.") % {"name": integration.name}, + ) + return redirect("settings:integrations") diff --git a/back/admin/settings/templates/settings_integrations.html b/back/admin/settings/templates/settings_integrations.html index 17174d36c..bf5bf8370 100644 --- a/back/admin/settings/templates/settings_integrations.html +++ b/back/admin/settings/templates/settings_integrations.html @@ -65,6 +65,15 @@ {% translate "Update credentials" %} {% endif %} + {% if integration.can_backfill_ids %} +
+ {% csrf_token %} + +
+ {% endif %} {% translate "Update manifest" %} From dd061576d40a7ec772058d6723c2446aa6bfa8b2 Mon Sep 17 00:00:00 2001 From: Jord Date: Wed, 6 May 2026 12:52:34 +0100 Subject: [PATCH 2/3] Adding extra_fields to be able to backfill data for integrations that only support usernames --- back/admin/integrations/models.py | 1 + 1 file changed, 1 insertion(+) diff --git a/back/admin/integrations/models.py b/back/admin/integrations/models.py index 421deff74..5f62a0308 100644 --- a/back/admin/integrations/models.py +++ b/back/admin/integrations/models.py @@ -525,6 +525,7 @@ def user_exists(self, new_hire, save_result=True): self.new_hire = new_hire self.has_user_context = new_hire is not None + self.params = new_hire.extra_fields # Renew token if necessary if not self.renew_key(): From 498c106922395c441fca2c078db666509b873098 Mon Sep 17 00:00:00 2001 From: Jord Date: Wed, 27 May 2026 10:52:23 +0100 Subject: [PATCH 3/3] Added Reporting --- back/admin/integrations/tasks.py | 55 ++++++ .../integrations/templates/access_report.html | 113 ++++++++++++ back/admin/integrations/tests.py | 29 ++++ back/admin/integrations/urls.py | 15 ++ back/admin/integrations/utils.py | 15 +- back/admin/integrations/views.py | 161 +++++++++++++++++- back/back/settings.py | 1 + back/users/templates/admin_base.html | 15 ++ 8 files changed, 400 insertions(+), 4 deletions(-) create mode 100644 back/admin/integrations/templates/access_report.html diff --git a/back/admin/integrations/tasks.py b/back/admin/integrations/tasks.py index 4de43fa32..92d30fa1a 100644 --- a/back/admin/integrations/tasks.py +++ b/back/admin/integrations/tasks.py @@ -1,6 +1,7 @@ import logging from django.contrib.auth import get_user_model +from django_q.tasks import async_task from admin.integrations.models import Integration from admin.integrations.sync_userinfo import SyncUsers @@ -66,3 +67,57 @@ def backfill_integration_ids(integration_id): "not_found": not_found, "errored": errored, } + + +def refresh_access_for_user_integration(integration_id, user_id): + # One `exists` lookup for a single (integration, user) pair. Enqueued + # individually so a slow or failing integration can't stall the whole + # refresh. Errors are swallowed and logged — one bad lookup must not + # prevent the other queued tasks from running. + try: + integration = Integration.objects.get(id=integration_id) + user = get_user_model().objects.get(id=user_id) + except (Integration.DoesNotExist, get_user_model().DoesNotExist): + return + try: + integration.user_exists(user, save_result=True) + except Exception as e: + logger.warning( + "Refresh error for integration %s, user %s: %s", + integration_id, user.email, e, + ) + + +def refresh_access_report(): + # Enqueue one background task per (integration, user) pair instead of + # running every lookup inline — a single big task was timing out on + # orgs with lots of staff. The worker pool processes the queue, which + # also provides natural throttling against per-service rate limits. + User = get_user_model() + integrations = ( + Integration.objects.account_provision_options().filter(is_active=True) + ) + user_ids = list( + User.objects.filter(is_active=True) + .exclude(email="") + .order_by("id") + .values_list("id", flat=True) + ) + + enqueued = 0 + for integration in integrations: + # Manual-provisioning integrations don't make HTTP calls; their + # IntegrationUser rows only change via the toggle button. Skip them. + if integration.skip_user_provisioning: + continue + for user_id in user_ids: + async_task( + "admin.integrations.tasks.refresh_access_for_user_integration", + integration.id, + user_id, + task_name=f"Refresh access: {integration.name} #{user_id}", + ) + enqueued += 1 + + logger.info("Access report refresh enqueued: %s tasks", enqueued) + return {"enqueued": enqueued} diff --git a/back/admin/integrations/templates/access_report.html b/back/admin/integrations/templates/access_report.html new file mode 100644 index 000000000..c2fc9f236 --- /dev/null +++ b/back/admin/integrations/templates/access_report.html @@ -0,0 +1,113 @@ +{% extends 'admin_base.html' %} +{% load i18n %} + +{% block content %} +
+
+
+
+
+ +
+
+ +
+
+ +
+
+ +
+
+ {% if request.user.is_admin %} + + {% endif %} + + {% translate "Download CSV" %} + +
+
+ {% if request.user.is_admin %} +
+ {% csrf_token %} +
+ {% endif %} +
+
+ + + + + {% for integration in integrations %} + + {% endfor %} + + + + {% for row in rows %} + + + {% for cell in row.cells %} + {% if cell == "active" %} + + {% elif cell == "revoked" %} + + {% else %} + + {% endif %} + {% endfor %} + + {% empty %} + + + + {% endfor %} + +
{% translate "User" %}{{ integration.name }}
+ + {{ row.user.full_name }} + +
{{ row.user.email }}
+
{% translate "Active" %}{% translate "Not Active" %}
{% translate "No users found" %}
+
+ {% if page_obj.has_other_pages %} + + {% endif %} +
+
+{% endblock %} diff --git a/back/admin/integrations/tests.py b/back/admin/integrations/tests.py index 8cec6df34..6694a3cb3 100644 --- a/back/admin/integrations/tests.py +++ b/back/admin/integrations/tests.py @@ -874,6 +874,35 @@ def test_get_value_from_notation(): with pytest.raises(KeyError): get_value_from_notation("two", test_data) + # filter syntax: top-level field (existing behavior, must keep working) + test_data = { + "data": [ + {"id": "A", "email": "a@x.com"}, + {"id": "B", "email": "b@x.com"}, + ] + } + assert get_value_from_notation("data[email=b@x.com].id", test_data) == "B" + + # filter syntax: nested field via dotted path (new behavior) + test_data = { + "data": [ + {"id": "1", "attributes": {"email": "a@x.com"}}, + {"id": "2", "attributes": {"email": "b@x.com"}}, + ] + } + assert ( + get_value_from_notation("data[attributes.email=b@x.com].id", test_data) == "2" + ) + + # filter syntax: nested miss raises KeyError, same as top-level miss + with pytest.raises(KeyError): + get_value_from_notation("data[attributes.email=nope@x.com].id", test_data) + + # filter syntax: dotted path that doesn't exist on items raises KeyError + test_data = {"data": [{"id": "1", "attributes": {"email": "a@x.com"}}]} + with pytest.raises(KeyError): + get_value_from_notation("data[attributes.missing=a@x.com].id", test_data) + @pytest.mark.django_db @patch( diff --git a/back/admin/integrations/urls.py b/back/admin/integrations/urls.py index b2117cb0c..c75ee0c70 100644 --- a/back/admin/integrations/urls.py +++ b/back/admin/integrations/urls.py @@ -48,6 +48,21 @@ views.IntegrationTrackerDetailView.as_view(), name="tracker", ), + path( + "access-report/", + views.IntegrationAccessReportView.as_view(), + name="access-report", + ), + path( + "access-report/csv/", + views.IntegrationAccessReportCSVView.as_view(), + name="access-report-csv", + ), + path( + "access-report/refresh/", + views.IntegrationAccessReportRefreshView.as_view(), + name="access-report-refresh", + ), path( "builder/", builder_views.IntegrationBuilderCreateView.as_view(), diff --git a/back/admin/integrations/utils.py b/back/admin/integrations/utils.py index 89a661859..53b63df12 100644 --- a/back/admin/integrations/utils.py +++ b/back/admin/integrations/utils.py @@ -45,8 +45,21 @@ def get_value_from_notation(notation, value): raise KeyError field, _, expected = filter_expr.partition("=") + # field may be a dotted path (e.g. attributes.email) for APIs + # that nest values under attributes, like HackerOne / JSON:API. + # Single-segment paths preserve the original top-level behavior. + field_path = field.split(".") for item in value: - if isinstance(item, dict) and str(item.get(field, "")) == expected: + if not isinstance(item, dict): + continue + candidate = item + for part in field_path: + if isinstance(candidate, dict) and part in candidate: + candidate = candidate[part] + else: + candidate = None + break + if candidate is not None and str(candidate) == expected: value = item break else: diff --git a/back/admin/integrations/views.py b/back/admin/integrations/views.py index bfe625fac..00c3d261a 100644 --- a/back/admin/integrations/views.py +++ b/back/admin/integrations/views.py @@ -1,16 +1,21 @@ +import csv import json from datetime import timedelta -from urllib.parse import urlparse +from urllib.parse import urlencode, urlparse import requests +from django.conf import settings from django.contrib import messages +from django.contrib.auth import get_user_model from django.contrib.messages.views import SuccessMessageMixin -from django.http import Http404, HttpResponseRedirect +from django.core.paginator import Paginator +from django.db.models import Q +from django.http import Http404, HttpResponse, HttpResponseRedirect from django.shortcuts import get_object_or_404, redirect from django.urls import reverse_lazy from django.utils import timezone from django.utils.translation import gettext as _ -from django.views.generic import View +from django.views.generic import TemplateView, View from django.views.generic.base import RedirectView from django.views.generic.detail import DetailView from django.views.generic.edit import CreateView, DeleteView, UpdateView @@ -18,6 +23,7 @@ from django_q.tasks import async_task from users.mixins import AdminOrManagerPermMixin, AdminPermMixin +from users.models import IntegrationUser from .forms import IntegrationExtraArgsForm, IntegrationForm from .models import Integration, IntegrationTracker @@ -267,3 +273,152 @@ def post(self, request, pk): "as the lookup runs in the background.") % {"name": integration.name}, ) return redirect("settings:integrations") + + +STATUS_ACTIVE = "active" +STATUS_REVOKED = "revoked" +STATUS_NONE = "none" + +ACCESS_REPORT_PAGE_SIZES = [10, 25, 50, 100] + + +def _resolve_per_page(value, default): + try: + per_page = int(value) + except (TypeError, ValueError): + return default + return per_page if per_page in ACCESS_REPORT_PAGE_SIZES else default + + +def _filtered_users_queryset(role_filter=None, search=None): + User = get_user_model() + users_qs = User.objects.filter(is_active=True).order_by("first_name", "last_name") + if role_filter == "newhire": + users_qs = users_qs.filter(role=User.Role.NEWHIRE) + elif role_filter == "colleague": + users_qs = users_qs.exclude(role=User.Role.NEWHIRE) + if search: + users_qs = users_qs.filter( + Q(first_name__icontains=search) + | Q(last_name__icontains=search) + | Q(email__icontains=search) + ) + return users_qs + + +def _build_access_matrix(role_filter=None, search=None, users=None): + integrations = list( + Integration.objects.account_provision_options() + .filter(is_active=True) + .order_by("name") + ) + integration_ids = [i.id for i in integrations] + + if users is None: + users = list(_filtered_users_queryset(role_filter, search)) + + user_ids = [u.id for u in users] + access_lookup = {} + for iu in IntegrationUser.objects.filter( + integration_id__in=integration_ids, + user_id__in=user_ids, + ).only("user_id", "integration_id", "revoked"): + access_lookup[(iu.user_id, iu.integration_id)] = ( + STATUS_REVOKED if iu.revoked else STATUS_ACTIVE + ) + + rows = [] + for user in users: + cells = [] + for integration in integrations: + cells.append( + access_lookup.get((user.id, integration.id), STATUS_NONE) + ) + rows.append({"user": user, "cells": cells}) + + return integrations, rows + + +class IntegrationAccessReportView(AdminOrManagerPermMixin, TemplateView): + template_name = "access_report.html" + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + role_filter = self.request.GET.get("role", "all") + search = self.request.GET.get("q", "").strip() + + per_page = _resolve_per_page( + self.request.GET.get("per_page"), settings.ACCESS_REPORT_PAGINATE_BY + ) + + users_qs = _filtered_users_queryset(role_filter=role_filter, search=search) + paginator = Paginator(users_qs, per_page) + page_obj = paginator.get_page(self.request.GET.get("page")) + + integrations, rows = _build_access_matrix( + role_filter=role_filter, + search=search, + users=list(page_obj.object_list), + ) + + preserved = {"role": role_filter, "per_page": per_page} + if search: + preserved["q"] = search + context["title"] = _("Integration access report") + context["subtitle"] = _("reports") + context["integrations"] = integrations + context["rows"] = rows + context["role_filter"] = role_filter + context["search"] = search + context["per_page"] = per_page + context["page_size_options"] = ACCESS_REPORT_PAGE_SIZES + context["page_obj"] = page_obj + context["paginator"] = paginator + context["preserved_query"] = urlencode(preserved) + return context + + +class IntegrationAccessReportRefreshView(AdminPermMixin, View): + def post(self, request, *args, **kwargs): + async_task( + "admin.integrations.tasks.refresh_access_report", + task_name="Refresh access report", + ) + messages.success( + request, + _( + "Refreshing access data in the background. The report will update " + "as each integration finishes its lookups." + ), + ) + return redirect("integrations:access-report") + + +class IntegrationAccessReportCSVView(AdminOrManagerPermMixin, View): + def get(self, request, *args, **kwargs): + role_filter = request.GET.get("role", "all") + search = request.GET.get("q", "").strip() + integrations, rows = _build_access_matrix( + role_filter=role_filter, search=search + ) + + response = HttpResponse(content_type="text/csv") + response["Content-Disposition"] = ( + 'attachment; filename="integration-access-report.csv"' + ) + cell_labels = { + STATUS_ACTIVE: "Active", + STATUS_REVOKED: "Not Active", + STATUS_NONE: "", + } + writer = csv.writer(response) + writer.writerow( + ["Name", "Email"] + [i.name for i in integrations] + ) + for row in rows: + user = row["user"] + writer.writerow( + [user.full_name, user.email] + + [cell_labels[c] for c in row["cells"]] + ) + return response diff --git a/back/back/settings.py b/back/back/settings.py index a40c86342..e7924466c 100644 --- a/back/back/settings.py +++ b/back/back/settings.py @@ -506,3 +506,4 @@ RESOURCE_PAGINATE_BY = env.int("RESOURCE_PAGINATE_BY", DEFAULT_PAGINATE_BY) SEQUENCE_PAGINATE_BY = env.int("SEQUENCE_PAGINATE_BY", DEFAULT_PAGINATE_BY) TODO_PAGINATE_BY = env.int("TODO_PAGINATE_BY", DEFAULT_PAGINATE_BY) +ACCESS_REPORT_PAGINATE_BY = env.int("ACCESS_REPORT_PAGINATE_BY", 25) diff --git a/back/users/templates/admin_base.html b/back/users/templates/admin_base.html index 4da338f8c..740b25187 100644 --- a/back/users/templates/admin_base.html +++ b/back/users/templates/admin_base.html @@ -153,6 +153,21 @@ +