Improved SAML auth handlers initialization (#570)

petruki · web-flow · commit 6ee4f08dd9f3 · 2025-09-15T21:09:35.000-07:00
diff --git a/.env-cmdrc-template b/.env-cmdrc-template
@@ -45,6 +45,7 @@
         "SWITCHER_GITOPS_URL": "http://localhost:8000"
     },
     "test": {
+        "ENV": "TEST",
         "NODE_OPTIONS": "--experimental-vm-modules",
         "PORT": "3000",
         "MONGODB_URI": "mongodb://mongodb:27017/switcher-api-test",
diff --git a/.github/workflows/master.yml b/.github/workflows/master.yml
@@ -14,12 +14,12 @@ jobs:
 
     steps:
       - name: Git checkout
-        uses: actions/checkout@v4
+        uses: actions/checkout@v5
         with:
           fetch-depth: 0
 
       - name: Use Node.js 24.x
-        uses: actions/setup-node@v4
+        uses: actions/setup-node@v5
         with:
           node-version: 24.x
 
@@ -102,12 +102,12 @@ jobs:
 
     steps:
       - name: Checkout
-        uses: actions/checkout@v4
+        uses: actions/checkout@v5
         with:
           ref: 'master'
 
       - name: Checkout Kustomize
-        uses: actions/checkout@v4
+        uses: actions/checkout@v5
         with:
           token: ${{ secrets.ARGOCD_PAT }}
           repository: switcherapi/switcher-deployment
diff --git a/.github/workflows/re-release.yml b/.github/workflows/re-release.yml
@@ -15,13 +15,13 @@ jobs:
 
     steps:
       - name: Git checkout
-        uses: actions/checkout@v4
+        uses: actions/checkout@v5
         with:
           fetch-depth: 0
           ref: ${{ github.event.inputs.tag }}
 
       - name: Use Node.js 24.x
-        uses: actions/setup-node@v4
+        uses: actions/setup-node@v5
         with:
           node-version: 24.x
 
@@ -68,7 +68,7 @@ jobs:
 
     steps:
       - name: Checkout code
-        uses: actions/checkout@v4
+        uses: actions/checkout@v5
         with:
           fetch-depth: 0
           ref: ${{ github.event.inputs.tag }}
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
@@ -11,12 +11,12 @@ jobs:
 
     steps:
       - name: Git checkout
-        uses: actions/checkout@v4
+        uses: actions/checkout@v5
         with:
           fetch-depth: 0
 
       - name: Use Node.js 24.x
-        uses: actions/setup-node@v4
+        uses: actions/setup-node@v5
         with:
           node-version: 24.x
 
@@ -63,7 +63,7 @@ jobs:
 
     steps:
       - name: Checkout code
-        uses: actions/checkout@v4
+        uses: actions/checkout@v5
 
       - name: Docker meta
         id: meta
diff --git a/.github/workflows/sonar.yml b/.github/workflows/sonar.yml
@@ -28,13 +28,13 @@ jobs:
             core.setOutput('base_ref', pr.data.base.ref);
             core.setOutput('head_sha', pr.data.head.sha);
 
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v5
         with:
           ref: ${{ steps.pr.outputs.head_sha }}
           fetch-depth: 0
 
       - name: Use Node.js 24.x
-        uses: actions/setup-node@v4
+        uses: actions/setup-node@v5
         with:
           node-version: 24.x
 
diff --git a/src/api-docs/paths/path-admin-saml.js b/src/api-docs/paths/path-admin-saml.js
@@ -8,6 +8,9 @@ export default {
             responses: {
                 '302': {
                     description: 'Redirect to SAML Identity Provider'
+                },
+                '404': {
+                    description: 'SAML not configured'
                 }
             }
         }
@@ -37,6 +40,9 @@ export default {
                 },
                 '401': {
                     description: 'SAML authentication failed'
+                },
+                '404': {
+                    description: 'SAML not configured'
                 }
             }
         }
@@ -52,6 +58,12 @@ export default {
                 '200': {
                     description: 'Success',
                     content: commonSchemaContent('AdminLoginResponse')
+                },
+                '401': {
+                    description: 'Authentication failed'
+                },
+                '404': {
+                    description: 'SAML not configured'
                 }
             }
         }
@@ -71,6 +83,9 @@ export default {
                             }
                         }
                     }
+                },
+                '404': {
+                    description: 'SAML not configured'
                 }
             }
         }
diff --git a/src/app.js b/src/app.js
@@ -11,7 +11,6 @@ import './db/mongoose.js';
 import mongoose from 'mongoose';
 import swaggerDocument from './api-docs/swagger-document.js';
 import adminRouter from './routers/admin.js';
-import adminSamlRouter from './routers/admin-saml.js';
 import environment from './routers/environment.js';
 import component from './routers/component.js';
 import domainRouter from './routers/domain.js';
@@ -42,22 +41,23 @@ app.disable('x-powered-by');
 /**
  * Session configuration for SAML
  */
-app.use(session({
-    secret: process.env.SESSION_SECRET || 'switcher-api-session',
-    resave: false,
-    saveUninitialized: false,
-    cookie: { 
-        secure: process.env.NODE_ENV === 'prod',
-        maxAge: 24 * 60 * 60 * 1000 // 24 hours
-    }
-}));
-app.use(passport.initialize());
+if (isSamlAvailable()) {
+    app.use(session({
+        secret: process.env.SESSION_SECRET,
+        resave: false,
+        saveUninitialized: false,
+        cookie: { 
+            secure: true,
+            maxAge: 5 * 60 * 1000 // 5 minutes
+        }
+    }));
+    app.use(passport.initialize());
+}
 
 /**
  * API Routes
  */
 app.use(adminRouter);
-app.use(adminSamlRouter);
 app.use(component);
 app.use(environment);
 app.use(domainRouter);
@@ -70,6 +70,14 @@ app.use(permissionRouter);
 app.use(slackRouter);
 app.use(gitOpsRouter);
 
+/**
+ * SAML Routes
+ */
+if (isSamlAvailable()) {
+    const adminSamlRouter = await import('./routers/admin-saml.js');
+    app.use(adminSamlRouter.default);
+}
+
 /**
  * GraphQL Routes
  */
@@ -109,19 +117,38 @@ app.get('/check', defaultLimiter, (req, res) => {
             release_time: process.env.RELEASE_TIME,
             env: process.env.ENV,
             db_state: mongoose.connection.readyState,
-            switcherapi: process.env.SWITCHER_API_ENABLE,
-            switcherapi_logger: process.env.SWITCHER_API_LOGGER,
-            relay_bypass_https: process.env.RELAY_BYPASS_HTTPS,
-            relay_bypass_verification: process.env.RELAY_BYPASS_VERIFICATION,
-            permission_cache: process.env.PERMISSION_CACHE_ACTIVATED,
-            history: process.env.HISTORY_ACTIVATED,
+            switcherapi: isEnabled('SWITCHER_API_ENABLE'),
+            switcherapi_logger: isEnabled('SWITCHER_API_LOGGER'),
+            relay_bypass_https: isEnabled('RELAY_BYPASS_HTTPS'),
+            relay_bypass_verification: isEnabled('RELAY_BYPASS_VERIFICATION'),
+            permission_cache: isEnabled('PERMISSION_CACHE_ACTIVATED'),
+            history: isEnabled('HISTORY_ACTIVATED'),
             max_metrics_pages: process.env.METRICS_MAX_PAGE,
             max_stretegy_op: process.env.MAX_STRATEGY_OPERATION,
-            max_rpm: process.env.MAX_REQUEST_PER_MINUTE || DEFAULT_RATE_LIMIT
+            max_rpm: process.env.MAX_REQUEST_PER_MINUTE || DEFAULT_RATE_LIMIT,
+            auth_providers: {
+                saml: isSamlAvailable(),
+                github: isOauthAvailableFor('GIT_OAUTH_CLIENT_ID', 'GIT_OAUTH_SECRET'),
+                bitbucket: isOauthAvailableFor('BITBUCKET_OAUTH_CLIENT_ID', 'BITBUCKET_OAUTH_SECRET'),
+            }
         };
     }
 
     res.status(200).send(response);
 });
 
+function isSamlAvailable() {
+    return (process.env.SAML_ENTRY_POINT && 
+        process.env.SAML_CALLBACK_ENDPOINT_URL && 
+        process.env.SAML_CERT)?.length > 0;
+}
+
+function isOauthAvailableFor(clientId, secret) {
+    return (process.env[clientId] && process.env[secret])?.length > 0;
+}
+
+function isEnabled(feature) {
+    return process.env[feature] && process.env[feature].toLowerCase() === 'true';
+}
+
 export default createServer(app);
diff --git a/src/external/saml.js b/src/external/saml.js
@@ -3,49 +3,42 @@ import passport from 'passport';
 import { signUpSaml } from '../services/admin.js';
 import Logger from '../helpers/logger.js';
 
-function isSamlAvailable() {
-    return process.env.SAML_ENTRY_POINT && process.env.SAML_CALLBACK_ENDPOINT_URL && process.env.SAML_CERT;
-}
+const samlOptions = {
+    entryPoint: process.env.SAML_ENTRY_POINT,
+    issuer: process.env.SAML_ISSUER || 'switcher-api',
+    callbackUrl: `${process.env.SAML_CALLBACK_ENDPOINT_URL}/admin/saml/callback`,
+    idpCert: Buffer.from(process.env.SAML_CERT, 'base64').toString('utf8'),
+    privateKey: process.env.SAML_PRIVATE_KEY ? Buffer.from(process.env.SAML_PRIVATE_KEY, 'base64').toString('utf8') : undefined,
+    identifierFormat: process.env.SAML_IDENTIFIER_FORMAT || 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress',
+    acceptedClockSkewMs: process.env.SAML_ACCEPTED_CLOCK_SKEW_MS ? parseInt(process.env.SAML_ACCEPTED_CLOCK_SKEW_MS, 10) : 5000,
+    signatureAlgorithm: 'sha256',
+    digestAlgorithm: 'sha256',
+    wantAssertionsSigned: true,
+    wantAuthnResponseSigned: false,
+};
 
-if (isSamlAvailable()) {
-    const samlOptions = {
-        entryPoint: process.env.SAML_ENTRY_POINT,
-        issuer: process.env.SAML_ISSUER || 'switcher-api',
-        callbackUrl: `${process.env.SAML_CALLBACK_ENDPOINT_URL}/admin/saml/callback`,
-        idpCert: Buffer.from(process.env.SAML_CERT, 'base64').toString('utf8'),
-        privateKey: process.env.SAML_PRIVATE_KEY ? Buffer.from(process.env.SAML_PRIVATE_KEY, 'base64').toString('utf8') : undefined,
-        identifierFormat: process.env.SAML_IDENTIFIER_FORMAT || 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress',
-        acceptedClockSkewMs: process.env.SAML_ACCEPTED_CLOCK_SKEW_MS ? parseInt(process.env.SAML_ACCEPTED_CLOCK_SKEW_MS, 10) : 5000,
-        signatureAlgorithm: 'sha256',
-        digestAlgorithm: 'sha256',
-        wantAssertionsSigned: true,
-        wantAuthnResponseSigned: false,
-    };
-
-    const samlStrategy = new SamlStrategy(samlOptions, async (profile, done) => {
-        try {
-            const userInfo = {
-                id: profile.nameID,
-                email: profile.email || profile['http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress'],
-                name: profile.firstName || profile.nameID
-            };
-            
-            const { jwt } = await signUpSaml(userInfo);
-            return done(null, { token: jwt.token });
-        } catch (error) {
-            Logger.error('SAML Strategy Error Event:', error);
-            return done(error);
-        }
-    });
-
-    passport.use('saml', samlStrategy);
-    Logger.info('SSO enabled: SAML strategy configured');
-    Logger.info(`   - Entry Point: ${samlOptions.entryPoint}`);
-    Logger.info(`   - Callback URL: ${samlOptions.callbackUrl}`);
-    Logger.info(`   - Issuer: ${samlOptions.issuer}`);
-    Logger.info(`   - Identifier Format: ${samlOptions.identifierFormat}`);
-    Logger.info(`   - Accepted Clock Skew (ms): ${samlOptions.acceptedClockSkewMs}`);
-    Logger.info(`   - Idp Cert: ${samlOptions.idpCert ? 'Provided' : 'Not Provided'}`);
-    Logger.info(`   - Private Key: ${samlOptions.privateKey ? 'Provided' : 'Not Provided'}`);
-}
+const samlStrategy = new SamlStrategy(samlOptions, async (profile, done) => {
+    try {
+        const userInfo = {
+            id: profile.nameID,
+            email: profile.email || profile['http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress'],
+            name: profile.firstName || profile.nameID
+        };
+        
+        const { jwt } = await signUpSaml(userInfo);
+        return done(null, { token: jwt.token });
+    } catch (error) {
+        Logger.error('SAML Strategy Error Event:', error);
+        return done(error);
+    }
+});
 
+passport.use('saml', samlStrategy);
+Logger.info('SSO enabled: SAML strategy configured');
+Logger.info(`   - Entry Point: ${samlOptions.entryPoint}`);
+Logger.info(`   - Callback URL: ${samlOptions.callbackUrl}`);
+Logger.info(`   - Issuer: ${samlOptions.issuer}`);
+Logger.info(`   - Identifier Format: ${samlOptions.identifierFormat}`);
+Logger.info(`   - Accepted Clock Skew (ms): ${samlOptions.acceptedClockSkewMs}`);
+Logger.info(`   - Idp Cert: ${samlOptions.idpCert ? 'Provided' : 'Not Provided'}`);
+Logger.info(`   - Private Key: ${samlOptions.privateKey ? 'Provided' : 'Not Provided'}`);
diff --git a/src/routers/admin-saml.js b/src/routers/admin-saml.js
@@ -42,7 +42,8 @@ router.get('/admin/saml/metadata', (_, res) => {
     const metadata = generateServiceProviderMetadata({
         issuer: process.env.SAML_ISSUER,
         callbackUrl: process.env.SAML_CALLBACK_URL,
-        publicCerts: process.env.SAML_CERT
+        publicCerts: Buffer.from(process.env.SAML_CERT, 'base64').toString('utf8'),
+        privateKey: process.env.SAML_PRIVATE_KEY ? Buffer.from(process.env.SAML_PRIVATE_KEY, 'base64').toString('utf8') : undefined
     });
     
     res.set('Content-Type', 'application/xml');
diff --git a/src/services/admin.js b/src/services/admin.js
@@ -59,6 +59,7 @@ export async function signUpSaml(userInfo) {
     let admin = await Admin.findUserBySamlId(userInfo.id);
     admin = await Admin.createThirdPartyAccount(
         admin, userInfo, 'saml', '_samlid', checkAdmin);
+
     const jwt = await admin.generateAuthToken();
     return { admin, jwt };
 }

Original file line number	Diff line number	Diff line change
`@@ -8,6 +8,9 @@ export default {`
`8`	`8`	`responses: {`
`9`	`9`	`'302': {`
`10`	`10`	`description: 'Redirect to SAML Identity Provider'`
	`11`	`+ },`
	`12`	`+ '404': {`
	`13`	`+ description: 'SAML not configured'`
`11`	`14`	`}`
`12`	`15`	`}`
`13`	`16`	`}`
`@@ -37,6 +40,9 @@ export default {`
`37`	`40`	`},`
`38`	`41`	`'401': {`
`39`	`42`	`description: 'SAML authentication failed'`
	`43`	`+ },`
	`44`	`+ '404': {`
	`45`	`+ description: 'SAML not configured'`
`40`	`46`	`}`
`41`	`47`	`}`
`42`	`48`	`}`
`@@ -52,6 +58,12 @@ export default {`
`52`	`58`	`'200': {`
`53`	`59`	`description: 'Success',`
`54`	`60`	`content: commonSchemaContent('AdminLoginResponse')`
	`61`	`+ },`
	`62`	`+ '401': {`
	`63`	`+ description: 'Authentication failed'`
	`64`	`+ },`
	`65`	`+ '404': {`
	`66`	`+ description: 'SAML not configured'`
`55`	`67`	`}`
`56`	`68`	`}`
`57`	`69`	`}`
`@@ -71,6 +83,9 @@ export default {`
`71`	`83`	`}`
`72`	`84`	`}`
`73`	`85`	`}`
	`86`	`+ },`
	`87`	`+ '404': {`
	`88`	`+ description: 'SAML not configured'`
`74`	`89`	`}`
`75`	`90`	`}`
`76`	`91`	`}`
Original file line number	Diff line number	Diff line change
`@@ -59,6 +59,7 @@ export async function signUpSaml(userInfo) {`
`59`	`59`	`let admin = await Admin.findUserBySamlId(userInfo.id);`
`60`	`60`	`admin = await Admin.createThirdPartyAccount(`
`61`	`61`	`admin, userInfo, 'saml', '_samlid', checkAdmin);`
	`62`	`+`
`62`	`63`	`const jwt = await admin.generateAuthToken();`
`63`	`64`	`return { admin, jwt };`
`64`	`65`	`}`