diff --git a/CHANGELOG.md b/CHANGELOG.md index 10657e5..6e33ba5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Refer to the `integrations.agent` section of the config file for more information and how to enable it. - Requests to `/admin` will now be redirected to `/admin/` to prevent issues with the React Router (works with custom prefixes, closes [#173](https://github.com/tale/headplane/issues/173)). - The Login page has been simplified and separately reports errors versus incorrect API keys (closes [#186](https://github.com/tale/headplane/issues/186)). +- The machine actions backend has been reworked to better handle errors and provide more information to the user (closes [#185](https://github.com/tale/headplane/issues/185)). ### 0.5.10 (April 4, 2025) - Fix an issue where other preferences to skip onboarding affected every user. diff --git a/app/routes/machines/dialogs/delete.tsx b/app/routes/machines/dialogs/delete.tsx index 0ab69ae..9087d06 100644 --- a/app/routes/machines/dialogs/delete.tsx +++ b/app/routes/machines/dialogs/delete.tsx @@ -22,8 +22,8 @@ export default function Delete({ machine, isOpen, setIsOpen }: DeleteProps) { This machine will be permanently removed from your network. To re-add it, you will need to reauthenticate to your tailnet from the device. - - + + ); diff --git a/app/routes/machines/dialogs/expire.tsx b/app/routes/machines/dialogs/expire.tsx index d04e458..12f78b7 100644 --- a/app/routes/machines/dialogs/expire.tsx +++ b/app/routes/machines/dialogs/expire.tsx @@ -16,8 +16,8 @@ export default function Expire({ machine, isOpen, setIsOpen }: ExpireProps) { This will disconnect the machine from your Tailnet. In order to reconnect, you will need to re-authenticate from the device. - - + + ); diff --git a/app/routes/machines/dialogs/move.tsx b/app/routes/machines/dialogs/move.tsx index ab13a9b..7ac3d92 100644 --- a/app/routes/machines/dialogs/move.tsx +++ b/app/routes/machines/dialogs/move.tsx @@ -17,11 +17,11 @@ export default function Move({ machine, users, isOpen, setIsOpen }: MoveProps) { The owner of the machine is the user associated with it. - - + + - + - + + { const form = new FormData(); - form.set('id', machine.id); - form.set('_method', 'routes'); - form.set('route', route.id); + form.set('action_id', 'update_routes'); + form.set('node_id', machine.id); + form.set('routes', [route.id].join(',')); form.set('enabled', String(checked)); fetcher.submit(form, { @@ -115,8 +115,8 @@ export default function Routes({ label="Enabled" onChange={(checked) => { const form = new FormData(); - form.set('id', machine.id); - form.set('_method', 'exit-node'); + form.set('action_id', 'update_routes'); + form.set('node_id', machine.id); form.set('routes', exit.map((route) => route.id).join(',')); form.set('enabled', String(checked)); diff --git a/app/routes/machines/dialogs/tags.tsx b/app/routes/machines/dialogs/tags.tsx index 286630e..8e0b99f 100644 --- a/app/routes/machines/dialogs/tags.tsx +++ b/app/routes/machines/dialogs/tags.tsx @@ -33,8 +33,8 @@ export default function Tags({ machine, isOpen, setIsOpen }: TagsProps) { {' '} for more information. - - + + {tags.length === 0 ? ( diff --git a/app/routes/machines/machine-actions.ts b/app/routes/machines/machine-actions.ts index 728bde8..1387bff 100644 --- a/app/routes/machines/machine-actions.ts +++ b/app/routes/machines/machine-actions.ts @@ -1,11 +1,8 @@ -import type { ActionFunctionArgs } from 'react-router'; +import { type ActionFunctionArgs, data, redirect } from 'react-router'; import type { LoadContext } from '~/server'; import { Capabilities } from '~/server/web/roles'; import { Machine } from '~/types'; -import log from '~/utils/log'; -import { data400, data403, data404, send } from '~/utils/res'; -// TODO: Clean this up like dns-actions and user-actions export async function machineAction({ request, context, @@ -16,14 +13,33 @@ export async function machineAction({ Capabilities.write_machines, ); - const apiKey = session.get('api_key')!; const formData = await request.formData(); + const apiKey = session.get('api_key')!; - // TODO: Rename this to 'action_id' and 'node_id' - const action = formData.get('_method')?.toString(); - const nodeId = formData.get('id')?.toString(); - if (!action || !nodeId) { - return data400('Missing required parameters: _method and id'); + const action = formData.get('action_id')?.toString(); + if (!action) { + throw data('Missing `action_id` in the form data.', { + status: 400, + }); + } + + // Fast track register since it doesn't require an existing machine + if (action === 'register') { + if (!check) { + throw data('You do not have permission to manage machines', { + status: 403, + }); + } + + return registerMachine(formData, apiKey, context); + } + + // Check if the user has permission to manage this machine + const nodeId = formData.get('node_id')?.toString(); + if (!nodeId) { + throw data('Missing `node_id` in the form data.', { + status: 400, + }); } const { nodes } = await context.client.get<{ nodes: Machine[] }>( @@ -33,215 +49,192 @@ export async function machineAction({ const node = nodes.find((node) => node.id === nodeId); if (!node) { - return data404(`Node with ID ${nodeId} not found`); + throw data(`Machine with ID ${nodeId} not found`, { + status: 404, + }); } - const subject = session.get('user')!.subject; - if (node.user.providerId?.split('/').pop() !== subject) { - if (!check) { - return data403('You do not have permission to act on this machine'); - } + if ( + node.user.providerId?.split('/').pop() !== session.get('user')!.subject && + !check + ) { + throw data('You do not have permission to act on this machine', { + status: 403, + }); } - // TODO: Split up into methods switch (action) { + case 'rename': { + return renameMachine(formData, apiKey, nodeId, context); + } + case 'delete': { - await context.client.delete(`v1/node/${nodeId}`, session.get('api_key')!); - return { message: 'Machine removed' }; + return deleteMachine(apiKey, nodeId, context); } case 'expire': { - await context.client.post( - `v1/node/${nodeId}/expire`, - session.get('api_key')!, - ); - return { message: 'Machine expired' }; + return expireMachine(apiKey, nodeId, context); } - case 'rename': { - if (!formData.has('name')) { - return send( - { message: 'No name provided' }, - { - status: 400, - }, - ); - } - - const name = String(formData.get('name')); - await context.client.post( - `v1/node/${nodeId}/rename/${name}`, - session.get('api_key')!, - ); - return { message: 'Machine renamed' }; + case 'update_tags': { + return updateTags(formData, apiKey, nodeId, context); } - case 'routes': { - if (!formData.has('route') || !formData.has('enabled')) { - return send( - { message: 'No route or enabled provided' }, - { - status: 400, - }, - ); - } - - const route = String(formData.get('route')); - const enabled = formData.get('enabled') === 'true'; - const postfix = enabled ? 'enable' : 'disable'; - - await context.client.post( - `v1/routes/${route}/${postfix}`, - session.get('api_key')!, - ); - return { message: 'Route updated' }; + case 'update_routes': { + return updateRoutes(formData, apiKey, nodeId, context); } - case 'exit-node': { - if (!formData.has('routes') || !formData.has('enabled')) { - return send( - { message: 'No route or enabled provided' }, - { - status: 400, - }, - ); - } - - const routes = formData.get('routes')?.toString().split(',') ?? []; - const enabled = formData.get('enabled') === 'true'; - const postfix = enabled ? 'enable' : 'disable'; - - await Promise.all( - routes.map(async (route) => { - await context.client.post( - `v1/routes/${route}/${postfix}`, - session.get('api_key')!, - ); - }), - ); - - return { message: 'Exit node updated' }; + case 'reassign': { + return reassignMachine(formData, apiKey, nodeId, context); } - case 'move': { - if (!formData.has('to')) { - return send( - { message: 'No destination provided' }, - { - status: 400, - }, - ); - } - - const to = String(formData.get('to')); - - try { - await context.client.post( - `v1/node/${nodeId}/user`, - session.get('api_key')!, - { - user: to, - }, - ); - - return { message: `Moved node ${nodeId} to ${to}` }; - } catch (error) { - console.error(error); - return send( - { message: `Failed to move node ${nodeId} to ${to}` }, - { - status: 500, - }, - ); - } - } - - case 'tags': { - const tags = - formData - .get('tags') - ?.toString() - .split(',') - .filter((tag) => tag.trim() !== '') ?? []; - - try { - await context.client.post( - `v1/node/${nodeId}/tags`, - session.get('api_key')!, - { - tags, - }, - ); - - return { message: 'Tags updated' }; - } catch (error) { - log.debug('api', 'Failed to update tags: %s', error); - return send( - { message: 'Failed to update tags' }, - { - status: 500, - }, - ); - } - } - - case 'register': { - const key = formData.get('mkey')?.toString(); - const user = formData.get('user')?.toString(); - - if (!key) { - return send( - { message: 'No machine key provided' }, - { - status: 400, - }, - ); - } - - if (!user) { - return send( - { message: 'No user provided' }, - { - status: 400, - }, - ); - } - - try { - const qp = new URLSearchParams(); - qp.append('user', user); - qp.append('key', key); - - const url = `v1/node/register?${qp.toString()}`; - await context.client.post(url, session.get('api_key')!, { - user, - key, - }); - - return { - success: true, - message: 'Machine registered', - }; - } catch { - return send( - { - success: false, - message: 'Failed to register machine', - }, - { - status: 500, - }, - ); - } - } - - default: { - return send( - { message: 'Invalid method' }, - { - status: 400, - }, - ); - } + default: + throw data('Invalid action', { + status: 400, + }); } } + +async function registerMachine( + formData: FormData, + apiKey: string, + context: LoadContext, +) { + const registrationKey = formData.get('register_key')?.toString(); + if (!registrationKey) { + throw data('Missing `register_key` in the form data.', { + status: 400, + }); + } + + const user = formData.get('user')?.toString(); + if (!user) { + throw data('Missing `user` in the form data.', { + status: 400, + }); + } + + const qp = new URLSearchParams(); + qp.append('user', user); + qp.append('key', registrationKey); + const url = `v1/node/register?${qp.toString()}`; + const { node } = await context.client.post<{ node: Machine }>(url, apiKey, { + user, + key: registrationKey, + }); + + return redirect(`/machines/${node.id}`); +} + +async function renameMachine( + formData: FormData, + apiKey: string, + nodeId: string, + context: LoadContext, +) { + const newName = formData.get('name')?.toString(); + if (!newName) { + throw data('Missing `name` in the form data.', { + status: 400, + }); + } + + const name = String(formData.get('name')); + await context.client.post(`v1/node/${nodeId}/rename/${name}`, apiKey); + return { message: 'Machine renamed' }; +} + +async function deleteMachine( + apiKey: string, + nodeId: string, + context: LoadContext, +) { + await context.client.delete(`v1/node/${nodeId}`, apiKey); + return redirect('/machines'); +} + +async function expireMachine( + apiKey: string, + nodeId: string, + context: LoadContext, +) { + await context.client.post(`v1/node/${nodeId}/expire`, apiKey); + return { message: 'Machine expired' }; +} + +async function updateTags( + formData: FormData, + apiKey: string, + nodeId: string, + context: LoadContext, +) { + const tags = formData.get('tags')?.toString().split(',') ?? []; + if (tags.length === 0) { + throw data('Missing `tags` in the form data.', { + status: 400, + }); + } + + await context.client.post(`v1/node/${nodeId}/tags`, apiKey, { + tags: tags.map((tag) => tag.trim()).filter((tag) => tag !== ''), + }); + + return { message: 'Tags updated' }; +} + +async function updateRoutes( + formData: FormData, + apiKey: string, + nodeId: string, + context: LoadContext, +) { + const routes = formData.get('routes')?.toString(); + if (!routes) { + throw data('Missing `routes` in the form data.', { + status: 400, + }); + } + + const allRoutes = routes.split(',').map((route) => route.trim()); + if (allRoutes.length === 0) { + throw data('No routes provided to update', { + status: 400, + }); + } + + const enabled = formData.get('enabled')?.toString(); + if (enabled === undefined) { + throw data('Missing `enabled` in the form data.', { + status: 400, + }); + } + + const postfix = enabled === 'true' ? 'enable' : 'disable'; + await Promise.all( + allRoutes.map(async (route) => { + await context.client.post(`v1/routes/${route}/${postfix}`, apiKey); + }), + ); + + return { message: 'Routes updated' }; +} + +async function reassignMachine( + formData: FormData, + apiKey: string, + nodeId: string, + context: LoadContext, +) { + const user = formData.get('user')?.toString(); + if (!user) { + throw data('Missing `user` in the form data.', { + status: 400, + }); + } + + await context.client.post(`v1/node/${nodeId}/user`, apiKey, { + user, + }); + + return { message: 'Machine reassigned' }; +}