balansse / homebridge-vivint

Integrates Vivint security system with Apple Home
Other
45 stars 15 forks source link

Handle session cookies by adding a filter on cookie name #128

Closed zackman0010 closed 8 months ago

zackman0010 commented 8 months ago

Vivint is now sending an extra cookie for every request. This means that trying to grab the first cookie isn't always guaranteed to grab the session cookie. In order to fix this, I added a filter to find the Set-Cookie headers that start with s=. This will make sure to only grab the correct cookie, ignoring any other cookies that may get added.

jgrimard commented 8 months ago

@zackman0010 Has this code been tested? I'm getting the following error when attempting to generate a new refresh token.

[10/21/2023, 2:35:03 PM] [Homebridge UI] [@balansse/homebridge-vivint] Error in MFALogin TypeError: loginResponse.headers.set-cookie[0].filter is not a function
    at VivintMFA.MFALogin (/home/homebridge/homebridge-vivint/homebridge-vivint/lib/vivint_mfa.js:28:72)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
main.js:23848
[10/21/2023, 2:35:03 PM] [Homebridge UI] [@balansse/homebridge-vivint] TypeError: loginResponse.headers.set-cookie[0].filter is not a function
    at VivintMFA.MFALogin (/home/homebridge/homebridge-vivint/homebridge-vivint/lib/vivint_mfa.js:28:72)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
main.js:23848
Error in MFALogin TypeError: loginResponse.headers.set-cookie[0].filter is not a function
    at VivintMFA.MFALogin (/home/homebridge/homebridge-vivint/homebridge-vivint/lib/vivint_mfa.js:28:72)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {stack: 'TypeError: loginResponse.headers.set-cookie[0…ions (node:internal/process/task_queues:95:5)', message: 'loginResponse.headers.set-cookie[0].filter is not a function'}

vivint_mfa.js:52
TypeError: loginResponse.headers.set-cookie[0].filter is not a function
    at VivintMFA.MFALogin (/home/homebridge/homebridge-vivint/homebridge-vivint/lib/vivint_mfa.js:28:72)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {stack: 'TypeError: loginResponse.headers.set-cookie[0…ions (node:internal/process/task_queues:95:5)', message: 'loginResponse.headers.set-cookie[0].filter is not a function'}
zackman0010 commented 8 months ago

Strange, I tested the change locally. I may have copied something wrong by mistake when making the PR. Let me take a look.

zackman0010 commented 8 months ago

@jgrimard Thanks for pointing that out, I did have a copy/paste error. I was in a rush this morning, my wife was standing next to me ready to leave the house, lol.

jgrimard commented 8 months ago

@jgrimard Thanks for pointing that out, I did have a copy/paste error. I was in a rush this morning, my wife was standing next to me ready to leave the house, lol.

I hear you. Thats why I couldn't fix it earlier yesterday.

jgrimard commented 8 months ago

@zackman0010 vivint_mfa_cli.js is still broken. We need to fix the second part. Can you replace line 51 with the following?

    newRefreshToken = response.headers["set-cookie"].filter((cookie) => cookie.startsWith("s="))[0];
    if (newRefreshToken) {
      refreshToken = newRefreshToken.split(";")[0];
    }

Unfortunately I haven't been able to test the CLI or the plugin with MFA yet. It isn't requiring an MFA code for either of my logins.

zackman0010 commented 8 months ago

@jgrimard Fixed that as well, thanks! Another consequence of the rush job. I also wasn't able to test MFA locally, although I really should enable it.

jgrimard commented 8 months ago

@zackman0010 Looks great. Thanks for the updates. I just tested with MFA turned on and all is good.

@balansse I have reviewed and tested these changes. Everything is good to merge and push to NPM whenever you have a moment.