Fix #559, improve security for secrets

CHANGELOG.md

@@ -42,6 +42,10 @@ Format:
 ### Added
 - Appends the results of the cucumber [summary formatter](https://github.com/cucumber/cucumber-js/blob/main/docs/formatters.md#summary) to the `debug_log.txt`, which includes useful stack traces into the Kiln code when tests fail.
 
+### Fixed
+- Prevent login info from being saved in the report or screenshots being taken on error. [#599](https://github.com/SuffolkLITLab/ALKiln/issues/559).
+- Prevent error screenshots of screens that used a secret.
+
 ## [4.3.0] - 2022-04-07
 ### Added
 - GitHub environment variable `MAX_SECONDS_FOR_SETUP` to set a custom maximum time for setup and takedown for packages that take a long time to load.

lib/scope.js

@@ -276,9 +276,15 @@ module.exports = {
       if ( options.waitForShowIf ) { await scope.waitForShowIf( scope ); }
     }  // ends if scope.page
 
-    // Reset navigation (TODO: discuss use for observational steps)
-    if ( options.navigated ) { scope.navigated = true; }  // Consider `daPageLoad` event
-    else { scope.navigated = false; }
+    if ( options.navigated ) {
+      // Reset navigation (TODO: discuss use for observational steps)
+      // TODO: Consider `daPageLoad` event
+      scope.navigated = true;
+      // Allow screenshots of the next page (may have been disabled in order to set secret values)
+      scope.disable_error_screenshot = false;
+    } else {
+      scope.navigated = false;
+    }
 
     // Plain old waiting. Here to avoid doing this in each individual place
     if ( options.waitForTimeout ) { await time.waitForTimeout( options.waitForTimeout ); }
@@ -1836,6 +1842,8 @@ module.exports = {
 
     set_secret_var: async ( scope, var_name, set_to_env_name ) => {
       /** Sets a non-story table variable to a "secret" value (i.e. an env var) */
+      // Prevent pictures of a screen with a secret.
+      scope.disable_error_screenshot = true;
       let var_data = await scope.normalizeTable( scope, {
               var_data: [{
                 var: var_name,
@@ -1963,6 +1971,9 @@ module.exports = {
       /** Allow the developer to log a user into their server using GitHub
       * secrets to authenticate.
       */
+      // Don't take a picture of a failed login in case one of the inputs is correct
+      scope.disable_error_screenshot = true;
+
       // If there is no browser open, start a new one
       if (!scope.browser) {
         scope.browser = await scope.driver.launch({ headless: !session_vars.DEBUG });
@@ -2003,8 +2014,10 @@ module.exports = {
 
       await scope.addToReport( scope, {
         type: `row`,
-        value: `Logged into ${ session_vars.get_da_server_url() }/user/sign-in as a ${ email } with ${ password }.`
+        value: `Signed into ${ session_vars.get_da_server_url() }/user/sign-in successfully.`
       });
+
+      await scope.afterStep(scope, {waitForShowIf: false});
     },  // Ends sign_in
 
   },  // ends scope.steps

lib/steps.js

@@ -83,6 +83,7 @@ Before(async (scenario) => {
   // Device interaction
   scope.device = 'pc';
   scope.activate = click_with[ scope.device ];
+  scope.disable_error_screenshot = false;
 
   scope.page_id = null;
 
@@ -882,11 +883,19 @@ After(async function(scenario) {
 
     await scope.addToReport(scope, { type: `outcome`, value: `**-- Scenario Failed --**` });
     if ( scope.page ) {
-      // Save/download a picture of the screen that's showing during the unexpected status
-      let safe_filename = await scope.getSafeScenarioFilename( scope, { prefix: `error` });
-      await scope.page.screenshot({ path: `${ safe_filename }.jpg`, type: `jpeg`, fullPage: true });
-    }
-  }  // Ends if not passed
+      if ( scope.disable_error_screenshot ) {
+        scope.addToReport(scope, {
+          type: `row`,
+          value: `For security, ALKiln will not create a screenshot for this error. It's possible a secret is being used on this screen.`
+        });
+      } else {
+        // Save/download a picture of the screen that's showing during the unexpected status
+        let safe_filename = await scope.getSafeScenarioFilename( scope, { prefix: `error` });
+        await scope.page.screenshot({ path: `${ safe_filename }.jpg`, type: `jpeg`, fullPage: true });
+      }  // ends if scope.disable_error_screenshot
+    }  // ends if scope.page exists
+
+  }  // ends if not passed
 
   if (scenario.result.status === `FAILED`) {
     await scope.addToReport(scope, { type: `outcome`, value: `**-- Scenario Failed --**` });

tests/features/reports.feature

@@ -346,7 +346,7 @@ Scenario: I can't match JSON page var to str
   """
 
 @fast @rf22 @signin
-Scenario: Fail with wrong email secret
+Scenario: Fail with wrong email secret name
   Given the final Scenario status should be "failed"
   Given the Scenario report should include:
   """
@@ -355,7 +355,7 @@ Scenario: Fail with wrong email secret
   Given I log on with the email "WRONG_EMAIL_NAME" and the password "USER1_PASSWORD"
 
 @fast @rf23 @signin
-Scenario: Fail with wrong password secret
+Scenario: Fail with wrong password secret name
   Given the final Scenario status should be "failed"
   Given the Scenario report should include:
   """
@@ -364,7 +364,7 @@ Scenario: Fail with wrong password secret
   Given I sign in with the email "USER1_EMAIL" and the password "WRONG_PASSWORD_NAME"
 
 @fast @rf24 @signin
-Scenario: Fail with 2 wrong signin secrets
+Scenario: Fail with 2 wrong signin secret names
   Given the final Scenario status should be "failed"
   Given the Scenario report should include:
   """
@@ -393,6 +393,10 @@ Scenario: Fail to find var while keeping value secret
   """
   Did not find a field on this page for the variable "missing_var" that could be set to "SECRET_FOR_MISSING_FIELD"
   """
+  And the Scenario report should include:
+  """
+  For security, ALKiln will not create a screenshot for this error.
+  """
   And I start the interview at "test_secret_vars"
   And I set the var "missing_var" to the secret "SECRET_FOR_MISSING_FIELD"
 
@@ -518,3 +522,11 @@ Scenario: Report lists unused table rows
     | button_continue | True |  |
     | buttons_other | button_2 |  |
     | buttons_yesnomaybe | True |  |
+
+@fast @rp3 @signin
+Scenario: Sign in to server successfully
+  Given the Scenario report should include:
+  """
+  Signed in
+  """
+  Given I sign in with the email "USER1_EMAIL" and the password "USER1_PASSWORD"