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
24 changes: 5 additions & 19 deletions src/routes/dashboard/admin/admin/users/+page.server.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { db } from '$lib/server/db/index.js';
import { project, user, devlog } from '$lib/server/db/schema.js';
import { user, session } from '$lib/server/db/schema.js';
import { error } from '@sveltejs/kit';
import { eq, sql } from 'drizzle-orm';
import type { Actions } from './$types';

export async function load({ locals }) {
Expand All @@ -20,29 +19,16 @@ export async function load({ locals }) {
}

export const actions = {
default: async ({ locals, request }) => {
logoutEveryone: async ({ locals }) => {
if (!locals.user) {
throw error(500);
}
if (!locals.user.hasAdmin) {
throw error(403, { message: 'get out, peasant' });
}

const data = await request.formData();
const statusFilter = data.getAll('status') as (typeof project.status._.data)[];

const userFilter = data.getAll('user').map((userId) => {
const parsedInt = parseInt(userId.toString());
if (!parsedInt) throw error(400, { message: 'malformed user filter' });
return parseInt(userId.toString());
});

return {
// users,
fields: {
status: statusFilter,
user: userFilter
}
};
await db.delete(session);

return {};
}
} satisfies Actions;
32 changes: 24 additions & 8 deletions src/routes/dashboard/admin/admin/users/+page.svelte
Original file line number Diff line number Diff line change
@@ -1,31 +1,47 @@
<script lang="ts">
import { enhance } from '$app/forms';
import Head from '$lib/components/Head.svelte';
import { projectStatuses } from '$lib/utils.js';
import { ExternalLink } from '@lucide/svelte';
import relativeDate from 'tiny-relative-date';

let { data, form } = $props();

let userSearch = $state('');

let users = $derived(data.users); //form?.users ??

let filteredUsers = $derived(
data.users.filter((user) => user.name?.toLowerCase().includes(userSearch.toLowerCase()))
);

let formPending = $state(false);
let logoutEveryonePending = $state(false);
</script>

<Head title="Review" />

<div class="flex h-full flex-col">
<h1 class="mt-5 mb-3 font-hero text-3xl font-medium">Users</h1>
<div class="mt-5 flex flex-row">
<h1 class="mb-3 grow font-hero text-3xl font-medium">Users</h1>
<form
method="POST"
class=""
action="?/logoutEveryone"
use:enhance={() => {
logoutEveryonePending = true;
return async ({ update }) => {
await update({ reset: false });
logoutEveryonePending = false;
};
}}
onsubmit={() => {
return confirm('really really log everyone out?');
}}
>
<button type="submit" class="button md red w-full" disabled={logoutEveryonePending}>
nuke all sessions
</button>
</form>
</div>

<p class="mb-3 text-lg">Showing {filteredUsers.length} users</p>

<input class="themed-box p-2 mb-3 w-full" placeholder="Search users..." bind:value={userSearch} />
<input class="themed-box mb-3 w-full p-2" placeholder="Search users..." bind:value={userSearch} />

{#if filteredUsers.length == 0}
<div class="flex grow items-center justify-center">
Expand Down
88 changes: 87 additions & 1 deletion src/routes/dashboard/admin/admin/users/[id]/+page.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,15 @@ import { user, devlog, session } from '$lib/server/db/schema.js';
import { error, fail, redirect } from '@sveltejs/kit';
import { and, eq, sql } from 'drizzle-orm';
import type { Actions } from './$types';
import { createSession, DAY_IN_MS, generateSessionToken, SESSION_EXPIRY_DAYS, setSessionTokenCookie } from '$lib/server/auth';
import {
createSession,
DAY_IN_MS,
generateSessionToken,
SESSION_EXPIRY_DAYS,
setSessionTokenCookie
} from '$lib/server/auth';
import { decrypt } from '$lib/server/encryption';
import { getUserData } from '$lib/server/idvUserData';

export async function load({ locals, params }) {
if (!locals.user) {
Expand Down Expand Up @@ -233,5 +241,83 @@ export const actions = {
);

return redirect(302, '/dashboard');
},

fetchPII: async (event) => {
const { locals, params } = event;

if (!locals.user) {
throw error(500);
}

// Pretty important line
if (!locals.user.hasAdmin) {
throw error(403, { message: 'get out, peasant' });
}

const id: number = parseInt(params.id);

const [queriedUser] = await db
.select({
idvToken: user.idvToken
})
.from(user)
.where(eq(user.id, id));

if (!queriedUser) {
throw error(404, { message: 'user not found' });
}

if (!queriedUser.idvToken) {
return fail(400, {
fetchPII: {
success: false,
errorMessage: 'IDV token not found, ask them to re-login',
first_name: null,
last_name: null,
primary_email: null,
phone_number: null,
birthday: null,
address: null
}
});
}

const token = decrypt(queriedUser.idvToken);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The decrypt() call for the idvToken is not wrapped in a try-catch block. If decryption fails, it will cause an unhandled exception.
Severity: HIGH | Confidence: High

🔍 Detailed Analysis

The decrypt() function on line 286 is called to decrypt the idvToken. This function is designed to throw an error if decryption fails, for example, due to a key rotation or data corruption. Unlike other parts of the action which handle potential failures gracefully by returning a fail() response, this call is not wrapped in a try-catch block. An unhandled exception here will crash the server action, resulting in a 500 error for the user instead of a clear error message.

💡 Suggested Fix

Wrap the decrypt(queriedUser.idvToken) call in a try-catch block. In the catch block, log the error and return a fail() response with an appropriate error message, similar to how other failures are handled in the same function.

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/routes/dashboard/admin/admin/users/[id]/+page.server.ts#L286

Potential issue: The `decrypt()` function on line 286 is called to decrypt the
`idvToken`. This function is designed to throw an error if decryption fails, for
example, due to a key rotation or data corruption. Unlike other parts of the action
which handle potential failures gracefully by returning a `fail()` response, this call
is not wrapped in a `try-catch` block. An unhandled exception here will crash the server
action, resulting in a 500 error for the user instead of a clear error message.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7737311

let userData;

try {
userData = await getUserData(token);
} catch {
return fail(400, {
fetchPII: {
success: false,
errorMessage: 'IDV token revoked/expired, ask them to re-login',
first_name: null,
last_name: null,
primary_email: null,
phone_number: null,
birthday: null,
address: null
}
});
}

const { first_name, last_name, primary_email, birthday, phone_number, addresses } = userData;

const address = addresses?.find((address: { primary: boolean }) => address.primary);

return {
fetchPII: {
success: true,
errorMessage: '',
first_name,
last_name,
primary_email,
phone_number,
birthday,
address
}
};
}
} satisfies Actions;
92 changes: 82 additions & 10 deletions src/routes/dashboard/admin/admin/users/[id]/+page.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
let privilegesPending = $state(false);
let impersonatePending = $state(false);
let logoutPending = $state(false);
let fetchPIIPending = $state(false);
</script>

<Head title={'User: ' + user.name} />
Expand All @@ -26,7 +27,7 @@
<img
src={user.profilePicture}
alt="user profile"
class="h-45 rounded-xl border-4 border-primary-800 shadow-lg aspect-square"
class="aspect-square h-45 rounded-xl border-4 border-primary-800 shadow-lg"
/>
</div>

Expand Down Expand Up @@ -146,7 +147,7 @@
>
<div class="grid grid-cols-2 gap-3 lg:grid-cols-3">
<label class="flex flex-col gap-1">
<span class="font-medium text-sm">Clay</span>
<span class="text-sm font-medium">Clay</span>
<input
type="number"
name="clay"
Expand All @@ -158,7 +159,7 @@
/>
</label>
<label class="flex flex-col gap-1">
<span class="font-medium text-sm">Brick</span>
<span class="text-sm font-medium">Brick</span>
<input
type="number"
name="brick"
Expand All @@ -170,7 +171,7 @@
/>
</label>
<label class="flex flex-col gap-1">
<span class="font-medium text-sm">Market score</span>
<span class="text-sm font-medium">Market score</span>
<input
type="number"
name="market_score"
Expand Down Expand Up @@ -206,12 +207,7 @@
>
<div class="grid grid-cols-2 lg:grid-cols-4">
<label class="flex flex-row items-center gap-1">
<input
type="checkbox"
name="is_printer"
checked={user.isPrinter}
class="checkbox"
/>
<input type="checkbox" name="is_printer" checked={user.isPrinter} class="checkbox" />
<span class="font-medium">Is printer</span>
</label>
<label class="flex flex-row items-center gap-1">
Expand Down Expand Up @@ -242,6 +238,82 @@
>
</form>
</div>

<h2 class="mt-2 text-2xl font-bold">yummy stuff</h2>
<div class="mb-5">
{#if form?.fetchPII?.success}
<div class="grid grid-cols-1 gap-3 md:grid-cols-2 xl:grid-cols-3 2xl:grid-cols-4">
<DataCard title="Name">
{form.fetchPII.first_name}
</DataCard>
<DataCard title="Surname">
{form.fetchPII.last_name}
</DataCard>
<DataCard title="Email">
{form.fetchPII.primary_email}
</DataCard>
<DataCard title="Phone number">
<code>{form.fetchPII.phone_number}</code>
</DataCard>
<DataCard title="Birthday">
<code>{form.fetchPII.birthday}</code>
</DataCard>
</div>

<h3 class="mt-3 mb-2 text-xl font-bold">address</h3>

{#if form?.fetchPII.address}
<div class="grid grid-cols-1 gap-3 md:grid-cols-2 xl:grid-cols-3 2xl:grid-cols-4">
<DataCard title="Address ID">
<code>{form.fetchPII.address.id}</code>
</DataCard>
<DataCard title="Address first name">
{form.fetchPII.address.first_name}
</DataCard>
<DataCard title="Address last name">
{form.fetchPII.address.last_name}
</DataCard>
<DataCard title="Address line 1">
{form.fetchPII.address.line_1}
</DataCard>
{#if form.fetchPII.address.line_2}
<DataCard title="Address line 2">
{form.fetchPII.address.line_2}
</DataCard>
{/if}
<DataCard title="City">
{form.fetchPII.address.city}
</DataCard>
<DataCard title="State">
{form.fetchPII.address.state}
</DataCard>
<DataCard title="Postcode">
{form.fetchPII.address.postal_code}
</DataCard>
<DataCard title="Country">
{form.fetchPII.address.country}
</DataCard>
</div>
{/if}
{:else}
<form
action="?/fetchPII"
method="POST"
use:enhance={() => {
fetchPIIPending = true;
return async ({ update }) => {
await update();
fetchPIIPending = false;
};
}}
>
<button type="submit" class="button md primary" disabled={fetchPIIPending}
>go fetch</button
>
<p class="mt-1">{form?.fetchPII?.errorMessage}</p>
</form>
{/if}
</div>
</div>
</div>
</div>
9 changes: 7 additions & 2 deletions src/routes/dashboard/admin/ysws-review/[id]/+page.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ export async function load({ locals, params }) {
uploadedFileUrl: project.uploadedFileUrl,
modelFile: project.modelFile,

submittedToAirtable: project.submittedToAirtable,

createdAt: project.createdAt,
updatedAt: project.updatedAt,
status: project.status
Expand Down Expand Up @@ -59,6 +61,7 @@ export async function load({ locals, params }) {
project.editorUrl,
project.uploadedFileUrl,
project.modelFile,
project.submittedToAirtable,
project.createdAt,
project.status,
user.id,
Expand Down Expand Up @@ -132,6 +135,7 @@ export const actions = {
name: user.name,
slackId: user.slackId,
idvId: user.idvId,
idvToken: user.idvToken,
trust: user.trust,
hackatimeTrust: user.hackatimeTrust
},
Expand All @@ -154,6 +158,7 @@ export const actions = {
user.name,
user.slackId,
user.idvId,
user.idvToken,
user.trust,
user.hackatimeTrust
)
Expand Down Expand Up @@ -184,13 +189,13 @@ export const actions = {
.orderBy(desc(devlog.createdAt))
.limit(1);

if (!locals.user.idvToken) {
if (!queriedProject.user?.idvToken) {
return fail(400, {
message: 'IDV token revoked/expired, ask them to reauthenticate'
});
}

const token = decrypt(locals.user.idvToken);
const token = decrypt(queriedProject.user.idvToken);
let userData;

try {
Expand Down
1 change: 1 addition & 0 deletions src/routes/dashboard/admin/ysws-review/[id]/+page.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
.project.timeSpent % 60}min
</p>
<p>Status: {projectStatuses[data.project.project.status]}</p>
<p>Submitted to Airtable: {data.project.project.submittedToAirtable ?? 'null (false)'}</p>
<div class="mt-1">
<ProjectLinks
url={data.project.project.url}
Expand Down
Loading