Skip to content

Commit

Permalink
Allow usernames to be case-insensitive (home-assistant#20558)
Browse files Browse the repository at this point in the history
* Allow usernames to be case-insensitive

* Fix typing

* FLAKE*
  • Loading branch information
balloob authored and pvizeli committed Jan 29, 2019
1 parent b0ff51b commit 73a0c66
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 11 deletions.
29 changes: 25 additions & 4 deletions homeassistant/auth/providers/homeassistant.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
import base64
from collections import OrderedDict
import logging
from typing import Any, Dict, List, Optional, cast

from typing import Any, Dict, List, Optional, Set, cast # noqa: F401

import bcrypt
import voluptuous as vol
Expand Down Expand Up @@ -52,6 +53,9 @@ def __init__(self, hass: HomeAssistant) -> None:
self._store = hass.helpers.storage.Store(STORAGE_VERSION, STORAGE_KEY,
private=True)
self._data = None # type: Optional[Dict[str, Any]]
# Legacy mode will allow usernames to start/end with whitespace
# and will compare usernames case-insensitive.
# Remove in 2020 or when we launch 1.0.
self.is_legacy = False

@callback
Expand All @@ -60,7 +64,7 @@ def normalize_username(self, username: str) -> str:
if self.is_legacy:
return username

return username.strip()
return username.strip().casefold()

async def async_load(self) -> None:
"""Load stored data."""
Expand All @@ -71,17 +75,34 @@ async def async_load(self) -> None:
'users': []
}

seen = set() # type: Set[str]

for user in data['users']:
username = user['username']

# check if we have duplicates
folded = username.casefold()

if folded in seen:
self.is_legacy = True

logging.getLogger(__name__).warning(
"Home Assistant auth provider is running in legacy mode "
"because we detected usernames that are case-insensitive"
"equivalent. Please change the username: '%s'.", username)

break

seen.add(folded)

# check if we have unstripped usernames
if username != username.strip():
self.is_legacy = True

logging.getLogger(__name__).warning(
"Home Assistant auth provider is running in legacy mode "
"because we detected usernames that start or end in a "
"space. Please change the username.")
"space. Please change the username: '%s'.", username)

break

Expand All @@ -103,7 +124,7 @@ def validate_login(self, username: str, password: str) -> None:

# Compare all users to avoid timing attacks.
for user in self.users:
if username == user['username']:
if self.normalize_username(user['username']) == username:
found = user

if found is None:
Expand Down
22 changes: 15 additions & 7 deletions tests/auth/providers/test_homeassistant.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,14 @@ async def test_changing_password_raises_invalid_user(data, hass):
async def test_adding_user(data, hass):
"""Test adding a user."""
data.add_auth('test-user', 'test-pass')
data.validate_login('test-user', 'test-pass')
data.validate_login(' test-user ', 'test-pass')


async def test_adding_user_duplicate_username(data, hass):
"""Test adding a user with duplicate username."""
data.add_auth('test-user', 'test-pass')
with pytest.raises(hass_auth.InvalidUser):
data.add_auth('test-user ', 'other-pass')
data.add_auth('TEST-user ', 'other-pass')


async def test_validating_password_invalid_password(data, hass):
Expand All @@ -91,16 +90,22 @@ async def test_validating_password_invalid_password(data, hass):
with pytest.raises(hass_auth.InvalidAuth):
data.validate_login(' test-user ', 'invalid-pass')

with pytest.raises(hass_auth.InvalidAuth):
data.validate_login('test-user', 'test-pass ')

with pytest.raises(hass_auth.InvalidAuth):
data.validate_login('test-user', 'Test-pass')


async def test_changing_password(data, hass):
"""Test adding a user."""
data.add_auth('test-user', 'test-pass')
data.change_password('test-user ', 'new-pass')
data.change_password('TEST-USER ', 'new-pass')

with pytest.raises(hass_auth.InvalidAuth):
data.validate_login('test-user', 'test-pass')

data.validate_login('test-user', 'new-pass')
data.validate_login('test-UsEr', 'new-pass')


async def test_login_flow_validates(data, hass):
Expand All @@ -122,18 +127,18 @@ async def test_login_flow_validates(data, hass):
assert result['errors']['base'] == 'invalid_auth'

result = await flow.async_step_init({
'username': 'test-user ',
'username': 'TEST-user ',
'password': 'incorrect-pass',
})
assert result['type'] == data_entry_flow.RESULT_TYPE_FORM
assert result['errors']['base'] == 'invalid_auth'

result = await flow.async_step_init({
'username': 'test-user',
'username': 'test-USER',
'password': 'test-pass',
})
assert result['type'] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY
assert result['data']['username'] == 'test-user'
assert result['data']['username'] == 'test-USER'


async def test_saving_loading(data, hass):
Expand Down Expand Up @@ -179,6 +184,9 @@ async def test_legacy_adding_user_duplicate_username(legacy_data, hass):
legacy_data.add_auth('test-user', 'test-pass')
with pytest.raises(hass_auth.InvalidUser):
legacy_data.add_auth('test-user', 'other-pass')
# Not considered duplicate
legacy_data.add_auth('test-user ', 'test-pass')
legacy_data.add_auth('Test-user', 'test-pass')


async def test_legacy_validating_password_invalid_password(legacy_data, hass):
Expand Down

0 comments on commit 73a0c66

Please sign in to comment.