various improvements (#2671)

* improve oauth config logic

* expire pending login

* move sort to backend

src/app/components/Clients/Sort.vue

@@ -1,6 +1,6 @@
 <template>
   <BasePrimaryButton @click="toggleSort">
-    <IconsArrowDown v-if="globalStore.sortClient === true" class="mr-2 w-4" />
+    <IconsArrowDown v-if="globalStore.sortClient === 'asc'" class="mr-2 w-4" />
     <IconsArrowUp v-else class="mr-2 w-4" />
     <span class="text-sm">{{ $t('client.sort') }}</span>
   </BasePrimaryButton>
@@ -11,7 +11,7 @@ const globalStore = useGlobalStore();
 const clientsStore = useClientsStore();
 
 function toggleSort() {
-  globalStore.sortClient = !globalStore.sortClient;
+  globalStore.sortClient = globalStore.sortClient === 'asc' ? 'desc' : 'asc';
   clientsStore.refresh().catch(console.error);
 }
 </script>

src/app/pages/login/2fa.vue

@@ -65,6 +65,14 @@ const _submit = useSubmit(
             type: 'error',
           });
           return;
+        } else if (data?.status === 'PENDING_LOGIN_EXPIRED') {
+          toast.showToast({
+            title: t('general.2fa'),
+            message: t('login.loginExpired'),
+            type: 'error',
+          });
+          await navigateTo('/login');
+          return;
         }
       }
       authenticating.value = false;

src/app/stores/clients.ts

@@ -31,9 +31,12 @@ export const useClientsStore = defineStore('Clients', () => {
   const clients = ref<null | LocalClient[]>(null);
   const clientsPersist = ref<Record<string, ClientPersist>>({});
 
-  const searchParams = ref({
+  const filter = ref<string | undefined>(undefined);
-    filter: undefined as string | undefined,
+
-  });
+  const searchParams = computed(() => ({
+    filter: filter.value,
+    sort: globalStore.sortClient,
+  }));
 
   const { data: _clients, refresh: _refresh } = useFetch('/api/client', {
     method: 'get',
@@ -43,7 +46,7 @@ export const useClientsStore = defineStore('Clients', () => {
   // TODO: rewrite
   async function refresh({ updateCharts = false } = {}) {
     await _refresh();
-    let transformedClients = _clients.value?.map((client) => {
+    const transformedClients = _clients.value?.map((client) => {
       let avatar = undefined;
       if (client.name.includes('@') && client.name.includes('.')) {
         avatar = `https://gravatar.com/avatar/${sha256(client.name.toLowerCase().trim())}.jpg`;
@@ -126,21 +129,12 @@ export const useClientsStore = defineStore('Clients', () => {
       };
     });
 
-    // TODO: move sort to backend
-    if (transformedClients !== undefined) {
-      transformedClients = sortByProperty(
-        transformedClients,
-        'name',
-        globalStore.sortClient
-      );
-    }
-
     clients.value = transformedClients ?? null;
   }
 
-  function setSearchQuery(filter: string) {
+  function setSearchQuery(query: string) {
     clients.value = null;
-    searchParams.value.filter = filter || undefined;
+    filter.value = query || undefined;
   }
 
   return { clients, clientsPersist, refresh, _clients, setSearchQuery };

src/app/stores/global.ts

@@ -6,7 +6,7 @@ export const useGlobalStore = defineStore('Global', () => {
     }
   );
 
-  const sortClient = ref(true); // Sort clients by name, true = asc, false = desc
+  const sortClient = ref<'asc' | 'desc'>('asc');
 
   const uiShowCharts = useCookie<boolean>('uiShowCharts', {
     default: () => false,

src/i18n/locales/en.json

@@ -84,7 +84,8 @@
     "rememberMeDesc": "Stay logged after closing the browser",
     "insecure": "You can't log in with an insecure connection. Use HTTPS.",
     "2faRequired": "Two Factor Authentication is required",
-    "2faWrong": "Two Factor Authentication is wrong"
+    "2faWrong": "Two Factor Authentication is wrong",
+    "loginExpired": "Login expired. Try again"
   },
   "client": {
     "empty": "There are no clients yet.",

src/server/api/auth/[provider]/callback.get.ts

@@ -40,6 +40,8 @@ export default defineEventHandler(async (event) => {
             type: 'oauth',
             userId: result.userId,
             remember: false,
+            // 5min
+            expires_at: Date.now() + 5 * 60 * 1000,
           },
           oauth_nonce: undefined,
           oauth_state: undefined,

src/server/api/auth/cancel.post.ts

@@ -1,10 +1,12 @@
 export default defineEventHandler(async (event) => {
-  const session = await useWGSession(event, false);
+  const session = await useWGSession(event);
+
   await session.update({
     pendingLogin: undefined,
     oauth_nonce: undefined,
     oauth_state: undefined,
     oauth_verifier: undefined,
   });
+
   return { success: true as const };
 });

src/server/api/auth/password.post.ts

@@ -32,6 +32,8 @@ export default defineEventHandler(async (event) => {
             type: 'password',
             userId: result.userId,
             remember,
+            // 5min
+            expires_at: Date.now() + 5 * 60 * 1000,
           },
         });
         return { status: 'TOTP_REQUIRED' as const };

src/server/api/auth/pending.get.ts

@@ -7,6 +7,16 @@ export default defineEventHandler(async (event) => {
       statusMessage: 'No pending authentication',
     });
   }
+  if (new Date() > new Date(session.data.pendingLogin.expires_at)) {
+    await session.update({
+      pendingLogin: undefined,
+    });
+
+    throw createError({
+      statusCode: 401,
+      statusMessage: 'No pending authentication',
+    });
+  }
 
   return {
     type: session.data.pendingLogin.type,

src/server/api/auth/verify-2fa.post.ts

@@ -14,6 +14,13 @@ export default defineEventHandler(async (event) => {
       statusMessage: 'No pending authentication',
     });
   }
+  if (new Date() > new Date(pendingLogin.expires_at)) {
+    await session.update({
+      pendingLogin: undefined,
+    });
+
+    return { status: 'PENDING_LOGIN_EXPIRED' as const };
+  }
 
   const totpStatus = await Database.users.validateTotpCode(
     pendingLogin.userId,

src/server/api/client/index.get.ts

@@ -4,14 +4,14 @@ export default definePermissionEventHandler(
   'clients',
   'custom',
   async ({ event, user }) => {
-    const { filter } = await getValidatedQuery(
+    const { filter, sort } = await getValidatedQuery(
       event,
       validateZod(ClientQuerySchema, event)
     );
 
     if (user.role === roles.ADMIN) {
-      return WireGuard.getAllClients(filter);
+      return WireGuard.getAllClients({ filter, sort });
     }
-    return WireGuard.getClientsForUser(user.id, filter);
+    return WireGuard.getClientsForUser(user.id, { filter, sort });
   }
 );

src/server/database/repositories/client/service.ts

@@ -4,6 +4,7 @@ import { client } from './schema';
 import type {
   ClientCreateFromExistingType,
   ClientCreateType,
+  ClientQueryType,
   UpdateClientType,
 } from './types';
 import type { DBType } from '#db/sqlite';
@@ -18,63 +19,9 @@ function createPreparedStatement(db: DBType) {
         },
       })
       .prepare(),
-    findAllPublic: db.query.client
-      .findMany({
-        with: {
-          oneTimeLink: true,
-        },
-        columns: {
-          privateKey: false,
-          preSharedKey: false,
-        },
-      })
-      .prepare(),
     findById: db.query.client
       .findFirst({ where: eq(client.id, sql.placeholder('id')) })
       .prepare(),
-    findByUserId: db.query.client
-      .findMany({
-        where: eq(client.userId, sql.placeholder('userId')),
-        with: { oneTimeLink: true },
-        columns: {
-          privateKey: false,
-          preSharedKey: false,
-        },
-      })
-      .prepare(),
-    findAllPublicFiltered: db.query.client
-      .findMany({
-        where: or(
-          like(client.name, sql.placeholder('filter')),
-          like(client.ipv4Address, sql.placeholder('filter')),
-          like(client.ipv6Address, sql.placeholder('filter'))
-        ),
-        with: {
-          oneTimeLink: true,
-        },
-        columns: {
-          privateKey: false,
-          preSharedKey: false,
-        },
-      })
-      .prepare(),
-    findByUserIdFiltered: db.query.client
-      .findMany({
-        where: and(
-          eq(client.userId, sql.placeholder('userId')),
-          or(
-            like(client.name, sql.placeholder('filter')),
-            like(client.ipv4Address, sql.placeholder('filter')),
-            like(client.ipv6Address, sql.placeholder('filter'))
-          )
-        ),
-        with: { oneTimeLink: true },
-        columns: {
-          privateKey: false,
-          preSharedKey: false,
-        },
-      })
-      .prepare(),
     toggle: db
       .update(client)
       .set({ enabled: sql.placeholder('enabled') as never as boolean })
@@ -96,15 +43,6 @@ export class ClientService {
     this.#statements = createPreparedStatement(db);
   }
 
-  async getForUser(userId: ID) {
-    const result = await this.#statements.findByUserId.execute({ userId });
-    return result.map((row) => ({
-      ...row,
-      createdAt: new Date(row.createdAt),
-      updatedAt: new Date(row.updatedAt),
-    }));
-  }
-
   /**
    * Never return values directly from this function. Use {@link getAllPublic} instead.
    */
@@ -120,25 +58,40 @@ export class ClientService {
   /**
    * Returns all clients without sensitive data
    */
-  async getAllPublic() {
+  async getAllPublic({ filter, sort }: ClientQueryType) {
-    const result = await this.#statements.findAllPublic.execute();
+    const filters = [];
-    return result.map((row) => ({
-      ...row,
-      createdAt: new Date(row.createdAt),
-      updatedAt: new Date(row.updatedAt),
-    }));
-  }
 
-  /**
+    if (filter?.trim()) {
-   * Get clients based on user ID and filter conditions
+      const filterPattern = `%${filter?.toLowerCase()}%`;
-   */
+      filters.push(
-  async getForUserFiltered(userId: ID, filter: string) {
+        or(
-    const filterPattern = `%${filter.toLowerCase()}%`;
+          like(client.name, filterPattern),
+          like(client.ipv4Address, filterPattern),
+          like(client.ipv6Address, filterPattern)
+        )
+      );
+    }
 
-    const result = await this.#statements.findByUserIdFiltered.execute({
+    const result = await this.#db.query.client
-      userId,
+      .findMany({
-      filter: filterPattern,
+        with: {
-    });
+          oneTimeLink: true,
+        },
+        where: and(...filters),
+        columns: {
+          privateKey: false,
+          preSharedKey: false,
+        },
+        orderBy: (t, { asc, desc }) => {
+          if (sort === 'desc') {
+            return desc(t.name);
+          } else {
+            // default to asc
+            return asc(t.name);
+          }
+        },
+      })
+      .execute();
 
     return result.map((row) => ({
       ...row,
@@ -148,14 +101,40 @@ export class ClientService {
   }
 
   /**
-   * Get all clients based on filter conditions without sensitive data
+   * Returns all clients without sensitive data belonging to user
    */
-  async getAllPublicFiltered(filter: string) {
+  async getAllForUser(userId: ID, { filter, sort }: ClientQueryType) {
-    const filterPattern = `%${filter.toLowerCase()}%`;
+    const filters = [];
 
-    const result = await this.#statements.findAllPublicFiltered.execute({
+    if (filter?.trim()) {
-      filter: filterPattern,
+      const filterPattern = `%${filter?.toLowerCase()}%`;
-    });
+      filters.push(
+        or(
+          like(client.name, filterPattern),
+          like(client.ipv4Address, filterPattern),
+          like(client.ipv6Address, filterPattern)
+        )
+      );
+    }
+
+    const result = await this.#db.query.client
+      .findMany({
+        where: and(eq(client.userId, userId), ...filters),
+        with: { oneTimeLink: true },
+        columns: {
+          privateKey: false,
+          preSharedKey: false,
+        },
+        orderBy: (t, { asc, desc }) => {
+          if (sort === 'desc') {
+            return desc(t.name);
+          } else {
+            // default to asc
+            return asc(t.name);
+          }
+        },
+      })
+      .execute();
 
     return result.map((row) => ({
       ...row,

src/server/database/repositories/client/types.ts

@@ -45,8 +45,6 @@ const address6 = z
   .pipe(controlStringRefine)
   .refine((v) => isIPv6(v));
 
-const filter = z.string().pipe(safeStringRefine).optional();
-
 const serverAllowedIps = z.array(AddressSchema, {
   message: t('zod.client.serverAllowedIps'),
 });
@@ -58,8 +56,13 @@ export const ClientCreateSchema = z.object({
 
 export type ClientCreateType = z.infer<typeof ClientCreateSchema>;
 
+const filter = z.string().pipe(safeStringRefine);
+
+const sort = z.enum(['asc', 'desc']);
+
 export const ClientQuerySchema = z.object({
-  filter: filter,
+  filter: filter.optional(),
+  sort: sort.optional(),
 });
 
 export type ClientQueryType = z.infer<typeof ClientQuerySchema>;
@@ -93,7 +96,6 @@ export const ClientUpdateSchema = schemaForType<UpdateClientType>()(
   })
 );
 
-// TODO: investigate if coerce is bad
 const clientId = z.coerce.number({ message: t('zod.client.id') });
 
 export const ClientGetSchema = z.object({

src/server/utils/WireGuard.ts

@@ -1,6 +1,7 @@
 import fs from 'node:fs/promises';
 import { createDebug } from 'obug';
 import type { InterfaceType } from '#db/repositories/interface/types';
+import type { ClientQueryType } from '#db/repositories/client/types';
 
 const WG_DEBUG = createDebug('WireGuard');
 
@@ -78,15 +79,10 @@ class WireGuard {
     WG_DEBUG('Config synced successfully.');
   }
 
-  async getClientsForUser(userId: ID, filter?: string) {
+  async getClientsForUser(userId: ID, query: ClientQueryType) {
     const wgInterface = await Database.interfaces.get();
 
-    let dbClients;
+    const dbClients = await Database.clients.getAllForUser(userId, query);
-    if (filter?.trim()) {
-      dbClients = await Database.clients.getForUserFiltered(userId, filter);
-    } else {
-      dbClients = await Database.clients.getForUser(userId);
-    }
 
     const clients = dbClients.map((client) => ({
       ...client,
@@ -126,15 +122,10 @@ class WireGuard {
     return clientDump;
   }
 
-  async getAllClients(filter?: string) {
+  async getAllClients(query: ClientQueryType = {}) {
     const wgInterface = await Database.interfaces.get();
 
-    let dbClients;
+    const dbClients = await Database.clients.getAllPublic(query);
-    if (filter?.trim()) {
-      dbClients = await Database.clients.getAllPublicFiltered(filter);
-    } else {
-      dbClients = await Database.clients.getAllPublic();
-    }
 
     const clients = dbClients.map((client) => ({
       ...client,

src/server/utils/config.ts

@@ -44,7 +44,7 @@ export const WG_ENV = {
   DISABLE_IPV6: process.env.DISABLE_IPV6 === 'true',
   WG_EXECUTABLE: await detectAwg(),
   DISABLE_VERSION_CHECK: process.env.DISABLE_VERSION_CHECK === 'true',
-  /** List of enabled OAuth providers */
+  /** List of enabled and configured OAuth providers */
   OAUTH_PROVIDERS: oauthProviders,
   /** List of allowed OAuth domains */
   OAUTH_ALLOWED_DOMAINS: process.env.OAUTH_ALLOWED_DOMAINS?.split(',').map(

src/server/utils/oauth.ts

@@ -12,6 +12,11 @@ type OAuthConfig = {
   userInfoFlow?: 'github';
 };
 
+type ConfiguredOAuthConfig = OAuthConfig & {
+  clientId: string;
+  clientSecret: string;
+};
+
 const GoogleConfig: OAuthConfig = {
   friendlyName: 'Google',
   server: 'https://accounts.google.com',
@@ -63,22 +68,23 @@ export function isValidOauthProvider(
 }
 
 export function isConfiguredOauthProvider(
-  oauthProvider: (typeof OAUTH_PROVIDERS)[OAUTH_PROVIDER]
+  oauthProvider: OAuthConfig
-): oauthProvider is (typeof OAUTH_PROVIDERS)[OAUTH_PROVIDER] & {
+): oauthProvider is ConfiguredOAuthConfig {
-  clientId: string;
-  clientSecret: string;
-} {
   if (!oauthProvider.clientId || !oauthProvider.clientSecret) {
     return false;
   }
   return true;
 }
 
-function isEnabledProvider(provider: OAUTH_PROVIDER) {
+function getEnabledOauthProvider(provider: OAUTH_PROVIDER) {
-  return WG_ENV.OAUTH_PROVIDERS?.includes(provider);
+  if (!WG_ENV.OAUTH_PROVIDERS?.includes(provider)) {
+    return;
+  }
+
+  // WG_ENV.OAUTH_PROVIDERS is filtered to configured providers during startup.
+  return OAUTH_PROVIDERS[provider] as ConfiguredOAuthConfig;
 }
 
-// TODO: simplify logic between WG_ENV.OAUTH_PROVIDERS and buildOauthConfig
 export async function buildOauthConfig(event: H3Event) {
   const provider = getRouterParam(event, 'provider');
   if (!provider || !isValidOauthProvider(provider)) {
@@ -88,22 +94,14 @@ export async function buildOauthConfig(event: H3Event) {
     });
   }
 
-  if (!isEnabledProvider(provider)) {
+  const oauthProvider = getEnabledOauthProvider(provider);
+  if (!oauthProvider) {
     throw createError({
       statusCode: 403,
       statusMessage: 'Provider is not enabled',
     });
   }
 
-  const oauthProvider = OAUTH_PROVIDERS[provider];
-
-  if (!isConfiguredOauthProvider(oauthProvider)) {
-    throw createError({
-      statusCode: 500,
-      statusMessage: 'Provider is not configured',
-    });
-  }
-
   const config = await client.discovery(
     new URL(oauthProvider.server),
     oauthProvider.clientId,

src/server/utils/session.ts

@@ -3,11 +3,12 @@ import type { UserType } from '#db/repositories/user/types';
 
 export type WGSession = Partial<{
   userId: ID;
-  // TODO: add pending login expiration
   pendingLogin: {
     type: 'password' | 'oauth';
     userId: ID;
     remember: boolean;
+    /** in milliseconds */
+    expires_at: number;
   };
   oauth_verifier: string;
   oauth_nonce: string;