diff --git a/backend/src/baserow/api/user/errors.py b/backend/src/baserow/api/user/errors.py index e866799c7..c418c151b 100644 --- a/backend/src/baserow/api/user/errors.py +++ b/backend/src/baserow/api/user/errors.py @@ -50,3 +50,9 @@ ERROR_DEACTIVATED_USER = ( HTTP_401_UNAUTHORIZED, "User account has been disabled.", ) + +ERROR_AUTH_PROVIDER_DISABLED = ( + "ERROR_AUTH_PROVIDER_DISABLED", + HTTP_401_UNAUTHORIZED, + "Authentication provider is disabled.", +) diff --git a/backend/src/baserow/api/user/serializers.py b/backend/src/baserow/api/user/serializers.py index 6775b9d86..4f9d805ba 100644 --- a/backend/src/baserow/api/user/serializers.py +++ b/backend/src/baserow/api/user/serializers.py @@ -18,6 +18,8 @@ from baserow.api.groups.invitations.serializers import UserGroupInvitationSerial from baserow.api.user.jwt import get_user_from_token from baserow.api.user.registries import user_data_registry from baserow.api.user.validators import language_validation, password_validation +from baserow.core.auth_provider.exceptions import AuthProviderDisabled +from baserow.core.auth_provider.handler import PasswordProviderHandler from baserow.core.models import Template from baserow.core.user.exceptions import DeactivatedUserException from baserow.core.user.handler import UserHandler @@ -201,13 +203,16 @@ class TokenObtainPairWithUserSerializer(TokenObtainPairSerializer): super().validate(attrs) user = self.user + password_provider = PasswordProviderHandler.get() + if not password_provider.enabled and user.is_staff is False: + raise AuthProviderDisabled() if not user.is_active: raise DeactivatedUserException() data = generate_session_tokens_for_user(user, include_refresh_token=True) data.update(**get_all_user_data_serialized(user, self.context["request"])) - UserHandler().user_signed_in_via_default_provider(user) + UserHandler().user_signed_in_via_provider(user, password_provider) return data diff --git a/backend/src/baserow/api/user/views.py b/backend/src/baserow/api/user/views.py index dc07eac0d..7aabca6a5 100644 --- a/backend/src/baserow/api/user/views.py +++ b/backend/src/baserow/api/user/views.py @@ -36,6 +36,8 @@ from baserow.api.sessions import get_untrusted_client_session_id from baserow.api.user.registries import user_data_registry from baserow.core.action.handler import ActionHandler from baserow.core.action.registries import ActionScopeStr +from baserow.core.auth_provider.exceptions import AuthProviderDisabled +from baserow.core.auth_provider.handler import PasswordProviderHandler from baserow.core.exceptions import ( BaseURLHostnameNotAllowed, GroupInvitationDoesNotExist, @@ -57,6 +59,7 @@ from baserow.core.user.utils import generate_session_tokens_for_user from .errors import ( ERROR_ALREADY_EXISTS, + ERROR_AUTH_PROVIDER_DISABLED, ERROR_CLIENT_SESSION_ID_HEADER_NOT_SET, ERROR_DEACTIVATED_USER, ERROR_DISABLED_RESET_PASSWORD, @@ -119,6 +122,7 @@ class ObtainJSONWebToken(TokenObtainPairView): { AuthenticationFailed: ERROR_INVALID_CREDENTIALS, DeactivatedUserException: ERROR_DEACTIVATED_USER, + AuthProviderDisabled: ERROR_AUTH_PROVIDER_DISABLED, } ) def post(self, *args, **kwargs): @@ -202,12 +206,16 @@ class UserView(APIView): GroupInvitationDoesNotExist: ERROR_GROUP_INVITATION_DOES_NOT_EXIST, GroupInvitationEmailMismatch: ERROR_GROUP_INVITATION_EMAIL_MISMATCH, DisabledSignupError: ERROR_DISABLED_SIGNUP, + AuthProviderDisabled: ERROR_AUTH_PROVIDER_DISABLED, } ) @validate_body(RegisterSerializer) def post(self, request, data): """Registers a new user.""" + if not PasswordProviderHandler.get().enabled: + raise AuthProviderDisabled() + template = ( Template.objects.get(pk=data["template_id"]) if data["template_id"] @@ -267,6 +275,7 @@ class SendResetPasswordEmailView(APIView): { BaseURLHostnameNotAllowed: ERROR_HOSTNAME_IS_NOT_ALLOWED, ResetPasswordDisabledError: ERROR_DISABLED_RESET_PASSWORD, + AuthProviderDisabled: ERROR_AUTH_PROVIDER_DISABLED, } ) def post(self, request, data): @@ -279,6 +288,8 @@ class SendResetPasswordEmailView(APIView): try: user = handler.get_active_user(email=data["email"]) + if not PasswordProviderHandler.get().enabled and user.is_staff is False: + raise AuthProviderDisabled() handler.send_reset_password_email(user, data["base_url"]) except UserNotFound: pass @@ -357,12 +368,15 @@ class ChangePasswordView(APIView): @map_exceptions( { InvalidPassword: ERROR_INVALID_OLD_PASSWORD, + AuthProviderDisabled: ERROR_AUTH_PROVIDER_DISABLED, } ) @validate_body(ChangePasswordBodyValidationSerializer) def post(self, request, data): """Changes the authenticated user's password if the old password is correct.""" + if not PasswordProviderHandler.get().enabled and request.user.is_staff is False: + raise AuthProviderDisabled() handler = UserHandler() handler.change_password( request.user, data["old_password"], data["new_password"] diff --git a/backend/src/baserow/core/apps.py b/backend/src/baserow/core/apps.py index 9b2614554..bd0881d97 100644 --- a/backend/src/baserow/core/apps.py +++ b/backend/src/baserow/core/apps.py @@ -180,7 +180,7 @@ class CoreConfig(AppConfig): ) from baserow.core.registries import auth_provider_type_registry - auth_provider_type_registry.register_default(PasswordAuthProviderType()) + auth_provider_type_registry.register(PasswordAuthProviderType()) # Clear the key after migration so we will trigger a new template sync. post_migrate.connect(start_sync_templates_task_after_migrate, sender=self) diff --git a/backend/src/baserow/core/auth_provider/auth_provider_types.py b/backend/src/baserow/core/auth_provider/auth_provider_types.py index ee6bcc0ae..95eb9954b 100644 --- a/backend/src/baserow/core/auth_provider/auth_provider_types.py +++ b/backend/src/baserow/core/auth_provider/auth_provider_types.py @@ -2,6 +2,7 @@ from typing import Any, Dict, Optional from rest_framework import serializers +from baserow.core.auth_provider.handler import PasswordProviderHandler from baserow.core.auth_provider.validators import validate_domain from baserow.core.registry import ( APIUrlsInstanceMixin, @@ -32,12 +33,19 @@ class AuthProviderType( raise NotImplementedError() - def can_create_new_providers(self): + def can_create_new_providers(self) -> bool: """ Returns True if it's possible to create an authentication provider of this type. """ - raise NotImplementedError() + return True + + def can_delete_existing_providers(self) -> bool: + """ + Returns True if it's possible to delete an authentication provider of this type. + """ + + return True def before_create(self, user, **values): """ @@ -126,6 +134,7 @@ class AuthProviderType( return { "type": self.type, "can_create_new": self.can_create_new_providers(), + "can_delete_existing": self.can_delete_existing_providers(), "auth_providers": [ self.get_serializer(provider, AuthProviderSerializer).data for provider in self.list_providers() @@ -142,6 +151,8 @@ class PasswordAuthProviderType(AuthProviderType): type = "password" model_class = PasswordAuthProviderModel + allowed_fields = ["id", "enabled"] + serializer_field_names = ["enabled"] serializer_field_overrides = { "domain": serializers.CharField( validators=[validate_domain], @@ -155,7 +166,12 @@ class PasswordAuthProviderType(AuthProviderType): } def get_login_options(self, **kwargs) -> Optional[Dict[str, Any]]: - return {} + if not PasswordProviderHandler.get().enabled: + return None + return {"type": self.type} def can_create_new_providers(self): return False + + def can_delete_existing_providers(self): + return False diff --git a/backend/src/baserow/core/auth_provider/exceptions.py b/backend/src/baserow/core/auth_provider/exceptions.py index 937fd63b6..c8865d5d2 100644 --- a/backend/src/baserow/core/auth_provider/exceptions.py +++ b/backend/src/baserow/core/auth_provider/exceptions.py @@ -1,2 +1,6 @@ class AuthProviderModelNotFound(Exception): """Raised if the requested authentication provider does not exist.""" + + +class AuthProviderDisabled(Exception): + """Raised when it is not possible to use a particular auth provider.""" diff --git a/backend/src/baserow/core/auth_provider/handler.py b/backend/src/baserow/core/auth_provider/handler.py new file mode 100644 index 000000000..149c62d50 --- /dev/null +++ b/backend/src/baserow/core/auth_provider/handler.py @@ -0,0 +1,14 @@ +from baserow.core.auth_provider.models import PasswordAuthProviderModel + + +class PasswordProviderHandler: + @classmethod + def get(cls) -> PasswordAuthProviderModel: + """ + Returns the password provider + + :return: The one and only password provider. + """ + + obj, created = PasswordAuthProviderModel.objects.get_or_create() + return obj diff --git a/backend/src/baserow/core/auth_provider/models.py b/backend/src/baserow/core/auth_provider/models.py index 395c149e6..3d7bad6a0 100644 --- a/backend/src/baserow/core/auth_provider/models.py +++ b/backend/src/baserow/core/auth_provider/models.py @@ -39,9 +39,4 @@ class AuthProviderModel( class PasswordAuthProviderModel(AuthProviderModel): - def save(self, *args, **kwargs): - if not self.enabled: - raise ValueError( - "The password authentication provider cannot be disabled. " - ) - super().save(*args, **kwargs) + ... diff --git a/backend/src/baserow/core/registries.py b/backend/src/baserow/core/registries.py index d4458cdd9..528e6ea96 100644 --- a/backend/src/baserow/core/registries.py +++ b/backend/src/baserow/core/registries.py @@ -356,17 +356,6 @@ class AuthenticationProviderTypeRegistry( super().__init__(*args, **kwargs) self._default = None - def register_default(self, instance): - super().register(instance) - self._default = instance - - def get_default_provider(self): - provider, _ = self._default.model_class.objects.get_or_create() - return provider - - def get_default(self): - return self._default - def get_all_available_login_options(self): login_options = {} for provider in self.get_all(): diff --git a/backend/src/baserow/core/user/handler.py b/backend/src/baserow/core/user/handler.py index a8d5c7d97..01a1d1a64 100644 --- a/backend/src/baserow/core/user/handler.py +++ b/backend/src/baserow/core/user/handler.py @@ -14,6 +14,7 @@ from django.utils.translation import gettext as _ from itsdangerous import URLSafeTimedSerializer +from baserow.core.auth_provider.handler import PasswordProviderHandler from baserow.core.auth_provider.models import AuthProviderModel from baserow.core.exceptions import ( BaseURLHostnameNotAllowed, @@ -21,7 +22,7 @@ from baserow.core.exceptions import ( ) from baserow.core.handler import CoreHandler from baserow.core.models import Group, GroupUser, Template, UserLogEntry, UserProfile -from baserow.core.registries import auth_provider_type_registry, plugin_registry +from baserow.core.registries import plugin_registry from baserow.core.signals import ( before_user_deleted, user_deleted, @@ -209,7 +210,7 @@ class UserHandler: # register the authentication provider used to create the user if auth_provider is None: - auth_provider = auth_provider_type_registry.get_default_provider() + auth_provider = PasswordProviderHandler.get() auth_provider.user_signed_in(user) return user @@ -347,16 +348,6 @@ class UserHandler: return user - def user_signed_in_via_default_provider(self, user: AbstractUser): - """ - Registers that a user has signed in via the default authentication provider. - - :param user: The user that has signed in. - """ - - default_provider = auth_provider_type_registry.get_default_provider() - self.user_signed_in_via_provider(user, default_provider) - def user_signed_in_via_provider( self, user: AbstractUser, authentication_provider: AuthProviderModel ): diff --git a/backend/src/baserow/test_utils/fixtures/__init__.py b/backend/src/baserow/test_utils/fixtures/__init__.py index b85a0b395..43ccfdc65 100644 --- a/backend/src/baserow/test_utils/fixtures/__init__.py +++ b/backend/src/baserow/test_utils/fixtures/__init__.py @@ -2,6 +2,7 @@ from faker import Faker from .airtable import AirtableFixtures from .application import ApplicationFixtures +from .auth_provider import AuthProviderFixtures from .field import FieldFixtures from .file_import import FileImportFixtures from .group import GroupFixtures @@ -35,5 +36,6 @@ class Fixtures( JobFixtures, FileImportFixtures, SnapshotFixtures, + AuthProviderFixtures, ): fake = Faker() diff --git a/backend/src/baserow/test_utils/fixtures/auth_provider.py b/backend/src/baserow/test_utils/fixtures/auth_provider.py new file mode 100644 index 000000000..8bb350551 --- /dev/null +++ b/backend/src/baserow/test_utils/fixtures/auth_provider.py @@ -0,0 +1,9 @@ +from baserow.core.auth_provider.models import PasswordAuthProviderModel + + +class AuthProviderFixtures: + def create_password_provider(self, **kwargs): + if "enabled" not in kwargs: + kwargs["enabled"] = True + + return PasswordAuthProviderModel.objects.create(**kwargs) diff --git a/backend/tests/baserow/api/settings/test_settings_views.py b/backend/tests/baserow/api/settings/test_settings_views.py index 3077517a3..9ca4f405b 100644 --- a/backend/tests/baserow/api/settings/test_settings_views.py +++ b/backend/tests/baserow/api/settings/test_settings_views.py @@ -32,7 +32,10 @@ def test_get_settings(api_client): @pytest.mark.django_db -def test_require_first_admin_user_is_false_after_admin_creation(api_client): +def test_require_first_admin_user_is_false_after_admin_creation( + api_client, data_fixture +): + data_fixture.create_password_provider() response = api_client.get(reverse("api:settings:get")) assert response.status_code == HTTP_200_OK response_json = response.json() diff --git a/backend/tests/baserow/api/users/test_token_auth.py b/backend/tests/baserow/api/users/test_token_auth.py index f3d94a73d..3bd45ead5 100644 --- a/backend/tests/baserow/api/users/test_token_auth.py +++ b/backend/tests/baserow/api/users/test_token_auth.py @@ -22,6 +22,8 @@ User = get_user_model() @pytest.mark.django_db def test_token_auth(api_client, data_fixture): + data_fixture.create_password_provider() + class TmpPlugin(Plugin): type = "tmp_plugin" called = False @@ -193,6 +195,48 @@ def test_token_auth(api_client, data_fixture): assert "user" in response_json +@pytest.mark.django_db +def test_token_password_auth_disabled(api_client, data_fixture): + data_fixture.create_password_provider(enabled=False) + user, token = data_fixture.create_user_and_token( + email="test@localhost", password="test" + ) + + response = api_client.post( + reverse("api:user:token_auth"), + {"email": "test@localhost", "password": "test"}, + format="json", + ) + + assert response.status_code == HTTP_401_UNAUTHORIZED + assert response.json() == { + "error": "ERROR_AUTH_PROVIDER_DISABLED", + "detail": "Authentication provider is disabled.", + } + + +@pytest.mark.django_db +def test_token_password_auth_disabled_superadmin(api_client, data_fixture): + data_fixture.create_password_provider(enabled=False) + user, token = data_fixture.create_user_and_token( + email="test@localhost", password="test", is_staff=True + ) + + response = api_client.post( + reverse("api:user:token_auth"), + {"email": "test@localhost", "password": "test"}, + format="json", + ) + + assert response.status_code == HTTP_200_OK + response_json = response.json() + assert "access_token" in response_json + assert "refresh_token" in response_json + assert "user" in response_json + assert response_json["user"]["id"] == user.id + assert response_json["user"]["is_staff"] is True + + @pytest.mark.django_db def test_token_refresh(api_client, data_fixture): class TmpPlugin(Plugin): diff --git a/backend/tests/baserow/api/users/test_user_views.py b/backend/tests/baserow/api/users/test_user_views.py index 3320baf73..c95a63c53 100644 --- a/backend/tests/baserow/api/users/test_user_views.py +++ b/backend/tests/baserow/api/users/test_user_views.py @@ -28,6 +28,7 @@ User = get_user_model() @pytest.mark.django_db def test_create_user(client, data_fixture): + data_fixture.create_password_provider() valid_password = "thisIsAValidPassword" short_password = "short" response = client.post( @@ -255,6 +256,7 @@ def test_user_account(data_fixture, api_client): @pytest.mark.django_db def test_create_user_with_invitation(data_fixture, client): + data_fixture.create_password_provider() core_handler = CoreHandler() valid_password = "thisIsAValidPassword" invitation = data_fixture.create_group_invitation(email="test0@test.nl") @@ -335,6 +337,7 @@ def test_create_user_with_invitation(data_fixture, client): @pytest.mark.django_db def test_create_user_with_template(data_fixture, client): + data_fixture.create_password_provider() old_templates = settings.APPLICATION_TEMPLATES_DIR valid_password = "thisIsAValidPassword" settings.APPLICATION_TEMPLATES_DIR = os.path.join( @@ -395,6 +398,7 @@ def test_create_user_with_template(data_fixture, client): @pytest.mark.django_db(transaction=True) def test_send_reset_password_email(data_fixture, client, mailoutbox): + data_fixture.create_password_provider() data_fixture.create_user(email="test@localhost.nl") response = client.post( @@ -454,6 +458,44 @@ def test_send_reset_password_email(data_fixture, client, mailoutbox): assert response.status_code == HTTP_400_BAD_REQUEST +@pytest.mark.django_db +def test_send_reset_password_email_password_auth_disabled( + api_client, data_fixture, mailoutbox +): + data_fixture.create_password_provider(enabled=False) + data_fixture.create_user(email="test@localhost.nl") + + response = api_client.post( + reverse("api:user:send_reset_password_email"), + {"email": "test@localhost.nl", "base_url": "http://localhost:3000"}, + format="json", + ) + + assert response.status_code == HTTP_401_UNAUTHORIZED + assert response.json() == { + "error": "ERROR_AUTH_PROVIDER_DISABLED", + "detail": "Authentication provider is disabled.", + } + assert len(mailoutbox) == 0 + + +@pytest.mark.django_db(transaction=True) +def test_send_reset_password_email_password_auth_disabled_staff( + api_client, data_fixture, mailoutbox +): + data_fixture.create_password_provider(enabled=False) + data_fixture.create_user(email="test@localhost.nl", is_staff=True) + + response = api_client.post( + reverse("api:user:send_reset_password_email"), + {"email": "test@localhost.nl", "base_url": "http://localhost:3000"}, + format="json", + ) + + assert response.status_code == HTTP_204_NO_CONTENT + assert len(mailoutbox) == 1 + + @pytest.mark.django_db def test_password_reset(data_fixture, client): user = data_fixture.create_user(email="test@localhost") @@ -581,6 +623,7 @@ def test_password_reset(data_fixture, client): @pytest.mark.django_db def test_change_password(data_fixture, client): + data_fixture.create_password_provider() valid_old_password = "thisIsAValidPassword" valid_new_password = "thisIsAValidNewPassword" short_password = "short" @@ -671,6 +714,50 @@ def test_change_password(data_fixture, client): assert user.check_password(valid_new_password) +@pytest.mark.django_db +def test_change_password_auth_disabled(api_client, data_fixture): + data_fixture.create_password_provider(enabled=False) + valid_old_password = "thisIsAValidPassword" + valid_new_password = "thisIsAValidNewPassword" + user, token = data_fixture.create_user_and_token( + email="test@localhost", password=valid_old_password + ) + + response = api_client.post( + reverse("api:user:change_password"), + {"old_password": valid_old_password, "new_password": valid_new_password}, + format="json", + HTTP_AUTHORIZATION=f"JWT {token}", + ) + + assert response.status_code == HTTP_401_UNAUTHORIZED + assert response.json() == { + "error": "ERROR_AUTH_PROVIDER_DISABLED", + "detail": "Authentication provider is disabled.", + } + + +@pytest.mark.django_db +def test_change_password_auth_disabled_staff(api_client, data_fixture): + data_fixture.create_password_provider(enabled=False) + valid_old_password = "thisIsAValidPassword" + valid_new_password = "thisIsAValidNewPassword" + user, token = data_fixture.create_user_and_token( + email="test@localhost", password=valid_old_password, is_staff=True + ) + + response = api_client.post( + reverse("api:user:change_password"), + {"old_password": valid_old_password, "new_password": valid_new_password}, + format="json", + HTTP_AUTHORIZATION=f"JWT {token}", + ) + + assert response.status_code == HTTP_204_NO_CONTENT + user.refresh_from_db() + assert user.check_password(valid_new_password) + + @pytest.mark.django_db def test_dashboard(data_fixture, client): user, token = data_fixture.create_user_and_token(email="test@localhost") @@ -699,6 +786,8 @@ def test_dashboard(data_fixture, client): @pytest.mark.django_db def test_additional_user_data(api_client, data_fixture): + data_fixture.create_password_provider() + class TmpUserDataType(UserDataType): type = "type" @@ -831,3 +920,23 @@ def test_token_error_if_user_deleted_or_disabled(api_client, data_fixture): "error": "ERROR_INVALID_REFRESH_TOKEN", "detail": "Refresh token is expired or invalid.", } + + +@pytest.mark.django_db +def test_create_user_password_auth_disabled(api_client, data_fixture): + data_fixture.create_password_provider(enabled=False) + user, token = data_fixture.create_user_and_token( + email="test@localhost", password="test" + ) + + response = api_client.post( + reverse("api:user:index"), + {"name": "Test1", "email": "test@test.nl", "password": "thisIsAValidPassword"}, + format="json", + ) + + assert response.status_code == HTTP_401_UNAUTHORIZED + assert response.json() == { + "error": "ERROR_AUTH_PROVIDER_DISABLED", + "detail": "Authentication provider is disabled.", + } diff --git a/changelog.md b/changelog.md index 4adf31093..965f1744a 100644 --- a/changelog.md +++ b/changelog.md @@ -11,6 +11,8 @@ For example: ### New Features +* Possibility to disable password authentication if another authentication provider is enabled. [#1317](https://gitlab.com/bramw/baserow/-/issues/1317) + ### Bug Fixes ### Refactors @@ -18,6 +20,7 @@ For example: ## Released (2022-12-8 1.13.2) ### New Features + * Add drag and drop zone for files to the row edit modal [#1161](https://gitlab.com/bramw/baserow/-/issues/1161) * Automatically enable/disable enterprise features upon activation/deactivation without needing a page refresh first. [#1306](https://gitlab.com/bramw/baserow/-/issues/1306) diff --git a/enterprise/backend/src/baserow_enterprise/api/admin/auth_provider/errors.py b/enterprise/backend/src/baserow_enterprise/api/admin/auth_provider/errors.py index 99122309a..bb23a3119 100644 --- a/enterprise/backend/src/baserow_enterprise/api/admin/auth_provider/errors.py +++ b/enterprise/backend/src/baserow_enterprise/api/admin/auth_provider/errors.py @@ -1,7 +1,25 @@ -from rest_framework.status import HTTP_404_NOT_FOUND +from rest_framework.status import HTTP_400_BAD_REQUEST, HTTP_404_NOT_FOUND ERROR_AUTH_PROVIDER_DOES_NOT_EXIST = ( "ERROR_AUTH_PROVIDER_DOES_NOT_EXIST", HTTP_404_NOT_FOUND, "The requested auth provider does not exist.", ) + +ERROR_AUTH_PROVIDER_CANNOT_BE_CREATED = ( + "ERROR_AUTH_PROVIDER_CANNOT_BE_CREATED", + HTTP_400_BAD_REQUEST, + "The provider type cannot be created.", +) + +ERROR_AUTH_PROVIDER_CANNOT_BE_DELETED = ( + "ERROR_AUTH_PROVIDER_CANNOT_BE_DELETED", + HTTP_400_BAD_REQUEST, + "The provider type cannot be deleted.", +) + +ERROR_CANNOT_DISABLE_ALL_AUTH_PROVIDERS = ( + "ERROR_CANNOT_DISABLE_ALL_AUTH_PROVIDERS", + HTTP_400_BAD_REQUEST, + "The last enabled provider cannot be disabled.", +) diff --git a/enterprise/backend/src/baserow_enterprise/api/admin/auth_provider/views.py b/enterprise/backend/src/baserow_enterprise/api/admin/auth_provider/views.py index da07270b8..64fe522cb 100644 --- a/enterprise/backend/src/baserow_enterprise/api/admin/auth_provider/views.py +++ b/enterprise/backend/src/baserow_enterprise/api/admin/auth_provider/views.py @@ -18,10 +18,20 @@ from baserow.api.utils import ( ) from baserow.core.auth_provider.exceptions import AuthProviderModelNotFound from baserow.core.registries import auth_provider_type_registry +from baserow_enterprise.auth_provider.exceptions import ( + CannotCreateAuthProvider, + CannotDeleteAuthProvider, + CannotDisableLastAuthProvider, +) from baserow_enterprise.auth_provider.handler import AuthProviderHandler from baserow_enterprise.sso.utils import check_sso_feature_is_active_or_raise -from .errors import ERROR_AUTH_PROVIDER_DOES_NOT_EXIST +from .errors import ( + ERROR_AUTH_PROVIDER_CANNOT_BE_CREATED, + ERROR_AUTH_PROVIDER_CANNOT_BE_DELETED, + ERROR_AUTH_PROVIDER_DOES_NOT_EXIST, + ERROR_CANNOT_DISABLE_ALL_AUTH_PROVIDERS, +) from .serializers import ( CreateAuthProviderSerializer, NextAuthProviderIdSerializer, @@ -52,6 +62,11 @@ class AdminAuthProvidersView(APIView): auth_provider_type_registry, base_serializer_class=CreateAuthProviderSerializer, ) + @map_exceptions( + { + CannotCreateAuthProvider: ERROR_AUTH_PROVIDER_CANNOT_BE_CREATED, + } + ) def post(self, request: Request, data: Dict[str, Any]) -> Response: """Create a new authentication provider.""" @@ -121,6 +136,7 @@ class AdminAuthProviderView(APIView): @map_exceptions( { AuthProviderModelNotFound: ERROR_AUTH_PROVIDER_DOES_NOT_EXIST, + CannotDisableLastAuthProvider: ERROR_CANNOT_DISABLE_ALL_AUTH_PROVIDERS, } ) def patch(self, request, auth_provider_id: int): @@ -203,6 +219,8 @@ class AdminAuthProviderView(APIView): @map_exceptions( { AuthProviderModelNotFound: ERROR_AUTH_PROVIDER_DOES_NOT_EXIST, + CannotDeleteAuthProvider: ERROR_AUTH_PROVIDER_CANNOT_BE_DELETED, + CannotDisableLastAuthProvider: ERROR_CANNOT_DISABLE_ALL_AUTH_PROVIDERS, } ) def delete(self, request: Request, auth_provider_id: int) -> Response: diff --git a/enterprise/backend/src/baserow_enterprise/api/sso/utils.py b/enterprise/backend/src/baserow_enterprise/api/sso/utils.py index 9b6b5a1df..db338ed48 100644 --- a/enterprise/backend/src/baserow_enterprise/api/sso/utils.py +++ b/enterprise/backend/src/baserow_enterprise/api/sso/utils.py @@ -182,3 +182,13 @@ def get_frontend_login_error_url() -> str: """ return urljoin(settings.PUBLIC_WEB_FRONTEND_URL, "/login/error") + + +def get_frontend_login_saml_url() -> str: + """ + Returns the url to the frontend SAML login page. + + :return: The absolute url to the Baserow SAML login page. + """ + + return urljoin(settings.PUBLIC_WEB_FRONTEND_URL, "/login/saml") diff --git a/enterprise/backend/src/baserow_enterprise/auth_provider/exceptions.py b/enterprise/backend/src/baserow_enterprise/auth_provider/exceptions.py index 89549f622..1a381f809 100644 --- a/enterprise/backend/src/baserow_enterprise/auth_provider/exceptions.py +++ b/enterprise/backend/src/baserow_enterprise/auth_provider/exceptions.py @@ -3,3 +3,21 @@ class DifferentAuthProvider(Exception): Raised when logging in an existing user that should not be logged in using a different than the approved auth provider. """ + + +class CannotCreateAuthProvider(Exception): + """ + Raised when a provider type cannot be created. + """ + + +class CannotDeleteAuthProvider(Exception): + """ + Raised when a provider type cannot be deleted. + """ + + +class CannotDisableLastAuthProvider(Exception): + """ + Raised during an attempt to disable last enabled auth provider. + """ diff --git a/enterprise/backend/src/baserow_enterprise/auth_provider/handler.py b/enterprise/backend/src/baserow_enterprise/auth_provider/handler.py index b6c3986a8..c4559a568 100644 --- a/enterprise/backend/src/baserow_enterprise/auth_provider/handler.py +++ b/enterprise/backend/src/baserow_enterprise/auth_provider/handler.py @@ -13,7 +13,12 @@ from baserow.core.handler import CoreHandler from baserow.core.registries import auth_provider_type_registry from baserow.core.user.exceptions import UserNotFound from baserow.core.user.handler import UserHandler -from baserow_enterprise.auth_provider.exceptions import DifferentAuthProvider +from baserow_enterprise.auth_provider.exceptions import ( + CannotCreateAuthProvider, + CannotDeleteAuthProvider, + CannotDisableLastAuthProvider, + DifferentAuthProvider, +) SpecificAuthProviderModel = Type[AuthProviderModel] @@ -59,6 +64,8 @@ class AuthProviderHandler: :return: The created authentication provider. """ + if not auth_provider_type.can_create_new_providers(): + raise CannotCreateAuthProvider() auth_provider_type.before_create(user, **values) return auth_provider_type.create(**values) @@ -79,6 +86,17 @@ class AuthProviderHandler: """ auth_provider_type = auth_provider_type_registry.get_by_model(auth_provider) + + enabled_next = values.get("enabled", None) + if enabled_next is False: + another_enabled = ( + AuthProviderModel.objects.filter(enabled=True) + .exclude(id=auth_provider.id) + .exists() + ) + if not another_enabled: + raise CannotDisableLastAuthProvider() + auth_provider_type.before_update(user, auth_provider, **values) return auth_provider_type.update(auth_provider, **values) @@ -93,6 +111,15 @@ class AuthProviderHandler: """ auth_provider_type = auth_provider_type_registry.get_by_model(auth_provider) + if not auth_provider_type.can_delete_existing_providers(): + raise CannotDeleteAuthProvider() + another_enabled = ( + AuthProviderModel.objects.filter(enabled=True) + .exclude(id=auth_provider.id) + .exists() + ) + if not another_enabled: + raise CannotDisableLastAuthProvider() auth_provider_type.before_delete(user, auth_provider) auth_provider_type.delete(auth_provider) diff --git a/enterprise/backend/src/baserow_enterprise/sso/oauth2/auth_provider_types.py b/enterprise/backend/src/baserow_enterprise/sso/oauth2/auth_provider_types.py index 33a6c59ce..9787d1cec 100644 --- a/enterprise/backend/src/baserow_enterprise/sso/oauth2/auth_provider_types.py +++ b/enterprise/backend/src/baserow_enterprise/sso/oauth2/auth_provider_types.py @@ -53,32 +53,35 @@ class OAuth2AuthProviderMixin: - self.SCOPE """ - def can_create_new_providers(self): - return True - def get_login_options(self, **kwargs) -> Optional[Dict[str, Any]]: if not is_sso_feature_active(): - return {} + return None + + instances = self.model_class.objects.filter(enabled=True) + if not instances: + return None - instances = self.model_class.objects.all() items = [] for instance in instances: - if instance.enabled: - items.append( - { - "redirect_url": urllib.parse.urljoin( - OAUTH_BACKEND_URL, - reverse( - "api:enterprise:sso:oauth2:login", args=(instance.id,) - ), - ), - "name": instance.name, - "type": self.type, - } - ) + items.append( + { + "redirect_url": urllib.parse.urljoin( + OAUTH_BACKEND_URL, + reverse("api:enterprise:sso:oauth2:login", args=(instance.id,)), + ), + "name": instance.name, + "type": self.type, + } + ) + + default_redirect_url = None + if len(items) == 1: + default_redirect_url = items[0]["redirect_url"] + return { "type": self.type, "items": items, + "default_redirect_url": default_redirect_url, } def get_base_url(self, instance: AuthProviderModel) -> str: diff --git a/enterprise/backend/src/baserow_enterprise/sso/saml/auth_provider_types.py b/enterprise/backend/src/baserow_enterprise/sso/saml/auth_provider_types.py index 9b73e1b35..c6a61475d 100644 --- a/enterprise/backend/src/baserow_enterprise/sso/saml/auth_provider_types.py +++ b/enterprise/backend/src/baserow_enterprise/sso/saml/auth_provider_types.py @@ -16,7 +16,10 @@ from baserow_enterprise.api.sso.saml.validators import ( validate_saml_metadata, validate_unique_saml_domain, ) -from baserow_enterprise.api.sso.utils import get_frontend_default_redirect_url +from baserow_enterprise.api.sso.utils import ( + get_frontend_default_redirect_url, + get_frontend_login_saml_url, +) from baserow_enterprise.sso.saml.exceptions import SamlProviderForDomainAlreadyExists from baserow_enterprise.sso.utils import is_sso_feature_active @@ -83,22 +86,32 @@ class SamlAuthProviderType(AuthProviderType): if not configured_domains: return None + default_redirect_url = None + if configured_domains == 1: + default_redirect_url = self.get_login_absolute_url() + if configured_domains > 1: + default_redirect_url = get_frontend_login_saml_url() + return { "type": self.type, # if configure_domains = 1, we can redirect directly the user to the # IdP login page without asking for the email "domain_required": configured_domains > 1, + "default_redirect_url": default_redirect_url, } - def can_create_new_providers(self): - return True - @classmethod def get_acs_absolute_url(cls): return urljoin( settings.PUBLIC_BACKEND_URL, reverse("api:enterprise:sso:saml:acs") ) + @classmethod + def get_login_absolute_url(cls): + return urljoin( + settings.PUBLIC_BACKEND_URL, reverse("api:enterprise:sso:saml:login") + ) + def export_serialized(self) -> Dict[str, Any]: serialized_data = super().export_serialized() serialized_data["relay_state_url"] = get_frontend_default_redirect_url() diff --git a/enterprise/backend/tests/baserow_enterprise_tests/api/admin/auth_provider/test_admin_auth_provider_views.py b/enterprise/backend/tests/baserow_enterprise_tests/api/admin/auth_provider/test_admin_auth_provider_views.py index e0ef974e8..9190fc945 100644 --- a/enterprise/backend/tests/baserow_enterprise_tests/api/admin/auth_provider/test_admin_auth_provider_views.py +++ b/enterprise/backend/tests/baserow_enterprise_tests/api/admin/auth_provider/test_admin_auth_provider_views.py @@ -41,6 +41,7 @@ def test_admin_cannot_list_saml_provider_without_an_enterprise_license( def test_admin_can_list_saml_provider_with_an_enterprise_license( api_client, data_fixture, enterprise_data_fixture ): + data_fixture.create_password_provider() auth_prov_1 = enterprise_data_fixture.create_saml_auth_provider(domain="test.com") auth_prov_2 = enterprise_data_fixture.create_saml_auth_provider(domain="acme.com") @@ -115,6 +116,7 @@ def test_admin_cannot_create_saml_provider_without_an_enterprise_license( def test_admin_can_create_saml_provider_with_an_enterprise_license( api_client, data_fixture, enterprise_data_fixture ): + data_fixture.create_password_provider() # create a valid SAML provider domain = "test.it" @@ -429,6 +431,7 @@ def test_admin_cannot_delete_saml_provider_without_an_enterprise_license( def test_admin_can_delete_saml_provider_with_an_enterprise_license( api_client, data_fixture, enterprise_data_fixture ): + unrelated_provider = enterprise_data_fixture.create_saml_auth_provider() saml_provider_1 = enterprise_data_fixture.create_saml_auth_provider() _, token = enterprise_data_fixture.create_enterprise_admin_user_and_token() @@ -463,7 +466,7 @@ def test_admin_can_delete_saml_provider_with_an_enterprise_license( HTTP_AUTHORIZATION=f"JWT {token}", ) assert response.status_code == HTTP_204_NO_CONTENT - assert SamlAuthProviderModel.objects.count() == 0 + assert SamlAuthProviderModel.objects.count() == 1 @pytest.mark.django_db @@ -555,6 +558,7 @@ def test_create_and_get_oauth2_provider( endpoint will output correct information for the created provider. """ + data_fixture.create_password_provider() admin, token = enterprise_data_fixture.create_enterprise_admin_user_and_token() # create provider @@ -667,6 +671,7 @@ def test_update_oauth2_provider( admin, token = enterprise_data_fixture.create_enterprise_admin_user_and_token() + unrelated_provider = enterprise_data_fixture.create_saml_auth_provider() provider = enterprise_data_fixture.create_oauth_provider( type=provider_type, **extra_params ) @@ -741,3 +746,120 @@ def test_update_oauth_provider_invalid_url( ] } ) + + +@pytest.mark.django_db +@override_settings(DEBUG=True) +def test_admin_cannot_create_password_provider( + api_client, data_fixture, enterprise_data_fixture +): + """ + Password provider cannot be created as to keep only one instance. + """ + + admin, token = enterprise_data_fixture.create_enterprise_admin_user_and_token() + auth_provider_1_url = reverse("api:enterprise:admin:auth_provider:list") + + response = api_client.post( + auth_provider_1_url, + {"type": "password", "enabled": True}, + format="json", + HTTP_AUTHORIZATION=f"JWT {token}", + ) + + assert response.status_code == HTTP_400_BAD_REQUEST + assert response.json()["error"] == "ERROR_AUTH_PROVIDER_CANNOT_BE_CREATED" + + +@pytest.mark.django_db +@override_settings(DEBUG=True) +def test_admin_cannot_delete_password_provider( + api_client, data_fixture, enterprise_data_fixture +): + """ + Password provider cannot be deleted, only enabled and disabled. + """ + + admin, token = enterprise_data_fixture.create_enterprise_admin_user_and_token() + password_provider = data_fixture.create_password_provider() + auth_provider_1_url = reverse( + "api:enterprise:admin:auth_provider:item", + kwargs={"auth_provider_id": password_provider.id}, + ) + + response = api_client.delete( + auth_provider_1_url, + format="json", + HTTP_AUTHORIZATION=f"JWT {token}", + ) + + assert response.status_code == HTTP_400_BAD_REQUEST + assert response.json()["error"] == "ERROR_AUTH_PROVIDER_CANNOT_BE_DELETED" + + +@pytest.mark.django_db +@override_settings(DEBUG=True) +def test_admin_cannot_delete_last_provider( + api_client, data_fixture, enterprise_data_fixture +): + """ + At least one auth provider needs to be always enabled. + """ + + admin, token = enterprise_data_fixture.create_enterprise_admin_user_and_token() + last_provider = enterprise_data_fixture.create_oauth_provider(type="github") + + # but not he last one + last_provider_url = reverse( + "api:enterprise:admin:auth_provider:item", + kwargs={"auth_provider_id": last_provider.id}, + ) + response = api_client.delete( + last_provider_url, + format="json", + HTTP_AUTHORIZATION=f"JWT {token}", + ) + + assert response.status_code == HTTP_400_BAD_REQUEST + assert response.json()["error"] == "ERROR_CANNOT_DISABLE_ALL_AUTH_PROVIDERS" + + +@pytest.mark.django_db +@override_settings(DEBUG=True) +def test_admin_cannot_disable_last_provider( + api_client, data_fixture, enterprise_data_fixture +): + """ + At least one auth provider needs to be always enabled. + """ + + admin, token = enterprise_data_fixture.create_enterprise_admin_user_and_token() + password_provider = data_fixture.create_password_provider() + github_provider = enterprise_data_fixture.create_oauth_provider(type="github") + + # it is possible to disable second provider + github_provider_url = reverse( + "api:enterprise:admin:auth_provider:item", + kwargs={"auth_provider_id": github_provider.id}, + ) + response = api_client.patch( + github_provider_url, + {"enabled": False}, + format="json", + HTTP_AUTHORIZATION=f"JWT {token}", + ) + + # but not he last one + password_provider_url = reverse( + "api:enterprise:admin:auth_provider:item", + kwargs={"auth_provider_id": password_provider.id}, + ) + response = api_client.patch( + password_provider_url, + {"enabled": False}, + format="json", + HTTP_AUTHORIZATION=f"JWT {token}", + ) + + assert response.status_code == HTTP_400_BAD_REQUEST + assert response.json()["error"] == "ERROR_CANNOT_DISABLE_ALL_AUTH_PROVIDERS" diff --git a/enterprise/backend/tests/baserow_enterprise_tests/api/sso/test_sso_login_options.py b/enterprise/backend/tests/baserow_enterprise_tests/api/sso/test_sso_login_options.py index df081766e..521696791 100644 --- a/enterprise/backend/tests/baserow_enterprise_tests/api/sso/test_sso_login_options.py +++ b/enterprise/backend/tests/baserow_enterprise_tests/api/sso/test_sso_login_options.py @@ -10,6 +10,8 @@ from rest_framework.status import HTTP_200_OK def test_saml_not_available_without_an_enterprise_license( api_client, data_fixture, enterprise_data_fixture ): + data_fixture.create_password_provider() + # create a valid SAML provider enterprise_data_fixture.create_saml_auth_provider(domain="test1.com") @@ -24,6 +26,8 @@ def test_saml_not_available_without_an_enterprise_license( def test_saml_available_with_an_enterprise_license( api_client, data_fixture, enterprise_data_fixture ): + data_fixture.create_password_provider() + # create a valid SAML provider enterprise_data_fixture.create_saml_auth_provider(domain="test1.com") enterprise_data_fixture.create_enterprise_admin_user_and_token() diff --git a/enterprise/backend/tests/baserow_enterprise_tests/sso/oauth2/test_auth_provider_types.py b/enterprise/backend/tests/baserow_enterprise_tests/sso/oauth2/test_auth_provider_types.py index 0ec5244cc..da089b11c 100644 --- a/enterprise/backend/tests/baserow_enterprise_tests/sso/oauth2/test_auth_provider_types.py +++ b/enterprise/backend/tests/baserow_enterprise_tests/sso/oauth2/test_auth_provider_types.py @@ -28,7 +28,10 @@ from baserow_enterprise.sso.oauth2.auth_provider_types import ( ) @pytest.mark.django_db @override_settings(DEBUG=True) -def test_get_login_options(provider_type, extra_params, enterprise_data_fixture): +def test_get_login_options( + provider_type, extra_params, data_fixture, enterprise_data_fixture +): + data_fixture.create_password_provider() provider = enterprise_data_fixture.create_oauth_provider( type=provider_type, client_id="test_client_id", @@ -59,6 +62,9 @@ def test_get_login_options(provider_type, extra_params, enterprise_data_fixture) "type": provider_type, } ], + "default_redirect_url": ( + f"{settings.PUBLIC_BACKEND_URL}" f"/api/sso/oauth2/login/{provider.id}/" + ), } diff --git a/enterprise/backend/tests/baserow_enterprise_tests/sso/saml/test_auth_provider_types.py b/enterprise/backend/tests/baserow_enterprise_tests/sso/saml/test_auth_provider_types.py index 25669c568..d828a096f 100644 --- a/enterprise/backend/tests/baserow_enterprise_tests/sso/saml/test_auth_provider_types.py +++ b/enterprise/backend/tests/baserow_enterprise_tests/sso/saml/test_auth_provider_types.py @@ -9,7 +9,8 @@ from baserow_enterprise.sso.saml.exceptions import SamlProviderForDomainAlreadyE @pytest.mark.django_db @override_settings(DEBUG=True) -def test_get_login_options(enterprise_data_fixture): +def test_get_login_options(data_fixture, enterprise_data_fixture): + data_fixture.create_password_provider() enterprise_data_fixture.create_saml_auth_provider(domain="test.com") login_options = auth_provider_type_registry.get_all_available_login_options() assert "saml" not in login_options @@ -19,6 +20,7 @@ def test_get_login_options(enterprise_data_fixture): assert login_options["saml"] == { "type": "saml", "domain_required": False, + "default_redirect_url": "http://localhost:8000/api/sso/saml/login/", } enterprise_data_fixture.create_saml_auth_provider(domain="acme.com") @@ -26,6 +28,7 @@ def test_get_login_options(enterprise_data_fixture): assert login_options["saml"] == { "type": "saml", "domain_required": True, + "default_redirect_url": "http://localhost:3000/login/saml", } diff --git a/enterprise/web-frontend/modules/baserow_enterprise/authProviderTypes.js b/enterprise/web-frontend/modules/baserow_enterprise/authProviderTypes.js index c4e71e04b..5655d00f9 100644 --- a/enterprise/web-frontend/modules/baserow_enterprise/authProviderTypes.js +++ b/enterprise/web-frontend/modules/baserow_enterprise/authProviderTypes.js @@ -8,6 +8,7 @@ import GitLabSettingsForm from '@baserow_enterprise/components/admin/forms/GitLa import OpenIdConnectSettingsForm from '@baserow_enterprise/components/admin/forms/OpenIdConnectSettingsForm.vue' import LoginButton from '@baserow_enterprise/components/admin/login/LoginButton.vue' +import PasswordAuthIcon from '@baserow/modules/core/assets/images/providers/Key.svg' import SAMLIcon from '@baserow_enterprise/assets/images/providers/LockKey.svg' import GoogleIcon from '@baserow_enterprise/assets/images/providers/Google.svg' import FacebookIcon from '@baserow_enterprise/assets/images/providers/Facebook.svg' @@ -16,6 +17,36 @@ import GitLabIcon from '@baserow_enterprise/assets/images/providers/GitLab.svg' import OpenIdIcon from '@baserow_enterprise/assets/images/providers/OpenID.svg' import VerifiedProviderIcon from '@baserow_enterprise/assets/images/providers/VerifiedProviderIcon.svg' +export class PasswordAuthProviderType extends AuthProviderType { + getType() { + return 'password' + } + + getIcon() { + return PasswordAuthIcon + } + + getName() { + return this.app.i18n.t('authProviderTypes.password') + } + + getProviderName(provider) { + return this.getName() + } + + getAdminListComponent() { + return AuthProviderItem + } + + getAdminSettingsFormComponent() { + return null + } + + getOrder() { + return 1 + } +} + export class SamlAuthProviderType extends AuthProviderType { getType() { return 'saml' diff --git a/enterprise/web-frontend/modules/baserow_enterprise/components/admin/AuthProviderItem.vue b/enterprise/web-frontend/modules/baserow_enterprise/components/admin/AuthProviderItem.vue index a151c6295..40c6f1229 100644 --- a/enterprise/web-frontend/modules/baserow_enterprise/components/admin/AuthProviderItem.vue +++ b/enterprise/web-frontend/modules/baserow_enterprise/components/admin/AuthProviderItem.vue @@ -5,22 +5,29 @@ {{ getName() }} </div> <div class="auth-provider-admin__item-menu"> - <a ref="editMenuContextLink" @click="openContext()"> + <a + v-if="hasContextMenu(authProvider.type)" + ref="editMenuContextLink" + @click="openContext()" + > <i class="fas fa-ellipsis-v"></i> </a> <EditAuthProviderMenuContext + v-if="hasContextMenu(authProvider.type)" ref="editMenuContext" :auth-provider="authProvider" @edit="showUpdateSettingsModal" @delete="showDeleteModal" /> <UpdateSettingsAuthProviderModal + v-if="canBeEdited(authProvider.type)" ref="updateSettingsModal" :auth-provider="authProvider" @settings-updated="onSettingsUpdated" @cancel="$refs.updateSettingsModal.hide()" /> <DeleteAuthProviderModal + v-if="canBeDeleted(authProvider.type)" ref="deleteModal" :auth-provider="authProvider" @deleteConfirmed="onDeleteConfirmed" @@ -31,6 +38,7 @@ class="auth-provider-admin__item-toggle" :value="authProvider.enabled" :large="true" + :disabled="isOneProviderEnabled && authProvider.enabled" @input="setEnabled($event)" ></SwitchInput> </div> @@ -42,6 +50,7 @@ import EditAuthProviderMenuContext from '@baserow_enterprise/components/admin/co import UpdateSettingsAuthProviderModal from '@baserow_enterprise/components/admin/modals/UpdateSettingsAuthProviderModal.vue' import DeleteAuthProviderModal from '@baserow_enterprise/components/admin/modals/DeleteAuthProviderModal.vue' import AuthProviderIcon from '@baserow_enterprise/components/AuthProviderIcon.vue' +import authProviderItemMixin from '@baserow_enterprise/mixins/authProviderItemMixin' export default { name: 'AuthProviderItem', @@ -52,12 +61,18 @@ export default { UpdateSettingsAuthProviderModal, AuthProviderIcon, }, + mixins: [authProviderItemMixin], props: { authProvider: { type: Object, required: true, }, }, + computed: { + isOneProviderEnabled() { + return this.$store.getters['authProviderAdmin/isOneProviderEnabled'] + }, + }, methods: { getIcon() { return this.$registry diff --git a/enterprise/web-frontend/modules/baserow_enterprise/components/admin/contexts/EditAuthProviderMenuContext.vue b/enterprise/web-frontend/modules/baserow_enterprise/components/admin/contexts/EditAuthProviderMenuContext.vue index 255ab0cf8..b746102df 100644 --- a/enterprise/web-frontend/modules/baserow_enterprise/components/admin/contexts/EditAuthProviderMenuContext.vue +++ b/enterprise/web-frontend/modules/baserow_enterprise/components/admin/contexts/EditAuthProviderMenuContext.vue @@ -1,13 +1,13 @@ <template> <Context ref="context"> <ul class="context__menu"> - <li> + <li v-if="canBeEdited(authProvider.type)"> <a @click="$emit('edit', authProvider.id)"> <i class="context__menu-icon fas fa-fw fa-pen"></i> {{ $t('editAuthProviderMenuContext.edit') }} </a> </li> - <li> + <li v-if="canBeDeleted(authProvider.type)"> <a @click="$emit('delete', authProvider.id)"> <i class="context__menu-icon fas fa-fw fa-trash-alt"></i> {{ $t('editAuthProviderMenuContext.delete') }} @@ -20,11 +20,12 @@ <script> import context from '@baserow/modules/core/mixins/context' +import authProviderItemMixin from '@baserow_enterprise/mixins/authProviderItemMixin' export default { name: 'EditAuthProviderMenuContext', components: {}, - mixins: [context], + mixins: [context, authProviderItemMixin], props: { authProvider: { type: Object, diff --git a/enterprise/web-frontend/modules/baserow_enterprise/locales/en.json b/enterprise/web-frontend/modules/baserow_enterprise/locales/en.json index 7c7ff1d93..b63856837 100644 --- a/enterprise/web-frontend/modules/baserow_enterprise/locales/en.json +++ b/enterprise/web-frontend/modules/baserow_enterprise/locales/en.json @@ -1,4 +1,8 @@ { + "clientHandler": { + "cannotDisableAllAuthProvidersTitle": "Last enabled provider", + "cannotDisableAllAuthProvidersDescription": "It is not possible to disable or delete last enabled provider." + }, "enterprise": { "license": "Enterprise", "sidebarTooltip": "Your account has access to the enterprise features globally", @@ -58,6 +62,9 @@ "noProviders": "No authentication providers configured yet.", "addProvider": "Add provider" }, + "authProviderTypes": { + "password": "Email and password authentication" + }, "editAuthProviderMenuContext": { "edit": "Edit", "delete": "Delete" @@ -211,4 +218,4 @@ "chatwootSuportSidebarGroup": { "directSupport": "Direct support" } -} +} \ No newline at end of file diff --git a/enterprise/web-frontend/modules/baserow_enterprise/mixins/authProviderItemMixin.js b/enterprise/web-frontend/modules/baserow_enterprise/mixins/authProviderItemMixin.js new file mode 100644 index 000000000..8e3ad4c50 --- /dev/null +++ b/enterprise/web-frontend/modules/baserow_enterprise/mixins/authProviderItemMixin.js @@ -0,0 +1,22 @@ +export default { + methods: { + canBeEdited(authProviderType) { + return ( + this.$registry + .get('authProvider', authProviderType) + .getAdminSettingsFormComponent() != null + ) + }, + canBeDeleted(authProviderType) { + const getType = this.$store.getters['authProviderAdmin/getType'] + const providerTypeData = getType(authProviderType) + return providerTypeData.canDeleteExistingProviders + }, + hasContextMenu(authProviderType) { + return ( + this.canBeEdited(authProviderType) || + this.canBeDeleted(authProviderType) + ) + }, + }, +} diff --git a/enterprise/web-frontend/modules/baserow_enterprise/pages/login/loginError.vue b/enterprise/web-frontend/modules/baserow_enterprise/pages/login/loginError.vue index d7f0804a1..c059c30df 100644 --- a/enterprise/web-frontend/modules/baserow_enterprise/pages/login/loginError.vue +++ b/enterprise/web-frontend/modules/baserow_enterprise/pages/login/loginError.vue @@ -17,7 +17,7 @@ <ul class="auth__action-links"> <li> {{ $t('loginError.loginText') }} - <nuxt-link :to="{ name: 'login' }"> + <nuxt-link :to="{ name: 'login', query: { noredirect: null } }"> {{ $t('action.login') }} </nuxt-link> </li> diff --git a/enterprise/web-frontend/modules/baserow_enterprise/pages/login/loginWithSAML.vue b/enterprise/web-frontend/modules/baserow_enterprise/pages/login/loginWithSAML.vue index 06a89108e..fd3a6bab7 100644 --- a/enterprise/web-frontend/modules/baserow_enterprise/pages/login/loginWithSAML.vue +++ b/enterprise/web-frontend/modules/baserow_enterprise/pages/login/loginWithSAML.vue @@ -54,7 +54,7 @@ </div> <div> <ul class="auth__action-links"> - <li> + <li v-if="passwordLoginEnabled"> {{ $t('loginWithSaml.loginText') }} <nuxt-link :to="{ name: 'login' }"> {{ $t('action.login') }} @@ -72,6 +72,7 @@ </template> <script> +import { mapGetters } from 'vuex' import decamelize from 'decamelize' import { required, email } from 'vuelidate/lib/validators' import form from '@baserow/modules/core/mixins/form' @@ -131,6 +132,11 @@ export default { }, } }, + computed: { + ...mapGetters({ + passwordLoginEnabled: 'authProvider/getPasswordLoginEnabled', + }), + }, mounted() { if (this.redirectImmediately) { window.location.href = this.getRedirectUrlWithValidQueryParams( diff --git a/enterprise/web-frontend/modules/baserow_enterprise/plugin.js b/enterprise/web-frontend/modules/baserow_enterprise/plugin.js index dea2208ef..466a5d496 100644 --- a/enterprise/web-frontend/modules/baserow_enterprise/plugin.js +++ b/enterprise/web-frontend/modules/baserow_enterprise/plugin.js @@ -2,7 +2,9 @@ import { registerRealtimeEvents } from '@baserow_enterprise/realtime' import { RolePermissionManagerType } from '@baserow_enterprise/permissionManagerTypes' import { AuthProvidersType } from '@baserow_enterprise/adminTypes' import authProviderAdminStore from '@baserow_enterprise/store/authProviderAdmin' +import { PasswordAuthProviderType as CorePasswordAuthProviderType } from '@baserow/modules/core/authProviderTypes' import { + PasswordAuthProviderType, SamlAuthProviderType, GitHubAuthProviderType, GoogleAuthProviderType, @@ -50,6 +52,11 @@ export default (context) => { store.registerModule('authProviderAdmin', authProviderAdminStore) app.$registry.register('admin', new AuthProvidersType(context)) + app.$registry.unregister( + 'authProvider', + new CorePasswordAuthProviderType(context) + ) + app.$registry.register('authProvider', new PasswordAuthProviderType(context)) app.$registry.register('authProvider', new SamlAuthProviderType(context)) app.$registry.register('authProvider', new GoogleAuthProviderType(context)) app.$registry.register('authProvider', new FacebookAuthProviderType(context)) diff --git a/enterprise/web-frontend/modules/baserow_enterprise/store/authProviderAdmin.js b/enterprise/web-frontend/modules/baserow_enterprise/store/authProviderAdmin.js index 9d7b2ae1a..b370a6a86 100644 --- a/enterprise/web-frontend/modules/baserow_enterprise/store/authProviderAdmin.js +++ b/enterprise/web-frontend/modules/baserow_enterprise/store/authProviderAdmin.js @@ -1,4 +1,5 @@ import authProviderAdmin from '@baserow_enterprise/services/authProviderAdmin' +import { notifyIf } from '@baserow/modules/core/utils/error' function populateProviderType(authProviderType, registry) { const type = registry.get('authProvider', authProviderType.type) @@ -75,8 +76,12 @@ export const actions = { return item }, async delete({ commit }, item) { - await authProviderAdmin(this.$client).delete(item.id) - commit('DELETE_ITEM', item) + try { + await authProviderAdmin(this.$client).delete(item.id) + commit('DELETE_ITEM', item) + } catch (error) { + notifyIf(error, 'authProvider') + } }, async fetchNextProviderId({ commit }) { const { data } = await authProviderAdmin(this.$client).fetchNextProviderId() @@ -84,7 +89,7 @@ export const actions = { commit('SET_NEXT_PROVIDER_ID', providerId) return providerId }, - async setEnabled({ commit }, { authProvider, enabled }) { + async setEnabled({ commit, dispatch }, { authProvider, enabled }) { // use optimistic update to enable/disable the auth provider const wasEnabled = authProvider.enabled commit('UPDATE_ITEM', { ...authProvider, enabled }) @@ -92,6 +97,7 @@ export const actions = { await authProviderAdmin(this.$client).update(authProvider.id, { enabled }) } catch (error) { commit('UPDATE_ITEM', { ...authProvider, enabled: wasEnabled }) + notifyIf(error, 'authProvider') } }, } @@ -127,6 +133,17 @@ export const getters = { getType: (state) => (type) => { return state.items[type] }, + isOneProviderEnabled: (state) => { + let nEnabled = 0 + for (const authProviderType of Object.values(state.items)) { + for (const authProvider of authProviderType.authProviders) { + if (authProvider.enabled) { + nEnabled += 1 + } + } + } + return nEnabled === 1 + }, } export default { diff --git a/web-frontend/locales/en.json b/web-frontend/locales/en.json index 79e9e6814..f8593f1c6 100644 --- a/web-frontend/locales/en.json +++ b/web-frontend/locales/en.json @@ -248,7 +248,9 @@ "snapshotNameNotUniqueTitle": "The snapshot name has to be unique.", "snapshotNameNotUniqueDescription": "All snapshot names have to be unique per application.", "snapshotOperationLimitExceededTitle": "Limit reached", - "snapshotOperationLimitExceededDescription": "You have reached a limit on the number of running snapshot operations. Please wait until previous operation finishes." + "snapshotOperationLimitExceededDescription": "You have reached a limit on the number of running snapshot operations. Please wait until previous operation finishes.", + "disabledPasswordProviderTitle": "Password authentication is disabled.", + "disabledPasswordProviderMessage": "Please use another authentication provider." }, "importerType": { "csv": "Import a CSV file", diff --git a/web-frontend/modules/core/authProviderTypes.js b/web-frontend/modules/core/authProviderTypes.js index cc7457be3..4311dd6e8 100644 --- a/web-frontend/modules/core/authProviderTypes.js +++ b/web-frontend/modules/core/authProviderTypes.js @@ -76,6 +76,7 @@ export class AuthProviderType extends Registerable { order: this.getOrder(), hasAdminSettings: this.getAdminListComponent() !== null, canCreateNewProviders: authProviderType.can_create_new, + canDeleteExistingProviders: authProviderType.can_delete_existing, authProviders: authProviderType.auth_providers, } } @@ -123,14 +124,22 @@ export class PasswordAuthProviderType extends AuthProviderType { } getName() { - return 'Password' + return this.app.i18n.t('authProviderTypes.password') } getProviderName(provider) { + return this.getName() + } + + getAdminListComponent() { + return null + } + + getAdminSettingsFormComponent() { return null } getOrder() { - return 100 + return 1 } } diff --git a/web-frontend/modules/core/components/auth/Login.vue b/web-frontend/modules/core/components/auth/Login.vue index 8a585a929..842d30855 100644 --- a/web-frontend/modules/core/components/auth/Login.vue +++ b/web-frontend/modules/core/components/auth/Login.vue @@ -13,28 +13,43 @@ <LangPicker /> </div> </div> - <LoginButtons - show-border="bottom" - :hide-if-no-buttons="loginButtonsCompact" - :invitation="invitation" - :original="original" - /> - <PasswordLogin :invitation="invitation" @success="success"> </PasswordLogin> - <LoginActions :invitation="invitation" :original="original"> - <li v-if="settings.allow_reset_password"> - <nuxt-link :to="{ name: 'forgot-password' }"> - {{ $t('login.forgotPassword') }} - </nuxt-link> - </li> - <li v-if="settings.allow_new_signups"> - <slot name="signup"> - {{ $t('login.signUpText') }} - <nuxt-link :to="{ name: 'signup' }"> - {{ $t('login.signUp') }} + <div v-if="redirectByDefault && defaultRedirectUrl"> + {{ $t('login.redirecting') }} + </div> + <div v-else> + <LoginButtons + show-border="bottom" + :hide-if-no-buttons="loginButtonsCompact" + :invitation="invitation" + :original="original" + /> + <PasswordLogin + v-if="!passwordLoginHidden" + :invitation="invitation" + @success="success" + > + </PasswordLogin> + <LoginActions :invitation="invitation" :original="original"> + <li v-if="passwordLoginHidden"> + <a @click="passwordLoginHiddenIfDisabled = false"> + {{ $t('login.displayPasswordLogin') }} + </a> + </li> + <li v-if="settings.allow_reset_password && !passwordLoginHidden"> + <nuxt-link :to="{ name: 'forgot-password' }"> + {{ $t('login.forgotPassword') }} </nuxt-link> - </slot> - </li> - </LoginActions> + </li> + <li v-if="settings.allow_new_signups"> + <slot name="signup"> + {{ $t('login.signUpText') }} + <nuxt-link :to="{ name: 'signup' }"> + {{ $t('login.signUp') }} + </nuxt-link> + </slot> + </li> + </LoginActions> + </div> </div> </template> @@ -45,7 +60,10 @@ import LoginButtons from '@baserow/modules/core/components/auth/LoginButtons' import LoginActions from '@baserow/modules/core/components/auth/LoginActions' import PasswordLogin from '@baserow/modules/core/components/auth/PasswordLogin' import LangPicker from '@baserow/modules/core/components/LangPicker' -import { isRelativeUrl } from '@baserow/modules/core/utils/url' +import { + isRelativeUrl, + addQueryParamsToRedirectUrl, +} from '@baserow/modules/core/utils/url' export default { components: { PasswordLogin, LoginButtons, LangPicker, LoginActions }, @@ -75,12 +93,23 @@ export default { required: false, default: false, }, + redirectByDefault: { + type: Boolean, + required: false, + default: false, + }, + }, + data() { + return { + passwordLoginHiddenIfDisabled: true, + } }, computed: { ...mapGetters({ settings: 'settings/get', loginActions: 'authProvider/getAllLoginActions', loginButtons: 'authProvider/getAllLoginButtons', + passwordLoginEnabled: 'authProvider/getPasswordLoginEnabled', }), computedOriginal() { let original = this.original @@ -89,6 +118,24 @@ export default { } return original }, + passwordLoginHidden() { + return this.passwordLoginHiddenIfDisabled && !this.passwordLoginEnabled + }, + defaultRedirectUrl() { + return this.$store.getters['authProvider/getDefaultRedirectUrl'] + }, + }, + mounted() { + if (this.redirectByDefault) { + if (this.defaultRedirectUrl !== null) { + const { groupInvitationToken } = this.$route.query + const url = addQueryParamsToRedirectUrl(this.defaultRedirectUrl, { + original: this.computedOriginal, + groupInvitationToken, + }) + window.location = url + } + } }, methods: { success() { diff --git a/web-frontend/modules/core/components/auth/PasswordLogin.vue b/web-frontend/modules/core/components/auth/PasswordLogin.vue index c7f49d2d0..030a2a6b6 100644 --- a/web-frontend/modules/core/components/auth/PasswordLogin.vue +++ b/web-frontend/modules/core/components/auth/PasswordLogin.vue @@ -178,6 +178,13 @@ export default { this.$t('error.disabledAccountTitle'), this.$t('error.disabledAccountMessage') ) + } else if ( + response.data?.error === 'ERROR_AUTH_PROVIDER_DISABLED' + ) { + this.showError( + this.$t('clientHandler.disabledPasswordProviderTitle'), + this.$t('clientHandler.disabledPasswordProviderMessage') + ) } else { this.showError( this.$t('error.incorrectCredentialTitle'), diff --git a/web-frontend/modules/core/components/settings/SettingsModal.vue b/web-frontend/modules/core/components/settings/SettingsModal.vue index d5e369ff4..7d53af215 100644 --- a/web-frontend/modules/core/components/settings/SettingsModal.vue +++ b/web-frontend/modules/core/components/settings/SettingsModal.vue @@ -49,7 +49,9 @@ export default { }, computed: { registeredSettings() { - return this.$registry.getOrderedList('settings') + return this.$registry + .getOrderedList('settings') + .filter((settings) => settings.isEnabled() === true) }, settingPageComponent() { const active = Object.values(this.$registry.getAll('settings')).find( @@ -61,6 +63,9 @@ export default { name: 'auth/getName', }), }, + async mounted() { + await this.$store.dispatch('authProvider/fetchLoginOptions') + }, methods: { setPage(page) { this.page = page diff --git a/web-frontend/modules/core/locales/en.json b/web-frontend/modules/core/locales/en.json index a988fe006..6f657b0c2 100644 --- a/web-frontend/modules/core/locales/en.json +++ b/web-frontend/modules/core/locales/en.json @@ -87,7 +87,9 @@ "disabledAccountMessage": "This user account has been disabled.", "incorrectCredentialTitle": "Incorrect credentials", "incorrectCredentialMessage": "The provided e-mail address or password is incorrect.", - "inputRequired": "Input is required." + "inputRequired": "Input is required.", + "disabledPasswordProviderTitle": "Password authentication is disabled.", + "disabledPasswordProviderMessage": "Please use another authentication provider." }, "field": { "language": "Language", @@ -291,7 +293,9 @@ "passwordPlaceholder": "Enter your password..", "forgotPassword": "Forgot password?", "signUpText": "Don't have an account?", - "signUp": "Sign Up" + "signUp": "Sign Up", + "displayPasswordLogin": "Log in using email and password", + "redirecting": "Redirecting to authentication provider..." }, "resetPassword": { "title": "Reset password", diff --git a/web-frontend/modules/core/pages/login.vue b/web-frontend/modules/core/pages/login.vue index b61dbb2e3..3f08eaf97 100644 --- a/web-frontend/modules/core/pages/login.vue +++ b/web-frontend/modules/core/pages/login.vue @@ -4,6 +4,7 @@ :display-header="true" :redirect-on-success="true" :invitation="invitation" + :redirect-by-default="redirectByDefault" ></Login> </div> </template> @@ -35,5 +36,13 @@ export default { ], } }, + computed: { + redirectByDefault() { + if (this.$route.query.noredirect === null) { + return false + } + return true + }, + }, } </script> diff --git a/web-frontend/modules/core/pages/signup.vue b/web-frontend/modules/core/pages/signup.vue index fb5fe06ee..1323148f7 100644 --- a/web-frontend/modules/core/pages/signup.vue +++ b/web-frontend/modules/core/pages/signup.vue @@ -30,27 +30,24 @@ </template> <template v-else> <PasswordRegister - v-if="afterSignupStep < 0" + v-if="afterSignupStep < 0 && passwordLoginEnabled" :invitation="invitation" @success="next" > - <LoginButtons - show-border="top" - :hide-if-no-buttons="true" - :invitation="invitation" - /> - <LoginActions - v-if="!shouldShowAdminSignupPage" - :invitation="invitation" - > - <li> - {{ $t('signup.loginText') }} - <nuxt-link :to="{ name: 'login' }"> - {{ $t('action.login') }} - </nuxt-link> - </li> - </LoginActions> </PasswordRegister> + <LoginButtons + show-border="top" + :hide-if-no-buttons="true" + :invitation="invitation" + /> + <LoginActions v-if="!shouldShowAdminSignupPage" :invitation="invitation"> + <li> + {{ $t('signup.loginText') }} + <nuxt-link :to="{ name: 'login' }"> + {{ $t('action.login') }} + </nuxt-link> + </li> + </LoginActions> <component :is="afterSignupStepComponents[afterSignupStep]" v-else @@ -110,6 +107,7 @@ export default { ...mapGetters({ settings: 'settings/get', loginActions: 'authProvider/getAllLoginActions', + passwordLoginEnabled: 'authProvider/getPasswordLoginEnabled', }), }, methods: { diff --git a/web-frontend/modules/core/plugins/clientHandler.js b/web-frontend/modules/core/plugins/clientHandler.js index f0a8556a9..80eb3339f 100644 --- a/web-frontend/modules/core/plugins/clientHandler.js +++ b/web-frontend/modules/core/plugins/clientHandler.js @@ -118,6 +118,15 @@ export class ClientErrorMap { app.i18n.t('clientHandler.snapshotOperationLimitExceededTitle'), app.i18n.t('clientHandler.snapshotOperationLimitExceededDescription') ), + ERROR_AUTH_PROVIDER_DISABLED: new ResponseErrorMessage( + app.i18n.t('clientHandler.disabledPasswordProviderTitle'), + app.i18n.t('clientHandler.disabledPasswordProviderMessage') + ), + // TODO: Move to enterprise module if possible + ERROR_CANNOT_DISABLE_ALL_AUTH_PROVIDERS: new ResponseErrorMessage( + app.i18n.t('clientHandler.cannotDisableAllAuthProvidersTitle'), + app.i18n.t('clientHandler.cannotDisableAllAuthProvidersDescription') + ), } } diff --git a/web-frontend/modules/core/settingsTypes.js b/web-frontend/modules/core/settingsTypes.js index 4ff534ed1..0bab776c1 100644 --- a/web-frontend/modules/core/settingsTypes.js +++ b/web-frontend/modules/core/settingsTypes.js @@ -33,6 +33,10 @@ export class SettingsType extends Registerable { throw new Error('The component of a settings type must be set.') } + isEnabled() { + return true + } + constructor(...args) { super(...args) this.type = this.getType() @@ -98,6 +102,13 @@ export class PasswordSettingsType extends SettingsType { return i18n.t('settingType.password') } + isEnabled() { + return ( + this.app.store.getters['authProvider/getPasswordLoginEnabled'] || + this.app.store.getters['auth/isStaff'] + ) + } + getComponent() { return PasswordSettings } diff --git a/web-frontend/modules/core/store/authProvider.js b/web-frontend/modules/core/store/authProvider.js index 58ac3030a..8cb9344be 100644 --- a/web-frontend/modules/core/store/authProvider.js +++ b/web-frontend/modules/core/store/authProvider.js @@ -62,6 +62,22 @@ export const getters = { } return loginActions }, + getPasswordLoginEnabled: (state) => { + return state.loginOptions.password + }, + getDefaultRedirectUrl: (state) => { + const loginOptionsArr = Object.values(state.loginOptions) + const possibleRedirectLoginOptions = loginOptionsArr.filter( + (loginOption) => loginOption.default_redirect_url + ) + if ( + loginOptionsArr.length === 1 && + possibleRedirectLoginOptions.length === 1 + ) { + return possibleRedirectLoginOptions[0].default_redirect_url + } + return null + }, } export default { diff --git a/web-frontend/modules/core/utils/auth.js b/web-frontend/modules/core/utils/auth.js index e5d1a0ac4..c0ff83601 100644 --- a/web-frontend/modules/core/utils/auth.js +++ b/web-frontend/modules/core/utils/auth.js @@ -46,7 +46,7 @@ export const logoutAndRedirectToLogin = ( showSessionExpiredNotification = false ) => { store.dispatch('auth/logoff') - router.push({ name: 'login' }, () => { + router.push({ name: 'login', query: { noredirect: null } }, () => { if (showSessionExpiredNotification) { store.dispatch('notification/setUserSessionExpired', true) } diff --git a/web-frontend/modules/core/utils/url.js b/web-frontend/modules/core/utils/url.js index f8de29b22..cd1055825 100644 --- a/web-frontend/modules/core/utils/url.js +++ b/web-frontend/modules/core/utils/url.js @@ -2,3 +2,26 @@ export function isRelativeUrl(url) { const absoluteUrlRegExp = /^(?:[a-z+]+:)?\/\//i return !absoluteUrlRegExp.test(url) } + +export function addQueryParamsToRedirectUrl(url, params) { + const parsedUrl = new URL(url) + + for (const [key, value] of Object.entries(params)) { + if (['language'].includes(key)) { + parsedUrl.searchParams.append(key, value) + } + } + + if (params.original && isRelativeUrl(params.original)) { + parsedUrl.searchParams.append('original', params.original) + } + + if (params.groupInvitationToken) { + parsedUrl.searchParams.append( + 'group_invitation_token', + params.groupInvitationToken + ) + } + + return parsedUrl.toString() +} diff --git a/web-frontend/test/server/core/pages/server.spec.js b/web-frontend/test/server/core/pages/server.spec.js index 2bbbf85d9..8cccce769 100644 --- a/web-frontend/test/server/core/pages/server.spec.js +++ b/web-frontend/test/server/core/pages/server.spec.js @@ -28,9 +28,12 @@ describe('index redirect', () => { }, }) - mock - .onGet('http://localhost/api/auth-provider/login-options/') - .reply(200, {}) + mock.onGet('http://localhost/api/auth-provider/login-options/').reply(200, { + password: { + type: 'password', + enabled: true, + }, + }) nuxt = await createNuxt(true) done()