Improve error handling on Authproto

.changeset/config.json

@@ -7,5 +7,8 @@
   "access": "public",
   "baseBranch": "main",
   "updateInternalDependencies": "patch",
-  "ignore": ["@fujocoded/astro-ao3-loader", "@fujocoded/astro-alt-text-toolkit"]
+  "ignore": [
+    "@fujocoded/astro-smooth-action",
+    "@fujocoded/astro-alt-text-toolkit"
+  ]
 }

astro-authproto/README_OLD.md

@@ -19,12 +19,12 @@ Bluesky and beyond.
 - [ ] Comment the whole Auth files
 - [ ] Improve config and make it make sense
 - [x] Figure out how to turn AstroDB into an option
-- [ ] Add option to redirect logged in/logged out user to a chosen page
+- [x] Add option to redirect logged in/logged out user to a chosen page
 - [ ] Explain how to intercept logged out/logged in events so you can do things
-- [ ] Figure out how to make types correctly signal Astro.locals.loggedInUser can be null
-- [ ] Fix the state bits in the oauth login route
+- [x] Figure out how to make types correctly signal Astro.locals.loggedInUser can be null
+- [x] Fix the state bits in the oauth login route
 - [x] Figure out what's up with weird amount of implementations in lib/
-- [ ] Clean up client-metadata duplication
+- [x] Clean up client-metadata duplication
 - [ ] Rename genericData to writeData
 
 // Tip!

astro-authproto/package.json

@@ -1,7 +1,7 @@
 {
   "name": "@fujocoded/authproto",
   "version": "0.1.3",
-  "description": "Astro integration to easily authenticateyour site visitors using ATproto. For Bluesky and beyond.",
+  "description": "Astro integration to easily authenticate your site visitors using ATproto. For Bluesky and beyond.",
   "main": "dist/index.js",
   "type": "module",
   "exports": {
@@ -44,7 +44,7 @@
   ],
   "scripts": {
     "build": "tsdown",
-    "dev": "tsdown --watch src/",
+    "dev": "tsdown --watch .src/",
     "validate": "npx publint",
     "test": "vitest"
   },
@@ -72,10 +72,11 @@
     "@atproto/jwk-jose": "^0.1.4",
     "@atproto/oauth-client-node": "^0.3.8",
     "astro-integration-kit": "^0.19.0",
+    "glob": "^13.0.0",
     "unstorage": "^1.16.1"
   },
   "devDependencies": {
-    "tsdown": "^0.17.2"
+    "tsdown": "^0.18.0"
   },
   "peerDependencies": {
     "@astrojs/db": "^0.17.1",

astro-authproto/src/components/Login.astro

@@ -8,9 +8,11 @@ interface Props {
 
 const { redirect: redirectUrl } = Astro.props;
 const currentUser = Astro.locals.loggedInUser;
+const currentError = Astro.locals.authproto?.errorDescription ?? Astro.locals.authproto?.errorCode;
 
 // @ts-expect-error
 const { class: cssClass, 'class:list': cssClassList } = Astro.props;
+
 ---
 
 {
@@ -32,3 +34,4 @@ const { class: cssClass, 'class:list': cssClassList } = Astro.props;
     </form>
   )
 }
+{currentError && <div style="color:red;">{currentError}</div>}

astro-authproto/src/lib/auth.ts

@@ -86,3 +86,18 @@ export const didToHandle = async (did: string) => {
   const atprotoData = await IDENTITY_RESOLVER.resolveAtprotoData(did);
   return atprotoData.handle;
 };
+
+export const extractAuthError = (
+  e: unknown
+): { code: string | "UNKNOWN"; description: string | undefined } => {
+  if (e instanceof Error) {
+    return {
+      code: e.name ?? "UNKNOWN",
+      description: e.message,
+    };
+  }
+  return {
+    code: "UNKNOWN",
+    description: undefined,
+  };
+};

astro-authproto/src/routes/middleware.ts

@@ -1,11 +1,25 @@
 import { didToHandle, oauthClient } from "../lib/auth.js";
 import { type MiddlewareHandler } from "astro";
 
+export const AUTHPROTO_ERROR_CODE = "authproto-error-code";
+export const AUTHPROTO_ERROR_DESCRIPTION = "authproto-error-description"
+
 export const onRequest: MiddlewareHandler = async (
   { locals, session },
   next
 ) => {
   const userDid = await session?.get("atproto-did");
+  const errorCode = await session?.get(AUTHPROTO_ERROR_CODE);
+  const errorDescription = await session?.get(AUTHPROTO_ERROR_DESCRIPTION);
+  if (errorCode || errorDescription) {
+        locals.authproto = {
+          // TODO: add input handler
+          errorCode,
+          errorDescription
+        };
+        await session?.delete(AUTHPROTO_ERROR_CODE);
+        await session?.delete(AUTHPROTO_ERROR_DESCRIPTION);
+  }
 
   if (!session || !userDid) {
     locals.loggedInUser = null;

astro-authproto/src/routes/oauth/callback.ts

@@ -1,15 +1,42 @@
 import type { APIRoute } from "astro";
-import { oauthClient } from "../../lib/auth.js";
+import { extractAuthError, oauthClient } from "../../lib/auth.js";
 import { getRedirectUrl } from "../../lib/redirects.js";
 import { redirectAfterLogin } from "fujocoded:authproto/config";
+import {
+  AUTHPROTO_ERROR_DESCRIPTION,
+  AUTHPROTO_ERROR_CODE,
+} from "../middleware.ts";
+import { type OAuthSession } from "@atproto/oauth-client-node";
 
-export const GET: APIRoute = async ({ params, request, redirect, session }) => {
+export const GET: APIRoute = async ({
+  params,
+  request,
+  redirect,
+  session,
+  locals,
+}) => {
   const requestUrl = new URL(request.url);
-  const { session: oauthSession } = await oauthClient.callback(
-    requestUrl.searchParams
-  );
 
-  session?.set("atproto-did", oauthSession.did);
+  let oauthSession: OAuthSession | null;
+  let error = requestUrl.searchParams.get("error") ?? "UNKNOWN";
+  let errorDescription = requestUrl.searchParams.get("error_description") ?? undefined;
+  try {
+    const clientCallback = await oauthClient.callback(requestUrl.searchParams);
+    oauthSession = clientCallback.session;
+    session?.set("atproto-did", oauthSession.did);
+  } catch (e) {
+    // If there is an error during session restoration then it takes precedence
+    // over the one in the searchParams
+    const authError = extractAuthError(e);
+    error = authError.code ?? error;
+    errorDescription = authError.description;
+    oauthSession = null;
+  }
+
+  if (error || errorDescription) {
+    session?.set(AUTHPROTO_ERROR_CODE, error);
+    session?.set(AUTHPROTO_ERROR_DESCRIPTION, errorDescription);
+  }
 
   // Check if a custom redirect or referer was passed in the state
   // Note: CSRF validation is already handled by oauthClient.callback() above,
@@ -30,11 +57,13 @@ export const GET: APIRoute = async ({ params, request, redirect, session }) => {
     }
   }
 
-  const redirectTo = await getRedirectUrl({
-    redirectToBase: customRedirect ?? redirectAfterLogin ?? "/",
-    did: oauthSession.did,
-    referer: referer ?? "",
-  });
+  const redirectTo = oauthSession
+    ? await getRedirectUrl({
+        redirectToBase: customRedirect ?? redirectAfterLogin ?? "/",
+        did: oauthSession.did,
+        referer: referer ?? "",
+      })
+    : (referer ?? "/");
 
   return redirect(redirectTo);
 };

astro-authproto/src/routes/oauth/login.ts

@@ -1,9 +1,13 @@
 import type { APIRoute } from "astro";
-import { oauthClient } from "../../lib/auth.js";
+import { extractAuthError, oauthClient } from "../../lib/auth.js";
 import { scopes } from "fujocoded:authproto/config";
 import { randomBytes } from "node:crypto";
+import {
+  AUTHPROTO_ERROR_CODE,
+  AUTHPROTO_ERROR_DESCRIPTION,
+} from "../../../src/routes/middleware.ts";
 
-export const POST: APIRoute = async ({ redirect, request }) => {
+export const POST: APIRoute = async ({ redirect, request, session }) => {
   const body = await request.formData();
   const atprotoId = body.get("atproto-id")?.toString();
   const customRedirect = body.get("redirect")?.toString();
@@ -19,17 +23,24 @@ export const POST: APIRoute = async ({ redirect, request }) => {
     ...(referer && !customRedirect && { referer }),
   };
 
-  const url = await oauthClient.authorize(atprotoId!, {
-    scope: scopes.join(" "),
-    // This random value protects against CSRF (Cross-Site Request
-    // Forgery) attacks. We send it along our authorization request, and the OAuth
-    // provider will send it back with the authentication response. By verifying
-    // it matches what we sent, we can be sure the callback is in response to
-    // OUR authorization request, not someone else's.
-    // We also encode the desired redirect URL if provided.
-    state: Buffer.from(JSON.stringify(stateData)).toString("base64url"),
-  });
+  try {
+    const url = await oauthClient.authorize(atprotoId!, {
+      scope: scopes.join(" "),
+      // This random value protects against CSRF (Cross-Site Request
+      // Forgery) attacks. We send it along our authorization request, and the OAuth
+      // provider will send it back with the authentication response. By verifying
+      // it matches what we sent, we can be sure the callback is in response to
+      // OUR authorization request, not someone else's.
+      // We also encode the desired redirect URL if provided.
+      state: Buffer.from(JSON.stringify(stateData)).toString("base64url"),
+    });
 
-  console.log(`Redirecting to PDS for Authorization`);
-  return redirect(url.toString());
+    return redirect(url.toString());
+  } catch (e) {
+    const authError = extractAuthError(e);
+    session?.set(AUTHPROTO_ERROR_CODE, authError.code);
+    session?.set(AUTHPROTO_ERROR_DESCRIPTION, authError.description);
+
+    return redirect(stateData.referer || "/");
+  }
 };

astro-authproto/src/types.d.ts

@@ -1,7 +1,10 @@
+import { AUTHPROTO_ERROR_CODE, AUTHPROTO_ERROR_DESCRIPTION } from "./routes/middleware.ts";
 declare global {
   namespace App {
     interface SessionData {
       "atproto-did": string | undefined;
+      [AUTHPROTO_ERROR_CODE]: string | undefined;
+      [AUTHPROTO_ERROR_DESCRIPTION]: string | undefined;
     }
 
     interface Locals {
@@ -10,6 +13,11 @@ declare global {
         handle: string;
         fetchHandler: import("@atproto/oauth-client-node").OAuthSession["fetchHandler"];
       } | null;
+      authproto: {
+        attemptedHandle?: string;
+        errorDescription?: string;
+        errorCode?: string;
+      } | null;
     }
   }
 }

astro-authproto/tsdown.config.ts

@@ -17,30 +17,14 @@ export default defineConfig([
   {
     name: "components",
     entry: ["src/components.ts"],
-    copy: [{ from: "src/components", to: "dist/components" }],
+    copy: [{ from: "src/components/", to: "dist/" }],
     external: astroFileExternal,
     ...COMMON_CONFIG,
   },
-  {
-    name: "tables",
-    entry: ["src/db/tables.ts"],
-    outputOptions: {
-      dir: `./dist/db/`,
-    },
-    external: astroFileExternal,
-    ...COMMON_CONFIG,
-    dts: false,
-  },
   {
     name: "integration",
-    entry: ["src/index.ts", "src/types.d.ts"],
-    external: baseExternal,
-    ...COMMON_CONFIG,
-  },
-  {
-    name: "helpers",
-    entry: ["src/helpers.ts", "src/types.d.ts"],
-    external: baseExternal,
+    entry: ["src/index.ts", "src/helpers.ts", "src/types.d.ts", "src/db/tables.ts"],
+    external: astroFileExternal,
     ...COMMON_CONFIG,
   },
   {
@@ -53,17 +37,8 @@ export default defineConfig([
     ...COMMON_CONFIG,
   },
   {
-    name: "stores-unstorage",
-    entry: ["src/stores/unstorage.ts"],
-    outputOptions: {
-      dir: `./dist/stores/`,
-    },
-    external: baseExternal,
-    ...COMMON_CONFIG,
-  },
-  {
-    name: "stores-db",
-    entry: ["src/stores/db.ts"],
+    name: "stores",
+    entry: ["src/stores/unstorage.ts", "src/stores/db.ts"],
     outputOptions: {
       dir: `./dist/stores/`,
     },