From bcd05c5bb95febce5371cf5a55e5313bc5a4c93d Mon Sep 17 00:00:00 2001 From: monsieurswag Date: Tue, 2 Apr 2024 21:47:30 +0200 Subject: [PATCH 1/4] Return an error message when an admin try to delete the only admin account of the application --- backend/core/views.py | 8 ++++++++ .../src/routes/(app)/[model=urlmodel]/+page.server.ts | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/backend/core/views.py b/backend/core/views.py index ead262ee9..44d822d55 100644 --- a/backend/core/views.py +++ b/backend/core/views.py @@ -835,6 +835,14 @@ def get_queryset(self): # TODO: Implement a proper filter for the queryset return User.objects.all() + def destroy(self, request, *args, **kwargs): + user = self.get_object() + if user.user_groups.filter(name="BI-UG-ADM").exists() : + number_of_admin_users = User.objects.filter(user_groups__name="BI-UG-ADM").distinct().count() + if number_of_admin_users == 1 : + return Response({"error":"You can't delete the only admin account of your application."},status=HTTP_403_FORBIDDEN) + + return super().destroy(request,*args,**kwargs) class UserGroupViewSet(BaseModelViewSet): """ diff --git a/frontend/src/routes/(app)/[model=urlmodel]/+page.server.ts b/frontend/src/routes/(app)/[model=urlmodel]/+page.server.ts index 0b1147e9f..5d5c227ab 100644 --- a/frontend/src/routes/(app)/[model=urlmodel]/+page.server.ts +++ b/frontend/src/routes/(app)/[model=urlmodel]/+page.server.ts @@ -155,6 +155,10 @@ export const actions: Actions = { if (!res.ok) { const response = await res.json(); console.log(response); + if (response.error) { + setFlash({ type: 'error', message: response.error }, event); + return fail(403, { form: deleteForm }); + } if (response.non_field_errors) { setError(deleteForm, 'non_field_errors', response.non_field_errors); } From 27034e5737b577981a1c49c8ab6ee94b89926bb7 Mon Sep 17 00:00:00 2001 From: monsieurswag Date: Tue, 2 Apr 2024 22:34:47 +0200 Subject: [PATCH 2/4] Return an error message when an admin try to remove the admin user group from the only admin account of the application --- backend/core/views.py | 18 +++++++++++++++--- backend/iam/models.py | 12 ++++++++++-- .../[id=uuid]/edit/+page.server.ts | 4 ++++ .../(app)/users/[id=uuid]/edit/+page.server.ts | 7 +++++-- 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/backend/core/views.py b/backend/core/views.py index 44d822d55..0e781ea0b 100644 --- a/backend/core/views.py +++ b/backend/core/views.py @@ -57,7 +57,7 @@ from django.db import models -from iam.models import User, RoleAssignment, Folder +from iam.models import User, UserGroup, RoleAssignment, Folder User = get_user_model() @@ -835,10 +835,22 @@ def get_queryset(self): # TODO: Implement a proper filter for the queryset return User.objects.all() + def update(self, request: Request, *args, **kwargs) -> Response: + user = self.get_object() + if user.is_admin() : + number_of_admin_users = User.get_admin_users().count() + admin_group = UserGroup.get_admin_group() + if number_of_admin_users == 1 : + new_user_groups = set(request.data["user_groups"]) + if str(admin_group.pk) not in new_user_groups : + return Response({"error":"You can't remove the admin user group from the only admin user of the application."},status=HTTP_403_FORBIDDEN) + + return super().update(request, *args, **kwargs) + def destroy(self, request, *args, **kwargs): user = self.get_object() - if user.user_groups.filter(name="BI-UG-ADM").exists() : - number_of_admin_users = User.objects.filter(user_groups__name="BI-UG-ADM").distinct().count() + if user.is_admin() : + number_of_admin_users = User.get_admin_users().count() if number_of_admin_users == 1 : return Response({"error":"You can't delete the only admin account of your application."},status=HTTP_403_FORBIDDEN) diff --git a/backend/iam/models.py b/backend/iam/models.py index a2d79613f..f8a76015b 100644 --- a/backend/iam/models.py +++ b/backend/iam/models.py @@ -2,7 +2,7 @@ Inspired from Azure IAM model """ from collections import defaultdict -from typing import Any, List, Self, Tuple +from typing import Any, List, Self, Tuple, Self import uuid from django.utils import timezone from django.db import models @@ -236,6 +236,9 @@ def get_user_groups(user): user_group_list.append(user_group) return user_group_list + @staticmethod + def get_admin_group(): + return UserGroup.objects.get(name="BI-UG-ADM",builtin=True) class UserManager(BaseUserManager): use_in_migrations = True @@ -291,7 +294,6 @@ def create_superuser(self, email, password=None, **extra_fields): UserGroup.objects.get(name="BI-UG-ADM").user_set.add(superuser) return superuser - class User(AbstractBaseUser, AbstractBaseModel, FolderMixin): """a user is a principal corresponding to a human""" @@ -471,6 +473,12 @@ def permissions(self): def set_username(self, username): self.email = username + @staticmethod + def get_admin_users() -> List[Self] : + return User.objects.filter(user_groups__name="BI-UG-ADM",user_groups__builtin=True) + + def is_admin(self) -> bool : + return self.user_groups.filter(name="BI-UG-ADM",builtin=True).exists() class Role(NameDescriptionMixin, FolderMixin): """A role is a list of permissions""" diff --git a/frontend/src/routes/(app)/[model=urlmodel]/[id=uuid]/edit/+page.server.ts b/frontend/src/routes/(app)/[model=urlmodel]/[id=uuid]/edit/+page.server.ts index b0f517801..85d5795ec 100644 --- a/frontend/src/routes/(app)/[model=urlmodel]/[id=uuid]/edit/+page.server.ts +++ b/frontend/src/routes/(app)/[model=urlmodel]/[id=uuid]/edit/+page.server.ts @@ -35,6 +35,10 @@ export const actions: Actions = { if (!res.ok) { const response = await res.json(); console.error('server response:', response); + if (response.error) { + setFlash({ type: 'error', message: response.error }, event); + return fail(403, { form: form }); + } if (response.non_field_errors) { setError(form, 'non_field_errors', response.non_field_errors); } diff --git a/frontend/src/routes/(app)/users/[id=uuid]/edit/+page.server.ts b/frontend/src/routes/(app)/users/[id=uuid]/edit/+page.server.ts index 9a2c99dca..efd879bc2 100644 --- a/frontend/src/routes/(app)/users/[id=uuid]/edit/+page.server.ts +++ b/frontend/src/routes/(app)/users/[id=uuid]/edit/+page.server.ts @@ -2,8 +2,7 @@ import { BASE_API_URL } from '$lib/utils/constants'; import { UserEditSchema } from '$lib/utils/schemas'; import { setError, superValidate } from 'sveltekit-superforms/server'; import type { PageServerLoad } from './$types'; -import { redirect, type Actions } from '@sveltejs/kit'; -import { fail } from 'assert'; +import { redirect, fail, type Actions } from '@sveltejs/kit'; import { getModelInfo } from '$lib/utils/crud'; import { setFlash } from 'sveltekit-flash-message/server'; import * as m from '$paraglide/messages'; @@ -58,6 +57,10 @@ export const actions: Actions = { if (!res.ok) { const response = await res.json(); console.error('server response:', response); + if (response.error) { + setFlash({ type: 'error', message: response.error }, event); + return fail(403, { form: form }); + } if (response.non_field_errors) { setError(form, 'non_field_errors', response.non_field_errors); } From 8051e5230cb7a0b9741c9635759f825fcdb2d3ee Mon Sep 17 00:00:00 2001 From: monsieurswag Date: Wed, 3 Apr 2024 11:32:41 +0200 Subject: [PATCH 3/4] Add translations to single-admin account deletion attempt error messages --- backend/core/views.py | 4 ++-- frontend/messages/en.json | 4 +++- frontend/messages/fr.json | 4 +++- frontend/src/lib/utils/locales.ts | 4 +++- frontend/src/routes/(app)/[model=urlmodel]/+page.server.ts | 2 +- .../src/routes/(app)/users/[id=uuid]/edit/+page.server.ts | 4 +++- 6 files changed, 15 insertions(+), 7 deletions(-) diff --git a/backend/core/views.py b/backend/core/views.py index 0e781ea0b..2fcffb3fa 100644 --- a/backend/core/views.py +++ b/backend/core/views.py @@ -843,7 +843,7 @@ def update(self, request: Request, *args, **kwargs) -> Response: if number_of_admin_users == 1 : new_user_groups = set(request.data["user_groups"]) if str(admin_group.pk) not in new_user_groups : - return Response({"error":"You can't remove the admin user group from the only admin user of the application."},status=HTTP_403_FORBIDDEN) + return Response({"error":"attemptToRemoveOnlyAdminUserGroup"},status=HTTP_403_FORBIDDEN) return super().update(request, *args, **kwargs) @@ -852,7 +852,7 @@ def destroy(self, request, *args, **kwargs): if user.is_admin() : number_of_admin_users = User.get_admin_users().count() if number_of_admin_users == 1 : - return Response({"error":"You can't delete the only admin account of your application."},status=HTTP_403_FORBIDDEN) + return Response({"error":"attemptToDeleteOnlyAdminAccountError"},status=HTTP_403_FORBIDDEN) return super().destroy(request,*args,**kwargs) diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 9ee365bf1..8e1cb7b6b 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -491,5 +491,7 @@ "libraryAlreadyImportedError": "This library has already been imported.", "invalidLibraryFileError": "Invalid library file. Please make sure the format is correct.", "taintedFormMessage": "Do you want to leave this page? Changes you made may not be saved.", - "riskScenariosStatus": "Risk scenarios status" + "riskScenariosStatus": "Risk scenarios status", + "attemptToDeleteOnlyAdminAccountError": "You can't delete the only admin account of your application.", + "attemptToRemoveOnlyAdminUserGroup": "You can't remove the only admin user of the application from the admin user group." } diff --git a/frontend/messages/fr.json b/frontend/messages/fr.json index 030a726db..775e010a0 100644 --- a/frontend/messages/fr.json +++ b/frontend/messages/fr.json @@ -491,5 +491,7 @@ "libraryAlreadyImportedError": "Cette libairie a été déjà été importée.", "invalidLibraryFileError": "Fichier de bibliothèque invalide. Veuillez vérifier le format du fichier.", "taintedFormMessage": "Voulez-vous vraiment quitter cette page ? Toutes les données non enregistrées seront perdues.", - "riskScenariosStatus": "Statut des scénarios de risque" + "riskScenariosStatus": "Statut des scénarios de risque", + "attemptToDeleteOnlyAdminAccountError": "Vous ne pouvez pas supprimer votre unique compte administrateur de l'application.", + "attemptToRemoveOnlyAdminUserGroup": "Vous ne pouvez pas retirer le seul compte administrateur de l'application du groupe des administrateurs." } diff --git a/frontend/src/lib/utils/locales.ts b/frontend/src/lib/utils/locales.ts index 44e8e03c5..86724080d 100644 --- a/frontend/src/lib/utils/locales.ts +++ b/frontend/src/lib/utils/locales.ts @@ -309,7 +309,9 @@ export function localItems(languageTag: string): LocalItems { highSOK: m.highSOK({ languageTag: languageTag }), libraryImportError: m.libraryImportError({ languageTag: languageTag }), libraryAlreadyExistsError: m.libraryAlreadyImportedError({ languageTag: languageTag }), - invalidLibraryFileError: m.invalidLibraryFileError({ languageTag: languageTag }) + invalidLibraryFileError: m.invalidLibraryFileError({ languageTag: languageTag }), + attemptToDeleteOnlyAdminAccountError: m.attemptToDeleteOnlyAdminAccountError({ languageTag: languageTag }), + attemptToRemoveOnlyAdminUserGroup: m.attemptToRemoveOnlyAdminUserGroup({ languageTag: languageTag }) }; return LOCAL_ITEMS; } diff --git a/frontend/src/routes/(app)/[model=urlmodel]/+page.server.ts b/frontend/src/routes/(app)/[model=urlmodel]/+page.server.ts index 5d5c227ab..e3d947766 100644 --- a/frontend/src/routes/(app)/[model=urlmodel]/+page.server.ts +++ b/frontend/src/routes/(app)/[model=urlmodel]/+page.server.ts @@ -156,7 +156,7 @@ export const actions: Actions = { const response = await res.json(); console.log(response); if (response.error) { - setFlash({ type: 'error', message: response.error }, event); + setFlash({ type: 'error', message: localItems(languageTag())[response.error] }, event); return fail(403, { form: deleteForm }); } if (response.non_field_errors) { diff --git a/frontend/src/routes/(app)/users/[id=uuid]/edit/+page.server.ts b/frontend/src/routes/(app)/users/[id=uuid]/edit/+page.server.ts index efd879bc2..3578753eb 100644 --- a/frontend/src/routes/(app)/users/[id=uuid]/edit/+page.server.ts +++ b/frontend/src/routes/(app)/users/[id=uuid]/edit/+page.server.ts @@ -6,6 +6,8 @@ import { redirect, fail, type Actions } from '@sveltejs/kit'; import { getModelInfo } from '$lib/utils/crud'; import { setFlash } from 'sveltekit-flash-message/server'; import * as m from '$paraglide/messages'; +import { languageTag } from '$paraglide/runtime'; +import { localItems } from '$lib/utils/locales'; export const load: PageServerLoad = async ({ params, fetch }) => { const URLModel = 'users'; @@ -58,7 +60,7 @@ export const actions: Actions = { const response = await res.json(); console.error('server response:', response); if (response.error) { - setFlash({ type: 'error', message: response.error }, event); + setFlash({ type: 'error', message: localItems(languageTag())[response.error] }, event); return fail(403, { form: form }); } if (response.non_field_errors) { From 3a0e05ee4adec7e6a2aa8a09540bfd95ff06db7d Mon Sep 17 00:00:00 2001 From: monsieurswag Date: Fri, 5 Apr 2024 08:18:16 +0200 Subject: [PATCH 4/4] Remove some bad useless code --- backend/core/views.py | 2 +- backend/iam/models.py | 10 +++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/backend/core/views.py b/backend/core/views.py index 2fcffb3fa..b53230040 100644 --- a/backend/core/views.py +++ b/backend/core/views.py @@ -839,7 +839,7 @@ def update(self, request: Request, *args, **kwargs) -> Response: user = self.get_object() if user.is_admin() : number_of_admin_users = User.get_admin_users().count() - admin_group = UserGroup.get_admin_group() + admin_group = UserGroup.objects.get(name="BI-UG-ADM") if number_of_admin_users == 1 : new_user_groups = set(request.data["user_groups"]) if str(admin_group.pk) not in new_user_groups : diff --git a/backend/iam/models.py b/backend/iam/models.py index f8a76015b..5cf7c5b0d 100644 --- a/backend/iam/models.py +++ b/backend/iam/models.py @@ -2,7 +2,7 @@ Inspired from Azure IAM model """ from collections import defaultdict -from typing import Any, List, Self, Tuple, Self +from typing import Any, List, Self, Tuple import uuid from django.utils import timezone from django.db import models @@ -236,10 +236,6 @@ def get_user_groups(user): user_group_list.append(user_group) return user_group_list - @staticmethod - def get_admin_group(): - return UserGroup.objects.get(name="BI-UG-ADM",builtin=True) - class UserManager(BaseUserManager): use_in_migrations = True @@ -475,10 +471,10 @@ def set_username(self, username): @staticmethod def get_admin_users() -> List[Self] : - return User.objects.filter(user_groups__name="BI-UG-ADM",user_groups__builtin=True) + return User.objects.filter(user_groups__name="BI-UG-ADM") def is_admin(self) -> bool : - return self.user_groups.filter(name="BI-UG-ADM",builtin=True).exists() + return self.user_groups.filter(name="BI-UG-ADM").exists() class Role(NameDescriptionMixin, FolderMixin): """A role is a list of permissions"""