Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow adding integration with master token alone #339

Closed
leikoilja opened this issue Sep 17, 2021 · 9 comments
Closed

Allow adding integration with master token alone #339

leikoilja opened this issue Sep 17, 2021 · 9 comments
Labels
feature New feature or request

Comments

@leikoilja
Copy link
Owner

I think it would be a useful option to allow the user to specify a master_token instead of the account username and app password as is suggested as best security practice in the glocaltokens readme. Having to store my Google account credentials on the home assistant server is the main thing keeping me from using this extension so any way around that is appreciated, this was just what I came up with to maybe achieve that.

Originally posted by @coleya in #124 (comment)

Related leikoilja/glocaltokens#168

@DurgNomis-drol
Copy link
Collaborator

DurgNomis-drol commented Sep 17, 2021

Should we rethink how we (and if we should) store the master_token? HA does not encrypt anything AFAIK, but i am no security expert. What do you guys think?

And by storing, I mean not saving the master_token in config flow, but rather get one each time the integration is loaded from username/password? 🤔

@leikoilja
Copy link
Owner Author

@DurgNomis-drol, I would love to and I think it's very important, but tbh, i don't know what security considerations can we do within our integration scope. Currently we store username, password, master_token and android_id in the integration instance. But so does all other HA integrations, leaving the rest of security vulnerabilities/encryption(?)/storage etc for HA to manage. Apart from that "pre-defined" flow I am not sure what we can do 🤷‍♂️

@DurgNomis-drol
Copy link
Collaborator

@leikoilja You are probably right, i was just thinking out loud after the back and forth in the other issue. 😃

@leikoilja
Copy link
Owner Author

Totally agree with you. We can still support master token as an auth method, thou i dont think it's a common use case and it's not going to make it easier to authenticate or use the integration.
Let's also hear from @KapJI and @Drakulix, maybe they have some thoughts on security and better practises :)

@KapJI
Copy link
Collaborator

KapJI commented Sep 17, 2021

I think encrypting any data is meaningless because it should be decrypted by HA, and the decryption key must be stored somewhere where HA can access it. If someone has access to these files you're pwned in any case, it doesn't matter if they're encrypted or not.

Not storing master token doesn't make any sense either if we still store username and password. Obtaining it every time should work but the only thing it'll do is making the initialization of the integration slower because of the extra request.

Allowing using master token in the config flow is ok if we can provide instructions on how to obtain it. Although I don't think this makes anything safer, probably it's only a bit easier to revoke such token.

@Drakulix
Copy link
Collaborator

Let's also hear from @KapJI and @Drakulix, maybe they have some thoughts on security and better practises :)

Honestly, I think securely storing this information is a problem, that the home-assistant core has to solve. Many integrations including official ones do store sensitive secrets in there and there is no good way to encrypt them as an integration, that is not easily circumvented. Of course this is not an easy problem for the core to solve as well, but systems like home-assistant OS could at least offer disk-encryption? Anyway, in my opinion this is way out of our reach.

Was there any research onto getting other authentification flows to work? Obviously we would like to have a normal web-based authentification flow with a limited set of scopes. gmusicapi had a similar problem and stuck with the master_token approach, although simon-weber found a way to a web-based auth flow. The problem is just, that the token is stored in a cookie and not easy to extract.

I am not sure, if that approach even allows to limit scopes, but if we find any more secure solution, we should maybe write a tutorial and additionally offer people to sign in with a token instead. That way they can generate and extract a more secure one, instead of relying on our more convenient login procedure.

@DurgNomis-drol
Copy link
Collaborator

I think encrypting any data is meaningless because it should be decrypted by HA, and the decryption key must be stored somewhere where HA can access it. If someone has access to these files you're pwned in any case, it doesn't matter if they're encrypted or not.

This is as far as I understand, also the reasoning behind why HA does not encrypt in the first place, which makes totally sense BTW.

And @Drakulix reply answered what i was gonna say 😆

But the cookie thing sounds interesting.

We can still support master token as an auth method, thou i dont think it's a common use case and it's not going to make it easier to authenticate or use the integration.

👍 from me.

@DurgNomis-drol DurgNomis-drol added the feature New feature or request label Oct 15, 2021
@ArnyminerZ
Copy link
Collaborator

Currently we store username, password, master_token and android_id in the integration instance.

And what about just storing master_token and android_id, for example? And just if the integration detects that there is an auth issue, request the user to log in again. Does HA provide any way of requesting the user to log in again? 🤔 I think this would solve the privacy concern until HA does something on cryptography, which actually should be one of their priorities, as I see it. HA should provide some kind of cryptographic module with access just from each integration or something like this.

It's clear that this issue is not just ours, there are other integrations with the same security concerns.

I believe HA secrets is the way they recommend to store secrets, but it's not a good method by any means, as far as I see it.

We can still support master token as an auth method, thou i dont think it's a common use case and it's not going to make it easier to authenticate or use the integration.

This would be quite easy if we do the "re-auth" workflow. Since master_token should not expire, at least in a reasonable amount of time, but it may still happen, so further investigation should be done.

@leikoilja
Copy link
Owner Author

Closed in #723

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants