From 4f4a14bd627937e82bc3b607728a3d49c0bda207 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Fri, 28 Sep 2018 12:53:57 +0200 Subject: [PATCH 1/5] Add group foundation --- homeassistant/auth/__init__.py | 12 +- homeassistant/auth/auth_store.py | 123 ++++++++++-- homeassistant/auth/models.py | 12 +- .../auth/providers/trusted_networks.py | 8 +- homeassistant/components/config/auth.py | 71 +++---- .../config/auth_provider_homeassistant.py | 183 ++++++++---------- homeassistant/components/websocket_api.py | 26 +-- tests/auth/test_auth_store.py | 32 +++ tests/auth/test_init.py | 6 +- tests/common.py | 14 +- tests/components/config/test_auth.py | 5 +- tests/components/hassio/test_init.py | 2 +- tests/fixtures/auth_v1.json | 24 +++ 13 files changed, 336 insertions(+), 182 deletions(-) create mode 100644 tests/auth/test_auth_store.py create mode 100644 tests/fixtures/auth_v1.json diff --git a/homeassistant/auth/__init__.py b/homeassistant/auth/__init__.py index c6f978640..d45ab0b5b 100644 --- a/homeassistant/auth/__init__.py +++ b/homeassistant/auth/__init__.py @@ -211,7 +211,7 @@ class AuthManager: async def async_enable_user_mfa(self, user: models.User, mfa_module_id: str, data: Any) -> None: """Enable a multi-factor auth module for user.""" - if user.system_generated: + if user.group.system_generated: raise ValueError('System generated users cannot enable ' 'multi-factor auth module.') @@ -225,7 +225,7 @@ class AuthManager: async def async_disable_user_mfa(self, user: models.User, mfa_module_id: str) -> None: """Disable a multi-factor auth module for user.""" - if user.system_generated: + if user.group.system_generated: raise ValueError('System generated users cannot disable ' 'multi-factor auth module.') @@ -255,18 +255,18 @@ class AuthManager: if not user.is_active: raise ValueError('User is not active') - if user.system_generated and client_id is not None: + if user.group.system_generated and client_id is not None: raise ValueError( 'System generated users cannot have refresh tokens connected ' 'to a client.') if token_type is None: - if user.system_generated: + if user.group.system_generated: token_type = models.TOKEN_TYPE_SYSTEM else: token_type = models.TOKEN_TYPE_NORMAL - if user.system_generated != (token_type == models.TOKEN_TYPE_SYSTEM): + if user.group.system_generated != (token_type == models.TOKEN_TYPE_SYSTEM): raise ValueError( 'System generated users can only have system type ' 'refresh tokens') @@ -414,7 +414,7 @@ class AuthManager: being created. """ for user in await self._store.async_get_users(): - if not user.system_generated: + if not user.group.system_generated: return False return True diff --git a/homeassistant/auth/auth_store.py b/homeassistant/auth/auth_store.py index c17034474..20044e43a 100644 --- a/homeassistant/auth/auth_store.py +++ b/homeassistant/auth/auth_store.py @@ -1,20 +1,61 @@ """Storage for auth models.""" from collections import OrderedDict from datetime import timedelta +import hmac from logging import getLogger from typing import Any, Dict, List, Optional # noqa: F401 -import hmac +import uuid from homeassistant.auth.const import ACCESS_TOKEN_EXPIRATION from homeassistant.core import HomeAssistant, callback +from homeassistant.helpers import storage from homeassistant.util import dt as dt_util from . import models -STORAGE_VERSION = 1 +STORAGE_VERSION = 2 STORAGE_KEY = 'auth' +class DataStore(storage.Store): + """Store the auth data on disk using JSON.""" + + def __init__(self, hass): + """Initialize the data store.""" + super().__init__(hass, STORAGE_VERSION, STORAGE_KEY, True) + + async def _async_migrate_func(self, old_version, old_data): + """Migrate to the new version.""" + if old_version <= 1: + system_group_id = old_data['system_user_group_id'] = \ + uuid.uuid4().hex + family_group_id = old_data['default_new_user_group_id'] = \ + uuid.uuid4().hex + + old_data['groups'] = [ + { + 'name': 'System', + 'id': system_group_id, + 'system_generated': True, + }, + { + 'name': 'Family', + 'id': family_group_id, + 'system_generated': False, + }, + ] + + for user_info in old_data['users']: + if user_info.pop('system_generated'): + group_id = system_group_id + else: + group_id = family_group_id + + user_info['group_id'] = group_id + + return old_data + + class AuthStore: """Stores authentication info. @@ -28,8 +69,10 @@ class AuthStore: """Initialize the auth store.""" self.hass = hass self._users = None # type: Optional[Dict[str, models.User]] - self._store = hass.helpers.storage.Store(STORAGE_VERSION, STORAGE_KEY, - private=True) + self._groups = None # type: Optional[Dict[str, models.Group]] + self.default_new_user_group_id = None # type: Optional[str] + self.system_user_group_id = None # type: Optional[str] + self._store = DataStore(hass) async def async_get_users(self) -> List[models.User]: """Retrieve all users.""" @@ -50,6 +93,7 @@ class AuthStore: async def async_create_user( self, name: Optional[str], is_owner: Optional[bool] = None, is_active: Optional[bool] = None, + group_id: Optional[str] = None, system_generated: Optional[bool] = None, credentials: Optional[models.Credentials] = None) -> models.User: """Create a new user.""" @@ -57,8 +101,14 @@ class AuthStore: await self._async_load() assert self._users is not None + if system_generated: + group_id = self.system_user_group_id + elif group_id is None: + group_id = self.default_new_user_group_id + kwargs = { - 'name': name + 'name': name, + 'group': self._groups[group_id], } # type: Dict[str, Any] if is_owner is not None: @@ -67,9 +117,6 @@ class AuthStore: if is_active is not None: kwargs['is_active'] = is_active - if system_generated is not None: - kwargs['system_generated'] = system_generated - new_user = models.User(**kwargs) self._users[new_user.id] = new_user @@ -214,14 +261,32 @@ class AuthStore: if self._users is not None: return - users = OrderedDict() # type: Dict[str, models.User] - if data is None: - self._users = users + self._set_defaults() return + users = OrderedDict() # type: Dict[str, models.User] + groups = OrderedDict() # type: Dict[str, models.Group] + + # When creating objects we mention each attribute explicetely. This + # prevents crashing if user rolls back HA version after a new property + # was added. + + for group_dict in data['groups']: + groups[group_dict['id']] = models.Group( + name=group_dict['name'], + id=group_dict['id'], + system_generated=group_dict['system_generated'], + ) + for user_dict in data['users']: - users[user_dict['id']] = models.User(**user_dict) + users[user_dict['id']] = models.User( + name=user_dict['name'], + group=groups[user_dict['group_id']], + id=user_dict['id'], + is_owner=user_dict['is_owner'], + is_active=user_dict['is_active'], + ) for cred_dict in data['credentials']: users[cred_dict['user_id']].credentials.append(models.Credentials( @@ -276,7 +341,10 @@ class AuthStore: ) users[rt_dict['user_id']].refresh_tokens[token.id] = token + self._groups = groups self._users = users + self.default_new_user_group_id = data['default_new_user_group_id'] + self.system_user_group_id = data['system_user_group_id'] @callback def _async_schedule_save(self) -> None: @@ -294,14 +362,23 @@ class AuthStore: users = [ { 'id': user.id, + 'group_id': user.group.id, 'is_owner': user.is_owner, 'is_active': user.is_active, 'name': user.name, - 'system_generated': user.system_generated, } for user in self._users.values() ] + groups = [ + { + 'name': group.name, + 'id': group.id, + 'system_generated': group.system_generated, + } + for group in self._groups.values() + ] + credentials = [ { 'id': credential.id, @@ -338,6 +415,26 @@ class AuthStore: return { 'users': users, + 'groups': groups, 'credentials': credentials, 'refresh_tokens': refresh_tokens, + 'default_new_user_group_id': self.default_new_user_group_id, + 'system_user_group_id': self.system_user_group_id, } + + def _set_defaults(self): + """Set default values for auth store.""" + self._users = OrderedDict() + + # Add default groups + system_group = models.Group(name='System', system_generated=True) + family_group = models.Group(name='Family') + + self.default_new_user_group_id = family_group.id + self.system_user_group_id = system_group.id + + groups = OrderedDict() + groups[system_group.id] = system_group + groups[family_group.id] = family_group + + self._groups = groups diff --git a/homeassistant/auth/models.py b/homeassistant/auth/models.py index b0f4024c3..460542b7e 100644 --- a/homeassistant/auth/models.py +++ b/homeassistant/auth/models.py @@ -14,15 +14,25 @@ TOKEN_TYPE_SYSTEM = 'system' TOKEN_TYPE_LONG_LIVED_ACCESS_TOKEN = 'long_lived_access_token' +@attr.s(slots=True) +class Group: + """A group.""" + + name = attr.ib(type=str) # type: Optional[str] + id = attr.ib(type=str, default=attr.Factory(lambda: uuid.uuid4().hex)) + # System generated groups cannot be changed + system_generated = attr.ib(type=bool, default=False) + + @attr.s(slots=True) class User: """A user.""" name = attr.ib(type=str) # type: Optional[str] + group = attr.ib(type=Group) id = attr.ib(type=str, default=attr.Factory(lambda: uuid.uuid4().hex)) is_owner = attr.ib(type=bool, default=False) is_active = attr.ib(type=bool, default=False) - system_generated = attr.ib(type=bool, default=False) # List of credentials of a user. credentials = attr.ib( diff --git a/homeassistant/auth/providers/trusted_networks.py b/homeassistant/auth/providers/trusted_networks.py index 8a7e1d67c..13c31efc6 100644 --- a/homeassistant/auth/providers/trusted_networks.py +++ b/homeassistant/auth/providers/trusted_networks.py @@ -44,9 +44,9 @@ class TrustedNetworksAuthProvider(AuthProvider): """Return a flow to login.""" assert context is not None users = await self.store.async_get_users() - available_users = {user.id: user.name - for user in users - if not user.system_generated and user.is_active} + available_users = { + user.id: user.name for user in users + if not user.group.system_generated and user.is_active} return TrustedNetworksLoginFlow( self, cast(str, context.get('ip_address')), available_users) @@ -58,7 +58,7 @@ class TrustedNetworksAuthProvider(AuthProvider): users = await self.store.async_get_users() for user in users: - if (not user.system_generated and + if (not user.group.system_generated and user.is_active and user.id == user_id): for credential in await self.async_credentials(): diff --git a/homeassistant/components/config/auth.py b/homeassistant/components/config/auth.py index 6f00b03de..826e7ff73 100644 --- a/homeassistant/components/config/auth.py +++ b/homeassistant/components/config/auth.py @@ -1,7 +1,6 @@ """Offer API to configure Home Assistant auth.""" import voluptuous as vol -from homeassistant.core import callback from homeassistant.components import websocket_api @@ -40,61 +39,49 @@ async def async_setup(hass): return True -@callback @websocket_api.require_owner -def websocket_list(hass, connection, msg): +@websocket_api.async_response +async def websocket_list(hass, connection, msg): """Return a list of users.""" - async def send_users(): - """Send users.""" - result = [_user_info(u) for u in await hass.auth.async_get_users()] + result = [_user_info(u) for u in await hass.auth.async_get_users()] - connection.send_message_outside( - websocket_api.result_message(msg['id'], result)) - - hass.async_add_job(send_users()) + connection.send_message_outside( + websocket_api.result_message(msg['id'], result)) -@callback @websocket_api.require_owner -def websocket_delete(hass, connection, msg): +@websocket_api.async_response +async def websocket_delete(hass, connection, msg): """Delete a user.""" - async def delete_user(): - """Delete user.""" - if msg['user_id'] == connection.request.get('hass_user').id: - connection.send_message_outside(websocket_api.error_message( - msg['id'], 'no_delete_self', - 'Unable to delete your own account')) - return + if msg['user_id'] == connection.request.get('hass_user').id: + connection.send_message_outside(websocket_api.error_message( + msg['id'], 'no_delete_self', + 'Unable to delete your own account')) + return - user = await hass.auth.async_get_user(msg['user_id']) + user = await hass.auth.async_get_user(msg['user_id']) - if not user: - connection.send_message_outside(websocket_api.error_message( - msg['id'], 'not_found', 'User not found')) - return + if not user: + connection.send_message_outside(websocket_api.error_message( + msg['id'], 'not_found', 'User not found')) + return - await hass.auth.async_remove_user(user) + await hass.auth.async_remove_user(user) - connection.send_message_outside( - websocket_api.result_message(msg['id'])) - - hass.async_add_job(delete_user()) + connection.send_message_outside( + websocket_api.result_message(msg['id'])) -@callback @websocket_api.require_owner -def websocket_create(hass, connection, msg): +@websocket_api.async_response +async def websocket_create(hass, connection, msg): """Create a user.""" - async def create_user(): - """Create a user.""" - user = await hass.auth.async_create_user(msg['name']) + user = await hass.auth.async_create_user(msg['name']) - connection.send_message_outside( - websocket_api.result_message(msg['id'], { - 'user': _user_info(user) - })) - - hass.async_add_job(create_user()) + connection.send_message_outside( + websocket_api.result_message(msg['id'], { + 'user': _user_info(user) + })) def _user_info(user): @@ -104,7 +91,9 @@ def _user_info(user): 'name': user.name, 'is_owner': user.is_owner, 'is_active': user.is_active, - 'system_generated': user.system_generated, + # Temp, backwards compat since 0.80, remove in 85 + 'system_generated': user.group.system_generated, + 'group_id': user.group.id, 'credentials': [ { 'type': c.auth_provider_type, diff --git a/homeassistant/components/config/auth_provider_homeassistant.py b/homeassistant/components/config/auth_provider_homeassistant.py index 960e8f5e7..55c4b7954 100644 --- a/homeassistant/components/config/auth_provider_homeassistant.py +++ b/homeassistant/components/config/auth_provider_homeassistant.py @@ -2,7 +2,6 @@ import voluptuous as vol from homeassistant.auth.providers import homeassistant as auth_ha -from homeassistant.core import callback from homeassistant.components import websocket_api @@ -54,121 +53,109 @@ def _get_provider(hass): raise RuntimeError('Provider not found') -@callback @websocket_api.require_owner -def websocket_create(hass, connection, msg): +@websocket_api.async_response +async def websocket_create(hass, connection, msg): """Create credentials and attach to a user.""" - async def create_creds(): - """Create credentials.""" - provider = _get_provider(hass) - await provider.async_initialize() + provider = _get_provider(hass) + await provider.async_initialize() - user = await hass.auth.async_get_user(msg['user_id']) + user = await hass.auth.async_get_user(msg['user_id']) - if user is None: - connection.send_message_outside(websocket_api.error_message( - msg['id'], 'not_found', 'User not found')) - return + if user is None: + connection.send_message_outside(websocket_api.error_message( + msg['id'], 'not_found', 'User not found')) + return - if user.system_generated: - connection.send_message_outside(websocket_api.error_message( - msg['id'], 'system_generated', - 'Cannot add credentials to a system generated user.')) - return + if user.group.system_generated: + connection.send_message_outside(websocket_api.error_message( + msg['id'], 'system_generated', + 'Cannot add credentials to a system generated user.')) + return - try: - await hass.async_add_executor_job( - provider.data.add_auth, msg['username'], msg['password']) - except auth_ha.InvalidUser: - connection.send_message_outside(websocket_api.error_message( - msg['id'], 'username_exists', 'Username already exists')) - return + try: + await hass.async_add_executor_job( + provider.data.add_auth, msg['username'], msg['password']) + except auth_ha.InvalidUser: + connection.send_message_outside(websocket_api.error_message( + msg['id'], 'username_exists', 'Username already exists')) + return - credentials = await provider.async_get_or_create_credentials({ - 'username': msg['username'] - }) - await hass.auth.async_link_user(user, credentials) + credentials = await provider.async_get_or_create_credentials({ + 'username': msg['username'] + }) + await hass.auth.async_link_user(user, credentials) - await provider.data.async_save() - connection.to_write.put_nowait(websocket_api.result_message(msg['id'])) - - hass.async_add_job(create_creds()) + await provider.data.async_save() + connection.to_write.put_nowait(websocket_api.result_message(msg['id'])) -@callback @websocket_api.require_owner -def websocket_delete(hass, connection, msg): +@websocket_api.async_response +async def websocket_delete(hass, connection, msg): """Delete username and related credential.""" - async def delete_creds(): - """Delete user credentials.""" - provider = _get_provider(hass) - await provider.async_initialize() + provider = _get_provider(hass) + await provider.async_initialize() - credentials = await provider.async_get_or_create_credentials({ - 'username': msg['username'] - }) + credentials = await provider.async_get_or_create_credentials({ + 'username': msg['username'] + }) - # if not new, an existing credential exists. - # Removing the credential will also remove the auth. - if not credentials.is_new: - await hass.auth.async_remove_credentials(credentials) - - connection.to_write.put_nowait( - websocket_api.result_message(msg['id'])) - return - - try: - provider.data.async_remove_auth(msg['username']) - await provider.data.async_save() - except auth_ha.InvalidUser: - connection.to_write.put_nowait(websocket_api.error_message( - msg['id'], 'auth_not_found', 'Given username was not found.')) - return + # if not new, an existing credential exists. + # Removing the credential will also remove the auth. + if not credentials.is_new: + await hass.auth.async_remove_credentials(credentials) connection.to_write.put_nowait( websocket_api.result_message(msg['id'])) + return - hass.async_add_job(delete_creds()) - - -@callback -def websocket_change_password(hass, connection, msg): - """Change user password.""" - async def change_password(): - """Change user password.""" - user = connection.request.get('hass_user') - if user is None: - connection.send_message_outside(websocket_api.error_message( - msg['id'], 'user_not_found', 'User not found')) - return - - provider = _get_provider(hass) - await provider.async_initialize() - - username = None - for credential in user.credentials: - if credential.auth_provider_type == provider.type: - username = credential.data['username'] - break - - if username is None: - connection.send_message_outside(websocket_api.error_message( - msg['id'], 'credentials_not_found', 'Credentials not found')) - return - - try: - await provider.async_validate_login( - username, msg['current_password']) - except auth_ha.InvalidAuth: - connection.send_message_outside(websocket_api.error_message( - msg['id'], 'invalid_password', 'Invalid password')) - return - - await hass.async_add_executor_job( - provider.data.change_password, username, msg['new_password']) + try: + provider.data.async_remove_auth(msg['username']) await provider.data.async_save() + except auth_ha.InvalidUser: + connection.to_write.put_nowait(websocket_api.error_message( + msg['id'], 'auth_not_found', 'Given username was not found.')) + return - connection.send_message_outside( - websocket_api.result_message(msg['id'])) + connection.to_write.put_nowait( + websocket_api.result_message(msg['id'])) - hass.async_add_job(change_password()) + +@websocket_api.async_response +async def websocket_change_password(hass, connection, msg): + """Change user password.""" + user = connection.request.get('hass_user') + if user is None: + connection.send_message_outside(websocket_api.error_message( + msg['id'], 'user_not_found', 'User not found')) + return + + provider = _get_provider(hass) + await provider.async_initialize() + + username = None + for credential in user.credentials: + if credential.auth_provider_type == provider.type: + username = credential.data['username'] + break + + if username is None: + connection.send_message_outside(websocket_api.error_message( + msg['id'], 'credentials_not_found', 'Credentials not found')) + return + + try: + await provider.async_validate_login( + username, msg['current_password']) + except auth_ha.InvalidAuth: + connection.send_message_outside(websocket_api.error_message( + msg['id'], 'invalid_password', 'Invalid password')) + return + + await hass.async_add_executor_job( + provider.data.change_password, username, msg['new_password']) + await provider.data.async_save() + + connection.send_message_outside( + websocket_api.result_message(msg['id'])) diff --git a/homeassistant/components/websocket_api.py b/homeassistant/components/websocket_api.py index 9bd4aac4b..85ab4664b 100644 --- a/homeassistant/components/websocket_api.py +++ b/homeassistant/components/websocket_api.py @@ -480,22 +480,24 @@ class ActiveConnection: return wsock +async def _handle_async_response(func, hass, connection, msg): + """Create a response and handle exception.""" + try: + await func(hass, connection, msg) + except Exception: # pylint: disable=broad-except + _LOGGER.exception("Unexpected exception") + connection.send_message_outside(error_message( + msg['id'], 'unknown', 'Unexpected error occurred')) + + def async_response(func): """Decorate an async function to handle WebSocket API messages.""" - async def handle_msg_response(hass, connection, msg): - """Create a response and handle exception.""" - try: - await func(hass, connection, msg) - except Exception: # pylint: disable=broad-except - _LOGGER.exception("Unexpected exception") - connection.send_message_outside(error_message( - msg['id'], 'unknown', 'Unexpected error occurred')) - @callback @wraps(func) def schedule_handler(hass, connection, msg): """Schedule the handler.""" - hass.async_create_task(handle_msg_response(hass, connection, msg)) + hass.async_create_task( + _handle_async_response(func, hass, connection, msg)) return schedule_handler @@ -619,13 +621,13 @@ def ws_require_user( return if (only_system_user and - not connection.user.system_generated): + not connection.user.group.system_generated): output_error('only_system_user', 'Only allowed as system user') return if (not allow_system_user - and connection.user.system_generated): + and connection.user.group.system_generated): output_error('not_system_user', 'Not allowed as system user') return diff --git a/tests/auth/test_auth_store.py b/tests/auth/test_auth_store.py new file mode 100644 index 000000000..b539d4e5c --- /dev/null +++ b/tests/auth/test_auth_store.py @@ -0,0 +1,32 @@ +"""Test the auth store.""" +import json + +from homeassistant.auth import auth_store + +from tests.common import load_fixture + + +async def test_migration_v1_v2(hass, hass_storage): + """Test migrating auth store to add groups.""" + hass_storage[auth_store.STORAGE_KEY] = json.loads( + load_fixture('auth_v1.json')) + store = auth_store.DataStore(hass) + data = await store.async_load() + + assert len(data['groups']) == 2 + system_group, family_group = data['groups'] + + assert system_group['system_generated'] is True + assert system_group['name'] == 'System' + + assert family_group['system_generated'] is False + assert family_group['name'] == 'Family' + + assert len(data['users']) == 2 + owner, hassio = data['users'] + + assert owner['is_owner'] is True + assert owner['group_id'] == family_group['id'] + + assert hassio['is_owner'] is False + assert hassio['group_id'] == system_group['id'] diff --git a/tests/auth/test_init.py b/tests/auth/test_init.py index 8fd9b8930..d59936f14 100644 --- a/tests/auth/test_init.py +++ b/tests/auth/test_init.py @@ -334,7 +334,7 @@ async def test_generating_system_user(hass): manager = await auth.auth_manager_from_config(hass, [], []) user = await manager.async_create_system_user('Hass.io') token = await manager.async_create_refresh_token(user) - assert user.system_generated + assert user.group.system_generated assert token is not None assert token.client_id is None @@ -343,7 +343,7 @@ async def test_refresh_token_requires_client_for_user(hass): """Test create refresh token for a user with client_id.""" manager = await auth.auth_manager_from_config(hass, [], []) user = MockUser().add_to_auth_manager(manager) - assert user.system_generated is False + assert user.group.system_generated is False with pytest.raises(ValueError): await manager.async_create_refresh_token(user) @@ -361,7 +361,7 @@ async def test_refresh_token_not_requires_client_for_system_user(hass): """Test create refresh token for a system user w/o client_id.""" manager = await auth.auth_manager_from_config(hass, [], []) user = await manager.async_create_system_user('Hass.io') - assert user.system_generated is True + assert user.group.system_generated is True with pytest.raises(ValueError): await manager.async_create_refresh_token(user, CLIENT_ID) diff --git a/tests/common.py b/tests/common.py index 0cb15d683..9cee9de9e 100644 --- a/tests/common.py +++ b/tests/common.py @@ -355,11 +355,13 @@ class MockUser(auth_models.User): 'is_owner': is_owner, 'is_active': is_active, 'name': name, - 'system_generated': system_generated, + # Filled in when added to hass/auth manager + 'group': None, } if id is not None: kwargs['id'] = id super().__init__(**kwargs) + self._mock_system_generated = system_generated def add_to_hass(self, hass): """Test helper to add entry to hass.""" @@ -368,6 +370,14 @@ class MockUser(auth_models.User): def add_to_auth_manager(self, auth_mgr): """Test helper to add entry to hass.""" ensure_auth_manager_loaded(auth_mgr) + + if self._mock_system_generated: + group_id = auth_mgr._store.system_user_group_id + else: + group_id = auth_mgr._store.default_new_user_group_id + + self.group = auth_mgr._store._groups[group_id] + auth_mgr._store._users[self.id] = self return self @@ -392,7 +402,7 @@ def ensure_auth_manager_loaded(auth_mgr): """Ensure an auth manager is considered loaded.""" store = auth_mgr._store if store._users is None: - store._users = OrderedDict() + store._set_defaults() class MockModule: diff --git a/tests/components/config/test_auth.py b/tests/components/config/test_auth.py index cd04eedf0..c630b562d 100644 --- a/tests/components/config/test_auth.py +++ b/tests/components/config/test_auth.py @@ -83,6 +83,7 @@ async def test_list(hass, hass_ws_client): 'is_owner': True, 'is_active': True, 'system_generated': False, + 'group_id': owner.group.id, 'credentials': [{'type': 'homeassistant'}] } assert data[1] == { @@ -91,6 +92,7 @@ async def test_list(hass, hass_ws_client): 'is_owner': False, 'is_active': True, 'system_generated': True, + 'group_id': system.group.id, 'credentials': [], } assert data[2] == { @@ -99,6 +101,7 @@ async def test_list(hass, hass_ws_client): 'is_owner': False, 'is_active': False, 'system_generated': False, + 'group_id': inactive.group.id, 'credentials': [], } @@ -201,7 +204,7 @@ async def test_create(hass, hass_ws_client, hass_access_token): assert user.name == data_user['name'] assert user.is_active assert not user.is_owner - assert not user.system_generated + assert not user.group.system_generated async def test_create_requires_owner(hass, hass_ws_client, hass_access_token): diff --git a/tests/components/hassio/test_init.py b/tests/components/hassio/test_init.py index 4fd59dd3f..f85024b00 100644 --- a/tests/components/hassio/test_init.py +++ b/tests/components/hassio/test_init.py @@ -105,7 +105,7 @@ async def test_setup_api_push_api_data_default(hass, aioclient_mock, hass_storage[STORAGE_KEY]['data']['hassio_user'] ) assert hassio_user is not None - assert hassio_user.system_generated + assert hassio_user.group.system_generated for token in hassio_user.refresh_tokens.values(): if token.token == refresh_token: break diff --git a/tests/fixtures/auth_v1.json b/tests/fixtures/auth_v1.json new file mode 100644 index 000000000..cc1530ede --- /dev/null +++ b/tests/fixtures/auth_v1.json @@ -0,0 +1,24 @@ +{ + "data": { + "credentials": [], + "refresh_tokens": [], + "users": [ + { + "id": "74f25fa868d649cd83114b321f5f0256", + "is_active": true, + "is_owner": true, + "name": "Paulus", + "system_generated": false + }, + { + "id": "b8e85152681d4611a13fb3ffe04b99f4", + "is_active": true, + "is_owner": false, + "name": "Hass.io", + "system_generated": true + } + ] + }, + "key": "auth", + "version": 1 +} From 48574cd98b2360a7bf2ec0c5bc1ac6670fbe349e Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Fri, 28 Sep 2018 13:03:09 +0200 Subject: [PATCH 2/5] Make group attrs private --- homeassistant/auth/auth_store.py | 20 ++++++++++---------- tests/common.py | 4 ++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/homeassistant/auth/auth_store.py b/homeassistant/auth/auth_store.py index 20044e43a..ceb8ea9a9 100644 --- a/homeassistant/auth/auth_store.py +++ b/homeassistant/auth/auth_store.py @@ -70,8 +70,8 @@ class AuthStore: self.hass = hass self._users = None # type: Optional[Dict[str, models.User]] self._groups = None # type: Optional[Dict[str, models.Group]] - self.default_new_user_group_id = None # type: Optional[str] - self.system_user_group_id = None # type: Optional[str] + self._default_new_user_group_id = None # type: Optional[str] + self._system_user_group_id = None # type: Optional[str] self._store = DataStore(hass) async def async_get_users(self) -> List[models.User]: @@ -102,9 +102,9 @@ class AuthStore: assert self._users is not None if system_generated: - group_id = self.system_user_group_id + group_id = self._system_user_group_id elif group_id is None: - group_id = self.default_new_user_group_id + group_id = self._default_new_user_group_id kwargs = { 'name': name, @@ -343,8 +343,8 @@ class AuthStore: self._groups = groups self._users = users - self.default_new_user_group_id = data['default_new_user_group_id'] - self.system_user_group_id = data['system_user_group_id'] + self._default_new_user_group_id = data['default_new_user_group_id'] + self._system_user_group_id = data['system_user_group_id'] @callback def _async_schedule_save(self) -> None: @@ -418,8 +418,8 @@ class AuthStore: 'groups': groups, 'credentials': credentials, 'refresh_tokens': refresh_tokens, - 'default_new_user_group_id': self.default_new_user_group_id, - 'system_user_group_id': self.system_user_group_id, + 'default_new_user_group_id': self._default_new_user_group_id, + 'system_user_group_id': self._system_user_group_id, } def _set_defaults(self): @@ -430,8 +430,8 @@ class AuthStore: system_group = models.Group(name='System', system_generated=True) family_group = models.Group(name='Family') - self.default_new_user_group_id = family_group.id - self.system_user_group_id = system_group.id + self._default_new_user_group_id = family_group.id + self._system_user_group_id = system_group.id groups = OrderedDict() groups[system_group.id] = system_group diff --git a/tests/common.py b/tests/common.py index 9cee9de9e..963f18781 100644 --- a/tests/common.py +++ b/tests/common.py @@ -372,9 +372,9 @@ class MockUser(auth_models.User): ensure_auth_manager_loaded(auth_mgr) if self._mock_system_generated: - group_id = auth_mgr._store.system_user_group_id + group_id = auth_mgr._store._system_user_group_id else: - group_id = auth_mgr._store.default_new_user_group_id + group_id = auth_mgr._store._default_new_user_group_id self.group = auth_mgr._store._groups[group_id] From b9f004b54901fdd47a1fbbcf3208d7ee57f8d637 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Fri, 28 Sep 2018 13:53:29 +0200 Subject: [PATCH 3/5] Lint --- homeassistant/auth/__init__.py | 3 ++- homeassistant/auth/auth_store.py | 18 ++++++++++++------ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/homeassistant/auth/__init__.py b/homeassistant/auth/__init__.py index d45ab0b5b..80bf81b31 100644 --- a/homeassistant/auth/__init__.py +++ b/homeassistant/auth/__init__.py @@ -266,7 +266,8 @@ class AuthManager: else: token_type = models.TOKEN_TYPE_NORMAL - if user.group.system_generated != (token_type == models.TOKEN_TYPE_SYSTEM): + if user.group.system_generated != (token_type == + models.TOKEN_TYPE_SYSTEM): raise ValueError( 'System generated users can only have system type ' 'refresh tokens') diff --git a/homeassistant/auth/auth_store.py b/homeassistant/auth/auth_store.py index ceb8ea9a9..308a5cfe8 100644 --- a/homeassistant/auth/auth_store.py +++ b/homeassistant/auth/auth_store.py @@ -20,11 +20,12 @@ STORAGE_KEY = 'auth' class DataStore(storage.Store): """Store the auth data on disk using JSON.""" - def __init__(self, hass): + def __init__(self, hass: HomeAssistant) -> None: """Initialize the data store.""" super().__init__(hass, STORAGE_VERSION, STORAGE_KEY, True) - async def _async_migrate_func(self, old_version, old_data): + async def _async_migrate_func(self, old_version: int, + old_data: Dict[str, Any]) -> Dict[str, Any]: """Migrate to the new version.""" if old_version <= 1: system_group_id = old_data['system_user_group_id'] = \ @@ -99,7 +100,11 @@ class AuthStore: """Create a new user.""" if self._users is None: await self._async_load() - assert self._users is not None + + assert self._users is not None + assert self._groups is not None + assert self._system_user_group_id is not None + assert self._default_new_user_group_id is not None if system_generated: group_id = self._system_user_group_id @@ -358,6 +363,7 @@ class AuthStore: def _data_to_save(self) -> Dict: """Return the data to store.""" assert self._users is not None + assert self._groups is not None users = [ { @@ -422,9 +428,9 @@ class AuthStore: 'system_user_group_id': self._system_user_group_id, } - def _set_defaults(self): + def _set_defaults(self) -> None: """Set default values for auth store.""" - self._users = OrderedDict() + self._users = OrderedDict() # type: Dict[str, models.User] # Add default groups system_group = models.Group(name='System', system_generated=True) @@ -433,7 +439,7 @@ class AuthStore: self._default_new_user_group_id = family_group.id self._system_user_group_id = system_group.id - groups = OrderedDict() + groups = OrderedDict() # type: Dict[str, models.Group] groups[system_group.id] = system_group groups[family_group.id] = family_group From 4f26935756a6ff4a45d882d0ff6059e020ae0d93 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Fri, 28 Sep 2018 14:12:20 +0200 Subject: [PATCH 4/5] Move backwards compat code to migration --- homeassistant/auth/auth_store.py | 62 ++++++++++++++++---------------- tests/auth/test_auth_store.py | 17 ++++++++- tests/fixtures/auth_v1.json | 29 ++++++++++++++- 3 files changed, 76 insertions(+), 32 deletions(-) diff --git a/homeassistant/auth/auth_store.py b/homeassistant/auth/auth_store.py index 308a5cfe8..971187da2 100644 --- a/homeassistant/auth/auth_store.py +++ b/homeassistant/auth/auth_store.py @@ -2,7 +2,6 @@ from collections import OrderedDict from datetime import timedelta import hmac -from logging import getLogger from typing import Any, Dict, List, Optional # noqa: F401 import uuid @@ -46,13 +45,36 @@ class DataStore(storage.Store): }, ] - for user_info in old_data['users']: - if user_info.pop('system_generated'): + for user_dict in old_data['users']: + if user_dict.pop('system_generated'): group_id = system_group_id else: group_id = family_group_id - user_info['group_id'] = group_id + user_dict['group_id'] = group_id + + refresh_tokens = [] + + for rt_dict in old_data['refresh_tokens']: + if 'jwt_key' not in rt_dict: + continue + + if 'token_type' not in rt_dict: + if rt_dict['client_id'] is None: + token_type = models.TOKEN_TYPE_SYSTEM + else: + token_type = models.TOKEN_TYPE_NORMAL + + rt_dict['token_type'] = token_type + + rt_dict.setdefault('last_used_at', None) + rt_dict.setdefault('client_name', None) + rt_dict.setdefault('client_icon', None) + rt_dict.setdefault('last_used_ip', None) + + refresh_tokens.append(rt_dict) + + old_data['refresh_tokens'] = refresh_tokens return old_data @@ -303,26 +325,7 @@ class AuthStore: )) for rt_dict in data['refresh_tokens']: - # Filter out the old keys that don't have jwt_key (pre-0.76) - if 'jwt_key' not in rt_dict: - continue - - created_at = dt_util.parse_datetime(rt_dict['created_at']) - if created_at is None: - getLogger(__name__).error( - 'Ignoring refresh token %(id)s with invalid created_at ' - '%(created_at)s for user_id %(user_id)s', rt_dict) - continue - - token_type = rt_dict.get('token_type') - if token_type is None: - if rt_dict['client_id'] is None: - token_type = models.TOKEN_TYPE_SYSTEM - else: - token_type = models.TOKEN_TYPE_NORMAL - - # old refresh_token don't have last_used_at (pre-0.78) - last_used_at_str = rt_dict.get('last_used_at') + last_used_at_str = rt_dict['last_used_at'] if last_used_at_str: last_used_at = dt_util.parse_datetime(last_used_at_str) else: @@ -332,17 +335,16 @@ class AuthStore: id=rt_dict['id'], user=users[rt_dict['user_id']], client_id=rt_dict['client_id'], - # use dict.get to keep backward compatibility - client_name=rt_dict.get('client_name'), - client_icon=rt_dict.get('client_icon'), - token_type=token_type, - created_at=created_at, + client_name=rt_dict['client_name'], + client_icon=rt_dict['client_icon'], + token_type=rt_dict['token_type'], + created_at=dt_util.parse_datetime(rt_dict['created_at']), access_token_expiration=timedelta( seconds=rt_dict['access_token_expiration']), token=rt_dict['token'], jwt_key=rt_dict['jwt_key'], last_used_at=last_used_at, - last_used_ip=rt_dict.get('last_used_ip'), + last_used_ip=rt_dict['last_used_ip'], ) users[rt_dict['user_id']].refresh_tokens[token.id] = token diff --git a/tests/auth/test_auth_store.py b/tests/auth/test_auth_store.py index b539d4e5c..d7278d979 100644 --- a/tests/auth/test_auth_store.py +++ b/tests/auth/test_auth_store.py @@ -1,7 +1,7 @@ """Test the auth store.""" import json -from homeassistant.auth import auth_store +from homeassistant.auth import auth_store, models from tests.common import load_fixture @@ -30,3 +30,18 @@ async def test_migration_v1_v2(hass, hass_storage): assert hassio['is_owner'] is False assert hassio['group_id'] == system_group['id'] + + assert len(data['refresh_tokens']) == 2 + user_token, system_token = data['refresh_tokens'] + + assert user_token['token_type'] == models.TOKEN_TYPE_NORMAL + assert user_token['last_used_at'] is None + assert user_token['client_name'] is None + assert user_token['client_icon'] is None + assert user_token['last_used_ip'] is None + + assert system_token['token_type'] == models.TOKEN_TYPE_SYSTEM + assert system_token['last_used_at'] is None + assert system_token['client_name'] is None + assert system_token['client_icon'] is None + assert system_token['last_used_ip'] is None diff --git a/tests/fixtures/auth_v1.json b/tests/fixtures/auth_v1.json index cc1530ede..f4ed4cb91 100644 --- a/tests/fixtures/auth_v1.json +++ b/tests/fixtures/auth_v1.json @@ -1,7 +1,34 @@ { "data": { "credentials": [], - "refresh_tokens": [], + "refresh_tokens": [ + { + "access_token_expiration": 1800.0, + "client_id": "http://localhost:8123/", + "created_at": "2018-09-28T10:35:47.969734+00:00", + "id": "aaaaf7deb7ab454685d33c50c319ae7e", + "jwt_key": "some-jwt-key", + "token": "some-token", + "user_id": "74f25fa868d649bf83114b321f5f0256" + }, + { + "access_token_expiration": 1800.0, + "client_id": "http://localhost:8123/", + "created_at": "2018-09-28T10:35:47.969734+00:00", + "id": "bbbbf7deb7ab454685d33c50c319ae7e", + "token": "some-token", + "user_id": "74f25fa868d649bf83114b321f5f0256" + }, + { + "access_token_expiration": 1800.0, + "client_id": null, + "created_at": "2018-09-28T10:35:47.969734+00:00", + "id": "aaaaf7deb7ab454685d33c50c319ae7e", + "jwt_key": "some-jwt-key", + "token": "some-token", + "user_id": "74f25fa868d649bf83114b321f5f0256" + } + ], "users": [ { "id": "74f25fa868d649cd83114b321f5f0256", From a1e4b98fa8c21f4113bfb198a929ed18d4531ef7 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Fri, 28 Sep 2018 15:16:41 +0200 Subject: [PATCH 5/5] Typing --- homeassistant/auth/auth_store.py | 10 +++++++++- homeassistant/helpers/storage.py | 4 ++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/homeassistant/auth/auth_store.py b/homeassistant/auth/auth_store.py index 971187da2..f1f249b3d 100644 --- a/homeassistant/auth/auth_store.py +++ b/homeassistant/auth/auth_store.py @@ -2,6 +2,7 @@ from collections import OrderedDict from datetime import timedelta import hmac +from logging import getLogger from typing import Any, Dict, List, Optional # noqa: F401 import uuid @@ -325,6 +326,13 @@ class AuthStore: )) for rt_dict in data['refresh_tokens']: + created_at = dt_util.parse_datetime(rt_dict['created_at']) + if created_at is None: + getLogger(__name__).error( + 'Ignoring refresh token %(id)s with invalid created_at ' + '%(created_at)s for user_id %(user_id)s', rt_dict) + continue + last_used_at_str = rt_dict['last_used_at'] if last_used_at_str: last_used_at = dt_util.parse_datetime(last_used_at_str) @@ -338,7 +346,7 @@ class AuthStore: client_name=rt_dict['client_name'], client_icon=rt_dict['client_icon'], token_type=rt_dict['token_type'], - created_at=dt_util.parse_datetime(rt_dict['created_at']), + created_at=created_at, access_token_expiration=timedelta( seconds=rt_dict['access_token_expiration']), token=rt_dict['token'], diff --git a/homeassistant/helpers/storage.py b/homeassistant/helpers/storage.py index 7d867b50e..cfe73d6d1 100644 --- a/homeassistant/helpers/storage.py +++ b/homeassistant/helpers/storage.py @@ -2,7 +2,7 @@ import asyncio import logging import os -from typing import Dict, Optional, Callable +from typing import Dict, Optional, Callable, Any from homeassistant.const import EVENT_HOMEASSISTANT_STOP from homeassistant.core import callback @@ -63,7 +63,7 @@ class Store: """Return the config path.""" return self.hass.config.path(STORAGE_DIR, self.key) - async def async_load(self): + async def async_load(self) -> Optional[Dict[str, Any]]: """Load data. If the expected version does not match the given version, the migrate