lwitzani / homebridgeStatusWidget

MIT License
371 stars 143 forks source link

Added 2FA, fix bugs, auto format file #37

Open LoSunny opened 3 years ago

LoSunny commented 3 years ago

The access token will be stored in the CONFIGURATION file, such that the token can be reused and doesn't need to be updated each time.

LoSunny commented 3 years ago

For those who want to use this fork before being pulled, please click the link below https://scriptdu.de/?name=Homebridge%20Status&source=https://raw.githubusercontent.com/LoSunny/homebridgeStatusWidget/main/homebridgeStatusWidget.js&docs=https://github.com/lwitzani/homebridgeStatusWidget#generator

lwitzani commented 3 years ago

Hey, Thanks for that. Will take a look at it and probably merge it 👍🏻

lwitzani commented 3 years ago

I looked at the code and run it on my phone. I don't quite get how this will work indefinitely? After your first successful authentication with an OTP the token is now saved in the configuration. Now in the getAuthToken() function you first check if the saved token is still valid (and if yes it is used). At the time when it is not valid anymore, the authentication without any credentials is checked and if that does not lead to a new token then the saved credentials are again used (like the very first time). But now the OTP is maybe not valid anymore so why would a request now lead to a new auth token?

I always thought that for 2FA support, i would need some logic in the code which calculates the current valid OTP based on the shared secret.

Thanks for explaining

LoSunny commented 3 years ago

I looked at the code and run it on my phone. I don't quite get how this will work indefinitely? This will not work indefinitely, once the access token expires, you'll need to re-enter the 2FA code and run the script again

But now the OTP is maybe not valid anymore so why would a request now lead to a new auth token? Because we still need to take care of those who didn't use 2FA to generate a new access token base on their username and password.

And for those who have 2FA enabled, this message popup image And they need to re-enter the 2FA code and run the script again

--EDIT-- To prevent the access token from expiring, we can increase the time for the session in the config.json of homebridge-config-ui-x as state in https://github.com/oznu/homebridge-config-ui-x/wiki/Config-Options#sessiontimeout The maximum time allowed is 86400000 seconds (source)

lwitzani commented 3 years ago

okay setting the session timeout to 86400000s seems to be a reasonable solution ;) would just have to add this to the readme then which would be no problem.

I noticed the problem that you cannot run the script inside the Scriptable app twice in a row when overwritePersistedConfig is true...this is probably not a good idea to have since playing around with e.g. the color settings (change color, run script, change color, run script, ...) would be a huge pain in the ass :) ...

LoSunny commented 3 years ago

Maybe we should add to the docs? change back overwritePersistedConfig to false once you finish running the script to save the config

lwitzani commented 3 years ago

I know that it works after that. But the User will be confused... The way it is in your branch -> it prevents playing around with settings

LoSunny commented 3 years ago

I'm actually debugging out the problem. It seems that Scriptable has some bugs when it goes to Reading files from cache, Reading files from disk, Saving files to disk.

What am I trying to do

But I've face some problems and I've pushed a commit to my dev branch. Here are the ways to reproduce the error

  1. Make sure there is a config.json file
  2. Run the script with overwritePersistedConfig true and a valid otp
  3. In the logs, you'll see the word Saving, and then the file content to be saved, which includes the accessToken
  4. You can double-check the config.json content in the Files app
  5. Re-run the script, which should have the word Read config, and Returning result, the line after Returning result won't have the accessToken
  6. But there isn't any "Save to disk" action before the "Read config" action? So why did it disappears
LoSunny commented 3 years ago

I've re-coded the whole 2FA run down, which now can fetch the 2FA code from Authy by my new script (Sorry for those who use Google Authenticator or others) The variable twoFactorAuthentication is to see if need to enable 2FA.

The function is to check if they use the parameter method and enable 2FA, which is impossible as need to use the access token. The reason we need to use an access token instead of re-auth(ing) each time is that a 2FA token can be only used once. If you try to re-auth with the same 2FA code, it will throw an error and you need to wait for 30 seconds for a new 2FA code

if (twoFactorAuthentication && foundCredentialsInParameter) {
  throw ('You cannot use this method if you want to enable 2FA');
}

If the user disables usePersistedConfiguration, it will only fetch the access token from the config and uses the configuration from the script

if (!usePersistedConfiguration && twoFactorAuthentication) {
  CONFIGURATION.access_token = (await getPersistedObject(fm, pathToConfig, CONFIGURATION_JSON_VERSION, CONFIGURATION, false)).access_token;
  log('Loaded access_token from config');
}