From 73e239b9f8f2acf46a6900a61b97dfd2f15301ac Mon Sep 17 00:00:00 2001 From: Abhijeet <41686026+abhvsn@users.noreply.github.com> Date: Wed, 26 Mar 2025 11:26:00 +0530 Subject: [PATCH] chore: Move signup_disabled and form_login_enabled from envs to DB (#39882) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description > [!TIP] > _Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team)._ > > _Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR._ /test Settings ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: 80445acc542f201c01a40a09323097a946959e50 > Cypress dashboard. > Tags: `@tag.Settings` > Spec: >
Tue, 25 Mar 2025 12:19:59 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **New Features** - Organizations now control form login and signup behavior with updated, clearer configuration flags. - **Refactor** - Renamed settings to reflect an enabled state for form login and a disabled state for signup. - Removed deprecated restart functionality and reorganized utility classes for improved maintainability. - **Chores/Tests** - Updated tests and environment examples to align with the new configuration settings and ensure consistent behavior. --- .../FormLogin/EnableFormLogin_spec.js | 6 +- app/client/cypress/locators/AdminsSettings.js | 4 +- .../src/ce/constants/organizationConstants.ts | 2 + .../AdminSettings/config/authentication.tsx | 4 +- app/client/src/ce/sagas/organizationSagas.tsx | 16 ++++++ .../ce/selectors/organizationSelectors.tsx | 3 + .../src/ce/utils/adminSettingsHelpers.ts | 3 +- .../src/pages/AdminSettings/SettingsForm.tsx | 37 +++++++++++- .../AuthenticationSuccessHandler.java | 2 +- .../ce/AuthenticationSuccessHandlerCE.java | 8 +-- .../server/configurations/CommonConfig.java | 9 --- .../server/constants/EnvVariables.java | 2 - .../appsmith/server/domains/Organization.java | 7 +-- .../ce/OrganizationConfigurationCE.java | 7 +-- .../server/exceptions/AppsmithError.java | 4 +- .../server/exceptions/AppsmithErrorCode.java | 2 +- .../constants/AllowedInstanceVariables.java | 3 + .../constants/AllowedInstanceVariablesCE.java | 7 +++ .../helpers/InstanceVariablesHelper.java | 7 +-- .../helpers}/InstanceVariablesHelperCE.java | 24 ++++---- ...inDisabledToOrganizationConfiguration.java | 51 +++++++++++++++++ ...upDisabledToOrganizationConfiguration.java | 52 +++++++++++++++++ .../ce/CustomOrganizationRepositoryCE.java | 1 - .../CustomOrganizationRepositoryCEImpl.java | 10 ---- .../services/OrganizationServiceImpl.java | 2 +- .../server/services/UserServiceImpl.java | 2 +- .../services/ce/OrganizationServiceCE.java | 2 - .../ce/OrganizationServiceCEImpl.java | 27 +-------- .../server/services/ce/UserServiceCEImpl.java | 56 ++++++++++--------- .../UserServiceCECompatibleImpl.java | 2 +- .../server/solutions/ce/EnvManagerCEImpl.java | 5 -- .../solutions/ce/ScheduledTaskCEImpl.java | 1 - .../helpers/ce/InstanceVariablesTest.java | 4 +- .../server/services/UserServiceTest.java | 2 +- .../UserServiceWithDisabledSignupTest.java | 21 ++++++- app/server/envs/dev.env.example | 3 - app/server/envs/docker.env.example | 2 - app/server/envs/test.env.example | 3 - 38 files changed, 260 insertions(+), 143 deletions(-) create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/instanceconfigs/constants/AllowedInstanceVariables.java create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/instanceconfigs/constants/AllowedInstanceVariablesCE.java rename app/server/appsmith-server/src/main/java/com/appsmith/server/{ => instanceconfigs}/helpers/InstanceVariablesHelper.java (64%) rename app/server/appsmith-server/src/main/java/com/appsmith/server/{helpers/ce => instanceconfigs/helpers}/InstanceVariablesHelperCE.java (76%) create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration071MoveFormLoginDisabledToOrganizationConfiguration.java create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration072MoveSignupDisabledToOrganizationConfiguration.java rename app/server/appsmith-server/src/test/java/com/appsmith/server/{ => instanceconfigs}/helpers/ce/InstanceVariablesTest.java (96%) diff --git a/app/client/cypress/e2e/Regression/ClientSide/FormLogin/EnableFormLogin_spec.js b/app/client/cypress/e2e/Regression/ClientSide/FormLogin/EnableFormLogin_spec.js index edca306ee7..7feadb22c0 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/FormLogin/EnableFormLogin_spec.js +++ b/app/client/cypress/e2e/Regression/ClientSide/FormLogin/EnableFormLogin_spec.js @@ -83,8 +83,8 @@ describe("Form Login test functionality", function () { cy.get(adminSettings.formloginButton).click(); cy.wait(2000); // disable form signup - cy.get(adminSettings.formLoginDisabled).should("have.value", "on"); - cy.get(adminSettings.formLoginDisabled).click({ force: true }); + cy.get(adminSettings.formLoginEnabled).should("have.value", "on"); + cy.get(adminSettings.formLoginEnabled).click({ force: true }); cy.wait(2000); // assert server is restarting cy.get(adminSettings.saveButton).should("be.visible"); @@ -109,7 +109,7 @@ describe("Form Login test functionality", function () { .should("be.visible") .should("contain", "Enable"); cy.get(adminSettings.formloginButton).click(); - cy.get(adminSettings.formLoginDisabled).click({ force: true }); + cy.get(adminSettings.formLoginEnabled).click({ force: true }); cy.wait(2000); // assert server is restarting cy.get(adminSettings.saveButton).should("be.visible"); diff --git a/app/client/cypress/locators/AdminsSettings.js b/app/client/cypress/locators/AdminsSettings.js index 6a03d6238b..1d4076a915 100644 --- a/app/client/cypress/locators/AdminsSettings.js +++ b/app/client/cypress/locators/AdminsSettings.js @@ -24,8 +24,8 @@ export default { loginWithGoogle: "[data-testid='login-with-Google']", loginWithGithub: "[data-testid='login-with-Github']", disconnectBtn: "[data-testid='disconnect-service-button']", - formSignupDisabled: "[data-testid='APPSMITH_SIGNUP_DISABLED']", - formLoginDisabled: "[data-testid='APPSMITH_FORM_LOGIN_DISABLED']", + formSignupDisabled: "[data-testid='isSignupDisabled']", + formLoginEnabled: "[data-testid='isFormLoginEnabled']", embedSettings: ".t--admin-settings-APPSMITH_ALLOWED_FRAME_ANCESTORS", upgrade: "[data-testid='t--button-upgrade']", accessControl: ".t--settings-category-access-control", diff --git a/app/client/src/ce/constants/organizationConstants.ts b/app/client/src/ce/constants/organizationConstants.ts index 0168039c00..c9a394d653 100644 --- a/app/client/src/ce/constants/organizationConstants.ts +++ b/app/client/src/ce/constants/organizationConstants.ts @@ -7,6 +7,8 @@ export const organizationConfigConnection: string[] = [ "hideWatermark", "userSessionTimeoutInMinutes", "isAtomicPushAllowed", + "isFormLoginEnabled", + "isSignupDisabled", ]; export const RESTART_POLL_TIMEOUT = 2 * 150 * 1000; diff --git a/app/client/src/ce/pages/AdminSettings/config/authentication.tsx b/app/client/src/ce/pages/AdminSettings/config/authentication.tsx index 8cb3b1544c..b7fbb2a3f6 100644 --- a/app/client/src/ce/pages/AdminSettings/config/authentication.tsx +++ b/app/client/src/ce/pages/AdminSettings/config/authentication.tsx @@ -48,13 +48,13 @@ const FormAuth: AdminConfigType = { canSave: true, settings: [ { - id: "APPSMITH_FORM_LOGIN_DISABLED", + id: "isFormLoginEnabled", category: SettingCategories.FORM_AUTH, controlType: SettingTypes.TOGGLE, label: "form login", }, { - id: "APPSMITH_SIGNUP_DISABLED", + id: "isSignupDisabled", category: SettingCategories.FORM_AUTH, controlType: SettingTypes.TOGGLE, label: "Form signup", diff --git a/app/client/src/ce/sagas/organizationSagas.tsx b/app/client/src/ce/sagas/organizationSagas.tsx index b1d5547bd1..a70ff9c5b4 100644 --- a/app/client/src/ce/sagas/organizationSagas.tsx +++ b/app/client/src/ce/sagas/organizationSagas.tsx @@ -69,6 +69,10 @@ export function* updateOrganizationConfigSaga( "emailVerificationEnabled", ); + const hasFormLoginSetting = settings.hasOwnProperty("isFormLoginEnabled"); + const hasSignupDisabledSetting = + settings.hasOwnProperty("isSignupDisabled"); + const response: ApiResponse = yield call( OrganizationApi.updateOrganizationConfig, action.payload, @@ -94,6 +98,18 @@ export function* updateOrganizationConfigSaga( }); } + if (hasFormLoginSetting) { + AnalyticsUtil.logEvent("GENERAL_SETTINGS_UPDATE", { + enabled: settings["isFormLoginEnabled"], + }); + } + + if (hasSignupDisabledSetting) { + AnalyticsUtil.logEvent("GENERAL_SETTINGS_UPDATE", { + enabled: settings["isSignupDisabled"], + }); + } + if (hasEmailVerificationSetting) { AnalyticsUtil.logEvent("EMAIL_VERIFICATION_SETTING_UPDATE", { enabled: settings["emailVerificationEnabled"], diff --git a/app/client/src/ce/selectors/organizationSelectors.tsx b/app/client/src/ce/selectors/organizationSelectors.tsx index 0011addfa3..15f96c7a17 100644 --- a/app/client/src/ce/selectors/organizationSelectors.tsx +++ b/app/client/src/ce/selectors/organizationSelectors.tsx @@ -48,6 +48,9 @@ export const getThirdPartyAuths = (state: AppState): string[] => export const getIsFormLoginEnabled = (state: AppState): boolean => state.organization?.organizationConfiguration?.isFormLoginEnabled ?? true; +export const getIsSignupDisabled = (state: AppState): boolean => + state.organization?.organizationConfiguration?.isSignupDisabled ?? false; + export const getInstanceId = (state: AppState): string => state.organization?.instanceId; diff --git a/app/client/src/ce/utils/adminSettingsHelpers.ts b/app/client/src/ce/utils/adminSettingsHelpers.ts index 56ea44af04..e0268e0857 100644 --- a/app/client/src/ce/utils/adminSettingsHelpers.ts +++ b/app/client/src/ce/utils/adminSettingsHelpers.ts @@ -18,8 +18,7 @@ export const saveAllowed = ( socialLoginList.length + (isFormLoginEnabled ? 1 : 0); if (connectedMethodsCount === 1) { - const checkFormLogin = - !("APPSMITH_FORM_LOGIN_DISABLED" in settings) && isFormLoginEnabled, + const checkFormLogin = isFormLoginEnabled, checkGoogleAuth = settings["APPSMITH_OAUTH2_GOOGLE_CLIENT_ID"] !== "" && socialLoginList.includes("google"), diff --git a/app/client/src/pages/AdminSettings/SettingsForm.tsx b/app/client/src/pages/AdminSettings/SettingsForm.tsx index 0fc1d3c5df..bec37d5745 100644 --- a/app/client/src/pages/AdminSettings/SettingsForm.tsx +++ b/app/client/src/pages/AdminSettings/SettingsForm.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useEffect, useMemo } from "react"; +import React, { useCallback, useEffect, useMemo, useState } from "react"; import { saveSettings } from "ee/actions/settingsAction"; import { SETTINGS_FORM_NAME } from "ee/constants/forms"; import { ReduxActionTypes } from "ee/constants/ReduxActionConstants"; @@ -89,6 +89,8 @@ export function SettingsForm( ); const isFormLoginEnabled = useSelector(getIsFormLoginEnabled); const socialLoginList = useSelector(getThirdPartyAuths); + const [initialFormLoginEnabled, setInitialFormLoginEnabled] = + useState(isFormLoginEnabled); const updatedOrganizationSettings = useMemo( () => Object.keys(props.settings).filter((s) => isOrganizationConfig(s)), @@ -103,6 +105,11 @@ export function SettingsForm( !isOrganizationConfig(s.id), ); + // Store the initial value of isFormLoginEnabled on component mount + useEffect(() => { + setInitialFormLoginEnabled(isFormLoginEnabled); + }, [category, subCategory]); + const saveChangedSettings = () => { const settingsKeyLength = Object.keys(props.settings).length; const isOnlyEnvSettings = @@ -151,7 +158,23 @@ export function SettingsForm( const onSave = () => { if (checkMandatoryFileds()) { - if (saveAllowed(props.settings, isFormLoginEnabled, socialLoginList)) { + // Use initialFormLoginEnabled instead of the current state value for the check + let effectiveFormLoginEnabled = initialFormLoginEnabled; + + // Check if isFormLoginEnabled is being updated in the current form + if (props.settings["isFormLoginEnabled"] !== undefined) { + // Convert to boolean if it's a string, otherwise use as is since we expect a boolean + const settingValue = props.settings["isFormLoginEnabled"]; + + effectiveFormLoginEnabled = + typeof settingValue === "string" + ? settingValue === "true" + : Boolean(settingValue); + } + + if ( + saveAllowed(props.settings, effectiveFormLoginEnabled, socialLoginList) + ) { AnalyticsUtil.logEvent("ADMIN_SETTINGS_SAVE", { method: pageTitle, }); @@ -224,6 +247,13 @@ export function SettingsForm( useEffect(onClear, [subCategory]); + useEffect(() => { + // Reset the initialFormLoginEnabled value when navigating to a different category + return () => { + setInitialFormLoginEnabled(isFormLoginEnabled); + }; + }, [category, subCategory]); + const onReleaseNotesClose = useCallback(() => { dispatch({ type: ReduxActionTypes.TOGGLE_RELEASE_NOTES, @@ -244,8 +274,9 @@ export function SettingsForm( // TODO: Fix this the next time the file is edited // eslint-disable-next-line @typescript-eslint/no-explicit-any const updatedSettings: any = {}; + // Use the initial value to determine if there are enough login methods const connectedMethodsCount = - socialLoginList.length + (isFormLoginEnabled ? 1 : 0); + socialLoginList.length + (initialFormLoginEnabled ? 1 : 0); if (connectedMethodsCount >= 2) { _.forEach(currentSettings, (setting: Setting) => { diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationSuccessHandler.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationSuccessHandler.java index e6917f1bad..55e4a8b040 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationSuccessHandler.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationSuccessHandler.java @@ -1,9 +1,9 @@ package com.appsmith.server.authentication.handlers; import com.appsmith.server.authentication.handlers.ce.AuthenticationSuccessHandlerCE; -import com.appsmith.server.helpers.InstanceVariablesHelper; import com.appsmith.server.helpers.RedirectHelper; import com.appsmith.server.helpers.WorkspaceServiceHelper; +import com.appsmith.server.instanceconfigs.helpers.InstanceVariablesHelper; import com.appsmith.server.ratelimiting.RateLimitService; import com.appsmith.server.repositories.UserRepository; import com.appsmith.server.repositories.WorkspaceRepository; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationSuccessHandlerCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationSuccessHandlerCE.java index 75534eec8c..78076db7ab 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationSuccessHandlerCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationSuccessHandlerCE.java @@ -10,9 +10,9 @@ import com.appsmith.server.domains.LoginSource; import com.appsmith.server.domains.User; import com.appsmith.server.domains.Workspace; import com.appsmith.server.dtos.ResendEmailVerificationDTO; -import com.appsmith.server.helpers.InstanceVariablesHelper; import com.appsmith.server.helpers.RedirectHelper; import com.appsmith.server.helpers.WorkspaceServiceHelper; +import com.appsmith.server.instanceconfigs.helpers.InstanceVariablesHelper; import com.appsmith.server.ratelimiting.RateLimitService; import com.appsmith.server.repositories.UserRepository; import com.appsmith.server.repositories.WorkspaceRepository; @@ -103,13 +103,13 @@ public class AuthenticationSuccessHandlerCE implements ServerAuthenticationSucce return Mono.just(FALSE); } else { return emailVerificationEnabledMono.flatMap(emailVerificationEnabled -> { - // email verification not enabled at the org + // email verification not enabled if (!TRUE.equals(emailVerificationEnabled)) { user.setEmailVerificationRequired(FALSE); return userRepository.save(user).then(Mono.just(FALSE)); } else { - // scenario when at the time of signup, the email verification was disabled at the org - // but later on turned on, now when this user logs in, it will not be prompted to verify + // scenario when at the time of signup, the email verification was disabled but later on + // turned on, now when this user logs in, it will not be prompted to verify // as the configuration at time of signup is considered for any user. // for old users, the login works as expected, without the need to verify if (!TRUE.equals(user.getEmailVerificationRequired())) { diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/CommonConfig.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/CommonConfig.java index c2f2de0b21..81c7a8d485 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/CommonConfig.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/CommonConfig.java @@ -36,9 +36,6 @@ public class CommonConfig { public static final Integer LATEST_INSTANCE_SCHEMA_VERSION = 2; - @Setter(AccessLevel.NONE) - private boolean isSignupDisabled = false; - @Setter(AccessLevel.NONE) private Set adminEmails = Collections.emptySet(); @@ -129,12 +126,6 @@ public class CommonConfig { adminEmails = Set.of(value.trim().split("\\s*,\\s*")); } - @Autowired - public void setSignupDisabled(@Value("${signup.disabled}") String value) { - // If `true`, then disable signup. If anything else, including empty string, then signups will be enabled. - isSignupDisabled = "true".equalsIgnoreCase(value); - } - public Long getCurrentTimeInstantEpochMilli() { return Instant.now().toEpochMilli(); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/EnvVariables.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/EnvVariables.java index 0ae80cad93..e4eb59ba6d 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/EnvVariables.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/EnvVariables.java @@ -13,7 +13,6 @@ public enum EnvVariables { APPSMITH_MAIL_USERNAME, APPSMITH_MAIL_PASSWORD, APPSMITH_MAIL_SMTP_TLS_ENABLED, - APPSMITH_SIGNUP_DISABLED, APPSMITH_SIGNUP_ALLOWED_DOMAINS, APPSMITH_ADMIN_EMAILS, APPSMITH_RECAPTCHA_SITE_KEY, @@ -24,7 +23,6 @@ public enum EnvVariables { APPSMITH_OAUTH2_GITHUB_CLIENT_ID, APPSMITH_OAUTH2_GITHUB_CLIENT_SECRET, APPSMITH_CUSTOM_DOMAIN, - APPSMITH_FORM_LOGIN_DISABLED, APPSMITH_ALLOWED_FRAME_ANCESTORS, APPSMITH_DISABLE_IFRAME_WIDGET_SANDBOX, APPSMITH_NEW_RELIC_ACCOUNT_ENABLE, diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Organization.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Organization.java index ecccaecd40..003689c8de 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Organization.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Organization.java @@ -12,8 +12,6 @@ import org.springframework.data.mongodb.core.mapping.Document; import java.io.Serializable; -import static com.appsmith.external.helpers.StringUtils.dotted; - @Getter @Setter @ToString @@ -38,8 +36,5 @@ public class Organization extends BaseDomain implements Serializable { // TODO add SSO and other configurations here after migrating from environment variables to database configuration - public static class Fields extends BaseDomain.Fields { - public static final String organizationConfiguration_isRestartRequired = - dotted(organizationConfiguration, OrganizationConfiguration.Fields.isRestartRequired); - } + public static class Fields extends BaseDomain.Fields {} } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/OrganizationConfigurationCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/OrganizationConfigurationCE.java index afd543933c..153447260a 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/OrganizationConfigurationCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/OrganizationConfigurationCE.java @@ -27,6 +27,8 @@ public class OrganizationConfigurationCE implements Serializable { private Boolean isFormLoginEnabled; + private Boolean isSignupDisabled; + @Transient @Deprecated(forRemoval = true, since = "v1.65") private String instanceName; @@ -58,10 +60,6 @@ public class OrganizationConfigurationCE implements Serializable { // 2. Because of grandfathering via cron where organization level feature flags are fetched Map featuresWithPendingMigration; - // This variable is used to indicate if the server needs to be restarted after the migration based on feature flags - // is complete. - Boolean isRestartRequired; - Boolean isStrongPasswordPolicyEnabled; private Boolean isAtomicPushAllowed = false; @@ -84,6 +82,7 @@ public class OrganizationConfigurationCE implements Serializable { googleMapsKey = ObjectUtils.defaultIfNull(organizationConfiguration.getGoogleMapsKey(), googleMapsKey); isFormLoginEnabled = ObjectUtils.defaultIfNull(organizationConfiguration.getIsFormLoginEnabled(), isFormLoginEnabled); + isSignupDisabled = ObjectUtils.defaultIfNull(organizationConfiguration.getIsSignupDisabled(), isSignupDisabled); instanceName = ObjectUtils.defaultIfNull(organizationConfiguration.getInstanceName(), instanceName); emailVerificationEnabled = ObjectUtils.defaultIfNull( organizationConfiguration.getEmailVerificationEnabled(), emailVerificationEnabled); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java index 49df751a00..5eccc61ec1 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java @@ -912,9 +912,9 @@ public enum AppsmithError { ErrorType.INTERNAL_ERROR, null), - ORGANIZATION_EMAIL_VERIFICATION_NOT_ENABLED( + EMAIL_VERIFICATION_NOT_ENABLED( 400, - AppsmithErrorCode.ORGANIZATION_EMAIL_VERIFICATION_NOT_ENABLED.getCode(), + AppsmithErrorCode.EMAIL_VERIFICATION_NOT_ENABLED.getCode(), "Email verification is not enabled. Please contact your admin", AppsmithErrorAction.DEFAULT, "Email verification not enabled", diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithErrorCode.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithErrorCode.java index 44b154674c..667438a61f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithErrorCode.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithErrorCode.java @@ -123,7 +123,7 @@ public enum AppsmithErrorCode { USER_EMAIL_ALREADY_VERIFIED("AE-EMV-4095", "User email already verified"), EMAIL_VERIFICATION_TOKEN_EXPIRED("AE-EMV-4096", "Email Verification Token expired"), - ORGANIZATION_EMAIL_VERIFICATION_NOT_ENABLED("AE-EMV-4097", "Email Verification not enabled"), + EMAIL_VERIFICATION_NOT_ENABLED("AE-EMV-4097", "Email Verification not enabled"), INVALID_EMAIL_VERIFICATION("AE-EMV-4098", "Invalid email verification request"), diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/instanceconfigs/constants/AllowedInstanceVariables.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/instanceconfigs/constants/AllowedInstanceVariables.java new file mode 100644 index 0000000000..b01597218a --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/instanceconfigs/constants/AllowedInstanceVariables.java @@ -0,0 +1,3 @@ +package com.appsmith.server.instanceconfigs.constants; + +public interface AllowedInstanceVariables extends AllowedInstanceVariablesCE {} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/instanceconfigs/constants/AllowedInstanceVariablesCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/instanceconfigs/constants/AllowedInstanceVariablesCE.java new file mode 100644 index 0000000000..17d6a44084 --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/instanceconfigs/constants/AllowedInstanceVariablesCE.java @@ -0,0 +1,7 @@ +package com.appsmith.server.instanceconfigs.constants; + +public interface AllowedInstanceVariablesCE { + String INSTANCE_NAME = "instanceName"; + String EMAIL_VERIFICATION_ENABLED = "emailVerificationEnabled"; + String GOOGLE_MAPS_KEY = "googleMapsKey"; +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/InstanceVariablesHelper.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/instanceconfigs/helpers/InstanceVariablesHelper.java similarity index 64% rename from app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/InstanceVariablesHelper.java rename to app/server/appsmith-server/src/main/java/com/appsmith/server/instanceconfigs/helpers/InstanceVariablesHelper.java index 8a01c6adc0..4afc9da561 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/InstanceVariablesHelper.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/instanceconfigs/helpers/InstanceVariablesHelper.java @@ -1,17 +1,12 @@ -package com.appsmith.server.helpers; +package com.appsmith.server.instanceconfigs.helpers; -import com.appsmith.server.helpers.ce.InstanceVariablesHelperCE; import com.appsmith.server.services.ConfigService; import lombok.extern.slf4j.Slf4j; import org.springframework.stereotype.Component; -/** - * Helper class for accessing instance variables from the instance config - */ @Component @Slf4j public class InstanceVariablesHelper extends InstanceVariablesHelperCE { - public InstanceVariablesHelper(ConfigService configService) { super(configService); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/InstanceVariablesHelperCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/instanceconfigs/helpers/InstanceVariablesHelperCE.java similarity index 76% rename from app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/InstanceVariablesHelperCE.java rename to app/server/appsmith-server/src/main/java/com/appsmith/server/instanceconfigs/helpers/InstanceVariablesHelperCE.java index 5ecd1bcf22..0b8355b110 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/InstanceVariablesHelperCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/instanceconfigs/helpers/InstanceVariablesHelperCE.java @@ -1,4 +1,4 @@ -package com.appsmith.server.helpers.ce; +package com.appsmith.server.instanceconfigs.helpers; import com.appsmith.server.constants.Appsmith; import com.appsmith.server.domains.OrganizationConfiguration; @@ -10,6 +10,10 @@ import reactor.core.publisher.Mono; import java.util.HashMap; import java.util.Map; +import static com.appsmith.server.instanceconfigs.constants.AllowedInstanceVariables.EMAIL_VERIFICATION_ENABLED; +import static com.appsmith.server.instanceconfigs.constants.AllowedInstanceVariables.GOOGLE_MAPS_KEY; +import static com.appsmith.server.instanceconfigs.constants.AllowedInstanceVariables.INSTANCE_NAME; + /** * Helper class for accessing instance variables from the instance config */ @@ -23,7 +27,7 @@ public class InstanceVariablesHelperCE { */ public Mono getInstanceName() { return configService.getInstanceVariables().map(instanceVariables -> { - Object value = instanceVariables.getOrDefault("instanceName", Appsmith.DEFAULT_INSTANCE_NAME); + Object value = instanceVariables.getOrDefault(INSTANCE_NAME, Appsmith.DEFAULT_INSTANCE_NAME); return value != null ? value.toString() : Appsmith.DEFAULT_INSTANCE_NAME; }); } @@ -34,7 +38,7 @@ public class InstanceVariablesHelperCE { */ public Mono isEmailVerificationEnabled() { return configService.getInstanceVariables().map(instanceVariables -> { - Object value = instanceVariables.getOrDefault("emailVerificationEnabled", false); + Object value = instanceVariables.getOrDefault(EMAIL_VERIFICATION_ENABLED, false); if (value instanceof Boolean) { return (Boolean) value; } @@ -48,24 +52,24 @@ public class InstanceVariablesHelperCE { */ public Mono getGoogleMapsKey() { return configService.getInstanceVariables().map(instanceVariables -> { - Object value = instanceVariables.getOrDefault("googleMapsKey", ""); + Object value = instanceVariables.getOrDefault(GOOGLE_MAPS_KEY, ""); return value != null ? value.toString() : ""; }); } public OrganizationConfiguration populateOrgConfigWithInstanceVariables( Map instanceVariables, OrganizationConfiguration organizationConfiguration) { - Object value = instanceVariables.getOrDefault("instanceName", Appsmith.DEFAULT_INSTANCE_NAME); + Object value = instanceVariables.getOrDefault(INSTANCE_NAME, Appsmith.DEFAULT_INSTANCE_NAME); organizationConfiguration.setInstanceName(value != null ? value.toString() : Appsmith.DEFAULT_INSTANCE_NAME); - value = instanceVariables.getOrDefault("emailVerificationEnabled", false); + value = instanceVariables.getOrDefault(EMAIL_VERIFICATION_ENABLED, false); if (value instanceof Boolean) { organizationConfiguration.setEmailVerificationEnabled((Boolean) value); } else { organizationConfiguration.setEmailVerificationEnabled(Boolean.FALSE); } - value = instanceVariables.getOrDefault("googleMapsKey", ""); + value = instanceVariables.getOrDefault(GOOGLE_MAPS_KEY, ""); organizationConfiguration.setGoogleMapsKey(value != null ? value.toString() : ""); return organizationConfiguration; @@ -85,13 +89,13 @@ public class InstanceVariablesHelperCE { protected Map updateAllowedInstanceVariables(OrganizationConfiguration orgConfig) { Map instanceVariables = new HashMap<>(); if (StringUtils.hasLength(orgConfig.getInstanceName())) { - instanceVariables.put("instanceName", orgConfig.getInstanceName()); + instanceVariables.put(INSTANCE_NAME, orgConfig.getInstanceName()); } if (orgConfig.getEmailVerificationEnabled() != null) { - instanceVariables.put("emailVerificationEnabled", orgConfig.getEmailVerificationEnabled()); + instanceVariables.put(EMAIL_VERIFICATION_ENABLED, orgConfig.getEmailVerificationEnabled()); } if (StringUtils.hasLength(orgConfig.getGoogleMapsKey())) { - instanceVariables.put("googleMapsKey", orgConfig.getGoogleMapsKey()); + instanceVariables.put(GOOGLE_MAPS_KEY, orgConfig.getGoogleMapsKey()); } return instanceVariables; } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration071MoveFormLoginDisabledToOrganizationConfiguration.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration071MoveFormLoginDisabledToOrganizationConfiguration.java new file mode 100644 index 0000000000..8374aeafa9 --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration071MoveFormLoginDisabledToOrganizationConfiguration.java @@ -0,0 +1,51 @@ +package com.appsmith.server.migrations.db.ce; + +import com.appsmith.server.configurations.CommonConfig; +import com.appsmith.server.domains.Organization; +import io.mongock.api.annotations.ChangeUnit; +import io.mongock.api.annotations.Execution; +import io.mongock.api.annotations.RollbackExecution; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.StringUtils; +import org.springframework.data.mongodb.core.MongoTemplate; +import org.springframework.data.mongodb.core.query.Query; +import org.springframework.data.mongodb.core.query.Update; + +import java.io.IOException; + +import static com.appsmith.server.migrations.db.ce.Migration021MoveGoogleMapsKeyToTenantConfiguration.commentEnvInFile; + +@Slf4j +@RequiredArgsConstructor +@ChangeUnit(order = "071", id = "move-form-login-disabled-to-organization-configuration") +public class Migration071MoveFormLoginDisabledToOrganizationConfiguration { + private final MongoTemplate mongoTemplate; + private final CommonConfig commonConfig; + + @RollbackExecution + public void rollbackExecution() {} + + @Execution + public void executeMigration() throws IOException { + final String envName = "APPSMITH_FORM_LOGIN_DISABLED"; + final String formLoginDisabledValue = System.getenv(envName); + + // Default to false if the environment variable is not present or empty + boolean formLoginDisabled = false; + + if (StringUtils.isNotEmpty(formLoginDisabledValue)) { + // Convert the string value to a boolean if env is present + formLoginDisabled = Boolean.parseBoolean(formLoginDisabledValue); + // Comment out the environment variable in the .env file + commentEnvInFile(envName, commonConfig.getEnvFilePath()); + } + + // Update all organizations to set the form login disabled configuration + // This happens whether the env var is present or not, defaulting to false + mongoTemplate.updateMulti( + new Query(), + new Update().set("organizationConfiguration.isFormLoginEnabled", !formLoginDisabled), + Organization.class); + } +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration072MoveSignupDisabledToOrganizationConfiguration.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration072MoveSignupDisabledToOrganizationConfiguration.java new file mode 100644 index 0000000000..9332ed5d2e --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration072MoveSignupDisabledToOrganizationConfiguration.java @@ -0,0 +1,52 @@ +package com.appsmith.server.migrations.db.ce; + +import com.appsmith.server.configurations.CommonConfig; +import com.appsmith.server.domains.Organization; +import io.mongock.api.annotations.ChangeUnit; +import io.mongock.api.annotations.Execution; +import io.mongock.api.annotations.RollbackExecution; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.StringUtils; +import org.springframework.data.mongodb.core.MongoTemplate; +import org.springframework.data.mongodb.core.query.Query; +import org.springframework.data.mongodb.core.query.Update; + +import java.io.IOException; + +import static com.appsmith.server.migrations.db.ce.Migration021MoveGoogleMapsKeyToTenantConfiguration.commentEnvInFile; + +@Slf4j +@RequiredArgsConstructor +@ChangeUnit(order = "072", id = "move-signup-disabled-to-organization-configuration") +public class Migration072MoveSignupDisabledToOrganizationConfiguration { + private final MongoTemplate mongoTemplate; + private final CommonConfig commonConfig; + + @RollbackExecution + public void rollbackExecution() {} + + @Execution + public void executeMigration() throws IOException { + final String envName = "APPSMITH_SIGNUP_DISABLED"; + final String signupDisabledValue = System.getenv(envName); + + // Default to false if the environment variable is not present or empty + boolean signupDisabled = false; + + if (StringUtils.isNotEmpty(signupDisabledValue)) { + // Convert the string value to a boolean if env is present + signupDisabled = Boolean.parseBoolean(signupDisabledValue); + + // Comment out the environment variable in the .env file + commentEnvInFile(envName, commonConfig.getEnvFilePath()); + } + + // Update all organizations to set the signup disabled configuration + // This happens whether the env var is present or not, defaulting to false + mongoTemplate.updateMulti( + new Query(), + new Update().set("organizationConfiguration.isSignupDisabled", signupDisabled), + Organization.class); + } +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomOrganizationRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomOrganizationRepositoryCE.java index 7fe1a4d45c..327c2ddfff 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomOrganizationRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomOrganizationRepositoryCE.java @@ -7,7 +7,6 @@ import com.appsmith.server.repositories.AppsmithRepository; import reactor.core.publisher.Mono; public interface CustomOrganizationRepositoryCE extends AppsmithRepository { - Mono disableRestartForAllOrganizations(); Mono findByIdAsUser(User user, String id, AclPermission permission); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomOrganizationRepositoryCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomOrganizationRepositoryCEImpl.java index e5a42590cb..102fd0b232 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomOrganizationRepositoryCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomOrganizationRepositoryCEImpl.java @@ -9,20 +9,10 @@ import com.appsmith.server.repositories.BaseAppsmithRepositoryImpl; import lombok.extern.slf4j.Slf4j; import reactor.core.publisher.Mono; -import static com.appsmith.server.domains.Organization.Fields.organizationConfiguration_isRestartRequired; - @Slf4j public class CustomOrganizationRepositoryCEImpl extends BaseAppsmithRepositoryImpl implements CustomOrganizationRepositoryCE { - @Override - public Mono disableRestartForAllOrganizations() { - log.info("Disabling restart for all organizations"); - return queryBuilder() - .criteria(Bridge.isTrue(organizationConfiguration_isRestartRequired)) - .updateAll(Bridge.update().set(organizationConfiguration_isRestartRequired, false)); - } - @Override public Mono findByIdAsUser(User user, String id, AclPermission permission) { return getAllPermissionGroupsForUser(user).flatMap(permissionGroups -> queryBuilder() diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/OrganizationServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/OrganizationServiceImpl.java index 59ec6e1cfd..2d0294e884 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/OrganizationServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/OrganizationServiceImpl.java @@ -2,8 +2,8 @@ package com.appsmith.server.services; import com.appsmith.server.configurations.CommonConfig; import com.appsmith.server.helpers.FeatureFlagMigrationHelper; -import com.appsmith.server.helpers.InstanceVariablesHelper; import com.appsmith.server.helpers.UserOrganizationHelper; +import com.appsmith.server.instanceconfigs.helpers.InstanceVariablesHelper; import com.appsmith.server.repositories.CacheableRepositoryHelper; import com.appsmith.server.repositories.OrganizationRepository; import com.appsmith.server.services.ce.OrganizationServiceCEImpl; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java index d11e3972ee..75d59671c0 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java @@ -2,9 +2,9 @@ package com.appsmith.server.services; import com.appsmith.server.configurations.CommonConfig; import com.appsmith.server.configurations.EmailConfig; -import com.appsmith.server.helpers.InstanceVariablesHelper; import com.appsmith.server.helpers.UserServiceHelper; import com.appsmith.server.helpers.UserUtils; +import com.appsmith.server.instanceconfigs.helpers.InstanceVariablesHelper; import com.appsmith.server.notifications.EmailSender; import com.appsmith.server.ratelimiting.RateLimitService; import com.appsmith.server.repositories.ApplicationRepository; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/OrganizationServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/OrganizationServiceCE.java index ed8c5090a0..6945ede783 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/OrganizationServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/OrganizationServiceCE.java @@ -30,7 +30,5 @@ public interface OrganizationServiceCE extends CrudService Mono retrieveById(String id); - Mono restartOrganization(); - Flux retrieveAll(); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/OrganizationServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/OrganizationServiceCEImpl.java index 53ca8bf934..1c85ddc95c 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/OrganizationServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/OrganizationServiceCEImpl.java @@ -14,8 +14,8 @@ import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.helpers.CollectionUtils; import com.appsmith.server.helpers.FeatureFlagMigrationHelper; -import com.appsmith.server.helpers.InstanceVariablesHelper; import com.appsmith.server.helpers.UserOrganizationHelper; +import com.appsmith.server.instanceconfigs.helpers.InstanceVariablesHelper; import com.appsmith.server.repositories.CacheableRepositoryHelper; import com.appsmith.server.repositories.OrganizationRepository; import com.appsmith.server.services.AnalyticsService; @@ -175,8 +175,6 @@ public class OrganizationServiceCEImpl extends BaseService instanceVariablesHelper.populateOrgConfigWithInstanceVariables( @@ -342,29 +340,6 @@ public class OrganizationServiceCEImpl extends BaseService - */ - @Override - public Mono restartOrganization() { - // TODO @CloudBilling: remove this method once we move the form login env to DB variable which is currently - // required as a part of downgrade migration for SSO - return this.retrieveAll() - .filter(organization -> - TRUE.equals(organization.getOrganizationConfiguration().getIsRestartRequired())) - .take(1) - .hasElements() - .flatMap(hasElement -> { - if (hasElement) { - return repository.disableRestartForAllOrganizations().then(envManager.restartWithoutAclCheck()); - } - return Mono.empty(); - }); - } - private boolean isMigrationRequired(Organization organization) { return organization.getOrganizationConfiguration() != null && (!CollectionUtils.isNullOrEmpty( diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java index fc03b0e94d..8b6d1277e4 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.java @@ -22,9 +22,9 @@ import com.appsmith.server.dtos.UserSignupDTO; import com.appsmith.server.dtos.UserUpdateDTO; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; -import com.appsmith.server.helpers.InstanceVariablesHelper; import com.appsmith.server.helpers.UserServiceHelper; import com.appsmith.server.helpers.UserUtils; +import com.appsmith.server.instanceconfigs.helpers.InstanceVariablesHelper; import com.appsmith.server.ratelimiting.RateLimitService; import com.appsmith.server.repositories.EmailVerificationTokenRepository; import com.appsmith.server.repositories.PasswordResetTokenRepository; @@ -557,37 +557,42 @@ public class UserServiceCEImpl extends BaseService */ @Override public Mono signupIfAllowed(User user) { - boolean isAdminUser = false; + Mono isAdminUserMono; if (!commonConfig.getAdminEmails().contains(user.getEmail())) { // If this is not an admin email address, only then do we check if signup should be allowed or not. Being an // explicitly set admin email address trumps all everything and signup for this email can never be disabled. - - if (commonConfig.isSignupDisabled()) { - // Signing up has been globally disabled. Reject. - return Mono.error(new AppsmithException(AppsmithError.SIGNUP_DISABLED, user.getUsername())); - } - - final List allowedDomains = user.getSource() == LoginSource.FORM - ? commonConfig.getAllowedDomains() - : commonConfig.getOauthAllowedDomains(); - if (!CollectionUtils.isEmpty(allowedDomains) - && StringUtils.hasText(user.getEmail()) - && user.getEmail().contains("@") - && !allowedDomains.contains(user.getEmail().split("@")[1])) { - // There is an explicit whitelist of email address domains that should be allowed. If the new email is - // of a different domain, reject. - return Mono.error(new AppsmithException(AppsmithError.SIGNUP_DISABLED, user.getUsername())); - } + isAdminUserMono = organizationService.getCurrentUserOrganization().map(organization -> { + OrganizationConfiguration organizationConfiguration = + organization.getOrganizationConfiguration() == null + ? new OrganizationConfiguration() + : organization.getOrganizationConfiguration(); + if (TRUE.equals(organizationConfiguration.getIsSignupDisabled())) { + // Signing up has been globally disabled. Reject. + throw new AppsmithException(AppsmithError.SIGNUP_DISABLED, user.getUsername()); + } + final List allowedDomains = user.getSource() == LoginSource.FORM + ? commonConfig.getAllowedDomains() + : commonConfig.getOauthAllowedDomains(); + if (!CollectionUtils.isEmpty(allowedDomains) + && StringUtils.hasText(user.getEmail()) + && user.getEmail().contains("@") + && !allowedDomains.contains(user.getEmail().split("@")[1])) { + // There is an explicit whitelist of email address domains that should be allowed. If the new email + // is + // of a different domain, reject. + throw new AppsmithException(AppsmithError.SIGNUP_DISABLED, user.getUsername()); + } + return FALSE; + }); } else { - isAdminUser = true; + isAdminUserMono = Mono.just(true); } - // First set the organization ID for the user - boolean finalIsAdminUser = isAdminUser; + // No special configurations found, allow signup for the new user. return setOrganizationIdForUser(user) - // Then proceed with user creation - .flatMap(userWithOrgId -> userCreate(userWithOrgId, finalIsAdminUser)) + .zipWhen(userWithOrgId -> isAdminUserMono) + .flatMap(tuple2 -> userCreate(tuple2.getT1(), tuple2.getT2())) .elapsed() .map(pair -> { log.debug("UserServiceCEImpl::Time taken for create user: {} ms", pair.getT1()); @@ -826,8 +831,7 @@ public class UserServiceCEImpl extends BaseService return instanceVariablesHelper.isEmailVerificationEnabled().flatMap(emailVerificationEnabled -> { // Email verification not enabled at instance level if (!TRUE.equals(emailVerificationEnabled)) { - return Mono.error( - new AppsmithException(AppsmithError.ORGANIZATION_EMAIL_VERIFICATION_NOT_ENABLED)); + return Mono.error(new AppsmithException(AppsmithError.EMAIL_VERIFICATION_NOT_ENABLED)); } return emailVerificationTokenRepository .findByEmail(user.getEmail()) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce_compatible/UserServiceCECompatibleImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce_compatible/UserServiceCECompatibleImpl.java index d6199b1a11..d8b838acaf 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce_compatible/UserServiceCECompatibleImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce_compatible/UserServiceCECompatibleImpl.java @@ -1,9 +1,9 @@ package com.appsmith.server.services.ce_compatible; import com.appsmith.server.configurations.CommonConfig; -import com.appsmith.server.helpers.InstanceVariablesHelper; import com.appsmith.server.helpers.UserServiceHelper; import com.appsmith.server.helpers.UserUtils; +import com.appsmith.server.instanceconfigs.helpers.InstanceVariablesHelper; import com.appsmith.server.ratelimiting.RateLimitService; import com.appsmith.server.repositories.EmailVerificationTokenRepository; import com.appsmith.server.repositories.PasswordResetTokenRepository; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java index 7c47496a64..9af6b6f33e 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java @@ -83,7 +83,6 @@ import static com.appsmith.server.constants.EnvVariables.APPSMITH_RECAPTCHA_SECR import static com.appsmith.server.constants.EnvVariables.APPSMITH_RECAPTCHA_SITE_KEY; import static com.appsmith.server.constants.EnvVariables.APPSMITH_REPLY_TO; import static com.appsmith.server.constants.EnvVariables.APPSMITH_SIGNUP_ALLOWED_DOMAINS; -import static com.appsmith.server.constants.EnvVariables.APPSMITH_SIGNUP_DISABLED; import static java.lang.Boolean.TRUE; @Slf4j @@ -371,10 +370,6 @@ public class EnvManagerCEImpl implements EnvManagerCE { // Try and update any at runtime, that can be. final Map changesCopy = new HashMap<>(changes); - if (changesCopy.containsKey(APPSMITH_SIGNUP_DISABLED.name())) { - commonConfig.setSignupDisabled(changesCopy.remove(APPSMITH_SIGNUP_DISABLED.name())); - } - if (changesCopy.containsKey(APPSMITH_SIGNUP_ALLOWED_DOMAINS.name())) { commonConfig.setAllowedDomainsString( changesCopy.remove(APPSMITH_SIGNUP_ALLOWED_DOMAINS.name())); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ScheduledTaskCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ScheduledTaskCEImpl.java index dbe29623fc..8276720efe 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ScheduledTaskCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ScheduledTaskCEImpl.java @@ -39,7 +39,6 @@ public class ScheduledTaskCEImpl implements ScheduledTaskCE { log.error("Error while fetching organization feature flags", error); return Flux.empty(); }) - .then(organizationService.restartOrganization()) .subscribeOn(LoadShifter.elasticScheduler) .subscribe(); } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ce/InstanceVariablesTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/instanceconfigs/helpers/ce/InstanceVariablesTest.java similarity index 96% rename from app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ce/InstanceVariablesTest.java rename to app/server/appsmith-server/src/test/java/com/appsmith/server/instanceconfigs/helpers/ce/InstanceVariablesTest.java index b0f0e17699..0e7ef8345f 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ce/InstanceVariablesTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/instanceconfigs/helpers/ce/InstanceVariablesTest.java @@ -1,7 +1,7 @@ -package com.appsmith.server.helpers.ce; +package com.appsmith.server.instanceconfigs.helpers.ce; import com.appsmith.server.constants.Appsmith; -import com.appsmith.server.helpers.InstanceVariablesHelper; +import com.appsmith.server.instanceconfigs.helpers.InstanceVariablesHelper; import com.appsmith.server.services.ConfigService; import net.minidev.json.JSONObject; import org.junit.jupiter.api.BeforeEach; diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java index 15534dbfc1..9eca74051f 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java @@ -693,7 +693,7 @@ public class UserServiceTest { Mono resultMono = userMono.then(organizationMono).then(emailVerificationMono); StepVerifier.create(resultMono) - .expectErrorMessage(AppsmithError.ORGANIZATION_EMAIL_VERIFICATION_NOT_ENABLED.getMessage()) + .expectErrorMessage(AppsmithError.EMAIL_VERIFICATION_NOT_ENABLED.getMessage()) .verify(); } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceWithDisabledSignupTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceWithDisabledSignupTest.java index 1953520cd1..b90e4619b9 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceWithDisabledSignupTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceWithDisabledSignupTest.java @@ -4,6 +4,8 @@ import com.appsmith.server.applications.base.ApplicationService; import com.appsmith.server.configurations.CommonConfig; import com.appsmith.server.configurations.WithMockAppsmithUser; import com.appsmith.server.domains.LoginSource; +import com.appsmith.server.domains.Organization; +import com.appsmith.server.domains.OrganizationConfiguration; import com.appsmith.server.domains.User; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; @@ -12,6 +14,7 @@ import com.appsmith.server.repositories.PermissionGroupRepository; import com.appsmith.server.repositories.UserRepository; import com.appsmith.server.repositories.WorkspaceRepository; import lombok.extern.slf4j.Slf4j; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -54,16 +57,32 @@ public class UserServiceWithDisabledSignupTest { @SpyBean CommonConfig commonConfig; + @Autowired + OrganizationService organizationService; + Mono userMono; @BeforeEach public void setup() { userMono = userService.findByEmail("usertest@usertest.com"); - Mockito.when(commonConfig.isSignupDisabled()).thenReturn(Boolean.TRUE); + Organization organization = + organizationService.getCurrentUserOrganization().block(); + assert organization != null; + organization.getOrganizationConfiguration().setIsSignupDisabled(true); + organizationService.save(organization).block(); Mockito.when(commonConfig.getAdminEmails()) .thenReturn(Set.of("dummy_admin@appsmith.com", "dummy2@appsmith.com")); } + @AfterAll + public static void cleanup(@Autowired OrganizationService organizationService) { + OrganizationConfiguration organizationConfiguration = new OrganizationConfiguration(); + organizationConfiguration.setIsSignupDisabled(false); + Organization organization = + organizationService.getCurrentUserOrganization().block(); + organizationService.save(organization).block(); + } + @Test @WithMockAppsmithUser public void createNewUserValidWhenDisabled() { diff --git a/app/server/envs/dev.env.example b/app/server/envs/dev.env.example index bcec41238b..63e7277f20 100644 --- a/app/server/envs/dev.env.example +++ b/app/server/envs/dev.env.example @@ -20,9 +20,6 @@ APPSMITH_CLOUD_SERVICES_BASE_URL="https://release-cs.appsmith.com" #APPSMITH_OAUTH2_GITHUB_CLIENT_ID="" #APPSMITH_OAUTH2_GITHUB_CLIENT_SECRET="" -#APPSMITH_FORM_LOGIN_DISABLED= -#APPSMITH_SIGNUP_DISABLED= - #APPSMITH_SENTRY_DSN= #APPSMITH_SENTRY_ENVIRONMENT= diff --git a/app/server/envs/docker.env.example b/app/server/envs/docker.env.example index e9d2238d9b..9c1550251b 100644 --- a/app/server/envs/docker.env.example +++ b/app/server/envs/docker.env.example @@ -4,8 +4,6 @@ # APPSMITH_OAUTH2_GOOGLE_CLIENT_SECRET="" # APPSMITH_OAUTH2_GITHUB_CLIENT_ID="" # APPSMITH_OAUTH2_GITHUB_CLIENT_SECRET="" -# APPSMITH_FORM_LOGIN_DISABLED= -# APPSMITH_SIGNUP_DISABLED= # APPSMITH_MAIL_ENABLED=true # APPSMITH_MAIL_HOST=localhost diff --git a/app/server/envs/test.env.example b/app/server/envs/test.env.example index 1069f4993f..444adcfee9 100644 --- a/app/server/envs/test.env.example +++ b/app/server/envs/test.env.example @@ -22,9 +22,6 @@ APPSMITH_CLOUD_SERVICES_BASE_URL="https://release-cs.appsmith.com" #APPSMITH_OAUTH2_GITHUB_CLIENT_ID="" #APPSMITH_OAUTH2_GITHUB_CLIENT_SECRET="" -#APPSMITH_FORM_LOGIN_DISABLED= -#APPSMITH_SIGNUP_DISABLED= - #APPSMITH_SENTRY_DSN= #APPSMITH_SENTRY_ENVIRONMENT=