Cleanups

src/lib/constants.ts

@@ -1 +1,3 @@
 export const allowedImages = ['image/png', 'image/jpg', 'image/jpeg'];
+export const OAUTH2_MAX_REDIRECTS = 5;
+export const OAUTH2_MAX_URLS = 1;

src/lib/i18n/en/admin.json

@@ -118,6 +118,7 @@
 			"invalidTitle": "Name must be between 3 and 32 characters long.",
 			"invalidDescription": "Description must be at most 1000 characters long.",
 			"deleteActivated": "Cannot delete an active application. Please deactivate it first.",
+			"illegalUrl": "You cannot add another URL of this type.",
 			"invalidUrlId": "Invalid URL ID for deletion.",
 			"invalidUrlType": "Invalid URL type provided.",
 			"invalidUrl": "Invalid URL provided.",

src/lib/server/api-utils.ts

@@ -14,7 +14,7 @@ export class ApiUtils {
 				const jsonBody = await request.json();
 				return jsonBody;
 			} catch {
-				// Try next...
+				return {};
 			}
 		}
 
@@ -22,9 +22,7 @@ export class ApiUtils {
 			const formBody = await request.formData();
 			return Object.fromEntries(formBody);
 		} catch (err) {
-			// Skip...
+			return {};
 		}
-
-		return {};
 	}
 }

src/lib/server/email/templates/forgot-password.email.ts

@@ -11,8 +11,7 @@ In order to change your password, please click on the following link.
 
 Change your password: ${url}
 
-If you did not request a password change on ${PUBLIC_SITE_NAME}, you can safely ignore this email.
+If you did not request a password change on ${PUBLIC_SITE_NAME}, you can safely ignore this email.`,
-  `,
 	html: /* html */ `
 <h1>${PUBLIC_SITE_NAME}</h1>
 
@@ -22,6 +21,5 @@ If you did not request a password change on ${PUBLIC_SITE_NAME}, you can safely
 
 <p>Change your password: <a href="${url}" target="_blank">${url}</a></p>
 
-<p>If you did not request a password change on ${PUBLIC_SITE_NAME}, you can safely ignore this email.</p>
+<p>If you did not request a password change on ${PUBLIC_SITE_NAME}, you can safely ignore this email.</p>`
-  `
 });

src/lib/server/email/templates/invitation.email.ts

@@ -9,8 +9,7 @@ Please click on the following link to create an account on ${PUBLIC_SITE_NAME}.
 
 Create your account here: ${url}
 
-This email was sent to you because you have requested an account on ${PUBLIC_SITE_NAME}. If you did not request this, you may safely ignore this email.
+This email was sent to you because you have requested an account on ${PUBLIC_SITE_NAME}. If you did not request this, you may safely ignore this email.`,
-  `,
 	html: /* html */ `
 <h1>${PUBLIC_SITE_NAME}</h1>
 
@@ -18,6 +17,5 @@ This email was sent to you because you have requested an account on ${PUBLIC_SIT
 
 <p>Create your account here: <a href="${url}" target="_blank">${url}</a></p>
 
-<p>This email was sent to you because you have requested an account on ${PUBLIC_SITE_NAME}. If you did not request this, you may safely ignore this email.</p>
+<p>This email was sent to you because you have requested an account on ${PUBLIC_SITE_NAME}. If you did not request this, you may safely ignore this email.</p>`
-  `
 });

src/lib/server/email/templates/oauth2-invitation.email.ts

@@ -15,8 +15,7 @@ Please use the following link to accept the invitation.
 
 Accept invitation: ${url}
 
-This email was sent to you because someone invited you to contribute to an application on ${PUBLIC_SITE_NAME}. If you believe that this was sent in error, you may safely ignore this email.
+This email was sent to you because someone invited you to contribute to an application on ${PUBLIC_SITE_NAME}. If you believe that this was sent in error, you may safely ignore this email.`,
-  `,
 	html: /* html */ `
 <h1>${PUBLIC_SITE_NAME}</h1>
 
@@ -26,6 +25,5 @@ This email was sent to you because someone invited you to contribute to an appli
 
 <p>Accept invitation: <a href="${url}" target="_blank">${url}</a></p>
 
-<p>This email was sent to you because someone invited you to contribute to an application on ${PUBLIC_SITE_NAME}. If you believe that this was sent in error, you may safely ignore this email.</p>
+<p>This email was sent to you because someone invited you to contribute to an application on ${PUBLIC_SITE_NAME}. If you believe that this was sent in error, you may safely ignore this email.</p>`
-  `
 });

src/lib/server/email/templates/registration.email.ts

@@ -11,8 +11,7 @@ In order to proceed with logging in, please click on the following link to activ
 
 Activate your account: ${url}
 
-This email was sent to you because you have created an account on ${PUBLIC_SITE_NAME}. If you did not create an account, you may contact us or just let the account expire.
+This email was sent to you because you have created an account on ${PUBLIC_SITE_NAME}. If you did not create an account, you may contact us or just let the account expire.`,
-  `,
 	html: /* html */ `
 <h1>${PUBLIC_SITE_NAME}</h1>
 
@@ -22,6 +21,5 @@ This email was sent to you because you have created an account on ${PUBLIC_SITE_
 
 <p>Activate your account: <a href="${url}" target="_blank">${url}</a></p>
 
-<p>This email was sent to you because you have created an account on ${PUBLIC_SITE_NAME}. If you did not create an account, you may contact us or just let the account expire.</p>
+<p>This email was sent to you because you have created an account on ${PUBLIC_SITE_NAME}. If you did not create an account, you may contact us or just let the account expire.</p>`
-  `
 });

src/lib/server/oauth2/controller/authorization.ts

@@ -20,7 +20,7 @@ import { OAuth2Users } from '../model/user';
 import { OAuth2Response } from '../response';
 
 export class OAuth2AuthorizationController {
-	static prehandle = async (url: URL, locals: App.Locals) => {
+	private static prehandle = async (url: URL, locals: App.Locals) => {
 		if (!url.searchParams.has('redirect_uri')) {
 			throw new InvalidRequest('redirect_uri field is mandatory for authorization endpoint');
 		}
@@ -52,8 +52,8 @@ export class OAuth2AuthorizationController {
 		// Support multiple types
 		const responseTypes = responseType.split(' ');
 		let grantTypes: string[] = [];
-		for (const i in responseTypes) {
+		for (const responseType of responseTypes) {
-			switch (responseTypes[i]) {
+			switch (responseType) {
 				case 'code':
 					grantTypes.push('authorization_code');
 					break;
@@ -61,10 +61,8 @@ export class OAuth2AuthorizationController {
 					grantTypes.push('implicit');
 					break;
 				case 'id_token':
-					grantTypes.push('id_token');
-					break;
 				case 'none':
-					grantTypes.push(responseTypes[i]);
+					grantTypes.push(responseType);
 					break;
 				default:
 					throw new UnsupportedResponseType('Unknown response_type parameter passed');
@@ -95,7 +93,7 @@ export class OAuth2AuthorizationController {
 
 		// The client needs to support all grant types
 		for (const grantType of grantTypes) {
-			if (!OAuth2Clients.checkGrantType(client, grantType) && grantType !== 'none') {
+			if (grantType !== 'none' && !OAuth2Clients.checkGrantType(client, grantType)) {
 				throw new UnauthorizedClient('This client does not support grant type ' + grantType);
 			}
 		}
@@ -127,7 +125,7 @@ export class OAuth2AuthorizationController {
 		};
 	};
 
-	static posthandle = async (
+	private static posthandle = async (
 		url: URL,
 		{
 			client,
@@ -141,9 +139,9 @@ export class OAuth2AuthorizationController {
 		}: Awaited<ReturnType<typeof OAuth2AuthorizationController.prehandle>>
 	) => {
 		let resObj: Record<string, string | number> = {};
-		for (const i in grantTypes) {
+		for (const grantType of grantTypes) {
 			let data = null;
-			switch (grantTypes[i]) {
+			switch (grantType) {
 				case 'authorization_code':
 					data = await OAuth2Codes.create(
 						user.id,

src/lib/server/oauth2/model/client.ts

@@ -215,6 +215,11 @@ export class OAuth2Clients {
 	) {
 		const filterText = `%${filters?.filter?.toLowerCase()}%`;
 		const limit = filters?.limit || 20;
+
+		// LEFT JOINs below will include more rows than we allow with LIMIT,
+		// so we need to do a subquery with the limiting first.
+		// The LEFT JOIN in the subquery only contributes to the WHERE clause
+		// and will not affect the returned row count.
 		const allowedClients = DB.drizzle
 			.select({ id: oauth2Client.id })
 			.from(oauth2Client)
@@ -404,7 +409,7 @@ export class OAuth2Clients {
 	static async sendManagerInvitationEmail(client: OAuth2Client, actor: User, email: string) {
 		const token = await UserTokens.create(
 			'invite',
-			new Date(Date.now() + 3600 * 1000),
+			new Date(Date.now() + 7 * 24 * 60 * 60 * 1000),
 			actor.id,
 			undefined,
 			`clientmanager=${client.client_id}`
@@ -413,7 +418,7 @@ export class OAuth2Clients {
 		const content = OAuth2InvitationEmail(
 			actor.display_name,
 			client.title,
-			`${PUBLIC_URL}/ssoadmin/oauth2/invite?${params.toString()}`
+			`${PUBLIC_URL}/account/accept-invite?${params.toString()}`
 		);
 
 		// TODO: logging

src/lib/server/oauth2/response.ts

@@ -15,14 +15,65 @@ export interface OAuth2TokenResponse {
 	expires_in?: number;
 	token_type?: string;
 	state?: string;
-	active?: boolean;
+}
+
+export interface OAuth2IntrospectResponse {
+	active: boolean;
 	scope?: string;
+	username?: string;
 	client_id?: string;
 	exp?: number;
 }
 
+export type OAuth2ResponseType = OAuth2TokenResponse | OAuth2IntrospectResponse;
+
 export class OAuth2Response {
-	static createResponse(code: number, data: unknown) {
+	static error(url: URL, err: OAuth2Error, redirectUri?: string) {
+		if (!(err instanceof OAuth2Error)) {
+			throw err;
+		}
+
+		OAuth2Response.doErrorRedirect(url, err, redirectUri);
+
+		return OAuth2Response.createErrorResponse(err);
+	}
+
+	static errorPlain(url: URL, err: OAuth2Error, redirectUri?: string) {
+		if (!(err instanceof OAuth2Error)) {
+			throw err;
+		}
+
+		OAuth2Response.doErrorRedirect(url, err, redirectUri);
+
+		return {
+			error: err.code,
+			error_description: err.message
+		};
+	}
+
+	static response(
+		url: URL,
+		obj: OAuth2ResponseType,
+		redirectUri?: string,
+		fragment: boolean = false
+	) {
+		OAuth2Response.doResponseRedirect(url, obj, redirectUri, fragment);
+
+		return OAuth2Response.createResponse(200, obj);
+	}
+
+	static responsePlain(
+		url: URL,
+		obj: OAuth2ResponseType,
+		redirectUri?: string,
+		fragment: boolean = false
+	) {
+		OAuth2Response.doResponseRedirect(url, obj, redirectUri, fragment);
+
+		return obj;
+	}
+
+	private static createResponse(code: number, data: unknown) {
 		const isJson = typeof data === 'object';
 		const body = isJson ? JSON.stringify(data) : (data as string);
 		return new Response(body, {
@@ -33,104 +84,44 @@ export class OAuth2Response {
 		});
 	}
 
-	static createErrorResponse(err: OAuth2Error) {
+	private static createErrorResponse(err: OAuth2Error) {
 		return OAuth2Response.createResponse(err.status, {
 			error: err.code,
 			error_description: err.message
 		});
 	}
 
-	static redirect(redirectUri: string) {
+	private static doResponseRedirect(
+		url: URL,
+		obj: OAuth2ResponseType,
+		redirectUri?: string,
+		fragment: boolean = false
+	) {
+		if (!redirectUri) return;
+		const searchJoinChar = redirectUri.includes('?') ? '&' : '?';
+		redirectUri += fragment ? '#' : searchJoinChar;
+
+		if (url.searchParams.has('state')) {
+			(obj as OAuth2TokenResponse).state = url.searchParams.get('state') as string;
+		}
+
+		redirectUri += new URLSearchParams(obj as Record<string, string>).toString();
 		return redirect(302, redirectUri);
 	}
 
-	static error(url: URL, err: OAuth2Error, redirectUri?: string) {
+	private static doErrorRedirect(url: URL, err: OAuth2Error, redirectUri?: string) {
-		if (!(err instanceof OAuth2Error)) {
+		if (!redirectUri) return;
-			throw err;
+		const obj: ErrorResponseData = {
-		}
-
-		if (redirectUri) {
-			const obj: ErrorResponseData = {
-				error: err.code,
-				error_description: err.message
-			};
-
-			if (url.searchParams.has('state')) {
-				obj.state = url.searchParams.get('state') as string;
-			}
-
-			redirectUri += '?' + new URLSearchParams(obj as Record<string, string>).toString();
-
-			return redirect(302, redirectUri);
-		}
-
-		return OAuth2Response.createErrorResponse(err);
-	}
-
-	static errorPlain(url: URL, err: OAuth2Error, redirectUri?: string) {
-		if (!(err instanceof OAuth2Error)) {
-			throw err;
-		}
-
-		if (redirectUri) {
-			const obj: ErrorResponseData = {
-				error: err.code,
-				error_description: err.message
-			};
-
-			if (url.searchParams.has('state')) {
-				obj.state = url.searchParams.get('state') as string;
-			}
-
-			redirectUri += '?' + new URLSearchParams(obj as Record<string, string>).toString();
-
-			return redirect(302, redirectUri);
-		}
-
-		return {
 			error: err.code,
 			error_description: err.message
 		};
-	}
 
-	static response(
+		if (url.searchParams.has('state')) {
-		url: URL,
+			obj.state = url.searchParams.get('state') as string;
-		obj: OAuth2TokenResponse,
-		redirectUri?: string,
-		fragment: boolean = false
-	) {
-		if (redirectUri) {
-			redirectUri += fragment ? '#' : redirectUri.indexOf('?') === -1 ? '?' : '&';
-
-			if (url.searchParams.has('state')) {
-				obj.state = url.searchParams.get('state') as string;
-			}
-
-			redirectUri += new URLSearchParams(obj as Record<string, string>).toString();
-			return redirect(302, redirectUri);
 		}
 
-		return OAuth2Response.createResponse(200, obj);
+		redirectUri += '?' + new URLSearchParams(obj as Record<string, string>).toString();
-	}
 
-	static responsePlain(
+		return redirect(302, redirectUri);
-		url: URL,
-		obj: OAuth2TokenResponse,
-		redirectUri?: string,
-		fragment: boolean = false
-	) {
-		if (redirectUri) {
-			const searchJoinChar = redirectUri.includes('?') ? '&' : '?';
-			redirectUri += fragment ? '#' : searchJoinChar;
-
-			if (url.searchParams.has('state')) {
-				obj.state = url.searchParams.get('state') as string;
-			}
-
-			redirectUri += new URLSearchParams(obj as Record<string, string>).toString();
-			return redirect(302, redirectUri);
-		}
-
-		return obj;
 	}
 }

src/lib/server/users/admin.ts

@@ -75,6 +75,8 @@ export class UsersAdmin {
 			.from(user)
 			.where(searchExpression);
 
+		// LEFT JOINs below will include more rows than we allow with LIMIT,
+		// so we need to do a subquery with the limiting first.
 		const baseQuery = DB.drizzle
 			.select({ id: user.id })
 			.from(user)

src/lib/server/users/index.ts

@@ -229,7 +229,7 @@ export class Users {
 	static async sendRegistrationEmail(user: User) {
 		const token = await UserTokens.create(
 			'activation',
-			new Date(Date.now() + 3600 * 1000),
+			new Date(Date.now() + 7 * 24 * 60 * 60 * 1000),
 			user.id
 		);
 
@@ -254,7 +254,11 @@ export class Users {
 	 * @param user User
 	 */
 	static async sendPasswordEmail(user: User) {
-		const token = await UserTokens.create('password', new Date(Date.now() + 3600 * 1000), user.id);
+		const token = await UserTokens.create(
+			'password',
+			new Date(Date.now() + 60 * 60 * 1000),
+			user.id
+		);
 		const params = new URLSearchParams({ token: token.token });
 		const content = ForgotPasswordEmail(
 			user.username,
@@ -280,7 +284,7 @@ export class Users {
 	static async sendInvitationEmail(email: string) {
 		const token = await UserTokens.create(
 			'invite',
-			new Date(Date.now() + 3600 * 1000),
+			new Date(Date.now() + 7 * 24 * 60 * 60 * 1000),
 			undefined,
 			undefined,
 			`register=${email}`

src/routes/account/accept-invite/+server.ts

@@ -3,9 +3,8 @@ import { OAuth2Clients } from '$lib/server/oauth2/index.js';
 import { UserTokens, Users } from '$lib/server/users';
 
 export const GET = async ({ locals, url }) => {
-	const userInfo = locals.session.data?.user;
+	const currentUser = await Users.getBySession(locals.session.data?.user);
-	const currentUser = await Users.getBySession(userInfo);
+	if (!currentUser) {
-	if (!userInfo || !currentUser) {
 		await locals.session.destroy();
 		return ApiUtils.redirect(`/login?redirectTo=${encodeURIComponent(url.pathname)}`);
 	}

src/routes/login/password/+page.server.ts

@@ -10,6 +10,8 @@ interface PasswordRequest {
 	repeatPassword: string;
 }
 
+// Sending an email asynchronously has a similar amount of delay,
+// so lets fake it. TODO: offload email sending somewhere else.
 const failDelay = () =>
 	new Promise((resolve) => setTimeout(resolve, 2000 + (Math.random() * 2000 - 1000)));
 
@@ -35,7 +37,7 @@ export const actions = {
 		const user = await Users.getByLogin(email);
 		if (!user || user.activated === 0) {
 			await failDelay();
-			return { success: true };
+			return { success: 'sent' };
 		}
 
 		try {

src/routes/ssoadmin/oauth2/[uuid]/+page.server.ts

@@ -1,8 +1,13 @@
+import { OAUTH2_MAX_REDIRECTS, OAUTH2_MAX_URLS } from '$lib/constants.js';
 import { AdminUtils } from '$lib/server/admin-utils';
 import { Changesets } from '$lib/server/changesets.js';
 import { CryptoUtils } from '$lib/server/crypto-utils.js';
 import type { OAuth2Client, User } from '$lib/server/drizzle';
-import { OAuth2ClientURLType, OAuth2Clients } from '$lib/server/oauth2';
+import {
+	OAuth2ClientURLType,
+	OAuth2Clients,
+	type OAuth2ClientAdminListItem
+} from '$lib/server/oauth2';
 import { Uploads } from '$lib/server/upload.js';
 import { Users } from '$lib/server/users';
 import { hasPrivileges } from '$lib/utils';
@@ -182,6 +187,15 @@ export const actions = {
 			return fail(400, { errors: ['invalidUrl'] });
 		}
 
+		// Do not allow adding multiple URLs of the same type, except for redirects.
+		const existingURLs = (details as OAuth2ClientAdminListItem).urls;
+		const existingOfType = existingURLs.filter((entry) => entry.type === type).length;
+		const maxOfType =
+			type === OAuth2ClientURLType.REDIRECT_URI ? OAUTH2_MAX_REDIRECTS : OAUTH2_MAX_URLS;
+		if (maxOfType < existingOfType) {
+			return fail(400, { errors: ['illegalUrl'] });
+		}
+
 		await OAuth2Clients.addUrl(details, type, url);
 
 		return { errors: [] };

src/routes/ssoadmin/oauth2/[uuid]/+page.svelte

@@ -9,13 +9,14 @@
 	import FormControl from '$lib/components/form/FormControl.svelte';
 	import FormSection from '$lib/components/form/FormSection.svelte';
 	import FormWrapper from '$lib/components/form/FormWrapper.svelte';
+	import FormErrors from '$lib/components/form/FormErrors.svelte';
+	import ActionButton from '$lib/components/ActionButton.svelte';
+	import type { ActionData, PageData } from './$types';
 	import { t } from '$lib/i18n';
 	import { page } from '$app/stores';
-	import type { ActionData, PageData } from './$types';
 	import { writable } from 'svelte/store';
-	import FormErrors from '$lib/components/form/FormErrors.svelte';
 	import { PUBLIC_SITE_NAME, PUBLIC_URL } from '$env/static/public';
-	import ActionButton from '$lib/components/ActionButton.svelte';
+	import { OAUTH2_MAX_REDIRECTS, OAUTH2_MAX_URLS } from '$lib/constants';
 
 	export let data: PageData;
 	export let form: ActionData;
@@ -31,9 +32,9 @@
 		// Can have up to five redirect URIs, only one of other types
 		const countOfType = data.details.urls.filter(({ type: subType }) => type === subType).length;
 		if (type === 'redirect_uri') {
-			return countOfType < 5;
+			return countOfType < OAUTH2_MAX_REDIRECTS;
 		}
-		return !countOfType;
+		return countOfType < OAUTH2_MAX_URLS;
 	});
 	$: splitScopes = data.details.scope?.split(' ') || [];
 	$: splitGrants = data.details.grants?.split(' ') || [];