refactor: refactor code

packages/core/src/saml-applications/routes/anonymous.ts

@@ -1,5 +1,6 @@
 import { z } from 'zod';
 
+import { EnvSet, getTenantEndpoint } from '#src/env-set/index.js';
 import RequestError from '#src/errors/RequestError/index.js';
 import koaGuard from '#src/middleware/koa-guard.js';
 import type { AnonymousRouter, RouterInitArgs } from '#src/routes/types.js';
@@ -10,6 +11,7 @@
   createSamlResponse,
   handleOidcCallbackAndGetUserInfo,
   setupSamlProviders,
+  buildSamlAppCallbackUrl,
 } from './utils.js';
 
 const samlApplicationSignInCallbackQueryParametersGuard = z.union([
@@ -23,12 +25,12 @@
 ]);
 
 export default function samlApplicationAnonymousRoutes<T extends AnonymousRouter>(
-  ...[router, { libraries, queries, envSet }]: RouterInitArgs<T>
+  ...[router, { id: tenantId, libraries, queries, envSet }]: RouterInitArgs<T>
 ) {
   const {
     samlApplications: { getSamlIdPMetadataByApplicationId },
   } = libraries;
   const { applications, samlApplicationSecrets, samlApplicationConfigs } = queries;
 
   router.get(
     '/saml-applications/:id/metadata',
@@ -49,11 +51,12 @@
       return next();
     }
   );
 
   router.get(
     '/saml-applications/:id/callback',
     koaGuard({
       params: z.object({ id: z.string() }),
+      // TODO: should be able to handle `state` and `redirectUri`
       query: samlApplicationSignInCallbackQueryParametersGuard,
       status: [200, 400],
     }),
@@ -77,9 +80,13 @@
         oidcClientMetadata: { redirectUris },
       } = await applications.findApplicationById(id);
 
-      assertThat(redirectUris[0], 'oidc.redirect_uri_not_set');
+      const tenantEndpoint = getTenantEndpoint(tenantId, EnvSet.values);
+      assertThat(
+        redirectUris[0] === buildSamlAppCallbackUrl(tenantEndpoint, id),
+        'oidc.invalid_redirect_uri'
+      );
 
       // TODO: should be able to handle `state` and code verifier etc.
       const { code } = query;
 
       // Handle OIDC callback and get user info

packages/core/src/saml-applications/routes/utils.ts

@@ -1,15 +1,18 @@
 import { parseJson } from '@logto/connector-kit';
 import { generateStandardId } from '@logto/shared';
-import { tryThat } from '@silverhand/essentials';
+import { tryThat, appendPath } from '@silverhand/essentials';
 import camelcaseKeys from 'camelcase-keys';
-import { got } from 'got';
 import saml from 'samlify';
 import { ZodError, z } from 'zod';
 
 import RequestError from '#src/errors/RequestError/index.js';
-import { fetchOidcConfigRaw, getRawUserInfoResponse } from '#src/sso/OidcConnector/utils.js';
+import {
+  fetchOidcConfigRaw,
+  getRawUserInfoResponse,
+  handleTokenExchange,
+} from '#src/sso/OidcConnector/utils.js';
 import { idTokenProfileStandardClaimsGuard } from '#src/sso/types/oidc.js';
-import { oidcTokenResponseGuard, type IdTokenProfileStandardClaims } from '#src/sso/types/oidc.js';
+import { type IdTokenProfileStandardClaims } from '#src/sso/types/oidc.js';
 import assertThat from '#src/utils/assert-that.js';
 
 import {
@@ -18,9 +21,18 @@
   samlValueXmlnsXsi,
 } from '../libraries/consts.js';
 
-// We only support email and persistent format at the moment.
-const getSamlNameId = (user: IdTokenProfileStandardClaims, idpNameIDFormat?: string | string[]) => {
-  // If IdP has specified nameIDFormat, use it
+/**
+ * Determines the SAML NameID format and value based on the user's claims and IdP's NameID format.
+ * Supports email and persistent formats.
+ *
+ * @param user - The user's standard claims
+ * @param idpNameIDFormat - The NameID format(s) specified by the IdP (optional)
+ * @returns An object containing the NameIDFormat and NameID
+ */
+const buildSamlAssertionNameId = (
+  user: IdTokenProfileStandardClaims,
+  idpNameIDFormat?: string | string[]
+): { NameIDFormat: string; NameID: string } => {
   if (idpNameIDFormat) {
     // Get the first name ID format
     const format = Array.isArray(idpNameIDFormat) ? idpNameIDFormat[0] : idpNameIDFormat;
@@ -30,27 +42,27 @@
       user.email &&
       user.email_verified
     ) {
       return {
         NameIDFormat: format,
         NameID: user.email,
       };
     }
     // For other formats or when email is not available, use sub
     if (format === saml.Constants.namespace.format.persistent) {
       return {
         NameIDFormat: format,
         NameID: user.sub,
       };
     }
   }
   // No nameIDFormat specified, use default logic
   // Use email if available
   if (user.email && user.email_verified) {
     return {
       NameIDFormat: saml.Constants.namespace.format.emailAddress,
       NameID: user.email,
     };
   }
   // Fallback to persistent format with user.sub
   return {
     NameIDFormat: saml.Constants.namespace.format.persistent,
@@ -70,7 +82,7 @@
     );
 
     const { nameIDFormat } = idp.entitySetting;
-    const { NameIDFormat, NameID } = getSamlNameId(user, nameIDFormat);
+    const { NameIDFormat, NameID } = buildSamlAssertionNameId(user, nameIDFormat);
 
     const id = `ID_${generateStandardId()}`;
     const now = new Date();
@@ -92,6 +104,7 @@
       SubjectConfirmationDataNotOnOrAfter: expireAt.toISOString(),
       NameIDFormat,
       NameID,
+      // TODO: should get the request ID from the input parameters, pending https://github.com/logto-io/logto/pull/6881.
       InResponseTo: 'null',
       /**
        * User attributes for SAML response
@@ -129,25 +142,13 @@
     redirectUri?: string;
   }
 ) => {
-  const headers = {
-    Authorization: `Basic ${Buffer.from(`${clientId}:${clientSecret}`, 'utf8').toString('base64')}`,
-    'Content-Type': 'application/x-www-form-urlencoded',
-  };
-
-  const tokenRequestParameters = new URLSearchParams({
-    grant_type: 'authorization_code',
+  const result = await handleTokenExchange(tokenEndpoint, {
     code,
-    client_id: clientId,
-    ...(redirectUri ? { redirect_uri: redirectUri } : {}),
-  });
-
-  const httpResponse = await got.post(tokenEndpoint, {
-    body: tokenRequestParameters.toString(),
-    headers,
+    clientId,
+    clientSecret,
+    redirectUri,
   });
 
-  const result = oidcTokenResponseGuard.safeParse(parseJson(httpResponse.body));
-
   if (!result.success) {
     throw new RequestError({
       code: 'oidc.invalid_token',
@@ -159,11 +160,11 @@
 };
 
 export const createSamlResponse = async (
   idp: saml.IdentityProviderInstance,
   sp: saml.ServiceProviderInstance,
   userInfo: IdTokenProfileStandardClaims
 ): Promise<{ context: string; entityEndpoint: string }> => {
   // TODO: fix binding method
   // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
   const { context, entityEndpoint } = await idp.createLoginResponse(
     sp,
@@ -179,20 +180,21 @@
 };
 
 export const generateAutoSubmitForm = (actionUrl: string, samlResponse: string): string => {
   return `
     <html>
       <body>
         <form id="redirectForm" action="${actionUrl}" method="POST">
           <input type="hidden" name="SAMLResponse" value="${samlResponse}" />
-          <input type="submit" value="Submit" /> 
         </form>
         <script>
-          document.getElementById('redirectForm').submit();
+          window.onload = function() {
+            document.getElementById('redirectForm').submit();
+          };
         </script>
       </body>
     </html>
   `;
 };
 
 export const getUserInfo = async (
   accessToken: string,
@@ -214,12 +216,12 @@
 
 // Helper functions for SAML callback
 export const handleOidcCallbackAndGetUserInfo = async (
   code: string,
   applicationId: string,
   secret: string,
   redirectUri: string,
   issuer: string
 ) => {
   // Get OIDC configuration
   const { tokenEndpoint, userinfoEndpoint } = await tryThat(
     async () => fetchOidcConfigRaw(issuer),
@@ -297,3 +299,6 @@
 
   return { idp, sp };
 };
+
+export const buildSamlAppCallbackUrl = (baseUrl: URL, samlApplicationId: string) =>
+  appendPath(baseUrl, `api/saml-applications/${samlApplicationId}/callback`).toString();

packages/core/src/sso/OidcConnector/utils.test.ts

@@ -159,14 +159,19 @@ describe('fetchToken', () => {
       })
     );
 
-    expect(postMock).toBeCalledWith({
-      url: oidcConfigResponseCamelCase.tokenEndpoint,
-      form: {
+    expect(postMock).toBeCalledWith(oidcConfigResponseCamelCase.tokenEndpoint, {
+      body: new URLSearchParams({
         grant_type: 'authorization_code',
-        client_id: oidcConfig.clientId,
-        client_secret: oidcConfig.clientSecret,
         code: data.code,
+        client_id: oidcConfig.clientId,
         redirect_uri: redirectUri,
+      }).toString(),
+      headers: {
+        'Content-Type': 'application/x-www-form-urlencoded',
+        Authorization: `Basic ${Buffer.from(
+          `${oidcConfig.clientId}:${oidcConfig.clientSecret}`,
+          'utf8'
+        ).toString('base64')}`,
       },
     });
   });

packages/core/src/sso/OidcConnector/utils.ts

@@ -20,6 +20,12 @@
   type OidcTokenResponse,
 } from '../types/oidc.js';
 
+/**
+ * Fetch the full-list of OIDC config from the issuer. Throws error if config is invalid.
+ *
+ * @param issuer The issuer URL
+ * @returns The full-list of OIDC config
+ */
 export const fetchOidcConfigRaw = async (issuer: string) => {
   const { body } = await got.get(`${issuer}/.well-known/openid-configuration`, {
     responseType: 'json',
@@ -50,6 +56,46 @@
   }
 };
 
+export const handleTokenExchange = async (
+  tokenEndpoint: string,
+  {
+    code,
+    clientId,
+    clientSecret,
+    redirectUri,
+  }: {
+    code: string;
+    clientId: string;
+    clientSecret: string;
+    redirectUri?: string;
+  }
+) => {
+  const tokenRequestParameters = new URLSearchParams({
+    grant_type: 'authorization_code',
+    code,
+    client_id: clientId,
+    ...(redirectUri ? { redirect_uri: redirectUri } : {}),
+  });
+
+  const headers = {
+    Authorization: `Basic ${Buffer.from(`${clientId}:${clientSecret}`, 'utf8').toString('base64')}`,
+    'Content-Type': 'application/x-www-form-urlencoded',
+  };
+
+  const httpResponse = await got.post(tokenEndpoint, {
+    body: tokenRequestParameters.toString(),
+    headers,
+  });
+
+  const result = oidcTokenResponseGuard.safeParse(parseJson(httpResponse.body));
+
+  if (!result.success) {
+    return { success: false as const, error: result.error, response: httpResponse };
+  }
+
+  return { success: true as const, data: result.data };
+};
+
 export const fetchToken = async (
   { tokenEndpoint, clientId, clientSecret }: BaseOidcConfig,
   data: unknown,
@@ -68,28 +114,22 @@
   const { code } = result.data;
 
   try {
-    const httpResponse = await got.post({
-      url: tokenEndpoint,
-      form: {
-        grant_type: 'authorization_code',
-        code,
-        redirect_uri: redirectUri,
-        client_id: clientId,
-        client_secret: clientSecret,
-      },
+    const exchangeResult = await handleTokenExchange(tokenEndpoint, {
+      code,
+      clientId,
+      clientSecret,
+      redirectUri,
     });
 
-    const result = oidcTokenResponseGuard.safeParse(parseJson(httpResponse.body));
-
-    if (!result.success) {
+    if (!exchangeResult.success) {
       throw new SsoConnectorError(SsoConnectorErrorCodes.AuthorizationFailed, {
         message: 'Invalid token response',
-        response: httpResponse.body,
-        error: result.error.flatten(),
+        response: exchangeResult.response.body,
+        error: exchangeResult.error.flatten(),
       });
     }
 
-    return camelcaseKeys(result.data);
+    return camelcaseKeys(exchangeResult.data);
   } catch (error: unknown) {
     if (error instanceof SsoConnectorError) {
       throw error;
@@ -172,16 +212,16 @@
  */
 export const getUserInfo = async (accessToken: string, userinfoEndpoint: string) => {
   try {
     const body = await getRawUserInfoResponse(accessToken, userinfoEndpoint);
 
     const result = idTokenProfileStandardClaimsGuard
       .catchall(z.unknown())
       .safeParse(parseJson(body));
 
     if (!result.success) {
       throw new SsoConnectorError(SsoConnectorErrorCodes.AuthorizationFailed, {
         message: 'Invalid user info response',
         response: body,
         error: result.error.flatten(),
       });
     }

packages/phrases/src/locales/en/errors/oidc.ts

@@ -8,7 +8,6 @@ const oidc = {
   invalid_grant: 'Grant request is invalid.',
   invalid_redirect_uri:
     "`redirect_uri` did not match any of the client's registered `redirect_uris`.",
-  redirect_uri_not_set: '`redirect_uri` is not set.',
   access_denied: 'Access denied.',
   invalid_target: 'Invalid resource indicator.',
   unsupported_grant_type: 'Unsupported `grant_type` requested.',