Skip to content

Replace api_password in Camera.Push #16339

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.

Sign up for GitHub

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

Merged
balloob merged 6 commits into from
Sep 11, 2018
Merged

Replace api_password in Camera.Push #16339

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
351fb72
Use access_token and user provided token instead of api_password
dgomes Aug 31, 2018
c57fab9
address comments by @awarecan
dgomes Sep 1, 2018
8eda035
new tests
dgomes Sep 1, 2018
42f370a
add extra checks and test
dgomes Sep 10, 2018
ef507fc
lint
dgomes Sep 10, 2018
413a93a
add comment
dgomes Sep 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 24 additions & 5 deletions homeassistant/components/camera/push.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
from homeassistant.components.camera import Camera, PLATFORM_SCHEMA,\
STATE_IDLE, STATE_RECORDING
from homeassistant.core import callback
from homeassistant.components.http.view import HomeAssistantView
from homeassistant.const import CONF_NAME, CONF_TIMEOUT, HTTP_BAD_REQUEST
from homeassistant.components.http.view import KEY_AUTHENTICATED,\
HomeAssistantView
from homeassistant.const import CONF_NAME, CONF_TIMEOUT,\
HTTP_NOT_FOUND, HTTP_UNAUTHORIZED, HTTP_BAD_REQUEST
from homeassistant.helpers import config_validation as cv
from homeassistant.helpers.event import async_track_point_in_utc_time
import homeassistant.util.dt as dt_util
Expand All @@ -25,11 +27,13 @@

CONF_BUFFER_SIZE = 'buffer'
CONF_IMAGE_FIELD = 'field'
CONF_TOKEN = 'token'

DEFAULT_NAME = "Push Camera"

ATTR_FILENAME = 'filename'
ATTR_LAST_TRIP = 'last_trip'
ATTR_TOKEN = 'token'

PUSH_CAMERA_DATA = 'push_camera'

Expand All @@ -39,6 +43,7 @@
vol.Optional(CONF_TIMEOUT, default=timedelta(seconds=5)): vol.All(
cv.time_period, cv.positive_timedelta),
vol.Optional(CONF_IMAGE_FIELD, default='image'): cv.string,
vol.Optional(CONF_TOKEN): vol.All(cv.string, vol.Length(min=8)),

@balloob balloob Sep 4, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be required

@dgomes dgomes Sep 4, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made it optional, since the implementation allows the use of the normal authentication.

})


Expand All @@ -50,7 +55,8 @@ async def async_setup_platform(hass, config, async_add_entities,

cameras = [PushCamera(config[CONF_NAME],
config[CONF_BUFFER_SIZE],
config[CONF_TIMEOUT])]
config[CONF_TIMEOUT],
config[CONF_TOKEN])]

@balloob balloob Sep 4, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This expects the CONF_TOKEN to be always set, which is not the case as it can be optional.


hass.http.register_view(CameraPushReceiver(hass,
config[CONF_IMAGE_FIELD]))
Expand All @@ -63,6 +69,7 @@ class CameraPushReceiver(HomeAssistantView):

url = "/api/camera_push/{entity_id}"
name = 'api:camera_push:camera_entity'
requires_auth = False

def __init__(self, hass, image_field):
"""Initialize CameraPushReceiver with camera entity."""
Expand All @@ -75,8 +82,18 @@ async def post(self, request, entity_id):

if _camera is None:
_LOGGER.error("Unknown %s", entity_id)
status = HTTP_NOT_FOUND if request[KEY_AUTHENTICATED]\
else HTTP_UNAUTHORIZED
return self.json_message('Unknown {}'.format(entity_id),
HTTP_BAD_REQUEST)
status)
Comment thread
dgomes marked this conversation as resolved.

authenticated = (request[KEY_AUTHENTICATED] or
request.query.get('token') == _camera.token)

@balloob balloob Sep 4, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not setting a token will result in token being None, so then not passing a token means we're checking None == None, allowing user to always access it.


if not authenticated:
return self.json_message(
'Invalid authorization credentials for {}'.format(entity_id),
HTTP_UNAUTHORIZED)

try:
data = await request.post()
Expand All @@ -95,7 +112,7 @@ async def post(self, request, entity_id):
class PushCamera(Camera):
"""The representation of a Push camera."""

def __init__(self, name, buffer_size, timeout):
def __init__(self, name, buffer_size, timeout, token):
"""Initialize push camera component."""
super().__init__()
self._name = name
Expand All @@ -106,6 +123,7 @@ def __init__(self, name, buffer_size, timeout):
self._timeout = timeout
self.queue = deque([], buffer_size)
self._current_image = None
self.token = token

async def async_added_to_hass(self):
"""Call when entity is added to hass."""
Expand Down Expand Up @@ -168,5 +186,6 @@ def device_state_attributes(self):
name: value for name, value in (
(ATTR_LAST_TRIP, self._last_trip),
(ATTR_FILENAME, self._filename),
(ATTR_TOKEN, self.token),
) if value is not None
}
20 changes: 14 additions & 6 deletions tests/components/camera/test_push.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from homeassistant import core as ha
from homeassistant.setup import async_setup_component
from homeassistant.util import dt as dt_util
from tests.components.auth import async_setup_auth


async def test_bad_posting(aioclient_mock, hass, aiohttp_client):
Expand All @@ -15,19 +14,25 @@ async def test_bad_posting(aioclient_mock, hass, aiohttp_client):
'camera': {
'platform': 'push',
'name': 'config_test',
'token': '12345678'
}})

client = await async_setup_auth(hass, aiohttp_client)
client = await aiohttp_client(hass.http.app)

# missing file
resp = await client.post('/api/camera_push/camera.config_test')
assert resp.status == 400

files = {'image': io.BytesIO(b'fake')}

# wrong entity
files = {'image': io.BytesIO(b'fake')}
resp = await client.post('/api/camera_push/camera.wrong', data=files)
assert resp.status == 400
assert resp.status == 404

# wrong token but authenticated user

@awarecan awarecan Sep 1, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did I missed something? How this be authenticated user? I didn't find the code to set Authorization header.

@dgomes dgomes Sep 1, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

client always makes authenticated requests... how can I disable it ?

@awarecan awarecan Sep 1, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/home-assistant/home-assistant/blob/a5d95dfbdcf0d9ca3da93f44e2cbb5d162ac3272/tests/components/http/test_auth.py#L158-L163

https://github.com/home-assistant/home-assistant/blob/a5d95dfbdcf0d9ca3da93f44e2cbb5d162ac3272/tests/components/conftest.py#L43-L44

files = {'image': io.BytesIO(b'fake')}
resp = await client.post('/api/camera_push/camera.config_test?token=1234',
data=files)
assert resp.status == 200


async def test_posting_url(hass, aiohttp_client):
Expand All @@ -36,6 +41,7 @@ async def test_posting_url(hass, aiohttp_client):
'camera': {
'platform': 'push',
'name': 'config_test',
'token': '12345678'
}})

client = await aiohttp_client(hass.http.app)
Expand All @@ -46,7 +52,9 @@ async def test_posting_url(hass, aiohttp_client):
assert camera_state.state == 'idle'

# post image
resp = await client.post('/api/camera_push/camera.config_test', data=files)
resp = await client.post(
'/api/camera_push/camera.config_test?token=12345678',
data=files)
assert resp.status == 200

# state recording
Expand Down