Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions crates/defguard_common/src/db/models/group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,26 @@ impl Group<Id> {
.await
}

pub async fn locations_with_one_allowed_group<'e, E>(&self, executor: E) -> sqlx::Result<Vec<String>>
where
E: PgExecutor<'e>,
{
query_scalar(
"SELECT wn.name FROM wireguard_network wn \
JOIN wireguard_network_allowed_group target_group \
ON target_group.network_id = wn.id AND target_group.group_id = $1 \
JOIN wireguard_network_allowed_group allowed_group \
ON allowed_group.network_id = wn.id \
WHERE NOT wn.allow_all_groups \
GROUP BY wn.id, wn.name \
HAVING COUNT(allowed_group.group_id) = 1 \
ORDER BY wn.name",
)
.bind(self.id)
.fetch_all(executor)
.await
}

pub async fn find_by_permission<'e, E>(
executor: E,
permission: Permission,
Expand Down
12 changes: 12 additions & 0 deletions crates/defguard_core/src/handlers/group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,18 @@ pub(crate) async fn delete_group(
return Ok(ApiResponse::with_status(StatusCode::BAD_REQUEST));
}
}
let blocked_locations = group
.locations_with_one_allowed_group(&appstate.pool)
.await?;
if !blocked_locations.is_empty() {
let msg = format!(
"Cannot delete group {} because it is the only allowed group in locations: {}",
group.name,
blocked_locations.join(", ")
);
error!("{msg}");
return Ok(ApiResponse::with_status(StatusCode::BAD_REQUEST));
}
group.clone().delete(&appstate.pool).await?;
ldap_delete_group(&group.name, &appstate.pool).await;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -919,7 +919,7 @@ DNS = 10.0.0.2
}

#[sqlx::test]
async fn test_delete_only_allowed_group(_: PgPoolOptions, options: PgConnectOptions) {
async fn test_delete_only_allowed_group_rejected(_: PgPoolOptions, options: PgConnectOptions) {
let pool = setup_pool(options).await;

let (client, client_state) = make_test_client(pool).await;
Expand Down Expand Up @@ -977,14 +977,76 @@ async fn test_delete_only_allowed_group(_: PgPoolOptions, options: PgConnectOpti
.delete(format!("/api/v1/group/{allowed_group_id}"))
.send()
.await;
assert_eq!(response.status(), StatusCode::OK);
assert_eq!(response.status(), StatusCode::BAD_REQUEST);

// network configuration was created only for the admin device
// network configuration remains unchanged
let peers = get_location_allowed_peers(&network, &client_state.pool)
.await
.unwrap();
assert_eq!(peers.len(), 1);
assert_eq!(peers.len(), 2);
assert_eq!(peers[0].pubkey, devices[0].wireguard_pubkey);
assert_eq!(peers[1].pubkey, devices[1].wireguard_pubkey);
}

#[sqlx::test]
async fn test_delete_allowed_group_when_location_keeps_other_groups(
_: PgPoolOptions,
options: PgConnectOptions,
) {
let pool = setup_pool(options).await;

let (client, client_state) = make_test_client(pool).await;
setup_test_users(&client_state.pool).await;

let mut wg_rx = client_state.wireguard_rx;

let auth = Auth::new("admin", "pass123");
let response = &client.post("/api/v1/auth").json(&auth).send().await;
assert_eq!(response.status(), StatusCode::OK);

let response = client
.post("/api/v1/network")
.json(&json!({
"name": "network",
"address": "10.1.1.1/24",
"port": 55555,
"endpoint": "192.168.4.14",
"allowed_ips": "10.1.1.0/24",
"dns": "1.1.1.1",
"mtu": 1420,
"fwmark": 0,
"allow_all_groups": false,
"allowed_groups": ["allowed group", "not allowed group"],
"keepalive_interval": 25,
"peer_disconnect_threshold": 300,
"acl_enabled": false,
"acl_default_allow": false,
"location_mfa_mode": "disabled",
"service_location_mode": "disabled"
}))
.send()
.await;
assert_eq!(response.status(), StatusCode::CREATED);
let network: WireguardNetwork<Id> = response.json().await;
let event = wg_rx.try_recv().unwrap();
assert_matches!(event, GatewayEvent::NetworkCreated(..));

let allowed_group_id = Group::find_by_name(&client_state.pool, "allowed group")
.await
.unwrap()
.unwrap()
.id;
let response = client
.delete(format!("/api/v1/group/{allowed_group_id}"))
.send()
.await;
assert_eq!(response.status(), StatusCode::OK);

let allowed_groups = network
.fetch_allowed_groups(&client_state.pool)
.await
.unwrap();
assert_eq!(allowed_groups, vec!["not allowed group"]);
}

#[sqlx::test]
Expand Down
2 changes: 2 additions & 0 deletions web/messages/en/modal.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
"modal_delete_edge_error": "Failed to delete edge",
"modal_delete_group_title": "Delete group",
"modal_delete_group_body": "Are you sure you want to delete group {name}? This action cannot be undone.",
"modal_delete_group_affected_locations_body": "Group {name} is used in location. Deleting it will also revoke location access for users in this group.",
"modal_delete_group_blocked_body": "Group {name} is the only allowed group in location. Deleting it would leave those locations with an empty Allowed groups list. Update locations manually before deleting this group.",
"modal_delete_group_success": "Group deleted",
"modal_delete_group_error": "Failed to delete group",
"modal_delete_user_device_title": "Delete device",
Expand Down
4 changes: 3 additions & 1 deletion web/src/pages/GroupsPage/GroupsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,22 @@ import { ThemeSpacing } from '../../shared/defguard-ui/types';
import { TablePageLayout } from '../../shared/layout/TablePageLayout/TablePageLayout';
import {
getGroupsInfoQueryOptions,
getLocationsQueryOptions,
getUsersOverviewQueryOptions,
} from '../../shared/query';
import { GroupsTable } from './components/GroupsTable/GroupsTable';
import { CEGroupModal } from './modals/CEGroupModal/CEGroupModal';

export const GroupsPage = () => {
const { data: groups } = useSuspenseQuery(getGroupsInfoQueryOptions);
const { data: locations } = useSuspenseQuery(getLocationsQueryOptions);
const { data: users } = useSuspenseQuery(getUsersOverviewQueryOptions);
return (
<>
<Page id="groups-page" title={m.groups_title()}>
<SizedBox height={ThemeSpacing.Xl3} />
<TablePageLayout>
<GroupsTable groups={groups} users={users} />
<GroupsTable groups={groups} locations={locations} users={users} />
</TablePageLayout>
</Page>
<CEGroupModal />
Expand Down
33 changes: 28 additions & 5 deletions web/src/pages/GroupsPage/components/GroupsTable/GroupsTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
import { useMemo, useState } from 'react';
import { m } from '../../../../paraglide/messages';
import api from '../../../../shared/api/api';
import type { GroupInfo, User } from '../../../../shared/api/types';
import type { GroupInfo, NetworkLocation, User } from '../../../../shared/api/types';
import { Badge } from '../../../../shared/defguard-ui/components/Badge/Badge';
import { Button } from '../../../../shared/defguard-ui/components/Button/Button';
import type { MenuItemProps } from '../../../../shared/defguard-ui/components/Menu/types';
Expand All @@ -23,14 +23,15 @@ import { ModalName } from '../../../../shared/hooks/modalControls/modalTypes';

type Props = {
groups: GroupInfo[];
locations: NetworkLocation[];
users: User[];
};

type RowData = GroupInfo;

const columnHelper = createColumnHelper<RowData>();

export const GroupsTable = ({ groups, users }: Props) => {
export const GroupsTable = ({ groups, locations, users }: Props) => {
const [search, setSearch] = useState('');
const reservedNames = useMemo(() => groups.map((g) => g.name), [groups]);

Expand Down Expand Up @@ -117,11 +118,33 @@ export const GroupsTable = ({ groups, users }: Props) => {
icon: 'delete',
variant: 'danger',
onClick: () => {
const affectedLocations = locations.filter(
(location) =>
!location.allow_all_groups &&
location.allowed_groups.includes(rowData.name),
);
const blockedLocations = affectedLocations.filter(
(location) => location.allowed_groups.length === 1,
);
if (blockedLocations.length > 0) {
openModal(ModalName.ConfirmAction, {
title: m.modal_delete_group_title(),
contentMd: m.modal_delete_group_blocked_body({ name: rowData.name }),
actionPromise: () => api.group.deleteGroup(rowData.id),
cancelProps: { text: m.controls_close() },
});
return;
}
openModal(ModalName.ConfirmAction, {
title: m.modal_delete_group_title(),
contentMd: m.modal_delete_group_body({ name: rowData.name }),
contentMd:
affectedLocations.length > 0
? m.modal_delete_group_affected_locations_body({
name: rowData.name,
})
: m.modal_delete_group_body({ name: rowData.name }),
actionPromise: () => api.group.deleteGroup(rowData.id),
invalidateKeys: [['group'], ['group-info']],
invalidateKeys: [['group'], ['group-info'], ['network']],
submitProps: { text: m.controls_delete(), variant: 'critical' },
onSuccess: () => Snackbar.default(m.modal_delete_group_success()),
onError: () => Snackbar.error(m.modal_delete_group_error()),
Expand All @@ -133,7 +156,7 @@ export const GroupsTable = ({ groups, users }: Props) => {
},
}),
],
[reservedNames, users],
[locations, reservedNames, users],
);

const table = useReactTable({
Expand Down
2 changes: 2 additions & 0 deletions web/src/routes/_authorized/_default/groups.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { createFileRoute } from '@tanstack/react-router';
import { GroupsPage } from '../../../pages/GroupsPage/GroupsPage';
import {
getGroupsInfoQueryOptions,
getLocationsQueryOptions,
getUsersOverviewQueryOptions,
} from '../../../shared/query';

Expand All @@ -10,6 +11,7 @@ export const Route = createFileRoute('/_authorized/_default/groups')({
loader: ({ context }) => {
return Promise.all([
context.queryClient.fetchQuery(getGroupsInfoQueryOptions),
context.queryClient.fetchQuery(getLocationsQueryOptions),
context.queryClient.fetchQuery(getUsersOverviewQueryOptions),
]);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,16 @@ const ModalContent = ({ data }: { data: ModalData }) => {
closeModal(modalNameValue);
}}
/>
<Button
text={m.controls_submit()}
variant="primary"
{...data.submitProps}
loading={isPending}
onClick={() => {
mutate();
}}
/>
{isPresent(data.submitProps) && (
<Button
variant="primary"
{...data.submitProps}
loading={isPending}
onClick={() => {
mutate();
}}
/>
)}
</div>
</Controls>
</>
Expand Down
2 changes: 1 addition & 1 deletion web/src/shared/validators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,6 @@ export const aclDestinationValidator = z
);

// Allows Unicode letters, Unicode digits, space, hyphen, underscore, dot, parentheses, colon, slash
const namePattern = /^[\p{L}\p{N} \-_.():\/]+$/u;
const namePattern = /^[\p{L}\p{N} \-_.():/]+$/u;

export const nameValidator = z.string().regex(namePattern, m.form_error_forbidden_char());