From 2521073dba2c7aa62b14a0859c1e717f3aee2d30 Mon Sep 17 00:00:00 2001 From: Jens L Date: Thu, 21 Dec 2023 14:43:40 +0100 Subject: [PATCH] providers/scim: use lock for sync (#7948) * providers/scim: use lock for sync Signed-off-by: Jens Langhammer * fix Signed-off-by: Jens Langhammer --------- Signed-off-by: Jens Langhammer --- authentik/providers/scim/api/providers.py | 22 ++++++-- authentik/providers/scim/models.py | 12 +++++ authentik/providers/scim/tasks.py | 4 ++ schema.yml | 17 +++++- .../admin-overview/charts/SyncStatusChart.ts | 19 +++---- .../providers/scim/SCIMProviderViewPage.ts | 53 ++++++++++++++----- web/xliff/de.xlf | 3 -- web/xliff/en.xlf | 4 -- web/xliff/es.xlf | 3 -- web/xliff/fr.xlf | 5 -- web/xliff/pl.xlf | 3 -- web/xliff/pseudo-LOCALE.xlf | 5 -- web/xliff/tr.xlf | 3 -- web/xliff/zh-Hans.xlf | 53 +++++++++---------- web/xliff/zh-Hant.xlf | 3 -- web/xliff/zh_TW.xlf | 5 -- 16 files changed, 123 insertions(+), 91 deletions(-) diff --git a/authentik/providers/scim/api/providers.py b/authentik/providers/scim/api/providers.py index 546e62e65..ddca8cfef 100644 --- a/authentik/providers/scim/api/providers.py +++ b/authentik/providers/scim/api/providers.py @@ -2,6 +2,7 @@ from django.utils.text import slugify from drf_spectacular.utils import OpenApiResponse, extend_schema from rest_framework.decorators import action +from rest_framework.fields import BooleanField from rest_framework.request import Request from rest_framework.response import Response from rest_framework.viewsets import ModelViewSet @@ -9,6 +10,7 @@ from rest_framework.viewsets import ModelViewSet from authentik.admin.api.tasks import TaskSerializer from authentik.core.api.providers import ProviderSerializer from authentik.core.api.used_by import UsedByMixin +from authentik.core.api.utils import PassiveSerializer from authentik.events.monitored_tasks import TaskInfo from authentik.providers.scim.models import SCIMProvider @@ -37,6 +39,13 @@ class SCIMProviderSerializer(ProviderSerializer): extra_kwargs = {} +class SCIMSyncStatusSerializer(PassiveSerializer): + """SCIM Provider sync status""" + + is_running = BooleanField(read_only=True) + tasks = TaskSerializer(many=True, read_only=True) + + class SCIMProviderViewSet(UsedByMixin, ModelViewSet): """SCIMProvider Viewset""" @@ -48,15 +57,18 @@ class SCIMProviderViewSet(UsedByMixin, ModelViewSet): @extend_schema( responses={ - 200: TaskSerializer(), + 200: SCIMSyncStatusSerializer(), 404: OpenApiResponse(description="Task not found"), } ) @action(methods=["GET"], detail=True, pagination_class=None, filter_backends=[]) def sync_status(self, request: Request, pk: int) -> Response: """Get provider's sync status""" - provider = self.get_object() + provider: SCIMProvider = self.get_object() task = TaskInfo.by_name(f"scim_sync:{slugify(provider.name)}") - if not task: - return Response(status=404) - return Response(TaskSerializer(task).data) + tasks = [task] if task else [] + status = { + "tasks": tasks, + "is_running": provider.sync_lock.locked(), + } + return Response(SCIMSyncStatusSerializer(status).data) diff --git a/authentik/providers/scim/models.py b/authentik/providers/scim/models.py index ac27288f3..b43a59375 100644 --- a/authentik/providers/scim/models.py +++ b/authentik/providers/scim/models.py @@ -1,11 +1,14 @@ """SCIM Provider models""" +from django.core.cache import cache from django.db import models from django.db.models import QuerySet from django.utils.translation import gettext_lazy as _ from guardian.shortcuts import get_anonymous_user +from redis.lock import Lock from rest_framework.serializers import Serializer from authentik.core.models import BackchannelProvider, Group, PropertyMapping, User, UserTypes +from authentik.providers.scim.clients import PAGE_TIMEOUT class SCIMProvider(BackchannelProvider): @@ -27,6 +30,15 @@ class SCIMProvider(BackchannelProvider): help_text=_("Property mappings used for group creation/updating."), ) + @property + def sync_lock(self) -> Lock: + """Redis lock for syncing SCIM to prevent multiple parallel syncs happening""" + return Lock( + cache.client.get_client(), + name=f"goauthentik.io/providers/scim/sync-{str(self.pk)}", + timeout=(60 * 60 * PAGE_TIMEOUT) * 3, + ) + def get_user_qs(self) -> QuerySet[User]: """Get queryset of all users with consistent ordering according to the provider's settings""" diff --git a/authentik/providers/scim/tasks.py b/authentik/providers/scim/tasks.py index c0c074ea4..7b16b293e 100644 --- a/authentik/providers/scim/tasks.py +++ b/authentik/providers/scim/tasks.py @@ -47,6 +47,10 @@ def scim_sync(self: MonitoredTask, provider_pk: int) -> None: ).first() if not provider: return + lock = provider.sync_lock + if lock.locked(): + LOGGER.debug("SCIM sync locked, skipping task", source=provider.name) + return self.set_uid(slugify(provider.name)) result = TaskResult(TaskResultStatus.SUCCESSFUL, []) result.messages.append(_("Starting full SCIM sync")) diff --git a/schema.yml b/schema.yml index fee6b8d3d..daded5c6d 100644 --- a/schema.yml +++ b/schema.yml @@ -17079,7 +17079,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/Task' + $ref: '#/components/schemas/SCIMSyncStatus' description: '' '404': description: Task not found @@ -40645,6 +40645,21 @@ components: - name - token - url + SCIMSyncStatus: + type: object + description: SCIM Provider sync status + properties: + is_running: + type: boolean + readOnly: true + tasks: + type: array + items: + $ref: '#/components/schemas/Task' + readOnly: true + required: + - is_running + - tasks SMSDevice: type: object description: Serializer for sms authenticator devices diff --git a/web/src/admin/admin-overview/charts/SyncStatusChart.ts b/web/src/admin/admin-overview/charts/SyncStatusChart.ts index 28747d682..d9a3cce6c 100644 --- a/web/src/admin/admin-overview/charts/SyncStatusChart.ts +++ b/web/src/admin/admin-overview/charts/SyncStatusChart.ts @@ -93,15 +93,16 @@ export class LDAPSyncStatusChart extends AKChart { const health = await api.providersScimSyncStatusRetrieve({ id: element.pk, }); - - if (health.status !== TaskStatusEnum.Successful) { - sourceKey = "failed"; - } - const now = new Date().getTime(); - const maxDelta = 3600000; // 1 hour - if (!health || now - health.taskFinishTimestamp.getTime() > maxDelta) { - sourceKey = "unsynced"; - } + health.tasks.forEach((task) => { + if (task.status !== TaskStatusEnum.Successful) { + sourceKey = "failed"; + } + const now = new Date().getTime(); + const maxDelta = 3600000; // 1 hour + if (!health || now - task.taskFinishTimestamp.getTime() > maxDelta) { + sourceKey = "unsynced"; + } + }); } catch { sourceKey = "unsynced"; } diff --git a/web/src/admin/providers/scim/SCIMProviderViewPage.ts b/web/src/admin/providers/scim/SCIMProviderViewPage.ts index 3998c7c81..d745ed55e 100644 --- a/web/src/admin/providers/scim/SCIMProviderViewPage.ts +++ b/web/src/admin/providers/scim/SCIMProviderViewPage.ts @@ -10,7 +10,7 @@ import "@goauthentik/elements/Tabs"; import "@goauthentik/elements/buttons/ActionButton"; import "@goauthentik/elements/buttons/ModalButton"; -import { msg } from "@lit/localize"; +import { msg, str } from "@lit/localize"; import { CSSResult, TemplateResult, html } from "lit"; import { customElement, property, state } from "lit/decorators.js"; @@ -31,7 +31,8 @@ import { ProvidersApi, RbacPermissionsAssignedByUsersListModelEnum, SCIMProvider, - Task, + SCIMSyncStatus, + TaskStatusEnum, } from "@goauthentik/api"; @customElement("ak-provider-scim-view") @@ -54,7 +55,7 @@ export class SCIMProviderViewPage extends AKElement { provider?: SCIMProvider; @state() - syncState?: Task; + syncState?: SCIMSyncStatus; static get styles(): CSSResult[] { return [ @@ -128,6 +129,41 @@ export class SCIMProviderViewPage extends AKElement { `; } + renderSyncStatus(): TemplateResult { + if (!this.syncState) { + return html`${msg("No sync status.")}`; + } + if (this.syncState.isRunning) { + return html`${msg("Sync currently running.")}`; + } + if (this.syncState.tasks.length < 1) { + return html`${msg("Not synced yet.")}`; + } + return html` +
    + ${this.syncState.tasks.map((task) => { + let header = ""; + if (task.status === TaskStatusEnum.Warning) { + header = msg("Task finished with warnings"); + } else if (task.status === TaskStatusEnum.Error) { + header = msg("Task finished with errors"); + } else { + header = msg(str`Last sync: ${task.taskFinishTimestamp.toLocaleString()}`); + } + return html`
  • +

    ${task.taskName}

    +
      +
    • ${header}
    • + ${task.messages.map((m) => { + return html`
    • ${m}
    • `; + })} +
    +
  • `; + })} +
+ `; + } + renderTabOverview(): TemplateResult { if (!this.provider) { return html``; @@ -186,16 +222,7 @@ export class SCIMProviderViewPage extends AKElement {

${msg("Sync status")}

-
- ${this.syncState - ? html`
    - ${this.syncState.messages.map((m) => { - return html`
  • ${m}
  • `; - })} -
` - : html` ${msg("Sync not run yet.")} `} -
- +
${this.renderSyncStatus()}