From ed0cfc4f31924ecf3f5a4a2cf7789e5c209563db Mon Sep 17 00:00:00 2001
From: Paulus Schoutsen <paulus@paulusschoutsen.nl>
Date: Sun, 15 Jul 2018 20:46:15 +0200
Subject: [PATCH] Add user via cmd line creates owner (#15470)
* Add user via cmd line creates owner
* Ensure access tokens are not verified for inactive users
* Stale print
* Lint
---
homeassistant/auth/__init__.py | 53 +++++++++++-------
homeassistant/auth/auth_store.py | 10 ++++
homeassistant/components/auth/__init__.py | 6 +++
homeassistant/components/http/auth.py | 5 --
tests/auth/test_init.py | 4 +-
tests/components/auth/test_init.py | 37 +++----------
tests/components/auth/test_init_link_user.py | 56 ++++----------------
tests/components/test_websocket_api.py | 27 ++++++++++
8 files changed, 97 insertions(+), 101 deletions(-)
diff --git a/homeassistant/auth/__init__.py b/homeassistant/auth/__init__.py
index fb35bd05c33..b05fca164a0 100644
--- a/homeassistant/auth/__init__.py
+++ b/homeassistant/auth/__init__.py
@@ -93,10 +93,15 @@ class AuthManager:
async def async_create_user(self, name):
"""Create a user."""
- return await self._store.async_create_user(
- name=name,
- is_active=True,
- )
+ kwargs = {
+ 'name': name,
+ 'is_active': True,
+ }
+
+ if await self._user_should_be_owner():
+ kwargs['is_owner'] = True
+
+ return await self._store.async_create_user(**kwargs)
async def async_get_or_create_user(self, credentials):
"""Get or create a user."""
@@ -116,20 +121,10 @@ class AuthManager:
info = await auth_provider.async_user_meta_for_credentials(
credentials)
- kwargs = {
- 'credentials': credentials,
- 'name': info.get('name')
- }
-
- # Make owner and activate user if it's the first user.
- if await self._store.async_get_users():
- kwargs['is_owner'] = False
- kwargs['is_active'] = False
- else:
- kwargs['is_owner'] = True
- kwargs['is_active'] = True
-
- return await self._store.async_create_user(**kwargs)
+ return await self._store.async_create_user(
+ credentials=credentials,
+ name=info.get('name'),
+ )
async def async_link_user(self, user, credentials):
"""Link credentials to an existing user."""
@@ -147,6 +142,14 @@ class AuthManager:
await self._store.async_remove_user(user)
+ async def async_activate_user(self, user):
+ """Activate a user."""
+ await self._store.async_activate_user(user)
+
+ async def async_deactivate_user(self, user):
+ """Deactivate a user."""
+ await self._store.async_deactivate_user(user)
+
async def async_remove_credentials(self, credentials):
"""Remove credentials."""
provider = self._async_get_auth_provider(credentials)
@@ -191,7 +194,7 @@ class AuthManager:
if tkn is None:
return None
- if tkn.expired:
+ if tkn.expired or not tkn.refresh_token.user.is_active:
self._access_tokens.pop(token)
return None
@@ -218,3 +221,15 @@ class AuthManager:
auth_provider_key = (credentials.auth_provider_type,
credentials.auth_provider_id)
return self._providers.get(auth_provider_key)
+
+ async def _user_should_be_owner(self):
+ """Determine if user should be owner.
+
+ A user should be an owner if it is the first non-system user that is
+ being created.
+ """
+ for user in await self._store.async_get_users():
+ if not user.system_generated:
+ return False
+
+ return True
diff --git a/homeassistant/auth/auth_store.py b/homeassistant/auth/auth_store.py
index ebd61140ac1..8fd66d4bbb7 100644
--- a/homeassistant/auth/auth_store.py
+++ b/homeassistant/auth/auth_store.py
@@ -81,6 +81,16 @@ class AuthStore:
self._users.pop(user.id)
await self.async_save()
+ async def async_activate_user(self, user):
+ """Activate a user."""
+ user.is_active = True
+ await self.async_save()
+
+ async def async_deactivate_user(self, user):
+ """Activate a user."""
+ user.is_active = False
+ await self.async_save()
+
async def async_remove_credentials(self, credentials):
"""Remove credentials."""
for user in self._users.values():
diff --git a/homeassistant/components/auth/__init__.py b/homeassistant/components/auth/__init__.py
index 1ead4cacdf0..f9588093933 100644
--- a/homeassistant/components/auth/__init__.py
+++ b/homeassistant/components/auth/__init__.py
@@ -275,6 +275,12 @@ class GrantTokenView(HomeAssistantView):
}, status_code=400)
user = await hass.auth.async_get_or_create_user(credentials)
+
+ if not user.is_active:
+ return self.json({
+ 'error': 'invalid_request',
+ }, status_code=400)
+
refresh_token = await hass.auth.async_create_refresh_token(user,
client_id)
access_token = hass.auth.async_create_access_token(refresh_token)
diff --git a/homeassistant/components/http/auth.py b/homeassistant/components/http/auth.py
index 46d77214160..2cc62dce38e 100644
--- a/homeassistant/components/http/auth.py
+++ b/homeassistant/components/http/auth.py
@@ -106,11 +106,6 @@ async def async_validate_auth_header(request, api_password=None):
if access_token is None:
return False
- user = access_token.refresh_token.user
-
- if not user.is_active:
- return False
-
request['hass_user'] = access_token.refresh_token.user
return True
diff --git a/tests/auth/test_init.py b/tests/auth/test_init.py
index f7187fd49fd..3e3662c13c4 100644
--- a/tests/auth/test_init.py
+++ b/tests/auth/test_init.py
@@ -80,7 +80,7 @@ async def test_create_new_user(hass, hass_storage):
credentials = step['result']
user = await manager.async_get_or_create_user(credentials)
assert user is not None
- assert user.is_owner is True
+ assert user.is_owner is False
assert user.name == 'Test Name'
@@ -198,7 +198,7 @@ async def test_saving_loading(hass, hass_storage):
'password': 'test-pass',
})
user = await manager.async_get_or_create_user(step['result'])
-
+ await manager.async_activate_user(user)
refresh_token = await manager.async_create_refresh_token(user, CLIENT_ID)
manager.async_create_access_token(refresh_token)
diff --git a/tests/components/auth/test_init.py b/tests/components/auth/test_init.py
index c5c46d55e39..59fc8714f77 100644
--- a/tests/components/auth/test_init.py
+++ b/tests/components/auth/test_init.py
@@ -10,7 +10,7 @@ from . import async_setup_auth
from tests.common import CLIENT_ID, CLIENT_REDIRECT_URI
-async def test_login_new_user_and_refresh_token(hass, aiohttp_client):
+async def test_login_new_user_and_trying_refresh_token(hass, aiohttp_client):
"""Test logging in with new user and refreshing tokens."""
client = await async_setup_auth(hass, aiohttp_client, setup_api=True)
resp = await client.post('/auth/login_flow', json={
@@ -34,36 +34,13 @@ async def test_login_new_user_and_refresh_token(hass, aiohttp_client):
# Exchange code for tokens
resp = await client.post('/auth/token', data={
- 'client_id': CLIENT_ID,
- 'grant_type': 'authorization_code',
- 'code': code
- })
-
- assert resp.status == 200
- tokens = await resp.json()
-
- assert hass.auth.async_get_access_token(tokens['access_token']) is not None
-
- # Use refresh token to get more tokens.
- resp = await client.post('/auth/token', data={
- 'client_id': CLIENT_ID,
- 'grant_type': 'refresh_token',
- 'refresh_token': tokens['refresh_token']
- })
-
- assert resp.status == 200
- tokens = await resp.json()
- assert 'refresh_token' not in tokens
- assert hass.auth.async_get_access_token(tokens['access_token']) is not None
-
- # Test using access token to hit API.
- resp = await client.get('/api/')
- assert resp.status == 401
-
- resp = await client.get('/api/', headers={
- 'authorization': 'Bearer {}'.format(tokens['access_token'])
+ 'client_id': CLIENT_ID,
+ 'grant_type': 'authorization_code',
+ 'code': code
})
- assert resp.status == 200
+
+ # User is not active
+ assert resp.status == 400
def test_credential_store_expiration():
diff --git a/tests/components/auth/test_init_link_user.py b/tests/components/auth/test_init_link_user.py
index 28a924bb43a..13515db87fa 100644
--- a/tests/components/auth/test_init_link_user.py
+++ b/tests/components/auth/test_init_link_user.py
@@ -25,40 +25,9 @@ async def async_get_code(hass, aiohttp_client):
}]
}]
client = await async_setup_auth(hass, aiohttp_client, config)
-
- resp = await client.post('/auth/login_flow', json={
- 'client_id': CLIENT_ID,
- 'handler': ['insecure_example', None],
- 'redirect_uri': CLIENT_REDIRECT_URI,
- })
- assert resp.status == 200
- step = await resp.json()
-
- resp = await client.post(
- '/auth/login_flow/{}'.format(step['flow_id']), json={
- 'client_id': CLIENT_ID,
- 'username': 'test-user',
- 'password': 'test-pass',
- })
-
- assert resp.status == 200
- step = await resp.json()
- code = step['result']
-
- # Exchange code for tokens
- resp = await client.post('/auth/token', data={
- 'client_id': CLIENT_ID,
- 'grant_type': 'authorization_code',
- 'code': code
- })
-
- assert resp.status == 200
- tokens = await resp.json()
-
- access_token = hass.auth.async_get_access_token(tokens['access_token'])
- assert access_token is not None
- user = access_token.refresh_token.user
- assert len(user.credentials) == 1
+ user = await hass.auth.async_create_user(name='Hello')
+ refresh_token = await hass.auth.async_create_refresh_token(user, CLIENT_ID)
+ access_token = hass.auth.async_create_access_token(refresh_token)
# Now authenticate with the 2nd flow
resp = await client.post('/auth/login_flow', json={
@@ -83,7 +52,7 @@ async def async_get_code(hass, aiohttp_client):
'user': user,
'code': step['result'],
'client': client,
- 'tokens': tokens,
+ 'access_token': access_token.token,
}
@@ -92,18 +61,17 @@ async def test_link_user(hass, aiohttp_client):
info = await async_get_code(hass, aiohttp_client)
client = info['client']
code = info['code']
- tokens = info['tokens']
# Link user
resp = await client.post('/auth/link_user', json={
'client_id': CLIENT_ID,
'code': code
}, headers={
- 'authorization': 'Bearer {}'.format(tokens['access_token'])
+ 'authorization': 'Bearer {}'.format(info['access_token'])
})
assert resp.status == 200
- assert len(info['user'].credentials) == 2
+ assert len(info['user'].credentials) == 1
async def test_link_user_invalid_client_id(hass, aiohttp_client):
@@ -111,36 +79,34 @@ async def test_link_user_invalid_client_id(hass, aiohttp_client):
info = await async_get_code(hass, aiohttp_client)
client = info['client']
code = info['code']
- tokens = info['tokens']
# Link user
resp = await client.post('/auth/link_user', json={
'client_id': 'invalid',
'code': code
}, headers={
- 'authorization': 'Bearer {}'.format(tokens['access_token'])
+ 'authorization': 'Bearer {}'.format(info['access_token'])
})
assert resp.status == 400
- assert len(info['user'].credentials) == 1
+ assert len(info['user'].credentials) == 0
async def test_link_user_invalid_code(hass, aiohttp_client):
"""Test linking a user to new credentials."""
info = await async_get_code(hass, aiohttp_client)
client = info['client']
- tokens = info['tokens']
# Link user
resp = await client.post('/auth/link_user', json={
'client_id': CLIENT_ID,
'code': 'invalid'
}, headers={
- 'authorization': 'Bearer {}'.format(tokens['access_token'])
+ 'authorization': 'Bearer {}'.format(info['access_token'])
})
assert resp.status == 400
- assert len(info['user'].credentials) == 1
+ assert len(info['user'].credentials) == 0
async def test_link_user_invalid_auth(hass, aiohttp_client):
@@ -156,4 +122,4 @@ async def test_link_user_invalid_auth(hass, aiohttp_client):
}, headers={'authorization': 'Bearer invalid'})
assert resp.status == 401
- assert len(info['user'].credentials) == 1
+ assert len(info['user'].credentials) == 0
diff --git a/tests/components/test_websocket_api.py b/tests/components/test_websocket_api.py
index 6ea90bcdb88..dc1688bae16 100644
--- a/tests/components/test_websocket_api.py
+++ b/tests/components/test_websocket_api.py
@@ -341,6 +341,33 @@ async def test_auth_active_with_token(hass, aiohttp_client, hass_access_token):
assert auth_msg['type'] == wapi.TYPE_AUTH_OK
+async def test_auth_active_user_inactive(hass, aiohttp_client,
+ hass_access_token):
+ """Test authenticating with a token."""
+ hass_access_token.refresh_token.user.is_active = False
+ assert await async_setup_component(hass, 'websocket_api', {
+ 'http': {
+ 'api_password': API_PASSWORD
+ }
+ })
+
+ client = await aiohttp_client(hass.http.app)
+
+ async with client.ws_connect(wapi.URL) as ws:
+ with patch('homeassistant.auth.AuthManager.active') as auth_active:
+ auth_active.return_value = True
+ auth_msg = await ws.receive_json()
+ assert auth_msg['type'] == wapi.TYPE_AUTH_REQUIRED
+
+ await ws.send_json({
+ 'type': wapi.TYPE_AUTH,
+ 'access_token': hass_access_token.token
+ })
+
+ auth_msg = await ws.receive_json()
+ assert auth_msg['type'] == wapi.TYPE_AUTH_INVALID
+
+
async def test_auth_active_with_password_not_allow(hass, aiohttp_client):
"""Test authenticating with a token."""
assert await async_setup_component(hass, 'websocket_api', {
--
GitLab