Merge pull request #41 from poloka/master

mhemesath · web-flow · commit 8c027aec1dbe · 2019-08-29T14:39:45.000-05:00
Fix verifyChallenge to properly handle and return the Promise.reject scenario
diff --git a/README.md b/README.md
@@ -199,6 +199,17 @@ XFC.Provider.init({
 })
 ```
 
+If the case of secret validation failures, acceptance is a new Error
+
+```js
+XFC.Provider.init({
+  acls: ['*'],
+  secret: function(secret) {
+    return Promise.reject(new Error('Failure'));
+  }
+})
+```
+
 If the app is using an alternate form a security and does require XFC to provide clickjacking support, a wildcard with no secret may be passed. Under these conditions, XFC will not hide the content and the consumer will automatically be authorized.
 
 ```js
diff --git a/example/initialization/1_b_index.html b/example/initialization/1_b_index.html
@@ -56,13 +56,14 @@ <h2>launched app</h2>
         <h2>authorized app</h2>
       </div>
     </div>
-    <p>The three examples below show usage of secret based authorization.  This means that the origin of the URL does not matter. The first example shows that a string literal can be used as a secret.  It shows that when the secret is incorrect, no content is displayed.  The second example shows a working example using string literal secret that works.  The third example shows that a secret function can be used to verify the secret.
+    <p>The four examples below show usage of secret based authorization.  This means that the origin of the URL does not matter. The first example shows that a string literal can be used as a secret.  It shows that when the secret is incorrect, no content is displayed.  The second example shows a working example using string literal secret that works.  The third example shows that a secret function can be used to verify the secret.  The fourth example shows how a failure response from the secret validation is handled and no content is displayed.
     </p>
     <script>
       XFC.Consumer.init()
       XFC.Consumer.mount(document.body, 'http://localprovider.com:8080/example/initialization/1_b_secret_string_literal.html', {secret: 'abc12367'});
       XFC.Consumer.mount(document.body, 'http://localprovider.com:8080/example/initialization/1_b_secret_string_literal.html', {secret: 'abc123'});
       XFC.Consumer.mount(document.body, 'http://localprovider.com:8080/example/initialization/1_b_secret_function.html', {secret: 'OAuth Header'});
+      XFC.Consumer.mount(document.body, 'http://localprovider.com:8080/example/initialization/1_b_secret_function_failure.html', {secret: 'OAuth Header'});
     </script>
   </body>
 </html>
diff --git a/example/initialization/1_b_secret_function_failure.html b/example/initialization/1_b_secret_function_failure.html
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<html lang="en-us">
+  <head>
+    <title>HealtheLife Trusted Site</title>
+    <meta charset="utf-8" />
+    <style>
+      body {
+        padding-bottom: 5px;
+      }
+      html[hidden] { display: none; }
+    </style>
+    <script src="http://localhost:8080/xfc.js"></script>
+    <script src="https://cdnjs.cloudflare.com/ajax/libs/babel-polyfill/6.23.0/polyfill.min.js"></script>
+  </head>
+  <body>
+    <div>
+      <h1>1. Initialization</h1>
+      <p>b. Enable parent frame authorization through a domain or a secret.</p>
+      <p>i. The app can set the secret as a function.</p>
+    </div>
+    <script type="text/javascript">
+      XFC.Provider.init({
+        acls: ['*'],
+        secret: function(secret) {
+          return Promise.reject(new Error('Failure'));
+        }
+      });
+    </script>
+  </body>
+</html>
diff --git a/package-lock.json b/package-lock.json
diff --git a/src/provider/application.js b/src/provider/application.js
@@ -246,8 +246,9 @@ class Application extends EventEmitter {
     if (typeof this.secret === 'string' && fixedTimeCompare(this.secret, secretAttempt)) {
       authorize();
     } else if (typeof this.secret === 'function') {
-      this.secret.call(this, secretAttempt).then(authorize);
+      return this.secret.call(this, secretAttempt).then(authorize);
     }
+    return Promise.resolve();
   }
 
   /**
diff --git a/test/application.js b/test/application.js
@@ -199,6 +199,17 @@ describe('Application', () => {
 
           application.verifyChallenge("123");
         });
+
+        it("handles failure from this.secret", (done) => {
+          const error = new Error('promise rejected');
+          const secret = (secretAttempt) => Promise.reject(error);
+          application.init({secret});
+          expect(application.emitError).to.have.been.called;
+          application.verifyChallenge("123").catch((err) => {
+            expect(err).to.equal(error);
+            done();
+          });
+        });
       });
     });
 

Original file line number	Diff line number	Diff line change
`@@ -246,8 +246,9 @@ class Application extends EventEmitter {`
`246`	`246`	`if (typeof this.secret === 'string' && fixedTimeCompare(this.secret, secretAttempt)) {`
`247`	`247`	`authorize();`
`248`	`248`	`} else if (typeof this.secret === 'function') {`
`249`		`- this.secret.call(this, secretAttempt).then(authorize);`
	`249`	`+ return this.secret.call(this, secretAttempt).then(authorize);`
`250`	`250`	`}`
	`251`	`+ return Promise.resolve();`
`251`	`252`	`}`
`252`	`253`
`253`	`254`	`/**`