Skip to content

chore: Add logging to SAML endpoints #22968

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
emrysal merged 3 commits into from
Aug 8, 2025
Merged

chore: Add logging to SAML endpoints #22968

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6867d3c
Add logging to SAML endpoints
joeauyeung Aug 8, 2025
0d02275
Merge branch 'main' into saml-add-logging
joeauyeung Aug 8, 2025
2a875d5
Update packages/features/auth/lib/next-auth-options.ts
hariombalhara Aug 8, 2025
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
4 changes: 4 additions & 0 deletions apps/web/app/api/auth/oidc/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@ import { NextResponse } from "next/server";

import jackson from "@calcom/features/ee/sso/lib/jackson";
import { HttpError } from "@calcom/lib/http-error";
import logger from "@calcom/lib/logger";

// This is the callback endpoint for the OIDC provider
// A team must set this endpoint in the OIDC provider's configuration
async function handler(req: NextRequest) {
const log = logger.getSubLogger({ prefix: ["[ODIC auth]"] });
const { searchParams } = req.nextUrl;
const code = searchParams.get("code");
const state = searchParams.get("state");
const tenant = searchParams.get("tenant");

if (!code || !state) {
return NextResponse.json({ message: "Code and state are required" }, { status: 400 });
Expand All @@ -30,6 +33,7 @@ async function handler(req: NextRequest) {

return NextResponse.redirect(redirect_url, 302);
} catch (err) {
log.error(`Error authorizing tenant ${tenant}: ${err}`);
Copy link
Contributor Author

@joeauyeung joeauyeung Aug 8, 2025

Choose a reason for hiding this comment

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

Tenant contains the org id

const { message, statusCode = 500 } = err as HttpError;

return NextResponse.json({ message }, { status: statusCode });
Expand Down
9 changes: 6 additions & 3 deletions apps/web/app/api/auth/saml/authorize/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,20 @@ import { NextResponse } from "next/server";
import type { OAuthReq } from "@calcom/features/ee/sso/lib/jackson";
import jackson from "@calcom/features/ee/sso/lib/jackson";
import type { HttpError } from "@calcom/lib/http-error";
import logger from "@calcom/lib/logger";

async function handler(req: NextRequest) {
const log = logger.getSubLogger({ prefix: ["[SAML authorize]"] });
const { oauthController } = await jackson();

const oAuthReq = Object.fromEntries(req.nextUrl.searchParams) as unknown as OAuthReq;

try {
const { redirect_url } = await oauthController.authorize(
Object.fromEntries(req.nextUrl.searchParams) as unknown as OAuthReq
);
const { redirect_url } = await oauthController.authorize(oAuthReq);

return NextResponse.redirect(redirect_url as string, 302);
} catch (err) {
log.error(`Error initaiting SAML login for tenant ${oAuthReq?.tenant}: ${err}`);
const { message, statusCode = 500 } = err as HttpError;

return NextResponse.json({ message }, { status: statusCode });
Expand Down
17 changes: 14 additions & 3 deletions apps/web/app/api/auth/saml/callback/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,32 @@ import { defaultResponderForAppDir } from "app/api/defaultResponderForAppDir";
import { parseRequestData } from "app/api/parseRequestData";
import type { NextRequest } from "next/server";
import { NextResponse } from "next/server";
import { uuid } from "short-uuid";

import jackson from "@calcom/features/ee/sso/lib/jackson";
import type { SAMLResponsePayload } from "@calcom/features/ee/sso/lib/jackson";
import logger from "@calcom/lib/logger";

async function handler(req: NextRequest) {
const log = logger.getSubLogger({ prefix: ["[SAML callback]"] });
const { oauthController } = await jackson();

const { redirect_url } = await oauthController.samlResponse(
(await parseRequestData(req)) as SAMLResponsePayload
);
const requestData = (await parseRequestData(req)) as SAMLResponsePayload;

const { redirect_url, error } = await oauthController.samlResponse(requestData);

if (redirect_url) {
return NextResponse.redirect(redirect_url, 302);
}

if (error) {
const uid = uuid();
log.error(
`Error authenticating user with error ${error} for relayState ${requestData?.RelayState} trace:${uid}`
);
return NextResponse.json({ message: `Error authorizing user. trace: ${uid}` }, { status: 400 });
Comment on lines +24 to +28
Copy link
Contributor Author

@joeauyeung joeauyeung Aug 8, 2025

Choose a reason for hiding this comment

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

Since we don't have any identifying information here, let's add a trace that the user can use when reaching out to support.

}

return NextResponse.json({ message: "No redirect URL provided" }, { status: 400 });
}

Expand Down
16 changes: 14 additions & 2 deletions apps/web/app/api/auth/saml/token/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,26 @@ import { defaultResponderForAppDir } from "app/api/defaultResponderForAppDir";
import { parseRequestData } from "app/api/parseRequestData";
import type { NextRequest } from "next/server";
import { NextResponse } from "next/server";
import { uuid } from "short-uuid";

import jackson from "@calcom/features/ee/sso/lib/jackson";
import type { OAuthTokenReq } from "@calcom/features/ee/sso/lib/jackson";
import logger from "@calcom/lib/logger";

async function handler(req: NextRequest) {
const { oauthController } = await jackson();
const tokenResponse = await oauthController.token((await parseRequestData(req)) as OAuthTokenReq);
return NextResponse.json(tokenResponse);
const log = logger.getSubLogger({ prefix: ["[SAML token]"] });

const oauthTokenReq = (await parseRequestData(req)) as OAuthTokenReq;

try {
const tokenResponse = await oauthController.token(oauthTokenReq);
return NextResponse.json(tokenResponse);
} catch (error) {
const uid = uuid();
log.error(`Error getting auth token for client id ${oauthTokenReq?.client_id}: ${error} trace: ${uid}`);
throw new Error(`Error getting auth token with error ${error} trace: ${uid}`);
}
}

export const POST = defaultResponderForAppDir(handler);
28 changes: 23 additions & 5 deletions apps/web/app/api/auth/saml/userinfo/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,34 +1,52 @@
import { defaultResponderForAppDir } from "app/api/defaultResponderForAppDir";
import type { NextRequest } from "next/server";
import { NextResponse } from "next/server";
import z from "zod";
import { uuid } from "short-uuid";
import { z } from "zod";

import jackson from "@calcom/features/ee/sso/lib/jackson";
import { HttpError } from "@calcom/lib/http-error";
import logger from "@calcom/lib/logger";

const extractAuthToken = (req: NextRequest) => {
const log = logger.getSubLogger({ prefix: ["SAML extractAuthToken"] });
const uid = uuid();
const authHeader = req.headers.get("authorization");
const parts = (authHeader || "").split(" ");
if (parts.length > 1) return parts[1];

// check for query param
let arr: string[] = [];
const { access_token } = requestQuery.parse(Object.fromEntries(req.nextUrl.searchParams));
const tokenParse = requestQuery.safeParse(Object.fromEntries(req.nextUrl.searchParams));
let access_token;
if (!tokenParse.success) {
log.error(`Error parsing request query: ${tokenParse.error} trace ${uid}`);
throw new HttpError({ statusCode: 401, message: `Unauthorized trace: ${uid}` });
}
access_token = tokenParse.data.access_token;
arr = arr.concat(access_token);
if (arr[0].length > 0) return arr[0];

throw new HttpError({ statusCode: 401, message: "Unauthorized" });
throw new HttpError({ statusCode: 401, message: `Unauthorized trace: ${uid}` });
};

const requestQuery = z.object({
access_token: z.string(),
});

async function handler(req: NextRequest) {
const log = logger.getSubLogger({ prefix: ["SAML userinfo"] });
const { oauthController } = await jackson();
const token = extractAuthToken(req);
Comment on lines 39 to 40
Copy link
Member

@hariombalhara hariombalhara Aug 8, 2025

Choose a reason for hiding this comment

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

How about we create traceId once here and pass the traceId to extractAuthToken too. That makes the traceId more usable.

const userInfo = await oauthController.userInfo(token);
return NextResponse.json(userInfo);

try {
const userInfo = await oauthController.userInfo(token);
return NextResponse.json(userInfo);
} catch (error) {
const uid = uuid();
log.error(`trace: ${uid} Error getting user info from token: ${error}`);
throw new Error(`Error getting user info from token. trace: ${uid}`);
}
Comment on lines +42 to +49
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 8, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix error handling - don't throw unhandled errors.

The error handling throws a new Error after logging, which will result in an unhandled error in the API route. The error should be returned as an HTTP response instead.

Apply this diff to properly handle the error:

  try {
    const userInfo = await oauthController.userInfo(token);
    return NextResponse.json(userInfo);
  } catch (error) {
    const uid = uuid();
    log.error(`trace: ${uid} Error getting user info from token: ${error}`);
-    throw new Error(`Error getting user info from token. trace: ${uid}`);
+    return NextResponse.json(
+      { message: `Error getting user info from token. trace: ${uid}` },
+      { status: 500 }
+    );
  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
try {
const userInfo = await oauthController.userInfo(token);
return NextResponse.json(userInfo);
} catch (error) {
const uid = uuid();
log.error(`trace: ${uid} Error getting user info from token: ${error}`);
throw new Error(`Error getting user info from token. trace: ${uid}`);
}
try {
const userInfo = await oauthController.userInfo(token);
return NextResponse.json(userInfo);
} catch (error) {
const uid = uuid();
log.error(`trace: ${uid} Error getting user info from token: ${error}`);
return NextResponse.json(
{ message: `Error getting user info from token. trace: ${uid}` },
{ status: 500 }
);
}
🤖 Prompt for AI Agents 
In apps/web/app/api/auth/saml/userinfo/route.ts around lines 42 to 49, the
current error handling throws a new Error after logging, causing unhandled
errors in the API route. Instead of throwing, modify the catch block to return
an HTTP error response using NextResponse with an appropriate status code and
error message, including the trace ID for debugging.

}

export const GET = defaultResponderForAppDir(handler);
Expand Down
2 changes: 1 addition & 1 deletion packages/features/auth/lib/next-auth-options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { isENVDev } from "@calcom/lib/env";
import logger from "@calcom/lib/logger";
import { randomString } from "@calcom/lib/random";
import { safeStringify } from "@calcom/lib/safeStringify";
import { hashEmail } from "@calcom/lib/server/PiiHasher";
import { CredentialRepository } from "@calcom/lib/server/repository/credential";
import { DeploymentRepository } from "@calcom/lib/server/repository/deployment";
import { OrganizationRepository } from "@calcom/lib/server/repository/organization";
Expand All @@ -48,7 +49,6 @@ import { dub } from "./dub";
import { isPasswordValid } from "./isPasswordValid";
import CalComAdapter from "./next-auth-custom-adapter";
import { verifyPassword } from "./verifyPassword";
import { hashEmail } from "@calcom/lib/server/PiiHasher";

const log = logger.getSubLogger({ prefix: ["next-auth-options"] });
const GOOGLE_API_CREDENTIALS = process.env.GOOGLE_API_CREDENTIALS || "{}";
Expand Down
Loading