home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
73.57k stars 30.74k forks source link

Use access_token instead api_password #15376

Closed awarecan closed 5 years ago

awarecan commented 6 years ago

It is an epic includes all files need to remove api_password, change to using access_token.

Separate issue/PR may create for each change.

All tests are not listed here.

balloob commented 6 years ago

I suggest we drop the API class in Remote and the related methods. Home Assistant is moving away from Rest to WebSocket calls, and we should not maintain these methods. It's also sync instead of async.

awarecan commented 6 years ago

Will that break lots of users?

awarecan commented 6 years ago

We probably need two rounds change. The first round, add access_token, keep api_password, and make sure access_token will be used if auth.active. Make a big announcement, switch auth.active from opt-in to opt-out. Then make the second round, remove api_password.

gerard33 commented 6 years ago

I guess Geofency should be added to the list as well. https://www.home-assistant.io/components/device_tracker.geofency/ components/device_tracker/geofency.py

awarecan commented 6 years ago

We may have to implement accept access token in query string (RFC 6750 2.3) to support web hook usage.

I will suggest to add an optional config in auth components to explicitly enable this support, since it is not recommend validation method

balloob commented 6 years ago

I don't think that I want to support that. Integrations can allow users to configure a token if they want to do auth via url

awarecan commented 6 years ago

I see.

@gerard33 So the Geofency will need change its view as follow example.

class GeofencyView(HomeAssistantView):
    """View to handle Geofency requests."""

    url = URL
    name = 'api:geofency'
    requires_auth = False

    async def post(self, request):
        if not your_custom_validation(request):
            return web.Response(status=401)

EDIT: So, for any 3rd party want to use HA native auth system, they have to official support OAuth 2 protocol, otherwise, the component need implement itself auth system.

To keep backward compatibility, it can still accept api_password in query url or basic auth header. However, the actually validation logic need be written by individual component, HA will eventually remove current api_password configuration and validation function from http component, do not rely on it.

balloob commented 6 years ago

When adding something to the url, it should not be a HASS access token, instead the integration should allow the user to specify some sort of auth in the config.

We don't have permissions yet, access tokens are very powerful, never expiring access tokens even more (as that is what a user would want to use). URLs are getting logged in proxies or other systems left and right. There is a very good reason auth should not be send via a url. So if an integration like Geofency wants to use auth in the url, it should define their own string, as that would only grant access to that one endpoint.

awarecan commented 6 years ago

Another issue for 3rd party use access token if they don't native support OAuth2 is that they cannot pass IndieAuth, since we need a redirect url match the client id. This will be a deadend.

I will edit my previous comment

pvizeli commented 6 years ago

It's very dificult to use websockets with curl in bash scripts. I like also to minimize the REST API support but complete remove in flavor of websockets are heavy.

A developer can write a short python script but a sysadmin ends with call curl.

There are also other integraton like nodered they run with eventstream and Rest API. I think a rest API and mqtt are a minimun support for IoT device. That will be also used for esphome.

But I also think we should remove the remote class.

awarecan commented 6 years ago

EDIT: misread

jedi7 commented 6 years ago

Hi, the warning "Please change to use bearer token access" is comming also from appdaemon calls. I'm also for to keep the rest api (ex: for simple router scripts to track devices)

Regarding appdaemon, how is auth solved for "system" 3rd party services? Like appdaemon? Thanks

balloob commented 6 years ago

Just like all other apps: migrate to the new auth.

balloob commented 6 years ago

Did a quick grep through the code. These are the components that register HTTP views and probably need to be updated to work with access tokens:

Might be less as I have not checked if all these HTTP views require authentication.

DavidFW1960 commented 6 years ago

My Google Assistant (manual setup) used the original gactions works fine still without legacy_api enabled. I also think you might have missed IFTTT in the list?

frog32 commented 6 years ago

binary_sensor.http is also missing in this list

awarecan commented 6 years ago

IFTTT can be categorized with REST API.

Maybe we shall retire sensor.http and binary_sesnor.http? EDIT: clearly the doc is outdated, those should be removed from components. They are part of REST API

dgomes commented 6 years ago

I'm responsible for camera.push

So currently I have:

POST /api/camera_push/camera.

This endpoint must be secured, and I relied so far in the rest API.

Since most calls are handled using curl as external call to a script, it makes no sense to move this to the websocket API.

Should I implement https://github.com/home-assistant/home-assistant/issues/15376#issuecomment-405105542 proposal ?

balloob commented 6 years ago

@dgomes yeah. I would suggest something like this:

# configuration.yaml entry
camera:
  - platform: push
    token: sdlkjadslkjadskjlad
    name: MotionEye Outdoor
    buffer: 3
    timeout: 5

And then accept pushes without authentication on the url

POST /api/camera_push/camera.<entity_id>?token=<token>

This is exactly how the Media Player thumbnail endpoint works:

https://github.com/home-assistant/home-assistant/blob/f20a3313b09129b725fec0ac4064bafe940db233/homeassistant/components/media_player/__init__.py#L995-L1014

dgomes commented 6 years ago

Tks @balloob !

Are we OK replicating this solution in all other components relying on api_password ?

Can't we create a special user with limited access to specific API's? (RBAC -> implement user based ACL's) I think this would also pacify the discussion on the last release blog post.

In the meanwhile I'll prepare this change to the camera.push component.

balloob commented 6 years ago

we'll get permissions eventually, not today though 😉

Yeah we're ok with replicating this logic. It's not a lot of logic so no need to create an abstraction for it.

jcconnell commented 6 years ago

I'd like to help update the components that need to use access tokens. I'm a novice programmer. Can someone help me understand what needs to be changed and also explain @balloob's example? Once I get the idea, I'll start submitting PRs for the updates.

PhracturedBlue commented 6 years ago

As per @awarecan I would like some pointers on the proper way to fix the camera proxy. Fundamentally it is fetching an image from another camera, processing it and returning the modified image. We aren't allowed to directly call one component from another, so today we use the old rest API (which requires either a whitelist or http.api_password). But rather than waiting for the legacy mode to disappear, I'd love to pro-actively fix this. However, prior pull requests indicate that any solution I come up with is likely to be rejected :) So 'm asking for advice first this time.

balloob commented 6 years ago

Migrate to use the new

image = await hass.components.camera.async_get_image('camera.bla')
print(image.content_type)
print(len(image.content))
PhracturedBlue commented 6 years ago

Thanks @balloob. That worked well for the still images. I tried to do something similar with the mjpeg stream , however, the resultant object is a ResponseStream, and I don't know how to get access to the raw data within. I could just capture still images and build the mjpeg from that, but I'd prefer to parse the MJPEG input from the camera directly if possible.

current pseudo code (all error checking removed)

components.camera:
@bind_hass
async def async_get_mjpeg_stream(hass, entity_id, request, timeout=10):
    component = hass.data.get(DOMAIN)
    camera = component.get_entity(entity_id)
    return await camera.handle_async_mjpeg_stream(request)

components.camera.proxy:
   async def handle_async_mjpeg_stream(self, request):
        stream_coro = self.hass.components.camera.async_get_mjpeg_stream(
            self._proxied_camera, request)

        req = await stream_coro  <-- code never gets past this line

        stream = req.content
        while True:
            chunk = await stream.read(102400)

When I do await stream_coro the frontend gets the original mpeg from the camera, and my proxy never gets a chance to modify the stream.

balloob commented 6 years ago

I don't know really. Also, we should move this to another issue please.

awarecan commented 6 years ago

Long-lived access token will be landed in 0.78 release. See https://github.com/home-assistant/home-assistant-polymer/pull/1656

balloob commented 6 years ago

For any component that requires a webhook to receive data, we have introduced a new webhook component that you can register with and be able to receive information.

I will write up the docs soon, but here is an example of how it works for the IFTTT component:

https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/ifttt/__init__.py#L77-L88

balloob commented 6 years ago

Made it even easier to set up webhooks for users by linking it to the automation component: https://github.com/home-assistant/home-assistant/pull/17246

rohankapoorcom commented 6 years ago

For mailgun, it seems like the best approach would be to switch it over to the use an autogenerated webhook similar to changes in ifttt that @balloob did for 0.8.0. I'd be willing to give that a shot.

rohankapoorcom commented 6 years ago

Once we get #17464 merged in, I'm willing to switch Twilio over to an auto-generated webhook url as well.

rohankapoorcom commented 6 years ago

I'm just starting migrating over the Prometheus component to an auto-generated webhook url and noticed that it (and several others) are using GET requests to scrape data from Home Assistant instead of POSTing data to Home Assistant. I'm going to extend the webhook component to support GET requests as well first.

balloob commented 6 years ago

We shouldn't do that. I think that it was wrong from us to even support responses to webhooks. We should just acknowledge that we receive them but not expose any info.

if an integration is fetching data from Home Assistant, it should implement an HTTP view and appropriate auth.

rohankapoorcom commented 6 years ago

Okay, that makes sense. I guess the correct approach for those components then is continuing to use a password from config just not the one from the HTTP component and then disabling auth on the view?

I disagree though regarding responses from webhooks. Some endpoints (Twilio in particular comes to mind) require a webhook to be acknowledged with a certain response (in twilio's case their specific XML format) so that they know you received the data.

balloob commented 6 years ago

Those components should start using long lived access tokens yes.

balloob commented 6 years ago

I Apologize, I did more research and saw the PR by @rohankapoorcom and changed my mind. Webhooks should be able to return responses, but we should make sure we only use them for webhooks.

rohankapoorcom commented 5 years ago

I was thinking about moving the Meraki device tracker platform (first to a split component/platform) and then to use the webhook component instead of the current API password approach. Unfortunately I found this in the Meraki docs:

Upon the first connection, the Meraki cloud will perform a single HTTP GET; the server must return the organization-specific validator string as a response, which will verify the organization's identity as the Cisco Meraki customer. The Meraki cloud will then begin performing JSON posts.

Since we don't allow the webhook component to respond to GET requests, what do you recommend for moving this component off of the API password? The simplest approach I can think of is to switch to an unauthenticated webview and perform our own authentication (via a custom API password) for this platform.

awarecan commented 5 years ago

After #21884, only api_password left are in either test code or legacy_api_password auth provider or http and websocket_api related codes. This issue can be closed.

RyuzakiKK commented 5 years ago

@awarecan What about components that still requires http.api_password to work, like alexa? I hope api_password will not get removed until all HA components switched away from it.

awarecan commented 5 years ago

I believe I have already checked all usage place, there should be no more dependency on http.api_password.

Could you please point to me where alexa need http.api_password

RyuzakiKK commented 5 years ago

For Alexa the wiki guide says that the api_password is still required as an endpoint https://www.home-assistant.io/components/alexa/#requirements and apparently we can't yet use a long lived access token for example.

awarecan commented 5 years ago

You can still use api_password in URL as long as you have legacy_api_password auth provider configured.

BTW, use api_password in alexa endpoint is not a good design, we should change it by verify the Amazon signature.

balloob commented 5 years ago

It should use a webhook.

awarecan commented 5 years ago

Actually, just like google assistant, alexa also can leverage our oauth2 authentication. I can look into it, https://developer.amazon.com/docs/account-linking/account-linking-for-sh-and-other.html

balloob commented 5 years ago

Oh there you go!

jerrychong25 commented 5 years ago

Based on the long conversation of this issue, it is confirmed that api_password will be dropped completely soon. A migration definitely is unavoidable.

Just curious, what is available method for this migration?

AFAIK, currently only have long live access token with curl command method.

awarecan commented 5 years ago

@jerrychong25 https://github.com/home-assistant/home-assistant/issues/15376#issuecomment-425876417

RyuzakiKK commented 5 years ago

@awarecan any news for the oauth2 authentication in the alexa component?

Sorry for being probably off topic, but as far as I know there isn't an open issue about that.

awarecan commented 5 years ago

Document has been updated. https://www.home-assistant.io/components/alexa.smart_home/

filikun commented 5 years ago

Looking at the Meraki integration it seems to still need the api_password? https://www.home-assistant.io/integrations/meraki/