fix(webex-core): re-evaluate credentials on app level redirects

packages/@webex/webex-core/src/interceptors/redirect.js

@@ -11,6 +11,34 @@ const requestHeaderName = 'cisco-no-http-redirect';
 const responseHeaderName = 'cisco-location';
 const LOCUS_REDIRECT_ERROR = 2000002;
 const APPAPI_REDIRECT_ERROR = 404100;
+// The status code both body based redirects are documented to arrive with, and
+// the one `HttpStatusInterceptor` pairs each of the error codes above with.
+const REDIRECT_ERROR_STATUS_CODE = 404;
+
+/**
+ * Copies the options of a request that is about to be re-issued against a uri
+ * taken from the previous response.
+ *
+ * The copy drops any `authorization` header carried over from the previous
+ * request. `AuthInterceptor` returns early when a request already carries that
+ * header, so a carried-over header means the interceptor never evaluates the
+ * new uri. Dropping it lets the interceptor make its normal decision against
+ * the uri that is actually about to be requested.
+ *
+ * @param {Object} options - The options of the request being redirected.
+ * @returns {Object} - A copy of the options, without inherited credentials.
+ */
+function cloneOptionsForRedirect(options) {
+  const redirectOptions = clone(options);
+
+  // `clone()` is shallow, so `headers` is still the previous request's object.
+  if (redirectOptions.headers) {
+    redirectOptions.headers = {...redirectOptions.headers};
+    Reflect.deleteProperty(redirectOptions.headers, 'authorization');
+  }
+
+  return redirectOptions;
+}
 
 /**
  * @class
@@ -65,7 +93,7 @@ export default class RedirectInterceptor extends Interceptor {
   onResponse(options, response) {
     /* eslint-disable no-else-return */
     if (response.headers && response.headers[responseHeaderName]) {
-      options = clone(options);
+      options = cloneOptionsForRedirect(options);
       options.uri = response.headers[responseHeaderName];
       options.$redirectCount += 1;
       if (options.$redirectCount > this.webex.config.maxAppLevelRedirects) {
@@ -74,12 +102,13 @@ export default class RedirectInterceptor extends Interceptor {
 
       return this.webex.request(options);
     } else if (
+      response.statusCode === REDIRECT_ERROR_STATUS_CODE &&
       response.headers &&
       response.body &&
       response.body.errorCode === LOCUS_REDIRECT_ERROR &&
       response.body.location
     ) {
-      options = clone(options);
+      options = cloneOptionsForRedirect(options);
 
       this.webex.logger.warn('redirect: url redirects needed from', options.uri);
       if (response.options && response.options.qs) {
@@ -100,13 +129,14 @@ export default class RedirectInterceptor extends Interceptor {
 
       return this.webex.request(options);
     } else if (
+      response.statusCode === REDIRECT_ERROR_STATUS_CODE &&
       response.headers &&
       response.body &&
       response.body.code === APPAPI_REDIRECT_ERROR &&
       response.body.data &&
       response.body.data.siteFullUrl
     ) {
-      options = clone(options);
+      options = cloneOptionsForRedirect(options);
 
       this.webex.logger.warn('redirect: url redirects needed from', options.uri);
       if (response.options && response.options.qs) {

packages/@webex/webex-core/test/unit/spec/interceptors/redirect.js

@@ -7,8 +7,16 @@
 import sinon from 'sinon';
 import {assert} from '@webex/test-helper-chai';
 import MockWebex from '@webex/test-helper-mock-webex';
-import {RedirectInterceptor, config, Credentials} from '@webex/webex-core';
-import {cloneDeep} from 'lodash';
+import {
+  AuthInterceptor,
+  RedirectInterceptor,
+  config,
+  Credentials,
+  Services,
+  ServicesV2,
+  Token,
+} from '@webex/webex-core';
+import {cloneDeep, merge} from 'lodash';
 
 describe('webex-core', () => {
   describe('Interceptors', () => {
@@ -20,8 +28,8 @@ describe('webex-core', () => {
           children: {
             credentials: Credentials,
           },
-          config: cloneDeep(config),
-          request: sinon.spy(),
+          config: merge(cloneDeep(config), {credentials: {client_secret: 'fake'}}),
+          request: sinon.stub().resolves({}),
         });
 
         interceptor = Reflect.apply(RedirectInterceptor.create, webex, []);
@@ -97,6 +105,20 @@ describe('webex-core', () => {
           assert.equal(interceptor.onResponse({$redirectCount: 5}, response), response);
         });
 
+        it('returns when a locus redirect body arrives on a response that is not a 404', () => {
+          const response = {
+            statusCode: 200,
+            headers: {},
+            body: {
+              errorCode: 2000002,
+              location: 'http://newlocus.example.com',
+            },
+          };
+
+          assert.equal(interceptor.onResponse({$redirectCount: 0}, response), response);
+          assert.notCalled(webex.request);
+        });
+
         it('redirects GET requests to new url on appapi redirect error', () => {
           const response = {
             statusCode: 404,
@@ -169,6 +191,28 @@ describe('webex-core', () => {
           });
         });
 
+        it('returns when an appapi redirect body arrives on a response that is not a 404', () => {
+          const response = {
+            statusCode: 200,
+            headers: {},
+            body: {
+              code: 404100,
+              data: {
+                siteFullUrl: 'newlocus.example.com'
+              },
+            },
+          };
+
+          assert.equal(
+            interceptor.onResponse(
+              {$redirectCount: 0, uri: 'https://test.webex.com/meet/v1/join'},
+              response
+            ),
+            response
+          );
+          assert.notCalled(webex.request);
+        });
+
         it('removes authorization header when redirecting preJoin request to webex-appapi-service', () => {
           const response = {
             statusCode: 404,
@@ -203,7 +247,7 @@ describe('webex-core', () => {
           }));
         });
 
-        it('keeps authorization header for non-preJoin requests on appapi redirect', () => {
+        it('does not forward the authorization header for non-preJoin requests on appapi redirect', () => {
           const response = {
             statusCode: 404,
             headers: {},
@@ -229,13 +273,13 @@ describe('webex-core', () => {
           sinon.assert.calledWith(webex.request, sinon.match({
             $redirectCount: 1,
             uri: 'https://newlocus.example.com/meet/v1/join',
-            headers: {
-              authorization: 'Bearer token123',
-            },
           }));
+          assert.notProperty(webex.request.firstCall.args[0].headers, 'authorization');
+          // the options of the request being redirected are left untouched
+          assert.equal(options.headers.authorization, 'Bearer token123');
         });
 
-        it('keeps authorization header for preJoin requests to non-webex-appapi-service', () => {
+        it('does not forward the authorization header for preJoin requests to non-webex-appapi-service', () => {
           const response = {
             statusCode: 404,
             headers: {},
@@ -261,10 +305,123 @@ describe('webex-core', () => {
           sinon.assert.calledWith(webex.request, sinon.match({
             $redirectCount: 1,
             uri: 'https://newlocus.example.com/meet/v1/preJoin',
-            headers: {
-              authorization: 'Bearer token123',
-            },
           }));
+          assert.notProperty(webex.request.firstCall.args[0].headers, 'authorization');
+          assert.equal(options.headers.authorization, 'Bearer token123');
+        });
+      });
+
+      describe('#onResponse() against a real service catalog', () => {
+        // Whether a request carries credentials is decided by the auth
+        // interceptor, and it returns early when the request already has an
+        // authorization header. So asserting that the header is not forwarded
+        // is only half of it: these cases run the redirected request back
+        // through the auth interceptor, with a real catalog and real
+        // allowed-domain methods, to show that the decision is remade against
+        // the uri taken from the response.
+        const inheritedToken = 'Bearer inherited-token';
+        const allowedHost = 'locus-eu.webex.com';
+        const otherHost = 'unrelated.example';
+
+        const responses = {
+          'a cisco-location header': (host) => ({
+            statusCode: 404,
+            headers: {'cisco-location': `https://${host}/resource`},
+          }),
+          'a locus redirect error': (host) => ({
+            statusCode: 404,
+            headers: {},
+            body: {
+              errorCode: 2000002,
+              location: `https://${host}/resource`,
+            },
+          }),
+          'an appapi redirect error': (host) => ({
+            statusCode: 404,
+            headers: {},
+            body: {
+              code: 404100,
+              data: {siteFullUrl: host},
+            },
+          }),
+        };
+
+        [
+          {name: 'Services', Constructor: Services},
+          {name: 'ServicesV2', Constructor: ServicesV2},
+        ].forEach(({name, Constructor}) => {
+          describe(name, () => {
+            let authInterceptor, getUserToken;
+
+            beforeEach(() => {
+              const services = new Constructor(undefined, {parent: webex});
+
+              services._getCatalog().setAllowedDomains(['webex.com']);
+              // the catalog holds no services, so a url that is not covered by
+              // an allowed domain has nothing else to authorize it
+              services.waitForService = sinon.stub().rejects(new Error('no such service'));
+
+              webex.internal.services = services;
+              webex.credentials.supertoken = new Token(
+                {
+                  access_token: 'ST1',
+                  token_type: 'Bearer',
+                },
+                {parent: webex}
+              );
+
+              getUserToken = sinon.spy(webex.credentials, 'getUserToken');
+              authInterceptor = Reflect.apply(AuthInterceptor.create, webex, []);
+            });
+
+            afterEach(() => {
+              getUserToken.restore();
+              delete webex.internal.services;
+            });
+
+            Object.keys(responses).forEach((trigger) => {
+              describe(`redirected by ${trigger}`, () => {
+                // Redirects the request and returns the options the redirected
+                // request was issued with.
+                const redirect = (host) => {
+                  const options = {
+                    $redirectCount: 0,
+                    uri: 'https://api.webex.com/resource',
+                    headers: {authorization: inheritedToken},
+                  };
+
+                  interceptor.onResponse(options, responses[trigger](host));
+
+                  assert.calledOnce(webex.request);
+                  // the options of the request being redirected are left untouched
+                  assert.equal(options.headers.authorization, inheritedToken);
+
+                  return webex.request.firstCall.args[0];
+                };
+
+                it('does not forward the authorization header', () => {
+                  assert.notProperty(redirect(allowedHost).headers, 'authorization');
+                });
+
+                it('authorizes the redirected request when the new uri is under an allowed domain', () =>
+                  authInterceptor.onRequest(redirect(allowedHost)).then((options) => {
+                    assert.equal(options.uri, `https://${allowedHost}/resource`);
+                    assert.equal(
+                      options.headers.authorization,
+                      webex.credentials.supertoken.toString()
+                    );
+                    assert.calledOnce(getUserToken);
+                  }));
+
+                it('does not authorize the redirected request when the new uri is not', () =>
+                  authInterceptor.onRequest(redirect(otherHost)).then((options) => {
+                    assert.equal(options.uri, `https://${otherHost}/resource`);
+                    assert.notProperty(options.headers, 'authorization');
+                    assert.notCalled(getUserToken);
+                  }));
+              });
+            });
+          });
         });
       });
     });