From 4882fed939863369a5fb9b7bbe4179266fe1686a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 9 Mar 2024 11:03:22 -1000 Subject: [PATCH] Avoid saving auth right after we load it during startup (#112008) --- homeassistant/auth/auth_store.py | 17 ++- tests/auth/test_auth_store.py | 192 +++++++++++++------------------ 2 files changed, 95 insertions(+), 114 deletions(-) diff --git a/homeassistant/auth/auth_store.py b/homeassistant/auth/auth_store.py index b9f7b3a7e14..b3481acca3c 100644 --- a/homeassistant/auth/auth_store.py +++ b/homeassistant/auth/auth_store.py @@ -31,6 +31,17 @@ GROUP_NAME_ADMIN = "Administrators" GROUP_NAME_USER = "Users" GROUP_NAME_READ_ONLY = "Read Only" +# We always save the auth store after we load it since +# we may migrate data and do not want to have to do it again +# but we don't want to do it during startup so we schedule +# the first save 5 minutes out knowing something else may +# want to save the auth store before then, and since Storage +# will honor the lower of the two delays, it will save it +# faster if something else saves it. +INITIAL_LOAD_SAVE_DELAY = 300 + +DEFAULT_SAVE_DELAY = 1 + class AuthStore: """Stores authentication info. @@ -468,12 +479,12 @@ class AuthStore: self._groups = groups self._users = users - self._async_schedule_save() + self._async_schedule_save(INITIAL_LOAD_SAVE_DELAY) @callback - def _async_schedule_save(self) -> None: + def _async_schedule_save(self, delay: float = DEFAULT_SAVE_DELAY) -> None: """Save users.""" - self._store.async_delay_save(self._data_to_save, 1) + self._store.async_delay_save(self._data_to_save, delay) @callback def _data_to_save(self) -> dict[str, list[dict[str, Any]]]: diff --git a/tests/auth/test_auth_store.py b/tests/auth/test_auth_store.py index 796f2335569..91048cd8568 100644 --- a/tests/auth/test_auth_store.py +++ b/tests/auth/test_auth_store.py @@ -6,71 +6,74 @@ from typing import Any from unittest.mock import patch from freezegun import freeze_time +from freezegun.api import FrozenDateTimeFactory import pytest from homeassistant.auth import auth_store from homeassistant.core import HomeAssistant from homeassistant.util import dt as dt_util +MOCK_STORAGE_DATA = { + "version": 1, + "data": { + "credentials": [], + "users": [ + { + "id": "user-id", + "is_active": True, + "is_owner": True, + "name": "Paulus", + "system_generated": False, + }, + { + "id": "system-id", + "is_active": True, + "is_owner": True, + "name": "Hass.io", + "system_generated": True, + }, + ], + "refresh_tokens": [ + { + "access_token_expiration": 1800.0, + "client_id": "http://localhost:8123/", + "created_at": "2018-10-03T13:43:19.774637+00:00", + "id": "user-token-id", + "jwt_key": "some-key", + "last_used_at": "2018-10-03T13:43:19.774712+00:00", + "token": "some-token", + "user_id": "user-id", + "version": "1.2.3", + }, + { + "access_token_expiration": 1800.0, + "client_id": None, + "created_at": "2018-10-03T13:43:19.774637+00:00", + "id": "system-token-id", + "jwt_key": "some-key", + "last_used_at": "2018-10-03T13:43:19.774712+00:00", + "token": "some-token", + "user_id": "system-id", + }, + { + "access_token_expiration": 1800.0, + "client_id": "http://localhost:8123/", + "created_at": "2018-10-03T13:43:19.774637+00:00", + "id": "hidden-because-no-jwt-id", + "last_used_at": "2018-10-03T13:43:19.774712+00:00", + "token": "some-token", + "user_id": "user-id", + }, + ], + }, +} + async def test_loading_no_group_data_format( hass: HomeAssistant, hass_storage: dict[str, Any] ) -> None: """Test we correctly load old data without any groups.""" - hass_storage[auth_store.STORAGE_KEY] = { - "version": 1, - "data": { - "credentials": [], - "users": [ - { - "id": "user-id", - "is_active": True, - "is_owner": True, - "name": "Paulus", - "system_generated": False, - }, - { - "id": "system-id", - "is_active": True, - "is_owner": True, - "name": "Hass.io", - "system_generated": True, - }, - ], - "refresh_tokens": [ - { - "access_token_expiration": 1800.0, - "client_id": "http://localhost:8123/", - "created_at": "2018-10-03T13:43:19.774637+00:00", - "id": "user-token-id", - "jwt_key": "some-key", - "last_used_at": "2018-10-03T13:43:19.774712+00:00", - "token": "some-token", - "user_id": "user-id", - "version": "1.2.3", - }, - { - "access_token_expiration": 1800.0, - "client_id": None, - "created_at": "2018-10-03T13:43:19.774637+00:00", - "id": "system-token-id", - "jwt_key": "some-key", - "last_used_at": "2018-10-03T13:43:19.774712+00:00", - "token": "some-token", - "user_id": "system-id", - }, - { - "access_token_expiration": 1800.0, - "client_id": "http://localhost:8123/", - "created_at": "2018-10-03T13:43:19.774637+00:00", - "id": "hidden-because-no-jwt-id", - "last_used_at": "2018-10-03T13:43:19.774712+00:00", - "token": "some-token", - "user_id": "user-id", - }, - ], - }, - } + hass_storage[auth_store.STORAGE_KEY] = MOCK_STORAGE_DATA store = auth_store.AuthStore(hass) await store.async_load() @@ -113,63 +116,7 @@ async def test_loading_all_access_group_data_format( hass: HomeAssistant, hass_storage: dict[str, Any] ) -> None: """Test we correctly load old data with single group.""" - hass_storage[auth_store.STORAGE_KEY] = { - "version": 1, - "data": { - "credentials": [], - "users": [ - { - "id": "user-id", - "is_active": True, - "is_owner": True, - "name": "Paulus", - "system_generated": False, - "group_ids": ["abcd-all-access"], - }, - { - "id": "system-id", - "is_active": True, - "is_owner": True, - "name": "Hass.io", - "system_generated": True, - }, - ], - "groups": [{"id": "abcd-all-access", "name": "All Access"}], - "refresh_tokens": [ - { - "access_token_expiration": 1800.0, - "client_id": "http://localhost:8123/", - "created_at": "2018-10-03T13:43:19.774637+00:00", - "id": "user-token-id", - "jwt_key": "some-key", - "last_used_at": "2018-10-03T13:43:19.774712+00:00", - "token": "some-token", - "user_id": "user-id", - "version": "1.2.3", - }, - { - "access_token_expiration": 1800.0, - "client_id": None, - "created_at": "2018-10-03T13:43:19.774637+00:00", - "id": "system-token-id", - "jwt_key": "some-key", - "last_used_at": "2018-10-03T13:43:19.774712+00:00", - "token": "some-token", - "user_id": "system-id", - "version": None, - }, - { - "access_token_expiration": 1800.0, - "client_id": "http://localhost:8123/", - "created_at": "2018-10-03T13:43:19.774637+00:00", - "id": "hidden-because-no-jwt-id", - "last_used_at": "2018-10-03T13:43:19.774712+00:00", - "token": "some-token", - "user_id": "user-id", - }, - ], - }, - } + hass_storage[auth_store.STORAGE_KEY] = MOCK_STORAGE_DATA store = auth_store.AuthStore(hass) await store.async_load() @@ -335,3 +282,26 @@ async def test_add_expire_at_property( assert token1.expire_at == now.timestamp() + timedelta(days=80).total_seconds() assert token2.expire_at assert token2.expire_at == now.timestamp() + timedelta(days=90).total_seconds() + + +async def test_loading_does_not_write_right_away( + hass: HomeAssistant, hass_storage: dict[str, Any], freezer: FrozenDateTimeFactory +) -> None: + """Test after calling load we wait five minutes to write.""" + hass_storage[auth_store.STORAGE_KEY] = MOCK_STORAGE_DATA + + store = auth_store.AuthStore(hass) + await store.async_load() + + # Wipe storage so we can verify if it was written + hass_storage[auth_store.STORAGE_KEY] = {} + + freezer.tick(auth_store.DEFAULT_SAVE_DELAY) + await hass.async_block_till_done() + assert hass_storage[auth_store.STORAGE_KEY] == {} + freezer.tick(auth_store.INITIAL_LOAD_SAVE_DELAY) + # Once for scheduling the task + await hass.async_block_till_done() + # Once for the task + await hass.async_block_till_done() + assert hass_storage[auth_store.STORAGE_KEY] != {}