Skip to content
This repository has been archived by the owner on Jun 27, 2019. It is now read-only.

8.x OAuth fails to find access_token which worked in 7.x #428

Closed
xt1 opened this issue May 9, 2019 · 3 comments · Fixed by zapier/zapier-platform-core#155
Closed

8.x OAuth fails to find access_token which worked in 7.x #428

xt1 opened this issue May 9, 2019 · 3 comments · Fixed by zapier/zapier-platform-core#155
Labels

Comments

@xt1
Copy link

xt1 commented May 9, 2019

With platform 7.6.1 and earlier our OAuth responses worked fine, but after upgrading our app to 8.0.1 or 8.1.0 the OAuth response is not parsed as before, and the connection attempt fails.

This means that no new users can set up zaps.
Existing users (with a refresh token) continue to work fine.
It is only when Connect an Account is opened that the login process runs, and this used to result in a valid access and refresh token.

module.exports = {
  type: 'oauth2',
  oauth2Config: {
    // Step 1 of the OAuth flow; specify where to send the user to authenticate with your API.
    // Zapier generates the state and redirect_uri, you are responsible for providing the rest.
    // Note: can also be a function that returns a string
    authorizeUrl: {
      method: 'GET',
      url: '{{process.env.BASE_URL}}/oauth/authorize',
      params: {
        client_id: '{{process.env.CLIENT_ID}}',
        state: '{{bundle.inputData.state}}',
        redirect_uri: '{{bundle.inputData.redirect_uri}}',
        response_type: 'code',
        response_mode: 'query',
        scope: 'openid'
      }
    },
    // Step 2 of the OAuth flow; Exchange a code for an access token.
    // This could also use the request shorthand.
    getAccessToken: getAccessToken,
    refreshAccessToken: refreshAccessToken,
    autoRefresh: true,
    scope: 'openid'
  },

results in

image

Tests that work fine under 7.6.1 fail under 8.1.0 with the following

Unhandled rejection TypeError [ERR_INVALID_ARG_TYPE]: The first argument must be one of type string, Buffer, ArrayBuffer, Array, or Array-like Object. Received type object
    at Function.from (buffer.js:232:9)
    at _.reduce (D:\Git\Apps\ZapierApp\node_modules\zapier-platform-core\src\tools\create-logger.js:114:21)
    at arrayReduce (D:\Git\Apps\ZapierApp\node_modules\lodash\lodash.js:683:21)
    at Function.reduce (D:\Git\Apps\ZapierApp\node_modules\lodash\lodash.js:9683:14)
    at makeSensitiveBank (D:\Git\Apps\ZapierApp\node_modules\zapier-platform-core\src\tools\create-logger.js:105:12)
    at sendLog (D:\Git\Apps\ZapierApp\node_modules\zapier-platform-core\src\tools\create-logger.js:129:25)
    at logErrorAndCallbackOnce (D:\Git\Apps\ZapierApp\node_modules\zapier-platform-core\src\tools\create-lambda-handler.js:173:7)
    at loadApp.then.then.catch.err (D:\Git\Apps\ZapierApp\node_modules\zapier-platform-core\src\tools\create-lambda-handler.js:217:11)
    at bound (domain.js:395:14)
    at runBound (domain.js:408:12)
From previous event:
    at Domain.handlerDomain.run (D:\Git\Apps\ZapierApp\node_modules\zapier-platform-core\src\tools\create-lambda-handler.js:213:15)
    at Domain.run (domain.js:342:14)
    at handler (D:\Git\Apps\ZapierApp\node_modules\zapier-platform-core\src\tools\create-lambda-handler.js:192:19)
    at ZapierPromise (D:\Git\Apps\ZapierApp\node_modules\zapier-platform-core\src\tools\create-app-tester.js:27:7)
From previous event:
    at event (D:\Git\Apps\ZapierApp\node_modules\zapier-platform-core\src\tools\create-app-tester.js:26:12)
    at D:\Git\Apps\ZapierApp\node_modules\zapier-platform-core\src\tools\create-app-tester.js:70:12
    at Context.it (D:\Git\Apps\ZapierApp\test\mock\authentication.js:210:9)
    at callFnAsync (D:\Git\Apps\ZapierApp\node_modules\mocha\lib\runnable.js:400:21)
    at Test.Runnable.run (D:\Git\Apps\ZapierApp\node_modules\mocha\lib\runnable.js:342:7)
    at Runner.runTest (D:\Git\Apps\ZapierApp\node_modules\mocha\lib\runner.js:455:10)
    at D:\Git\Apps\ZapierApp\node_modules\mocha\lib\runner.js:573:12
    at next (D:\Git\Apps\ZapierApp\node_modules\mocha\lib\runner.js:369:14)
    at D:\Git\Apps\ZapierApp\node_modules\mocha\lib\runner.js:379:7
    at next (D:\Git\Apps\ZapierApp\node_modules\mocha\lib\runner.js:303:14)
    at Immediate.<anonymous> (D:\Git\Apps\ZapierApp\node_modules\mocha\lib\runner.js:347:5)
    at runCallback (timers.js:705:18)
    at tryOnImmediate (timers.js:676:5)
    at processImmediate (timers.js:658:5)
    at process.topLevelDomainCallback (domain.js:120:23)

Test source:

   let apiMock = nock('https://sod.superoffice.com/login/common');
   it('Requesting for refresh token', (done) => {
        const bundle = {
            inputData: {
                id: 578,
                redirect_uri: 'https://zapier.com/dashboard/auth/oauth/return/App4291CLIAPI/'
            },
            authData: {
                refresh_token: '123REFRESH4567890123456789012345678901234567890',
                url: 'https://sod.superoffice.com/Cust23619/api/'
            },
            environment: {
                CLIENT_ID: 'b2b1047e7984',
                CLIENT_SECRET: '68e089bf65310'
            }
        };
        apiMock.post('/oauth/tokens').reply(200, {
            access_token: '123ACCESS4567890123456789012345678901234567890'
        });
        appTester(App.authentication.oauth2Config.refreshAccessToken, bundle)
            .then((result) => {
                should.exist(result.access_token);
                result.access_token.should.eql('123ACCESS4567890123456789012345678901234567890');
                done();
            })
            .catch(done);
    });

What would you like to change?

The OAuth handler should continue to handle responses as before (in platform 7.x)

Your Environment

  • CLI Version (zapier -v): zapier-platform-cli/8.1.0 zapier-platform-core/8.1.0 node/v10.15.3
  • App id: App4291 - v1.0.3 is 8.1.0
  • npm 6.9.0
  • node 10.15.3
@xavdid
Copy link
Contributor

xavdid commented May 9, 2019

@xt1 thanks for reporting, I think I see where we added the bug. We'll get that taken care of.

In the meantime, you should be able to re-promote an older version of the app that worked as expected. That way, users won't notice an issue. Thanks for your patience here!

@xavdid xavdid added the bug label May 9, 2019
@zapzap
Copy link

zapzap commented May 9, 2019

This issue has been copied into our private issue (as PDE-887) tracker. Thanks for the report! We'll update this as we learn more.

@xavdid
Copy link
Contributor

xavdid commented May 29, 2019

EDIT: I've figured it out, no need to do the below.


Ok! So i've found the root issue here and have fixed it, but i'm not sure what we're tripping over.

When you have a chance, do you mind editing your test to try to pull out what we're tripping over?

Make the following changes:

  1. change it('Re... to it.only('Re... in the test
  2. In this file in node_modules, add: console.dir(val) directly before the linked line
  3. run zapier-test and hopefully there'll be an object right before the program crashes. I'm curious what it is or where it came from.

Either way we can fix the bug, but knowing what's sneaking in there will help us prevent it in the future.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants