From b3c41a4bd0033ee5af14cd543c4ad9a6da2cf06b Mon Sep 17 00:00:00 2001 From: Hayden <64056131+hay-kot@users.noreply.github.com> Date: Sat, 13 Aug 2022 13:18:12 -0800 Subject: [PATCH] security: implement user lockout (#1552) * add data-types required for login security * implement user lockout checking at login * cleanup legacy patterns * expose passwords in test_user * test user lockout after bad attempts * test user service * bump alembic version * save increment to database * add locked_at to datetime transformer on import * do proper test cleanup * implement scheduled task * spelling * document env variables * implement context manager for session * use context manager * implement reset script * cleanup generator * run generator * implement API endpoint for resetting locked users * add button to reset all locked users * add info when account is locked * use ignore instead of expect-error --- ..._add_login_attemps_and_locked_at_field_.py | 26 ++++++++ dev/code-generation/gen_frontend_types.py | 18 +++--- .../installation/backend-config.md | 7 ++- frontend/api/admin/admin-users.ts | 7 ++- frontend/pages/admin/manage/users/index.vue | 33 ++++++++-- frontend/pages/login.vue | 5 +- frontend/types/api-types/admin.ts | 6 ++ frontend/types/api-types/cookbook.ts | 6 ++ frontend/types/api-types/group.ts | 24 ++++++- frontend/types/api-types/meal-plan.ts | 6 ++ frontend/types/api-types/recipe.ts | 24 +++++++ frontend/types/api-types/response.ts | 9 +++ frontend/types/api-types/user.ts | 11 ++++ mealie/app.py | 7 ++- mealie/core/security/security.py | 19 +++++- mealie/core/settings/settings.py | 6 ++ mealie/db/db_setup.py | 12 ++++ mealie/db/models/users/users.py | 4 +- mealie/repos/repository_users.py | 11 ++-- mealie/routes/admin/admin_management_users.py | 12 +++- mealie/routes/auth/auth.py | 6 +- mealie/schema/user/auth.py | 6 ++ mealie/schema/user/user.py | 29 ++++++--- mealie/scripts/reset_locked_users.py | 34 ++++++++++ .../services/backups_v2/alchemy_exporter.py | 2 +- mealie/services/scheduler/tasks/__init__.py | 2 + .../scheduler/tasks/reset_locked_users.py | 17 +++++ .../user_services/password_reset_service.py | 11 ++-- mealie/services/user_services/user_service.py | 39 ++++++++++++ tests/fixtures/fixture_admin.py | 1 + tests/fixtures/fixture_users.py | 4 ++ .../user_tests/test_user_login.py | 26 ++++++++ .../backup_v2_tests/test_alchemy_exporter.py | 2 +- .../user_services/test_user_service.py | 63 +++++++++++++++++++ tests/utils/fixture_schemas.py | 1 + 35 files changed, 450 insertions(+), 46 deletions(-) create mode 100644 alembic/versions/2022-08-12-19.05.59_188374910655_add_login_attemps_and_locked_at_field_.py create mode 100644 mealie/scripts/reset_locked_users.py create mode 100644 mealie/services/scheduler/tasks/reset_locked_users.py create mode 100644 mealie/services/user_services/user_service.py create mode 100644 tests/unit_tests/services_tests/user_services/test_user_service.py diff --git a/alembic/versions/2022-08-12-19.05.59_188374910655_add_login_attemps_and_locked_at_field_.py b/alembic/versions/2022-08-12-19.05.59_188374910655_add_login_attemps_and_locked_at_field_.py new file mode 100644 index 0000000000..9db5b91b1d --- /dev/null +++ b/alembic/versions/2022-08-12-19.05.59_188374910655_add_login_attemps_and_locked_at_field_.py @@ -0,0 +1,26 @@ +"""add login_attemps and locked_at field to user table + +Revision ID: 188374910655 +Revises: f30cf048c228 +Create Date: 2022-08-12 19:05:59.776361 + +""" +import sqlalchemy as sa + +from alembic import op + +# revision identifiers, used by Alembic. +revision = "188374910655" +down_revision = "f30cf048c228" +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column("users", sa.Column("login_attemps", sa.Integer(), nullable=True)) + op.add_column("users", sa.Column("locked_at", sa.DateTime(), nullable=True)) + + +def downgrade(): + op.drop_column("users", "locked_at") + op.drop_column("users", "login_attemps") diff --git a/dev/code-generation/gen_frontend_types.py b/dev/code-generation/gen_frontend_types.py index a4a6693f48..172e8a42b4 100644 --- a/dev/code-generation/gen_frontend_types.py +++ b/dev/code-generation/gen_frontend_types.py @@ -99,24 +99,24 @@ def path_to_module(path: Path): generate_typescript_defs(path_as_module, str(out_path), exclude=("MealieModel")) # type: ignore except Exception as e: failed_modules.append(module) - log.error(f"Module Error: {e}") # noqa + log.error(f"Module Error: {e}") - log.info("\nšŸ“ Skipped Directories:") # noqa + log.info("\nšŸ“ Skipped Directories:") for skipped_dir in skipped_dirs: - log.info(" šŸ“", skipped_dir.name) # noqa + log.info(f" šŸ“ {skipped_dir.name}") - log.info("šŸ“„ Skipped Files:") # noqa + log.info("šŸ“„ Skipped Files:") for f in skipped_files: - log.info(" šŸ“„", f.name) # noqa + log.info(f" šŸ“„ {f.name}") - log.error("āŒ Failed Modules:") # noqa + log.error("āŒ Failed Modules:") for f in failed_modules: - log.error(" āŒ", f.name) # noqa + log.error(f" āŒ {f.name}") if __name__ == "__main__": - log.info("\n-- Starting Global Components Generator --") # noqa + log.info("\n-- Starting Global Components Generator --") generate_global_components_types() - log.info("\n-- Starting Pydantic To Typescript Generator --") # noqa + log.info("\n-- Starting Pydantic To Typescript Generator --") generate_typescript_types() diff --git a/docs/docs/documentation/getting-started/installation/backend-config.md b/docs/docs/documentation/getting-started/installation/backend-config.md index b565dae0ae..051bc1f6a3 100644 --- a/docs/docs/documentation/getting-started/installation/backend-config.md +++ b/docs/docs/documentation/getting-started/installation/backend-config.md @@ -18,7 +18,12 @@ | ALLOW_SIGNUP | true | Allow user sign-up without token (should match frontend env) | +### Security +| Variables | Default | Description | +| --------------------------- | :-----: | ----------------------------------------------------------------------------------- | +| SECURITY_MAX_LOGIN_ATTEMPTS | 5 | Maximum times a user can provide an invalid password before their account is locked | +| SECURITY_USER_LOCKOUT_TIME | 24 | Time in hours for how long a users account is locked | ### Database @@ -39,7 +44,7 @@ | SMTP_HOST | None | Required For email | | SMTP_PORT | 587 | Required For email | | SMTP_FROM_NAME | Mealie | Required For email | -| SMTP_AUTH_STRATEGY | TLS | Required For email, Options: 'TLS', 'SSL', 'NONE' | +| SMTP_AUTH_STRATEGY | TLS | Required For email, Options: 'TLS', 'SSL', 'NONE' | | SMTP_FROM_EMAIL | None | Required For email | | SMTP_USER | None | Required if SMTP_AUTH_STRATEGY is 'TLS' or 'SSL' | | SMTP_PASSWORD | None | Required if SMTP_AUTH_STRATEGY is 'TLS' or 'SSL' | diff --git a/frontend/api/admin/admin-users.ts b/frontend/api/admin/admin-users.ts index 5df7a82873..9e863d3bfb 100644 --- a/frontend/api/admin/admin-users.ts +++ b/frontend/api/admin/admin-users.ts @@ -1,14 +1,19 @@ import { BaseCRUDAPI } from "../_base"; -import { UserIn, UserOut } from "~/types/api-types/user"; +import { UnlockResults, UserIn, UserOut } from "~/types/api-types/user"; const prefix = "/api"; const routes = { adminUsers: `${prefix}/admin/users`, adminUsersId: (tag: string) => `${prefix}/admin/users/${tag}`, + adminResetLockedUsers: (force: boolean) => `${prefix}/admin/users/unlock?force=${force ? "true" : "false"}`, }; export class AdminUsersApi extends BaseCRUDAPI { baseRoute: string = routes.adminUsers; itemRoute = routes.adminUsersId; + + async unlockAllUsers(force = false) { + return await this.requests.post(routes.adminResetLockedUsers(force), {}); + } } diff --git a/frontend/pages/admin/manage/users/index.vue b/frontend/pages/admin/manage/users/index.vue index 4b34e090e6..a24a3dc38d 100644 --- a/frontend/pages/admin/manage/users/index.vue +++ b/frontend/pages/admin/manage/users/index.vue @@ -10,9 +10,22 @@
- + {{ $t("general.create") }} + + + import { defineComponent, reactive, ref, toRefs, useContext, useRouter } from "@nuxtjs/composition-api"; -import { useUserApi } from "~/composables/api"; +import { useAdminApi } from "~/composables/api"; +import { alert } from "~/composables/use-toast"; import { useUser, useAllUsers } from "~/composables/use-user"; import { UserOut } from "~/types/api-types/user"; export default defineComponent({ layout: "admin", setup() { - const api = useUserApi(); + const api = useAdminApi(); const refUserDialog = ref(); const { i18n } = useContext(); @@ -97,9 +111,20 @@ export default defineComponent({ { text: i18n.t("general.delete"), value: "actions", sortable: false, align: "center" }, ]; + async function unlockAllUsers(): Promise { + const { data } = await api.users.unlockAllUsers(true); + + if (data) { + const unlocked = data.unlocked ?? 0; + + alert.success(`${unlocked} user(s) unlocked`); + refreshAllUsers(); + } + } + return { + unlockAllUsers, ...toRefs(state), - api, headers, deleteUser, loading, diff --git a/frontend/pages/login.vue b/frontend/pages/login.vue index 159cd248e3..ea550e0a88 100644 --- a/frontend/pages/login.vue +++ b/frontend/pages/login.vue @@ -157,9 +157,12 @@ export default defineComponent({ // See https://github.com/nuxt-community/axios-module/issues/550 // Import $axios from useContext() // if ($axios.isAxiosError(error) && error.response?.status === 401) { - // @ts-ignore - see above + // @ts-ignore- see above if (error.response?.status === 401) { alert.error("Invalid Credentials"); + // @ts-ignore - see above + } else if (error.response?.status === 423) { + alert.error("Account Locked. Please try again later"); } else { alert.error("Something Went Wrong!"); } diff --git a/frontend/types/api-types/admin.ts b/frontend/types/api-types/admin.ts index 034db6b6b5..62dcb4a92e 100644 --- a/frontend/types/api-types/admin.ts +++ b/frontend/types/api-types/admin.ts @@ -101,6 +101,8 @@ export interface RecipeSummary { recipeIngredient?: RecipeIngredient[]; dateAdded?: string; dateUpdated?: string; + createdAt?: string; + updateAt?: string; } export interface RecipeCategory { id?: string; @@ -135,6 +137,8 @@ export interface IngredientUnit { abbreviation?: string; useAbbreviation?: boolean; id: string; + createdAt?: string; + updateAt?: string; } export interface CreateIngredientUnit { name: string; @@ -149,6 +153,8 @@ export interface IngredientFood { labelId?: string; id: string; label?: MultiPurposeLabelSummary; + createdAt?: string; + updateAt?: string; } export interface MultiPurposeLabelSummary { name: string; diff --git a/frontend/types/api-types/cookbook.ts b/frontend/types/api-types/cookbook.ts index 91965f039e..904b7acdd1 100644 --- a/frontend/types/api-types/cookbook.ts +++ b/frontend/types/api-types/cookbook.ts @@ -86,6 +86,8 @@ export interface RecipeSummary { recipeIngredient?: RecipeIngredient[]; dateAdded?: string; dateUpdated?: string; + createdAt?: string; + updateAt?: string; } export interface RecipeCategory { id?: string; @@ -114,6 +116,8 @@ export interface IngredientUnit { abbreviation?: string; useAbbreviation?: boolean; id: string; + createdAt?: string; + updateAt?: string; } export interface CreateIngredientUnit { name: string; @@ -128,6 +132,8 @@ export interface IngredientFood { labelId?: string; id: string; label?: MultiPurposeLabelSummary; + createdAt?: string; + updateAt?: string; } export interface MultiPurposeLabelSummary { name: string; diff --git a/frontend/types/api-types/group.ts b/frontend/types/api-types/group.ts index c61613bcb6..b20a5bb4b4 100644 --- a/frontend/types/api-types/group.ts +++ b/frontend/types/api-types/group.ts @@ -197,6 +197,8 @@ export interface IngredientFood { labelId?: string; id: string; label?: MultiPurposeLabelSummary; + createdAt?: string; + updateAt?: string; } export interface MultiPurposeLabelSummary { name: string; @@ -211,6 +213,8 @@ export interface IngredientUnit { abbreviation?: string; useAbbreviation?: boolean; id: string; + createdAt?: string; + updateAt?: string; } export interface ReadGroupPreferences { privateGroup?: boolean; @@ -259,6 +263,8 @@ export interface RecipeSummary { recipeIngredient?: RecipeIngredient[]; dateAdded?: string; dateUpdated?: string; + createdAt?: string; + updateAt?: string; } export interface RecipeCategory { id?: string; @@ -322,6 +328,8 @@ export interface SetPermissions { } export interface ShoppingListCreate { name?: string; + createdAt?: string; + updateAt?: string; } export interface ShoppingListItemCreate { shoppingListId: string; @@ -336,6 +344,8 @@ export interface ShoppingListItemCreate { food?: IngredientFood; labelId?: string; recipeReferences?: ShoppingListItemRecipeRef[]; + createdAt?: string; + updateAt?: string; } export interface ShoppingListItemRecipeRef { recipeId: string; @@ -353,7 +363,9 @@ export interface ShoppingListItemOut { foodId?: string; food?: IngredientFood; labelId?: string; - recipeReferences?: ShoppingListItemRecipeRefOut[]; + recipeReferences?: (ShoppingListItemRecipeRef | ShoppingListItemRecipeRefOut)[]; + createdAt?: string; + updateAt?: string; id: string; label?: MultiPurposeLabelSummary; } @@ -376,10 +388,14 @@ export interface ShoppingListItemUpdate { food?: IngredientFood; labelId?: string; recipeReferences?: ShoppingListItemRecipeRef[]; + createdAt?: string; + updateAt?: string; id: string; } export interface ShoppingListOut { name?: string; + createdAt?: string; + updateAt?: string; groupId: string; id: string; listItems?: ShoppingListItemOut[]; @@ -394,15 +410,21 @@ export interface ShoppingListRecipeRefOut { } export interface ShoppingListSave { name?: string; + createdAt?: string; + updateAt?: string; groupId: string; } export interface ShoppingListSummary { name?: string; + createdAt?: string; + updateAt?: string; groupId: string; id: string; } export interface ShoppingListUpdate { name?: string; + createdAt?: string; + updateAt?: string; groupId: string; id: string; listItems?: ShoppingListItemOut[]; diff --git a/frontend/types/api-types/meal-plan.ts b/frontend/types/api-types/meal-plan.ts index df387e5b9a..a914a8fa71 100644 --- a/frontend/types/api-types/meal-plan.ts +++ b/frontend/types/api-types/meal-plan.ts @@ -116,6 +116,8 @@ export interface RecipeSummary { recipeIngredient?: RecipeIngredient[]; dateAdded?: string; dateUpdated?: string; + createdAt?: string; + updateAt?: string; } export interface RecipeCategory { id?: string; @@ -150,6 +152,8 @@ export interface IngredientUnit { abbreviation?: string; useAbbreviation?: boolean; id: string; + createdAt?: string; + updateAt?: string; } export interface CreateIngredientUnit { name: string; @@ -164,6 +168,8 @@ export interface IngredientFood { labelId?: string; id: string; label?: MultiPurposeLabelSummary; + createdAt?: string; + updateAt?: string; } export interface MultiPurposeLabelSummary { name: string; diff --git a/frontend/types/api-types/recipe.ts b/frontend/types/api-types/recipe.ts index ae23040f9e..358d287d72 100644 --- a/frontend/types/api-types/recipe.ts +++ b/frontend/types/api-types/recipe.ts @@ -7,6 +7,7 @@ export type ExportTypes = "json"; export type RegisteredParser = "nlp" | "brute"; +export type OrderDirection = "asc" | "desc"; export interface AssignCategories { recipes: string[]; @@ -96,6 +97,8 @@ export interface IngredientFood { labelId?: string; id: string; label?: MultiPurposeLabelSummary; + createdAt?: string; + updateAt?: string; } export interface MultiPurposeLabelSummary { name: string; @@ -120,6 +123,8 @@ export interface IngredientUnit { abbreviation?: string; useAbbreviation?: boolean; id: string; + createdAt?: string; + updateAt?: string; } export interface IngredientsRequest { parser?: RegisteredParser & string; @@ -142,6 +147,13 @@ export interface Nutrition { sodiumContent?: string; sugarContent?: string; } +export interface PaginationQuery { + page?: number; + perPage?: number; + orderBy?: string; + orderDirection?: OrderDirection & string; + queryFilter?: string; +} export interface ParsedIngredient { input?: string; confidence?: IngredientConfidence; @@ -178,6 +190,8 @@ export interface Recipe { recipeIngredient?: RecipeIngredient[]; dateAdded?: string; dateUpdated?: string; + createdAt?: string; + updateAt?: string; recipeInstructions?: RecipeStep[]; nutrition?: Nutrition; settings?: RecipeSettings; @@ -259,6 +273,8 @@ export interface RecipeSummary { recipeIngredient?: RecipeIngredient[]; dateAdded?: string; dateUpdated?: string; + createdAt?: string; + updateAt?: string; } export interface RecipeCommentCreate { recipeId: string; @@ -273,6 +289,14 @@ export interface RecipeCommentUpdate { id: string; text: string; } +export interface RecipePaginationQuery { + page?: number; + perPage?: number; + orderBy?: string; + orderDirection?: OrderDirection & string; + queryFilter?: string; + loadFood?: boolean; +} export interface RecipeShareToken { recipeId: string; expiresAt?: string; diff --git a/frontend/types/api-types/response.ts b/frontend/types/api-types/response.ts index 8e1cb0f5d0..0f2c7dab84 100644 --- a/frontend/types/api-types/response.ts +++ b/frontend/types/api-types/response.ts @@ -5,6 +5,8 @@ /* Do not modify it by hand - just update the pydantic models and then re-run the script */ +export type OrderDirection = "asc" | "desc"; + export interface ErrorResponse { message: string; error?: boolean; @@ -13,6 +15,13 @@ export interface ErrorResponse { export interface FileTokenResponse { fileToken: string; } +export interface PaginationQuery { + page?: number; + perPage?: number; + orderBy?: string; + orderDirection?: OrderDirection & string; + queryFilter?: string; +} export interface SuccessResponse { message: string; error?: boolean; diff --git a/frontend/types/api-types/user.ts b/frontend/types/api-types/user.ts index 9246e41645..99a71ce038 100644 --- a/frontend/types/api-types/user.ts +++ b/frontend/types/api-types/user.ts @@ -107,6 +107,8 @@ export interface PrivateUser { tokens?: LongLiveTokenOut[]; cacheKey: string; password: string; + loginAttemps?: number; + lockedAt?: string; } export interface PrivatePasswordResetToken { userId: string; @@ -134,6 +136,8 @@ export interface RecipeSummary { recipeIngredient?: RecipeIngredient[]; dateAdded?: string; dateUpdated?: string; + createdAt?: string; + updateAt?: string; } export interface RecipeCategory { id?: string; @@ -168,6 +172,8 @@ export interface IngredientUnit { abbreviation?: string; useAbbreviation?: boolean; id: string; + createdAt?: string; + updateAt?: string; } export interface CreateIngredientUnit { name: string; @@ -182,6 +188,8 @@ export interface IngredientFood { labelId?: string; id: string; label?: MultiPurposeLabelSummary; + createdAt?: string; + updateAt?: string; } export interface MultiPurposeLabelSummary { name: string; @@ -212,6 +220,9 @@ export interface TokenData { user_id?: string; username?: string; } +export interface UnlockResults { + unlocked?: number; +} export interface UpdateGroup { name: string; id: string; diff --git a/mealie/app.py b/mealie/app.py index d6d7a022b7..720b1d641c 100644 --- a/mealie/app.py +++ b/mealie/app.py @@ -10,7 +10,6 @@ from mealie.routes.media import media_router from mealie.services.scheduler import SchedulerRegistry, SchedulerService, tasks -logger = get_logger() settings = get_app_settings() description = f""" @@ -61,6 +60,10 @@ async def start_scheduler(): tasks.post_group_webhooks, ) + SchedulerRegistry.register_hourly( + tasks.locked_user_reset, + ) + SchedulerRegistry.print_jobs() await SchedulerService.start() @@ -77,6 +80,8 @@ def api_routers(): @app.on_event("startup") async def system_startup(): + logger = get_logger() + await start_scheduler() logger.info("-----SYSTEM STARTUP----- \n") diff --git a/mealie/core/security/security.py b/mealie/core/security/security.py index 0e9188a267..fe4118021b 100644 --- a/mealie/core/security/security.py +++ b/mealie/core/security/security.py @@ -9,10 +9,15 @@ from mealie.repos.all_repositories import get_repositories from mealie.repos.repository_factory import AllRepositories from mealie.schema.user import PrivateUser +from mealie.services.user_services.user_service import UserService ALGORITHM = "HS256" +class UserLockedOut(Exception): + ... + + def create_access_token(data: dict, expires_delta: timedelta = None) -> str: settings = get_app_settings() @@ -35,7 +40,7 @@ def create_recipe_slug_token(file_path: str | Path) -> str: return create_access_token(token_data, expires_delta=timedelta(minutes=30)) -def user_from_ldap(db: AllRepositories, session, username: str, password: str) -> PrivateUser | bool: +def user_from_ldap(db: AllRepositories, username: str, password: str) -> PrivateUser | bool: """Given a username and password, tries to authenticate by BINDing to an LDAP server @@ -85,7 +90,7 @@ def authenticate_user(session, email: str, password: str) -> PrivateUser | bool: user = db.users.get_one(email, "username", any_case=True) if settings.LDAP_AUTH_ENABLED and (not user or user.password == "LDAP"): - return user_from_ldap(db, session, email, password) + return user_from_ldap(db, email, password) if not user: # To prevent user enumeration we perform the verify_password computation to ensure @@ -93,7 +98,17 @@ def authenticate_user(session, email: str, password: str) -> PrivateUser | bool: verify_password("abc123cba321", "$2b$12$JdHtJOlkPFwyxdjdygEzPOtYmdQF5/R5tHxw5Tq8pxjubyLqdIX5i") return False + if user.login_attemps >= settings.SECURITY_MAX_LOGIN_ATTEMPTS or user.is_locked: + raise UserLockedOut() + elif not verify_password(password, user.password): + user.login_attemps += 1 + db.users.update(user.id, user) + + if user.login_attemps >= settings.SECURITY_MAX_LOGIN_ATTEMPTS: + user_service = UserService(db) + user_service.lock_user(user) + return False return user diff --git a/mealie/core/settings/settings.py b/mealie/core/settings/settings.py index 704ab373eb..d1740c93a1 100644 --- a/mealie/core/settings/settings.py +++ b/mealie/core/settings/settings.py @@ -36,6 +36,12 @@ class AppSettings(BaseSettings): ALLOW_SIGNUP: bool = True + # =============================================== + # Security Configuration + + SECURITY_MAX_LOGIN_ATTEMPTS: int = 5 + SECURITY_USER_LOCKOUT_TIME: int = 24 # Time in Hours + @property def DOCS_URL(self) -> str | None: return "/docs" if self.API_DOCS else None diff --git a/mealie/db/db_setup.py b/mealie/db/db_setup.py index 84b9b9f9b2..6e92751be5 100644 --- a/mealie/db/db_setup.py +++ b/mealie/db/db_setup.py @@ -1,4 +1,5 @@ from collections.abc import Generator +from contextlib import contextmanager import sqlalchemy as sa from sqlalchemy.orm import sessionmaker @@ -29,6 +30,17 @@ def sql_global_init(db_url: str): SessionLocal, engine = sql_global_init(settings.DB_URL) # type: ignore +@contextmanager +def with_session() -> Session: + global SessionLocal + sess = SessionLocal() + + try: + yield sess + finally: + sess.close() + + def create_session() -> Session: global SessionLocal return SessionLocal() diff --git a/mealie/db/models/users/users.py b/mealie/db/models/users/users.py index ec01a43bc8..71a3b43e92 100644 --- a/mealie/db/models/users/users.py +++ b/mealie/db/models/users/users.py @@ -1,4 +1,4 @@ -from sqlalchemy import Boolean, Column, ForeignKey, String, orm +from sqlalchemy import Boolean, Column, DateTime, ForeignKey, Integer, String, orm from mealie.core.config import get_app_settings from mealie.db.models._model_utils.guid import GUID @@ -36,6 +36,8 @@ class User(SqlAlchemyBase, BaseMixins): group = orm.relationship("Group", back_populates="users") cache_key = Column(String, default="1234") + login_attemps = Column(Integer, default=0) + locked_at = Column(DateTime, default=None) # Group Permissions can_manage = Column(Boolean, default=False) diff --git a/mealie/repos/repository_users.py b/mealie/repos/repository_users.py index afbde76b30..6b9b588603 100644 --- a/mealie/repos/repository_users.py +++ b/mealie/repos/repository_users.py @@ -1,6 +1,5 @@ import random import shutil -from typing import Optional from pydantic import UUID4 @@ -38,8 +37,10 @@ def delete(self, value: str | UUID4, match_key: str | None = None) -> User: shutil.rmtree(PrivateUser.get_directory(value)) return entry # type: ignore - def get_by_username(self, username: str, limit=1) -> Optional[User]: + def get_by_username(self, username: str) -> PrivateUser | None: dbuser = self.session.query(User).filter(User.username == username).one_or_none() - if dbuser is None: - return None - return self.schema.from_orm(dbuser) # type: ignore + return None if dbuser is None else self.schema.from_orm(dbuser) + + def get_locked_users(self) -> list[PrivateUser]: + results = self.session.query(User).filter(User.locked_at != None).all() # noqa E711 + return [self.schema.from_orm(x) for x in results] diff --git a/mealie/routes/admin/admin_management_users.py b/mealie/routes/admin/admin_management_users.py index ab26ac7984..ff907c3b57 100644 --- a/mealie/routes/admin/admin_management_users.py +++ b/mealie/routes/admin/admin_management_users.py @@ -8,7 +8,9 @@ from mealie.routes._base.mixins import HttpRepo from mealie.schema.response.pagination import PaginationQuery from mealie.schema.response.responses import ErrorResponse +from mealie.schema.user.auth import UnlockResults from mealie.schema.user.user import UserIn, UserOut, UserPagination +from mealie.services.user_services.user_service import UserService router = APIRouter(prefix="/users", tags=["Admin: Users"]) @@ -17,9 +19,6 @@ class AdminUserManagementRoutes(BaseAdminController): @cached_property def repo(self): - if not self.user: - raise Exception("No user is logged in.") - return self.repos.users # ======================================================================= @@ -44,6 +43,13 @@ def create_one(self, data: UserIn): data.password = security.hash_password(data.password) return self.mixins.create_one(data) + @router.post("/unlock", response_model=UnlockResults) + def unlock_users(self, force: bool = False) -> UnlockResults: + user_service = UserService(self.repos) + unlocked = user_service.reset_locked_users(force=force) + + return UnlockResults(unlocked=unlocked) + @router.get("/{item_id}", response_model=UserOut) def get_one(self, item_id: UUID4): return self.mixins.get_one(item_id) diff --git a/mealie/routes/auth/auth.py b/mealie/routes/auth/auth.py index c5ed2885bd..3c69ecd911 100644 --- a/mealie/routes/auth/auth.py +++ b/mealie/routes/auth/auth.py @@ -10,6 +10,7 @@ from mealie.core import security from mealie.core.dependencies import get_current_user from mealie.core.security import authenticate_user +from mealie.core.security.security import UserLockedOut from mealie.db.db_setup import generate_session from mealie.routes._base.routers import UserAPIRouter from mealie.schema.user import PrivateUser @@ -53,7 +54,10 @@ def get_token(data: CustomOAuth2Form = Depends(), session: Session = Depends(gen email = data.username password = data.password - user = authenticate_user(session, email, password) # type: ignore + try: + user = authenticate_user(session, email, password) # type: ignore + except UserLockedOut as e: + raise HTTPException(status_code=status.HTTP_423_LOCKED, detail="User is locked out") from e if not user: raise HTTPException( diff --git a/mealie/schema/user/auth.py b/mealie/schema/user/auth.py index cc3f648449..97abe7c3cd 100644 --- a/mealie/schema/user/auth.py +++ b/mealie/schema/user/auth.py @@ -3,6 +3,8 @@ from pydantic import UUID4, BaseModel from pydantic.types import constr +from mealie.schema._mealie.mealie_model import MealieModel + class Token(BaseModel): access_token: str @@ -12,3 +14,7 @@ class Token(BaseModel): class TokenData(BaseModel): user_id: Optional[UUID4] username: Optional[constr(to_lower=True, strip_whitespace=True)] = None # type: ignore + + +class UnlockResults(MealieModel): + unlocked: int = 0 diff --git a/mealie/schema/user/user.py b/mealie/schema/user/user.py index 9ee5eb62eb..222b9a5a89 100644 --- a/mealie/schema/user/user.py +++ b/mealie/schema/user/user.py @@ -1,8 +1,9 @@ +from datetime import datetime, timedelta from pathlib import Path from typing import Any, Optional from uuid import UUID -from pydantic import UUID4 +from pydantic import UUID4, validator from pydantic.types import constr from pydantic.utils import GetterDict @@ -137,16 +138,30 @@ def getter_dict(cls, ormModel: User): class PrivateUser(UserOut): password: str group_id: UUID4 + login_attemps: int = 0 + locked_at: datetime | None = None class Config: orm_mode = True + @validator("login_attemps", pre=True) + def none_to_zero(cls, v): + return 0 if v is None else v + @staticmethod def get_directory(user_id: UUID4 | str) -> Path: user_dir = get_app_dirs().USER_DIR / str(user_id) user_dir.mkdir(parents=True, exist_ok=True) return user_dir + @property + def is_locked(self) -> bool: + if self.locked_at is None: + return False + + lockout_expires_at = self.locked_at + timedelta(hours=get_app_settings().SECURITY_USER_LOCKOUT_TIME) + return lockout_expires_at > datetime.now() + def directory(self) -> Path: return PrivateUser.get_directory(self.id) @@ -168,15 +183,15 @@ class Config: @staticmethod def get_directory(id: UUID4) -> Path: - dir = get_app_dirs().GROUPS_DIR / str(id) - dir.mkdir(parents=True, exist_ok=True) - return dir + group_dir = get_app_dirs().GROUPS_DIR / str(id) + group_dir.mkdir(parents=True, exist_ok=True) + return group_dir @staticmethod def get_export_directory(id: UUID) -> Path: - dir = GroupInDB.get_directory(id) / "export" - dir.mkdir(parents=True, exist_ok=True) - return dir + export_dir = GroupInDB.get_directory(id) / "export" + export_dir.mkdir(parents=True, exist_ok=True) + return export_dir @property def directory(self) -> Path: diff --git a/mealie/scripts/reset_locked_users.py b/mealie/scripts/reset_locked_users.py new file mode 100644 index 0000000000..34cfb55455 --- /dev/null +++ b/mealie/scripts/reset_locked_users.py @@ -0,0 +1,34 @@ +from mealie.core import root_logger +from mealie.db.db_setup import with_session +from mealie.repos.repository_factory import AllRepositories +from mealie.services.user_services.user_service import UserService + + +def main(): + confirmed = input("Are you sure you want to reset all locked users? (y/n) ") + + if confirmed != "y": + print("aborting") # noqa + exit(0) + + logger = root_logger.get_logger() + + with with_session() as session: + repos = AllRepositories(session) + user_service = UserService(repos) + + locked_users = user_service.get_locked_users() + + if not locked_users: + logger.error("no locked users found") + + for user in locked_users: + logger.info(f"unlocking user {user.username}") + user_service.unlock_user(user) + + input("press enter to exit ") + exit(0) + + +if __name__ == "__main__": + main() diff --git a/mealie/services/backups_v2/alchemy_exporter.py b/mealie/services/backups_v2/alchemy_exporter.py index e6d6cc1f38..73dc07be3f 100644 --- a/mealie/services/backups_v2/alchemy_exporter.py +++ b/mealie/services/backups_v2/alchemy_exporter.py @@ -16,7 +16,7 @@ class AlchemyExporter(BaseService): engine: base.Engine meta: MetaData - look_for_datetime = {"created_at", "update_at", "date_updated", "timestamp", "expires_at"} + look_for_datetime = {"created_at", "update_at", "date_updated", "timestamp", "expires_at", "locked_at"} look_for_date = {"date_added", "date"} look_for_time = {"scheduled_time"} diff --git a/mealie/services/scheduler/tasks/__init__.py b/mealie/services/scheduler/tasks/__init__.py index f258380227..b771f8748d 100644 --- a/mealie/services/scheduler/tasks/__init__.py +++ b/mealie/services/scheduler/tasks/__init__.py @@ -2,12 +2,14 @@ from .purge_group_exports import purge_group_data_exports from .purge_password_reset import purge_password_reset_tokens from .purge_registration import purge_group_registration +from .reset_locked_users import locked_user_reset __all__ = [ "post_group_webhooks", "purge_password_reset_tokens", "purge_group_data_exports", "purge_group_registration", + "locked_user_reset", ] """ diff --git a/mealie/services/scheduler/tasks/reset_locked_users.py b/mealie/services/scheduler/tasks/reset_locked_users.py new file mode 100644 index 0000000000..1e3c513226 --- /dev/null +++ b/mealie/services/scheduler/tasks/reset_locked_users.py @@ -0,0 +1,17 @@ +from mealie.core import root_logger +from mealie.db.db_setup import with_session +from mealie.repos.repository_factory import AllRepositories +from mealie.services.user_services.user_service import UserService + + +def locked_user_reset(): + logger = root_logger.get_logger() + logger.info("resetting locked users") + + with with_session() as session: + repos = AllRepositories(session) + user_service = UserService(repos) + + unlocked = user_service.reset_locked_users() + logger.info(f"scheduled task unlocked {unlocked} users in the database") + logger.info("locked users reset") diff --git a/mealie/services/user_services/password_reset_service.py b/mealie/services/user_services/password_reset_service.py index f1674cedf7..7a9a3c7896 100644 --- a/mealie/services/user_services/password_reset_service.py +++ b/mealie/services/user_services/password_reset_service.py @@ -1,15 +1,12 @@ from fastapi import HTTPException, status from sqlalchemy.orm.session import Session -from mealie.core.root_logger import get_logger from mealie.core.security import hash_password, url_safe_token from mealie.repos.all_repositories import get_repositories from mealie.schema.user.user_passwords import SavePasswordResetToken from mealie.services._base_service import BaseService from mealie.services.email import EmailService -logger = get_logger(__name__) - class PasswordResetService(BaseService): def __init__(self, session: Session) -> None: @@ -20,7 +17,7 @@ def generate_reset_token(self, email: str) -> SavePasswordResetToken | None: user = self.db.users.get_one(email, "email", any_case=True) if user is None: - logger.error(f"failed to create password reset for {email=}: user doesn't exists") + self.logger.error(f"failed to create password reset for {email=}: user doesn't exists") # Do not raise exception here as we don't want to confirm to the client that the Email doens't exists return None @@ -41,7 +38,7 @@ def send_reset_email(self, email: str): try: email_servive.send_forgot_password(email, reset_url) except Exception as e: - logger.error(f"failed to send reset email: {e}") + self.logger.error(f"failed to send reset email: {e}") raise HTTPException(status.HTTP_500_INTERNAL_SERVER_ERROR, "Failed to send reset email") def reset_password(self, token: str, new_password: str): @@ -49,7 +46,7 @@ def reset_password(self, token: str, new_password: str): token_entry = self.db.tokens_pw_reset.get_one(token, "token") if token_entry is None: - logger.error("failed to reset password: invalid token") + self.logger.error("failed to reset password: invalid token") raise HTTPException(status.HTTP_400_BAD_REQUEST, "Invalid token") user = self.db.users.get_one(token_entry.user_id) @@ -59,7 +56,7 @@ def reset_password(self, token: str, new_password: str): new_user = self.db.users.update_password(user.id, password_hash) # Confirm Password if new_user.password != password_hash: - logger.error("failed to reset password: invalid password") + self.logger.error("failed to reset password: invalid password") raise HTTPException(status.HTTP_400_BAD_REQUEST, "Invalid password") # Delete Token from DB diff --git a/mealie/services/user_services/user_service.py b/mealie/services/user_services/user_service.py new file mode 100644 index 0000000000..301762910e --- /dev/null +++ b/mealie/services/user_services/user_service.py @@ -0,0 +1,39 @@ +from datetime import datetime + +from mealie.repos.repository_factory import AllRepositories +from mealie.schema.user.user import PrivateUser +from mealie.services._base_service import BaseService + + +class UserService(BaseService): + def __init__(self, repos: AllRepositories) -> None: + self.repos = repos + super().__init__() + + def get_locked_users(self) -> list[PrivateUser]: + return self.repos.users.get_locked_users() + + def reset_locked_users(self, force: bool = False) -> int: + """ + Queriers that database for all locked users and resets their locked_at field to None + if more than the set time has passed since the user was locked + """ + locked_users = self.get_locked_users() + + unlocked = 0 + + for user in locked_users: + if force or user.is_locked and user.locked_at is not None: + self.unlock_user(user) + unlocked += 1 + + return unlocked + + def lock_user(self, user: PrivateUser) -> PrivateUser: + user.locked_at = datetime.now() + return self.repos.users.update(user.id, user) + + def unlock_user(self, user: PrivateUser) -> PrivateUser: + user.locked_at = None + user.login_attemps = 0 + return self.repos.users.update(user.id, user) diff --git a/tests/fixtures/fixture_admin.py b/tests/fixtures/fixture_admin.py index 46409d90d0..bb5ded9e13 100644 --- a/tests/fixtures/fixture_admin.py +++ b/tests/fixtures/fixture_admin.py @@ -32,6 +32,7 @@ def admin_user(api_client: TestClient, api_routes: utils.AppRoutes): yield utils.TestUser( _group_id=user_data.get("groupId"), user_id=user_data.get("id"), + password=settings.DEFAULT_PASSWORD, username=user_data.get("username"), email=user_data.get("email"), token=token, diff --git a/tests/fixtures/fixture_users.py b/tests/fixtures/fixture_users.py index 87e206c5cb..e28f46bff0 100644 --- a/tests/fixtures/fixture_users.py +++ b/tests/fixtures/fixture_users.py @@ -26,6 +26,7 @@ def build_unique_user(group: str, api_client: TestClient) -> utils.TestUser: _group_id=user_data.get("groupId"), user_id=user_data.get("id"), email=user_data.get("email"), + password=registration.password, username=user_data.get("username"), token=token, ) @@ -67,6 +68,7 @@ def g2_user(admin_token, api_client: TestClient, api_routes: utils.AppRoutes): user_id=user_id, _group_id=group_id, token=token, + password="useruser", email=create_data["email"], username=create_data.get("username"), ) @@ -92,6 +94,7 @@ def unique_user(api_client: TestClient, api_routes: utils.AppRoutes): yield utils.TestUser( _group_id=user_data.get("groupId"), user_id=user_data.get("id"), + password=registration.password, email=user_data.get("email"), username=user_data.get("username"), token=token, @@ -144,6 +147,7 @@ def user_tuple(admin_token, api_client: TestClient, api_routes: utils.AppRoutes) _group_id=user_data.get("groupId"), user_id=user_data.get("id"), username=user_data.get("username"), + password="useruser", email=user_data.get("email"), token=token, ) diff --git a/tests/integration_tests/user_tests/test_user_login.py b/tests/integration_tests/user_tests/test_user_login.py index 83931460bf..60f6afe4be 100644 --- a/tests/integration_tests/user_tests/test_user_login.py +++ b/tests/integration_tests/user_tests/test_user_login.py @@ -3,6 +3,8 @@ from fastapi.testclient import TestClient from mealie.core.config import get_app_settings +from mealie.repos.repository_factory import AllRepositories +from mealie.services.user_services.user_service import UserService from tests.utils.app_routes import AppRoutes from tests.utils.fixture_schemas import TestUser @@ -35,3 +37,27 @@ def test_user_token_refresh(api_client: TestClient, api_routes: AppRoutes, admin response = api_client.post(api_routes.auth_refresh, headers=admin_user.token) response = api_client.get(api_routes.users_self, headers=admin_user.token) assert response.status_code == 200 + + +def test_user_lockout_after_bad_attemps(api_client: TestClient, unique_user: TestUser, database: AllRepositories): + """ + if the user has more than 5 bad login attemps the user will be locked out for 4 hours + This only applies if there is a user in the database with the same username + """ + routes = AppRoutes() + settings = get_app_settings() + + for _ in range(settings.SECURITY_MAX_LOGIN_ATTEMPTS): + form_data = {"username": unique_user.email, "password": "bad_password"} + response = api_client.post(routes.auth_token, form_data) + + assert response.status_code == 401 + + valid_data = {"username": unique_user.email, "password": unique_user.password} + response = api_client.post(routes.auth_token, valid_data) + assert response.status_code == 423 + + # Cleanup + user_service = UserService(database) + user = database.users.get_one(unique_user.user_id) + user_service.unlock_user(user) diff --git a/tests/unit_tests/services_tests/backup_v2_tests/test_alchemy_exporter.py b/tests/unit_tests/services_tests/backup_v2_tests/test_alchemy_exporter.py index 35f04969ff..5fbbb6e5c9 100644 --- a/tests/unit_tests/services_tests/backup_v2_tests/test_alchemy_exporter.py +++ b/tests/unit_tests/services_tests/backup_v2_tests/test_alchemy_exporter.py @@ -4,7 +4,7 @@ from mealie.services.backups_v2.alchemy_exporter import AlchemyExporter ALEMBIC_VERSIONS = [ - {"version_num": "f30cf048c228"}, + {"version_num": "188374910655"}, ] diff --git a/tests/unit_tests/services_tests/user_services/test_user_service.py b/tests/unit_tests/services_tests/user_services/test_user_service.py new file mode 100644 index 0000000000..7f6a50e850 --- /dev/null +++ b/tests/unit_tests/services_tests/user_services/test_user_service.py @@ -0,0 +1,63 @@ +from datetime import datetime, timedelta + +from mealie.repos.repository_factory import AllRepositories +from mealie.services.user_services.user_service import UserService +from tests.utils.fixture_schemas import TestUser + + +def test_get_locked_users(database: AllRepositories, user_tuple: list[TestUser]) -> None: + usr_1, usr_2 = user_tuple + + # Setup + user_service = UserService(database) + + user_1 = database.users.get_one(usr_1.user_id) + user_2 = database.users.get_one(usr_2.user_id) + + locked_users = user_service.get_locked_users() + assert len(locked_users) == 0 + + user_1 = user_service.lock_user(user_1) + + locked_users = user_service.get_locked_users() + assert len(locked_users) == 1 + assert locked_users[0].id == user_1.id + + user_2 = user_service.lock_user(user_2) + + locked_users = user_service.get_locked_users() + assert len(locked_users) == 2 + + for locked_user in locked_users: + if locked_user.id == user_1.id: + assert locked_user.locked_at == user_1.locked_at + elif locked_user.id == user_2.id: + assert locked_user.locked_at == user_2.locked_at + else: + assert False + + # Cleanup + user_service.unlock_user(user_1) + user_service.unlock_user(user_2) + + +def test_lock_unlocker_user(database: AllRepositories, unique_user: TestUser) -> None: + user_service = UserService(database) + + # Test that the user is unlocked + user = database.users.get_one(unique_user.user_id) + assert not user.locked_at + + # Test that the user is locked + locked_user = user_service.lock_user(user) + + assert locked_user.locked_at + assert locked_user.is_locked + + unlocked_user = user_service.unlock_user(locked_user) + assert not unlocked_user.locked_at + assert not unlocked_user.is_locked + + # Sanity check that the is_locked property is working + user.locked_at = datetime.now() - timedelta(days=2) + assert not user.is_locked diff --git a/tests/utils/fixture_schemas.py b/tests/utils/fixture_schemas.py index c00023d839..6bb3abe6ec 100644 --- a/tests/utils/fixture_schemas.py +++ b/tests/utils/fixture_schemas.py @@ -8,6 +8,7 @@ class TestUser: email: str user_id: UUID username: str + password: str _group_id: UUID token: Any