From 0944f647ebc04412bc4bbe02808478f63eaab754 Mon Sep 17 00:00:00 2001
From: "Robert St. John" <restjohn@users.noreply.github.com>
Date: Wed, 25 Oct 2023 12:15:41 -0600
Subject: [PATCH] [web-app][service] stop erroneously using old options key in
 saml settings

---
 service/src/authentication/saml.js            | 22 +++++++++++--------
 .../admin-authentication-saml.component.html  | 10 ++++-----
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/service/src/authentication/saml.js b/service/src/authentication/saml.js
index 636a86a69..1a7484393 100644
--- a/service/src/authentication/saml.js
+++ b/service/src/authentication/saml.js
@@ -14,8 +14,8 @@ function configure(strategy) {
 
   const options = {
     path: `/auth/${strategy.name}/callback`,
-    entryPoint: strategy.settings.options.entryPoint,
-    issuer: strategy.settings.options.issuer
+    entryPoint: strategy.settings.entryPoint,
+    issuer: strategy.settings.issuer
   }
 
   if (strategy.settings.cert) {
@@ -128,7 +128,10 @@ function configure(strategy) {
 
   function authenticate(req, res, next) {
     AuthenticationInitializer.passport.authenticate(strategy.name, function (err, user, info = {}) {
-      if (err) return next(err);
+      if (err) {
+        console.error('saml: authentication error', err);
+        return next(err);
+      }
 
       req.user = user;
 
@@ -150,6 +153,9 @@ function configure(strategy) {
 
       // DEPRECATED session authorization, remove req.login which creates session in next version
       req.login(user, function (err) {
+        if (err) {
+          return next(err);
+        }
         AuthenticationInitializer.tokenService.generateToken(user._id.toString(), TokenAssertion.Authorized, 60 * 5)
           .then(token => {
             req.token = token;
@@ -170,7 +176,9 @@ function configure(strategy) {
       let state = {};
       try {
         state = JSON.parse(req.body.RelayState)
-      } catch(ignore) {}
+      } catch (ignore) {
+        console.warn('saml: error parsing RelayState', ignore)
+      }
 
       if (state.initiator === 'mage') {
         if (state.client === 'mobile') {
@@ -210,9 +218,6 @@ function setDefaults(strategy) {
   if (!strategy.settings.profile.id) {
     strategy.settings.profile.id = 'uid';
   }
-  if (!strategy.settings.options) {
-    strategy.settings.options = {};
-  }
 }
 
 function initialize(strategy) {
@@ -321,8 +326,7 @@ function initialize(strategy) {
       req.session = null;
     }
   );
-
-};
+}
 
 module.exports = {
   initialize
diff --git a/web-app/src/app/admin/admin-authentication/admin-authentication-saml/admin-authentication-saml.component.html b/web-app/src/app/admin/admin-authentication/admin-authentication-saml/admin-authentication-saml.component.html
index 142296477..80e044b00 100644
--- a/web-app/src/app/admin/admin-authentication/admin-authentication-saml/admin-authentication-saml.component.html
+++ b/web-app/src/app/admin/admin-authentication/admin-authentication-saml/admin-authentication-saml.component.html
@@ -2,7 +2,7 @@
   <div class="mat-subheading-2">Identity Provider (IdP)</div>
   <mat-form-field appearance="outline">
     <mat-label>Entry Point</mat-label>
-    <input matInput type="text" [readonly]="!editable" [(ngModel)]="strategy.settings.options.entryPoint"
+    <input matInput type="text" [readonly]="!editable" [(ngModel)]="strategy.settings.entryPoint"
       (click)="setDirty(true)">
     <mat-hint align="end">
       Location of the IdP. This can be the location of the single sign-on identity provider.
@@ -11,7 +11,7 @@
 
   <mat-form-field appearance="outline">
     <mat-label>Issuer</mat-label>
-    <input matInput type="text" [readonly]="!editable" [(ngModel)]="strategy.settings.options.issuer"
+    <input matInput type="text" [readonly]="!editable" [(ngModel)]="strategy.settings.issuer"
       (click)="setDirty(true)">
     <mat-hint align="end">
       This is given to the IdP.
@@ -69,7 +69,7 @@
 
   <mat-form-field appearance="outline">
     <mat-label>IdP Issuer</mat-label>
-    <input matInput type="text" [readonly]="!editable" [(ngModel)]="strategy.settings.options.idpIssuer"
+    <input matInput type="text" [readonly]="!editable" [(ngModel)]="strategy.settings.idpIssuer"
       (click)="setDirty(true)">
     <mat-hint align="end">
       The IdP issuer will be validated for incoming Logout Requests/Responses.
@@ -91,7 +91,7 @@
   <mat-form-field appearance="outline">
     <mat-label>Request ID Expiration Period</mat-label>
     <input matInput type="number" [readonly]="!editable"
-      [(ngModel)]="strategy.settings.options.requestIdExpirationPeriodMs" (click)="setDirty(true)">
+      [(ngModel)]="strategy.settings.requestIdExpirationPeriodMs" (click)="setDirty(true)">
     <div class="suffix-spacing" matSuffix>Milliseconds</div>
     <mat-hint align="end">
       Defines the expiration time when a Request ID generated for a SAML request will not be valid if seen in a SAML
@@ -103,7 +103,7 @@
 
   <mat-form-field appearance="outline">
     <mat-label>Logout URL</mat-label>
-    <input matInput type="url" [readonly]="!editable" [(ngModel)]="strategy.settings.options.logoutUrl"
+    <input matInput type="url" [readonly]="!editable" [(ngModel)]="strategy.settings.logoutUrl"
       (click)="setDirty(true)">
     <mat-hint align="end">
       Base address to call with logout requests. Default is Entry Point.