From 80caa5ffafd6b543311c9633f37a675ee36ed40f Mon Sep 17 00:00:00 2001 From: Carter <35710697+cmintey@users.noreply.github.com> Date: Wed, 16 Oct 2024 09:34:51 -0500 Subject: [PATCH] fix: Prevent login via credentials when Auth Method is Mealie (#4370) --- .../getting-started/authentication/oidc-v2.md | 2 ++ .../providers/credentials_provider.py | 21 +++++++++++++---- .../core/security/providers/ldap_provider.py | 8 +++++-- .../providers/test_credentials_provider.py | 23 +++++++++++++++++++ 4 files changed, 47 insertions(+), 7 deletions(-) create mode 100644 tests/unit_tests/core/security/providers/test_credentials_provider.py diff --git a/docs/docs/documentation/getting-started/authentication/oidc-v2.md b/docs/docs/documentation/getting-started/authentication/oidc-v2.md index 1e7c3770e..b78902181 100644 --- a/docs/docs/documentation/getting-started/authentication/oidc-v2.md +++ b/docs/docs/documentation/getting-started/authentication/oidc-v2.md @@ -18,6 +18,8 @@ Mealie supports 3rd party authentication via [OpenID Connect (OIDC)](https://ope Signing in with OAuth will automatically find your account in Mealie and link to it. If a user does not exist in Mealie, then one will be created (if enabled), but will be unable to log in with any other authentication method. An admin can configure another authentication method for such a user. +If a user previously accessed Mealie via credentials and you want to no longer allow users to log in with `LDAP` or `Mealie` credentials, then you can set the user's *Authentication Method* to `OIDC`. Conversely, if a user's auth method is not `OIDC`, then they can still log in with whatever their auth method is as well as OIDC. + ## Provider Setup Before you can start using OIDC Authentication, you must first configure a new client application in your identity provider. Your identity provider must support the OAuth **Authorization Code flow with PKCE**. The steps will vary by provider, but generally, the steps are as follows. diff --git a/mealie/core/security/providers/credentials_provider.py b/mealie/core/security/providers/credentials_provider.py index c7b2aaa37..14db5c6e1 100644 --- a/mealie/core/security/providers/credentials_provider.py +++ b/mealie/core/security/providers/credentials_provider.py @@ -7,6 +7,7 @@ from mealie.core.config import get_app_settings from mealie.core.exceptions import UserLockedOut from mealie.core.security.hasher import get_hasher from mealie.core.security.providers.auth_provider import AuthProvider +from mealie.db.models.users.users import AuthMethod from mealie.repos.all_repositories import get_repositories from mealie.schema.user.auth import CredentialsRequest from mealie.services.user_services.user_service import UserService @@ -27,11 +28,13 @@ class CredentialsProvider(AuthProvider[CredentialsRequest]): user = self.try_get_user(self.data.username) if not user: - # To prevent user enumeration we perform the verify_password computation to ensure - # server side time is relatively constant and not vulnerable to timing attacks. - CredentialsProvider.verify_password( - "abc123cba321", - "$2b$12$JdHtJOlkPFwyxdjdygEzPOtYmdQF5/R5tHxw5Tq8pxjubyLqdIX5i", + self.verify_fake_password() + return None + + if user.auth_method != AuthMethod.MEALIE: + self.verify_fake_password() + self._logger.warning( + "Found user but their auth method is not 'Mealie'. Unable to continue with credentials login" ) return None @@ -52,6 +55,14 @@ class CredentialsProvider(AuthProvider[CredentialsRequest]): user = db.users.update(user.id, user) return self.get_access_token(user, self.data.remember_me) # type: ignore + def verify_fake_password(self): + # To prevent user enumeration we perform the verify_password computation to ensure + # server side time is relatively constant and not vulnerable to timing attacks. + CredentialsProvider.verify_password( + "abc123cba321", + "$2b$12$JdHtJOlkPFwyxdjdygEzPOtYmdQF5/R5tHxw5Tq8pxjubyLqdIX5i", + ) + @staticmethod def verify_password(plain_password: str, hashed_password: str) -> bool: """Compares a plain string to a hashed password""" diff --git a/mealie/core/security/providers/ldap_provider.py b/mealie/core/security/providers/ldap_provider.py index 67467b831..f78e56567 100644 --- a/mealie/core/security/providers/ldap_provider.py +++ b/mealie/core/security/providers/ldap_provider.py @@ -23,9 +23,13 @@ class LDAPProvider(CredentialsProvider): self.conn = None def authenticate(self) -> tuple[str, timedelta] | None: - """Attempt to authenticate a user given a username and password""" + """Attempt to authenticate a user given a username and password against an LDAP provider""" + # When LDAP is enabled, we need to still also support authentication with Mealie backend + # First we look to see if we have a user. If we don't we'll attempt to create one with LDAP + # If we do find a user, we will check if their auth method is LDAP and attempt to authenticate + # Otherwise, we will proceed with Mealie authentication user = self.try_get_user(self.data.username) - if not user or user.password == "LDAP" or user.auth_method == AuthMethod.LDAP: + if not user or user.auth_method == AuthMethod.LDAP: user = self.get_user() if user: return self.get_access_token(user, self.data.remember_me) diff --git a/tests/unit_tests/core/security/providers/test_credentials_provider.py b/tests/unit_tests/core/security/providers/test_credentials_provider.py new file mode 100644 index 000000000..af7184d47 --- /dev/null +++ b/tests/unit_tests/core/security/providers/test_credentials_provider.py @@ -0,0 +1,23 @@ +from mealie.core.security.providers.credentials_provider import CredentialsProvider +from mealie.db.models.users.users import AuthMethod +from mealie.schema.user.auth import CredentialsRequest +from tests.utils.fixture_schemas import TestUser + + +def test_login(unique_user: TestUser): + data = {"username": unique_user.username, "password": unique_user.password} + auth_provider = CredentialsProvider(unique_user.repos.session, CredentialsRequest(**data)) + + assert auth_provider.authenticate() is not None + + +def test_login_incorrect_auth_method(unique_user: TestUser): + db = unique_user.repos + user = db.users.get_by_username(unique_user.username) + user.auth_method = AuthMethod.OIDC + db.users.update(unique_user.user_id, user) + + data = {"username": unique_user.username, "password": unique_user.password} + auth_provider = CredentialsProvider(db.session, CredentialsRequest(**data)) + + assert auth_provider.authenticate() is None