From ea214c254fdf7c064931c4217c2ad7711c468ee7 Mon Sep 17 00:00:00 2001 From: Pawan Kumar Date: Wed, 23 Apr 2025 15:16:08 +0530 Subject: [PATCH 1/4] chore: new agent creation flow (#40351) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /ok-to-test tags="@tag.Templates, @tag.Sanity" > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: 6e4c5af948c633376f5219d95e5763309f9929a0 > Cypress dashboard. > Tags: `@tag.Templates, @tag.Sanity` > Spec: >
Wed, 23 Apr 2025 08:23:24 UTC ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Introduced conditional UI and messaging support for an AI agent flow across modals, buttons, and onboarding screens. - Added agent-specific text and button variants for datasource connection and navigation. - Adjusted styles and layouts in editors and modals to accommodate the AI agent flow. - Added controls to close the "Create Agent" modal post template import when AI agent flow is enabled. - **Bug Fixes** - Improved template filtering to exclude the "AI Agent" template where appropriate. - **Enhancements** - Updated template and modal components to support additional conditional rendering and styling based on the AI agent flow. - Enhanced template import and navigation logic to handle agent-specific flows and URLs. --- app/client/src/ce/constants/messages.ts | 4 + .../Editor/gitSync/useReconnectModalData.ts | 17 ++++- app/client/src/ce/sagas/ApplicationSagas.tsx | 14 +++- app/client/src/ee/actions/aiAgentActions.ts | 6 ++ .../pages/Editor/DataSourceEditor/index.tsx | 12 ++- .../Editor/gitSync/ImportSuccessModal.tsx | 5 +- .../gitSync/ReconnectDatasourceModal.tsx | 46 +++++++++--- .../src/pages/Templates/Template/index.tsx | 74 ++++++++++--------- .../pages/Templates/TemplateContent/index.tsx | 10 ++- .../src/pages/common/datasourceAuth/index.tsx | 10 ++- app/client/src/sagas/TemplatesSagas.ts | 11 +++ .../src/selectors/templatesSelectors.tsx | 22 ++---- 12 files changed, 158 insertions(+), 73 deletions(-) diff --git a/app/client/src/ce/constants/messages.ts b/app/client/src/ce/constants/messages.ts index 2bc1cf1bfb..f52663c740 100644 --- a/app/client/src/ce/constants/messages.ts +++ b/app/client/src/ce/constants/messages.ts @@ -704,6 +704,8 @@ export const RECONNECT_MISSING_DATASOURCE_CREDENTIALS = () => "Reconnect missing datasource credentials"; export const RECONNECT_MISSING_DATASOURCE_CREDENTIALS_DESCRIPTION = () => "Fill these with utmost care as the application will not behave normally otherwise"; +export const RECONNECT_MISSING_DATASOURCE_CREDENTIALS_DESCRIPTION_FOR_AGENTS = + () => "Ensure your agent is ready by integrating the required datasources."; export const RECONNECT_DATASOURCE_SUCCESS_MESSAGE1 = () => "These datasources were imported successfully!"; export const RECONNECT_DATASOURCE_SUCCESS_MESSAGE2 = () => @@ -714,6 +716,7 @@ export const SKIP_TO_APPLICATION_TOOLTIP_HEADER = () => export const SKIP_TO_APPLICATION_TOOLTIP_DESCRIPTION = () => `Skip this step to configure datasources later`; export const SKIP_TO_APPLICATION = () => "Go to application"; +export const SKIP_TO_APPLICATION_FOR_AGENTS = () => "Go to agent"; export const SKIP_CONFIGURATION = () => "Skip configuration"; export const SELECT_A_METHOD_TO_ADD_CREDENTIALS = () => "Select a method to add credentials"; @@ -2038,6 +2041,7 @@ export const SAVE_BUTTON_TEXT = () => "Save"; export const TEST_BUTTON_TEXT = () => "Test configuration"; export const SAVE_AND_AUTHORIZE_BUTTON_TEXT = () => "Save & Authorize"; export const CONNECT_DATASOURCE_BUTTON_TEXT = () => "Connect Datasource"; +export const CONNECT_DATASOURCE_BUTTON_TEXT_FOR_AGENTS = () => "Connect"; export const SAVE_AND_RE_AUTHORIZE_BUTTON_TEXT = () => "Save & Re-Authorize"; export const DISCARD_POPUP_DONT_SAVE_BUTTON_TEXT = () => "Don't save"; export const GSHEET_AUTHORISED_FILE_IDS_KEY = () => "userAuthorizedSheetIds"; diff --git a/app/client/src/ce/pages/Editor/gitSync/useReconnectModalData.ts b/app/client/src/ce/pages/Editor/gitSync/useReconnectModalData.ts index f2be964e6d..f96b03a4dd 100644 --- a/app/client/src/ce/pages/Editor/gitSync/useReconnectModalData.ts +++ b/app/client/src/ce/pages/Editor/gitSync/useReconnectModalData.ts @@ -2,9 +2,12 @@ import { IDE_TYPE } from "ee/IDE/Interfaces/IDETypes"; import { builderURL } from "ee/RouteBuilder"; import { RECONNECT_MISSING_DATASOURCE_CREDENTIALS_DESCRIPTION, + RECONNECT_MISSING_DATASOURCE_CREDENTIALS_DESCRIPTION_FOR_AGENTS, SKIP_TO_APPLICATION, + SKIP_TO_APPLICATION_FOR_AGENTS, createMessage, } from "ee/constants/messages"; +import { getIsAiAgentFlowEnabled } from "ee/selectors/aiAgentSelectors"; import { getApplicationByIdFromWorkspaces } from "ee/selectors/applicationSelectors"; import { useSelector } from "react-redux"; @@ -14,6 +17,7 @@ interface UseReconnectModalDataProps { } function useReconnectModalData({ appId, pageId }: UseReconnectModalDataProps) { + const isAiAgentFlowEnabled = useSelector(getIsAiAgentFlowEnabled); const application = useSelector((state) => getApplicationByIdFromWorkspaces(state, appId ?? ""), ); @@ -26,12 +30,21 @@ function useReconnectModalData({ appId, pageId }: UseReconnectModalDataProps) { builderURL({ basePageId, branch, + params: { + type: isAiAgentFlowEnabled ? "agent" : undefined, + }, }); return { - skipMessage: createMessage(SKIP_TO_APPLICATION), + skipMessage: createMessage( + isAiAgentFlowEnabled + ? SKIP_TO_APPLICATION_FOR_AGENTS + : SKIP_TO_APPLICATION, + ), missingDsCredentialsDescription: createMessage( - RECONNECT_MISSING_DATASOURCE_CREDENTIALS_DESCRIPTION, + isAiAgentFlowEnabled + ? RECONNECT_MISSING_DATASOURCE_CREDENTIALS_DESCRIPTION_FOR_AGENTS + : RECONNECT_MISSING_DATASOURCE_CREDENTIALS_DESCRIPTION, ), editorURL, editorId: appId, diff --git a/app/client/src/ce/sagas/ApplicationSagas.tsx b/app/client/src/ce/sagas/ApplicationSagas.tsx index 64fbba0b3c..1602813ab3 100644 --- a/app/client/src/ce/sagas/ApplicationSagas.tsx +++ b/app/client/src/ce/sagas/ApplicationSagas.tsx @@ -25,7 +25,7 @@ import type { UploadNavigationLogoRequest, } from "ee/api/ApplicationApi"; import ApplicationApi from "ee/api/ApplicationApi"; -import { all, call, put, select, take } from "redux-saga/effects"; +import { all, call, fork, put, select, take } from "redux-saga/effects"; import { validateResponse } from "sagas/ErrorSagas"; import { @@ -121,6 +121,7 @@ import type { Page } from "entities/Page"; import type { ApplicationPayload } from "entities/Application"; import { objectKeys } from "@appsmith/utils"; import { findDefaultPage } from "pages/utils"; +import { getIsAiAgentFlowEnabled } from "ee/selectors/aiAgentSelectors"; export let windowReference: Window | null = null; @@ -1004,6 +1005,7 @@ export function* initDatasourceConnectionDuringImport( }>, ) { const workspaceId = action.payload.workspaceId; + const isAgentFlowEnabled: boolean = yield select(getIsAiAgentFlowEnabled); const pluginsAndDatasourcesCalls: boolean = yield failFastApiCalls( [fetchPlugins({ workspaceId }), fetchDatasources({ workspaceId })], @@ -1032,9 +1034,13 @@ export function* initDatasourceConnectionDuringImport( }); yield all( - datasources.map((datasource: Datasource) => - call(initializeDatasourceWithDefaultValues, datasource), - ), + datasources.map((datasource: Datasource) => { + if (isAgentFlowEnabled) { + return fork(initializeDatasourceWithDefaultValues, datasource); + } + + return call(initializeDatasourceWithDefaultValues, datasource); + }), ); if (!action.payload.isPartialImport) { diff --git a/app/client/src/ee/actions/aiAgentActions.ts b/app/client/src/ee/actions/aiAgentActions.ts index 4f82e6cb73..802b88e5e5 100644 --- a/app/client/src/ee/actions/aiAgentActions.ts +++ b/app/client/src/ee/actions/aiAgentActions.ts @@ -1,3 +1,9 @@ import { noop } from "lodash"; export const toggleAISupportModal = noop; + +// just a placeholder action to avoid type errors +export const setCreateAgentModalOpen = ({ isOpen }: { isOpen: boolean }) => ({ + type: "", + payload: { isOpen }, +}); diff --git a/app/client/src/pages/Editor/DataSourceEditor/index.tsx b/app/client/src/pages/Editor/DataSourceEditor/index.tsx index d3c531ced9..1c9565f62d 100644 --- a/app/client/src/pages/Editor/DataSourceEditor/index.tsx +++ b/app/client/src/pages/Editor/DataSourceEditor/index.tsx @@ -103,6 +103,7 @@ import DatasourceTabs from "../DatasourceInfo/DatasorceTabs"; import DatasourceInformation, { ViewModeWrapper } from "./DatasourceSection"; import { convertToPageIdSelector } from "selectors/pageListSelectors"; import { getApplicationByIdFromWorkspaces } from "ee/selectors/applicationSelectors"; +import { getIsAiAgentFlowEnabled } from "ee/selectors/aiAgentSelectors"; interface ReduxStateProps { canDeleteDatasource: boolean; @@ -143,6 +144,7 @@ interface ReduxStateProps { featureFlags?: FeatureFlags; isPluginAllowedToPreviewData: boolean; isOnboardingFlow?: boolean; + isAiAgentFlowEnabled?: boolean; } const Form = styled.div` @@ -160,8 +162,11 @@ type Props = ReduxStateProps & basePageId: string; }>; -export const DSEditorWrapper = styled.div` - height: calc(100vh - ${(props) => props.theme.headerHeight}); +export const DSEditorWrapper = styled.div<{ isAiAgentFlowEnabled?: boolean }>` + height: ${(props) => + props.isAiAgentFlowEnabled + ? `auto` + : `calc(100vh - ${props.theme.headerHeight})`}; overflow: hidden; display: flex; flex-direction: row; @@ -1022,6 +1027,7 @@ class DatasourceEditorRouter extends React.Component { > {viewMode && !isInsideReconnectModal ? ( this.renderTabsForViewMode() @@ -1155,6 +1161,7 @@ const mapStateToProps = (state: AppState, props: any): ReduxStateProps => { const featureFlags = selectFeatureFlags(state); const isFeatureEnabled = isGACEnabled(featureFlags); + const isAiAgentFlowEnabled = getIsAiAgentFlowEnabled(state); const canManageDatasource = getHasManageDatasourcePermission( isFeatureEnabled, @@ -1223,6 +1230,7 @@ const mapStateToProps = (state: AppState, props: any): ReduxStateProps => { defaultKeyValueArrayConfig, initialValue, showDebugger, + isAiAgentFlowEnabled, }; }; diff --git a/app/client/src/pages/Editor/gitSync/ImportSuccessModal.tsx b/app/client/src/pages/Editor/gitSync/ImportSuccessModal.tsx index bea5a7726b..9a535f3952 100644 --- a/app/client/src/pages/Editor/gitSync/ImportSuccessModal.tsx +++ b/app/client/src/pages/Editor/gitSync/ImportSuccessModal.tsx @@ -15,6 +15,8 @@ import { ModalHeader, Text, } from "@appsmith/ads"; +import { useSelector } from "react-redux"; +import { getIsAiAgentFlowEnabled } from "ee/selectors/aiAgentSelectors"; const BodyContainer = styled.div` display: flex; @@ -40,6 +42,7 @@ function ImportSuccessModal(props: ImportSuccessModalProps) { const importedAppSuccess = localStorage.getItem("importSuccess"); // const isOpen = importedAppSuccess === "true"; const [isOpen, setIsOpen] = useState(importedAppSuccess === "true"); + const isAgentFlowEnabled = useSelector(getIsAiAgentFlowEnabled); const onClose = (open: boolean) => { if (!open) { @@ -53,7 +56,7 @@ function ImportSuccessModal(props: ImportSuccessModalProps) { }; return ( - + Datasource configured diff --git a/app/client/src/pages/Editor/gitSync/ReconnectDatasourceModal.tsx b/app/client/src/pages/Editor/gitSync/ReconnectDatasourceModal.tsx index 065c1473e3..6977460818 100644 --- a/app/client/src/pages/Editor/gitSync/ReconnectDatasourceModal.tsx +++ b/app/client/src/pages/Editor/gitSync/ReconnectDatasourceModal.tsx @@ -76,6 +76,7 @@ import useReconnectModalData from "ee/pages/Editor/gitSync/useReconnectModalData import { resetImportData } from "ee/actions/workspaceActions"; import { getLoadingTokenForDatasourceId } from "selectors/datasourceSelectors"; import ReconnectDatasourceForm from "Datasource/components/ReconnectDatasourceForm"; +import { getIsAiAgentFlowEnabled } from "ee/selectors/aiAgentSelectors"; const Section = styled.div` display: flex; @@ -86,10 +87,10 @@ const Section = styled.div` width: calc(100% - 206px); `; -const BodyContainer = styled.div` +const BodyContainer = styled.div<{ isAiAgentFlowEnabled?: boolean }>` flex: 3; - height: 640px; - max-height: 82vh; + height: ${(props) => (props.isAiAgentFlowEnabled ? "auto" : "640px")}; + max-height: ${(props) => (props.isAiAgentFlowEnabled ? "auto" : "82vh")}; `; // TODO: Removed usage of "t--" classes since they clash with the test classes. @@ -190,6 +191,8 @@ const Message = styled.div` const DBFormWrapper = styled.div` width: calc(100% - 206px); + min-width: 400px; + min-height: 360px; overflow: auto; display: flex; overflow: hidden; @@ -215,8 +218,17 @@ const DBFormWrapper = styled.div` } `; -const ModalContentWrapper = styled(ModalContent)` - width: 100%; +const ModalHeaderWrapper = styled.div<{ isAgentFlowEnabled: boolean }>` + & > div { + padding-bottom: ${(props) => + props.isAgentFlowEnabled ? "0px" : undefined}; + } +`; + +const ModalContentWrapper = styled(ModalContent)<{ + isAiAgentFlowEnabled?: boolean; +}>` + width: ${(props) => (props.isAiAgentFlowEnabled ? "auto" : "100%")}; `; const ModalBodyWrapper = styled(ModalBody)` overflow-y: hidden; @@ -330,6 +342,7 @@ function ReconnectDatasourceModal() { parentEntityId, // appId or packageId from query params skipMessage, } = useReconnectModalData({ pageId, appId }); + const isAgentFlowEnabled = useSelector(getIsAiAgentFlowEnabled); // when redirecting from oauth, processing the status if (isImport) { @@ -614,17 +627,28 @@ function ReconnectDatasourceModal() { - Reconnect datasources - - - - {createMessage(RECONNECT_MISSING_DATASOURCE_CREDENTIALS)} - + + + {isAgentFlowEnabled + ? "Connect Datasources" + : "Reconnect datasources"} + + + + + {!isAgentFlowEnabled && ( + + {createMessage(RECONNECT_MISSING_DATASOURCE_CREDENTIALS)} + + )} {missingDsCredentialsDescription} diff --git a/app/client/src/pages/Templates/Template/index.tsx b/app/client/src/pages/Templates/Template/index.tsx index 87127360bd..cb950ce627 100644 --- a/app/client/src/pages/Templates/Template/index.tsx +++ b/app/client/src/pages/Templates/Template/index.tsx @@ -75,11 +75,13 @@ const TemplateDatasources = styled.div` `; export interface TemplateProps { - hideForkTemplateButton: boolean; + hideForkTemplateButton?: boolean; template: TemplateInterface; size?: string; onClick?: (id: string) => void; onForkTemplateClick?: (template: TemplateInterface) => void; + hideFooter?: boolean; + hideDescription?: boolean; } const Template = (props: TemplateProps) => { @@ -152,42 +154,46 @@ export function TemplateLayout(props: TemplateLayoutProps) { {functions.join(" • ")} - - {description} - + {!props.hideDescription && ( + + {description} + + )} - - - {datasources.map((pluginPackageName) => { - return ( - + + {datasources.map((pluginPackageName) => { + return ( + + ); + })} + + {!props.hideForkTemplateButton && ( + + ), }[buttonType]; diff --git a/app/client/src/sagas/TemplatesSagas.ts b/app/client/src/sagas/TemplatesSagas.ts index 687d4eccfc..584a12f4ac 100644 --- a/app/client/src/sagas/TemplatesSagas.ts +++ b/app/client/src/sagas/TemplatesSagas.ts @@ -46,6 +46,8 @@ import { import { validateResponse } from "./ErrorSagas"; import { failFastApiCalls } from "./InitSagas"; import { getAllPageIdentities } from "./selectors"; +import { setCreateAgentModalOpen } from "ee/actions/aiAgentActions"; +import { getIsAiAgentFlowEnabled } from "ee/selectors/aiAgentSelectors"; const isAirgappedInstance = isAirgapped(); const AI_DATASOURCE_NAME = "AI Datasource"; @@ -76,6 +78,8 @@ function* getAllTemplatesSaga() { function* importTemplateToWorkspaceSaga( action: ReduxAction<{ templateId: string; workspaceId: string }>, ) { + const isAiAgentFlowEnabled: boolean = yield select(getIsAiAgentFlowEnabled); + try { const response: ImportTemplateResponse = yield call( TemplatesAPI.importTemplate, @@ -97,6 +101,10 @@ function* importTemplateToWorkspaceSaga( payload: response.data.application, }); + if (isAiAgentFlowEnabled) { + yield put(setCreateAgentModalOpen({ isOpen: false })); + } + // Temporary fix to remove AI Datasource from the unConfiguredDatasourceList // so we can avoid showing the AI Datasource in reconnect datasource modal const filteredUnConfiguredDatasourceList = ( @@ -117,6 +125,9 @@ function* importTemplateToWorkspaceSaga( } else { const pageURL = builderURL({ basePageId: application.defaultBasePageId, + params: { + type: isAiAgentFlowEnabled ? "agent" : undefined, + }, }); history.push(pageURL); diff --git a/app/client/src/selectors/templatesSelectors.tsx b/app/client/src/selectors/templatesSelectors.tsx index af7ee7146c..82dbb2a7d3 100644 --- a/app/client/src/selectors/templatesSelectors.tsx +++ b/app/client/src/selectors/templatesSelectors.tsx @@ -26,7 +26,6 @@ const fuzzySearchOptions = { }; const AGENT_TEMPLATES_USE_CASE = "Agent"; -const AGENT_TEMPLATES_TITLE = "AI Agent"; export const getTemplatesSelector = (state: AppState) => state.ui.templates.templates; @@ -36,22 +35,15 @@ export const getTemplatesByFlagSelector = createSelector( getIsAiAgentFlowEnabled, (templates, isAiAgentFlowEnabled) => { // For agents, we only show the templates that have the use case "Agent". - // The "Agent" use case acts as a filter for use to just show the templates + // The "Agent" use case acts as a filter for us to just show the templates // that are relevant to agents. - return ( - templates - .filter((template) => { - if (isAiAgentFlowEnabled) { - return template.useCases.includes(AGENT_TEMPLATES_USE_CASE); - } + return templates.filter((template) => { + if (isAiAgentFlowEnabled) { + return template.useCases.includes(AGENT_TEMPLATES_USE_CASE); + } - return true; - }) - // We are using AI Agent template for creating ai agent app, - // so we are not showing it in the templates list. - // TODO: Once we have a new entity for ai agent, we need to remove this filter. - .filter((template) => template.title !== AGENT_TEMPLATES_TITLE) - ); + return template.useCases.includes(AGENT_TEMPLATES_USE_CASE) === false; + }); }, ); From 68354422eb3b17d3395ccff5d2dee2526750e2e5 Mon Sep 17 00:00:00 2001 From: Aman Agarwal Date: Wed, 23 Apr 2025 15:23:10 +0530 Subject: [PATCH 2/4] fix: icon ds missing when plugin not present (#40327) --- .../components/PluginActionNameEditor.tsx | 4 +-- .../src/ce/selectors/entitiesSelector.ts | 26 +++++++++++++------ .../ActionCreator/helpers.tsx | 9 +++++-- .../ErrorLogs/components/LogEntityLink.tsx | 5 ++-- .../ActionSelectorControl.tsx | 3 +++ .../Editor/DataSidePane/DataSidePane.tsx | 11 +++----- .../gitSync/components/DatasourceListItem.tsx | 6 ++++- app/client/src/pages/Editor/utils.tsx | 13 +--------- app/client/src/sagas/selectors.tsx | 12 --------- 9 files changed, 42 insertions(+), 47 deletions(-) diff --git a/app/client/src/PluginActionEditor/components/PluginActionNameEditor.tsx b/app/client/src/PluginActionEditor/components/PluginActionNameEditor.tsx index c34b4a14a5..cebec4ad61 100644 --- a/app/client/src/PluginActionEditor/components/PluginActionNameEditor.tsx +++ b/app/client/src/PluginActionEditor/components/PluginActionNameEditor.tsx @@ -12,7 +12,7 @@ import { Flex } from "@appsmith/ads"; import styled from "styled-components"; import { noop } from "lodash"; import { EditableName, useIsRenaming } from "IDE"; - +import ImageAlt from "assets/images/placeholder-image.svg"; export interface SaveActionNameParams { id: string; name: string; @@ -64,7 +64,7 @@ const PluginActionNameEditor = ({ isFeatureEnabled, action?.userPermissions, ); - const iconUrl = getAssetUrl(plugin?.iconLocation) || ""; + const iconUrl = getAssetUrl(plugin?.iconLocation ?? ImageAlt); const icon = ActionUrlIcon(iconUrl); const handleDoubleClick = isChangePermitted ? enterEditMode : noop; diff --git a/app/client/src/ce/selectors/entitiesSelector.ts b/app/client/src/ce/selectors/entitiesSelector.ts index c33a71ef95..e81be33503 100644 --- a/app/client/src/ce/selectors/entitiesSelector.ts +++ b/app/client/src/ce/selectors/entitiesSelector.ts @@ -598,15 +598,25 @@ export const getDatasourcePlugins = createSelector(getPlugins, (plugins) => { return plugins.filter((plugin) => plugin?.allowUserDatasources ?? true); }); -export const getPluginImages = createSelector(getPlugins, (plugins) => { - const pluginImages: Record = {}; +export const getPluginImages = createSelector( + getPlugins, + getDatasources, + (plugins, datasources) => { + const pluginImages: Record = {}; - plugins.forEach((plugin) => { - pluginImages[plugin.id] = plugin?.iconLocation ?? ImageAlt; - }); + plugins.forEach((plugin) => { + pluginImages[plugin.id] = plugin?.iconLocation ?? ImageAlt; + }); - return pluginImages; -}); + datasources.forEach((datasource) => { + if (!pluginImages[datasource.pluginId]) { + pluginImages[datasource.pluginId] = ImageAlt; + } + }); + + return pluginImages; + }, +); export const getPluginNames = createSelector(getPlugins, (plugins) => { const pluginNames: Record = {}; @@ -1676,7 +1686,7 @@ export const getQuerySegmentItems = createSelector( const items: EntityItem[] = actions.map((action) => { let group; const iconUrl = getAssetUrl( - pluginGroups[action.config.pluginId]?.iconLocation, + pluginGroups[action.config.pluginId]?.iconLocation ?? ImageAlt, ); if (action.config.pluginType === PluginType.API) { diff --git a/app/client/src/components/editorComponents/ActionCreator/helpers.tsx b/app/client/src/components/editorComponents/ActionCreator/helpers.tsx index bef3973930..eb4a067b7e 100644 --- a/app/client/src/components/editorComponents/ActionCreator/helpers.tsx +++ b/app/client/src/components/editorComponents/ActionCreator/helpers.tsx @@ -32,6 +32,7 @@ import { getCurrentJSCollections, getQueryModuleInstances, getJSModuleInstancesData, + getPluginImages, } from "ee/selectors/entitiesSelector"; import { getModalDropdownList, @@ -64,7 +65,7 @@ import { selectEvaluationVersion } from "ee/selectors/applicationSelectors"; import { isJSAction } from "ee/workers/Evaluation/evaluationUtils"; import type { DataTreeEntity } from "entities/DataTree/dataTreeTypes"; import type { ModuleInstanceDataState } from "ee/constants/ModuleInstanceConstants"; -import { getModuleIcon, getPluginImagesFromPlugins } from "pages/Editor/utils"; +import { getModuleIcon } from "pages/Editor/utils"; import { getAllModules } from "ee/selectors/modulesSelector"; import type { Module } from "ee/constants/ModuleConstants"; import { @@ -420,6 +421,7 @@ export function getApiQueriesAndJSActionOptionsWithChildren( queryModuleInstances: ModuleInstanceDataState, jsModuleInstances: ReturnType, modules: Record, + pluginImages: Record, ) { // this function gets a list of all the queries/apis and attaches it to actionList getApiAndQueryOptions( @@ -429,6 +431,7 @@ export function getApiQueriesAndJSActionOptionsWithChildren( handleClose, queryModuleInstances, modules, + pluginImages, ); // this function gets a list of all the JS Objects and attaches it to actionList @@ -446,8 +449,8 @@ function getApiAndQueryOptions( handleClose: () => void, queryModuleInstances: ModuleInstanceDataState, modules: Record, + pluginImages: Record, ) { - const pluginImages = getPluginImagesFromPlugins(plugins); // TODO: Fix this the next time the file is edited // eslint-disable-next-line @typescript-eslint/no-explicit-any const pluginGroups: any = keyBy(plugins, "id"); @@ -686,6 +689,7 @@ export function useApisQueriesAndJsActionOptions(handleClose: () => void) { ) as unknown as ModuleInstanceDataState; const jsModuleInstancesData = useSelector(getJSModuleInstancesData); const modules = useSelector(getAllModules); + const pluginImages = useSelector(getPluginImages); // this function gets all the Queries/API's/JS Objects and attaches it to actionList return getApiQueriesAndJSActionOptionsWithChildren( @@ -698,5 +702,6 @@ export function useApisQueriesAndJsActionOptions(handleClose: () => void) { queryModuleInstances, jsModuleInstancesData, modules, + pluginImages, ); } diff --git a/app/client/src/components/editorComponents/Debugger/ErrorLogs/components/LogEntityLink.tsx b/app/client/src/components/editorComponents/Debugger/ErrorLogs/components/LogEntityLink.tsx index d3bab47655..ff72f48489 100644 --- a/app/client/src/components/editorComponents/Debugger/ErrorLogs/components/LogEntityLink.tsx +++ b/app/client/src/components/editorComponents/Debugger/ErrorLogs/components/LogEntityLink.tsx @@ -4,11 +4,10 @@ import { useSelector } from "react-redux"; import { keyBy } from "lodash"; import type { LogItemProps } from "../ErrorLogItem"; import { Colors } from "constants/Colors"; -import { getPlugins } from "ee/selectors/entitiesSelector"; +import { getPluginImages, getPlugins } from "ee/selectors/entitiesSelector"; import EntityLink from "../../EntityLink"; import { DebuggerLinkUI } from "components/editorComponents/Debugger/DebuggerEntityLink"; import { getIconForEntity } from "ee/components/editorComponents/Debugger/ErrorLogs/getLogIconForEntity"; -import { getPluginImagesFromPlugins } from "pages/Editor/utils"; const EntityLinkWrapper = styled.div` display: flex; @@ -50,7 +49,7 @@ const getIcon = (props: LogItemProps, pluginImages: Record) => { export default function LogEntityLink(props: LogItemProps) { const plugins = useSelector(getPlugins); const pluginGroups = useMemo(() => keyBy(plugins, "id"), [plugins]); - const pluginImages = getPluginImagesFromPlugins(plugins); + const pluginImages = useSelector(getPluginImages); const plugin = props.iconId ? pluginGroups[props.iconId] : undefined; diff --git a/app/client/src/components/propertyControls/ActionSelectorControl.tsx b/app/client/src/components/propertyControls/ActionSelectorControl.tsx index 911cc733b8..7a2824fcf6 100644 --- a/app/client/src/components/propertyControls/ActionSelectorControl.tsx +++ b/app/client/src/components/propertyControls/ActionSelectorControl.tsx @@ -19,6 +19,7 @@ import { getAllJSCollections, getJSModuleInstancesData, getModuleInstances, + getPluginImages, getPlugins, } from "ee/selectors/entitiesSelector"; import store from "store"; @@ -115,6 +116,7 @@ class ActionSelectorControl extends BaseControl { const queryModuleInstances = [] as ModuleInstanceDataState; const jsModuleInstances = getJSModuleInstancesData(state); const modules = getAllModules(state); + const pluginImages = getPluginImages(state); if (!!moduleInstances) { for (const moduleInstance of Object.values(moduleInstances)) { @@ -174,6 +176,7 @@ class ActionSelectorControl extends BaseControl { queryModuleInstances, jsModuleInstances, modules, + pluginImages, ); try { diff --git a/app/client/src/pages/Editor/DataSidePane/DataSidePane.tsx b/app/client/src/pages/Editor/DataSidePane/DataSidePane.tsx index fa6107dd51..980d8b1ad9 100644 --- a/app/client/src/pages/Editor/DataSidePane/DataSidePane.tsx +++ b/app/client/src/pages/Editor/DataSidePane/DataSidePane.tsx @@ -5,12 +5,12 @@ import { useSelector } from "react-redux"; import { getDatasources, getDatasourcesGroupedByPluginCategory, - getPlugins, + getPluginImages, } from "ee/selectors/entitiesSelector"; import history from "utils/history"; import { datasourcesEditorIdURL, integrationEditorURL } from "ee/RouteBuilder"; import { getSelectedDatasourceId } from "ee/navigation/FocusSelectors"; -import { get, keyBy } from "lodash"; +import { get } from "lodash"; import CreateDatasourceButton from "./CreateDatasourceButton"; import { useLocation } from "react-router"; import { @@ -53,8 +53,7 @@ export const DataSidePane = (props: DataSidePaneProps) => { >(""); const datasources = useSelector(getDatasources); const groupedDatasources = useSelector(getDatasourcesGroupedByPluginCategory); - const plugins = useSelector(getPlugins); - const groupedPlugins = keyBy(plugins, "id"); + const pluginImages = useSelector(getPluginImages); const location = useLocation(); const goToDatasource = useCallback((id: string) => { history.push(datasourcesEditorIdURL({ datasourceId: id })); @@ -123,9 +122,7 @@ export const DataSidePane = (props: DataSidePaneProps) => { title: data.name, startIcon: ( ), description: get(dsUsageMap, data.id, ""), diff --git a/app/client/src/pages/Editor/gitSync/components/DatasourceListItem.tsx b/app/client/src/pages/Editor/gitSync/components/DatasourceListItem.tsx index b88ef6da43..30a1cb9eff 100644 --- a/app/client/src/pages/Editor/gitSync/components/DatasourceListItem.tsx +++ b/app/client/src/pages/Editor/gitSync/components/DatasourceListItem.tsx @@ -6,6 +6,7 @@ import styled from "styled-components"; import { getAssetUrl } from "ee/utils/airgapHelpers"; import { PluginImage } from "pages/Editor/DataSourceEditor/DSFormHeader"; import { isEnvironmentConfigured } from "ee/utils/Environments"; +import ImageAlt from "assets/images/placeholder-image.svg"; import type { Plugin } from "entities/Plugin"; import { isDatasourceAuthorizedForQueryCreation, @@ -76,7 +77,10 @@ function ListItemWrapper(props: { className={`t--ds-list ${selected ? "active" : ""}`} onClick={() => onClick(ds)} > - + diff --git a/app/client/src/pages/Editor/utils.tsx b/app/client/src/pages/Editor/utils.tsx index 501c800b3b..6bc8d9ac84 100644 --- a/app/client/src/pages/Editor/utils.tsx +++ b/app/client/src/pages/Editor/utils.tsx @@ -28,8 +28,7 @@ import { JsFileIconV2, } from "pages/Editor/Explorer/ExplorerIcons"; import { getAssetUrl } from "ee/utils/airgapHelpers"; -import { type Plugin, PluginType } from "entities/Plugin"; -import ImageAlt from "assets/images/placeholder-image.svg"; +import { PluginType } from "entities/Plugin"; import { Icon } from "@appsmith/ads"; import { objectKeys } from "@appsmith/utils"; @@ -438,13 +437,3 @@ export function getModuleIcon( ); } - -export function getPluginImagesFromPlugins(plugins: Plugin[]) { - const pluginImages: Record = {}; - - plugins.forEach((plugin) => { - pluginImages[plugin.id] = plugin?.iconLocation ?? ImageAlt; - }); - - return pluginImages; -} diff --git a/app/client/src/sagas/selectors.tsx b/app/client/src/sagas/selectors.tsx index 0a4dd8469a..f1230fe456 100644 --- a/app/client/src/sagas/selectors.tsx +++ b/app/client/src/sagas/selectors.tsx @@ -169,18 +169,6 @@ export const getExistingActionNames = createSelector( }, ); -export const getPluginIdToImageLocation = createSelector( - getPlugins, - (plugins) => - // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-explicit-any - plugins.reduce((acc: any, p: Plugin) => { - acc[p.id] = p.iconLocation; - - return acc; - }, {}), -); - /** * returns a objects of existing page name in data tree * From 84f3df4511ff7b2d65c6addddd372292c1e681f0 Mon Sep 17 00:00:00 2001 From: vadim Date: Thu, 24 Apr 2025 10:06:34 +0200 Subject: [PATCH 3/4] chore: Adjust WDS accent subtle (#40353) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Fixes #39784 ## Automation /ok-to-test tags="@tag.Sanity" ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: c2f3eb1e5379ef8cc6a75d2b81f19ef0d60c4fe4 > Cypress dashboard. > Tags: `@tag.Sanity` > Spec: >
Wed, 23 Apr 2025 09:30:18 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No ## Summary by CodeRabbit - **Style** - Refined accent background colors for improved visual consistency, especially in teal and cold color ranges. - **Tests** - Updated expected color values in tests to match the new accent background color adjustments. --- .../theming/src/color/src/LightModeTheme.ts | 9 +++++++-- .../theming/src/color/tests/LightModeTheme.test.ts | 12 ++++++------ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/app/client/packages/design-system/theming/src/color/src/LightModeTheme.ts b/app/client/packages/design-system/theming/src/color/src/LightModeTheme.ts index 3b8474d04b..6b480526ea 100644 --- a/app/client/packages/design-system/theming/src/color/src/LightModeTheme.ts +++ b/app/client/packages/design-system/theming/src/color/src/LightModeTheme.ts @@ -263,11 +263,16 @@ export class LightModeTheme implements ColorModeTheme { // Colder seeds require a bit more chroma to not seem completely washed out if (this.seedChroma > 0.09 && this.seedIsCold) { - color.oklch.c = 0.09; + color.oklch.c = 0.06; + } + + // Teal is quite intense in the perceived chroma, possibly because of the narrow range on both lighntess and chroma + if (color.oklch.h >= 160 && color.oklch.h <= 235) { + color.oklch.c = 0.04; } if (this.seedChroma > 0.06 && !this.seedIsCold) { - color.oklch.c = 0.06; + color.oklch.c = 0.03; } if (this.seedIsAchromatic) { diff --git a/app/client/packages/design-system/theming/src/color/tests/LightModeTheme.test.ts b/app/client/packages/design-system/theming/src/color/tests/LightModeTheme.test.ts index 70ca3800bf..da4a6f7557 100644 --- a/app/client/packages/design-system/theming/src/color/tests/LightModeTheme.test.ts +++ b/app/client/packages/design-system/theming/src/color/tests/LightModeTheme.test.ts @@ -154,7 +154,7 @@ describe("bgAccentSubtle color", () => { "oklch(0.95 0.09 231)", ).getColors(); - expect(bgAccentSubtle).toBe("rgb(80.68% 97.025% 100%)"); + expect(bgAccentSubtle).toBe("rgb(84.002% 96.389% 100%)"); }); it("should return correct color when seedLightness < 0.93", () => { @@ -162,7 +162,7 @@ describe("bgAccentSubtle color", () => { "oklch(0.92 0.09 231)", ).getColors(); - expect(bgAccentSubtle).toBe("rgb(73.159% 94.494% 100%)"); + expect(bgAccentSubtle).toBe("rgb(80.804% 93.121% 99.673%)"); }); it("should return correct color when seedChroma > 0.09 and hue is between 116 and 165", () => { @@ -170,7 +170,7 @@ describe("bgAccentSubtle color", () => { "oklch(0.95 0.10 120)", ).getColors(); - expect(bgAccentSubtle).toBe("rgb(90.964% 97.964% 71.119%)"); + expect(bgAccentSubtle).toBe("rgb(91.961% 96.786% 79.187%)"); }); it("should return correct color when seedChroma > 0.06 and hue is not between 116 and 165", () => { @@ -178,7 +178,7 @@ describe("bgAccentSubtle color", () => { "oklch(0.95 0.07 170)", ).getColors(); - expect(bgAccentSubtle).toBe("rgb(75.944% 100% 91.359%)"); + expect(bgAccentSubtle).toBe("rgb(84.267% 97.811% 92.543%)"); }); it("should return correct color when seedChroma < 0.04", () => { @@ -196,7 +196,7 @@ describe("bgAccentSubtleHover color", () => { "oklch(0.35 0.09 70)", ).getColors(); - expect(bgAccentSubtleHover).toBe("rgb(100% 91.101% 76.695%)"); + expect(bgAccentSubtleHover).toBe("rgb(98.845% 92.363% 85.26%)"); }); }); @@ -206,7 +206,7 @@ describe("bgAccentSubtleActive color", () => { "oklch(0.35 0.09 70)", ).getColors(); - expect(bgAccentSubtleActive).toBe("rgb(100% 87.217% 72.911%)"); + expect(bgAccentSubtleActive).toBe("rgb(94.908% 88.479% 81.43%)"); }); }); From 4dbd235ac41bef94a15f45131a009cfe7409a0d8 Mon Sep 17 00:00:00 2001 From: Apeksha Bhosale <7846888+ApekshaBhosale@users.noreply.github.com> Date: Thu, 24 Apr 2025 14:16:26 +0530 Subject: [PATCH 4/4] chore: login metric added (#40325) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Added custom login metrics image Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.All" ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: e36ee2c4f9432935a3dc75c30596a5042e5fee47 > Cypress dashboard. > Tags: `@tag.All` > Spec: >
Wed, 23 Apr 2025 12:57:57 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Introduced login metrics tracking, including timing and failure counts for login attempts. - Added a new filter to monitor login attempts and record related metrics. - Enhanced authentication failure handling with detailed metrics for different failure sources and error redirects. - Improved rate limiting feedback with integrated metric recording. - **Bug Fixes** - More accurate tracking and reporting of login failures and their sources. - **Chores** - Updated security configuration to integrate new metrics and failure handling components. --- .../external/constants/spans/LoginSpan.java | 8 +++ .../AuthenticationFailureHandler.java | 6 ++- .../ce/AuthenticationFailureHandlerCE.java | 31 ++++++++++++ .../configurations/NoTagsMeterFilter.java | 6 ++- .../server/configurations/SecurityConfig.java | 44 ++++++++++++----- .../server/filters/LoginMetricsFilter.java | 49 +++++++++++++++++++ .../server/filters/LoginRateLimitFilter.java | 18 ++++++- 7 files changed, 145 insertions(+), 17 deletions(-) create mode 100644 app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/LoginSpan.java create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/filters/LoginMetricsFilter.java diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/LoginSpan.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/LoginSpan.java new file mode 100644 index 0000000000..fff4b0e7d1 --- /dev/null +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/LoginSpan.java @@ -0,0 +1,8 @@ +package com.appsmith.external.constants.spans; + +import static com.appsmith.external.constants.spans.BaseSpan.APPSMITH_SPAN_PREFIX; + +public class LoginSpan { + public static final String LOGIN_FAILURE = APPSMITH_SPAN_PREFIX + "login_failure"; + public static final String LOGIN_ATTEMPT = APPSMITH_SPAN_PREFIX + "login_total"; +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationFailureHandler.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationFailureHandler.java index c8fffa933f..56bedc1cdb 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationFailureHandler.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/AuthenticationFailureHandler.java @@ -2,12 +2,14 @@ package com.appsmith.server.authentication.handlers; import com.appsmith.server.authentication.handlers.ce.AuthenticationFailureHandlerCE; import com.appsmith.server.authentication.helpers.AuthenticationFailureRetryHandler; +import io.micrometer.core.instrument.MeterRegistry; import org.springframework.stereotype.Component; @Component public class AuthenticationFailureHandler extends AuthenticationFailureHandlerCE { - public AuthenticationFailureHandler(AuthenticationFailureRetryHandler authenticationFailureRetryHandler) { - super(authenticationFailureRetryHandler); + public AuthenticationFailureHandler( + AuthenticationFailureRetryHandler authenticationFailureRetryHandler, MeterRegistry meterRegistry) { + super(authenticationFailureRetryHandler, meterRegistry); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationFailureHandlerCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationFailureHandlerCE.java index 258ad6c16e..32614f4b4b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationFailureHandlerCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationFailureHandlerCE.java @@ -1,21 +1,52 @@ package com.appsmith.server.authentication.handlers.ce; import com.appsmith.server.authentication.helpers.AuthenticationFailureRetryHandler; +import io.micrometer.core.instrument.MeterRegistry; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.security.core.AuthenticationException; +import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.web.server.WebFilterExchange; import org.springframework.security.web.server.authentication.ServerAuthenticationFailureHandler; import reactor.core.publisher.Mono; +import static com.appsmith.external.constants.spans.LoginSpan.LOGIN_FAILURE; + @Slf4j @RequiredArgsConstructor public class AuthenticationFailureHandlerCE implements ServerAuthenticationFailureHandler { private final AuthenticationFailureRetryHandler authenticationFailureRetryHandler; + private final MeterRegistry meterRegistry; + + private static final String SOURCE_FORM = "form"; @Override public Mono onAuthenticationFailure(WebFilterExchange webFilterExchange, AuthenticationException exception) { + String source = exception instanceof OAuth2AuthenticationException + ? ((OAuth2AuthenticationException) exception).getError().getErrorCode() + : SOURCE_FORM; + + String errorMessage = exception.getMessage(); + + meterRegistry + .counter(LOGIN_FAILURE, "source", source, "message", errorMessage) + .increment(); return authenticationFailureRetryHandler.retryAndRedirectOnAuthenticationFailure(webFilterExchange, exception); } + + public Mono handleErrorRedirect(WebFilterExchange webFilterExchange) { + String error = + webFilterExchange.getExchange().getRequest().getQueryParams().getFirst("error"); + String message = + webFilterExchange.getExchange().getRequest().getQueryParams().getFirst("message"); + + if ("true".equals(error)) { + meterRegistry + .counter(LOGIN_FAILURE, "source", "redirect", "message", message) + .increment(); + } + + return Mono.empty(); + } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/NoTagsMeterFilter.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/NoTagsMeterFilter.java index 2a23a7cd2f..cfdba9a711 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/NoTagsMeterFilter.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/NoTagsMeterFilter.java @@ -8,6 +8,8 @@ import io.micrometer.core.instrument.config.MeterFilter; import java.util.List; import java.util.Map; +import static com.appsmith.external.constants.spans.LoginSpan.LOGIN_ATTEMPT; +import static com.appsmith.external.constants.spans.LoginSpan.LOGIN_FAILURE; import static com.appsmith.external.constants.spans.ce.ActionSpanCE.*; import static com.appsmith.external.git.constants.ce.GitSpanCE.FS_FETCH_REMOTE; import static com.appsmith.external.git.constants.ce.GitSpanCE.FS_RESET; @@ -28,7 +30,9 @@ public class NoTagsMeterFilter implements MeterFilter { Map.entry(FS_RESET, List.of()), Map.entry(JGIT_RESET_HARD, List.of()), Map.entry(FS_FETCH_REMOTE, List.of()), - Map.entry(JGIT_FETCH_REMOTE, List.of())); + Map.entry(JGIT_FETCH_REMOTE, List.of()), + Map.entry(LOGIN_FAILURE, List.of("source", "message")), + Map.entry(LOGIN_ATTEMPT, List.of("source"))); @Override public Meter.Id map(Meter.Id id) { diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java index 9f787fd267..2b00f5b483 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/SecurityConfig.java @@ -2,6 +2,7 @@ package com.appsmith.server.configurations; import com.appsmith.external.exceptions.ErrorDTO; import com.appsmith.server.authentication.handlers.AccessDeniedHandler; +import com.appsmith.server.authentication.handlers.AuthenticationFailureHandler; import com.appsmith.server.authentication.handlers.CustomServerOAuth2AuthorizationRequestResolver; import com.appsmith.server.authentication.handlers.LogoutSuccessHandler; import com.appsmith.server.authentication.oauth2clientrepositories.CustomOauth2ClientRepositoryManager; @@ -11,6 +12,7 @@ import com.appsmith.server.domains.User; import com.appsmith.server.dtos.ResponseDTO; import com.appsmith.server.exceptions.AppsmithErrorCode; import com.appsmith.server.filters.ConditionalFilter; +import com.appsmith.server.filters.LoginMetricsFilter; import com.appsmith.server.filters.LoginRateLimitFilter; import com.appsmith.server.helpers.RedirectHelper; import com.appsmith.server.ratelimiting.RateLimitService; @@ -18,6 +20,7 @@ import com.appsmith.server.services.AnalyticsService; import com.appsmith.server.services.UserService; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; +import io.micrometer.core.instrument.MeterRegistry; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Bean; @@ -42,7 +45,6 @@ import org.springframework.security.oauth2.client.registration.ReactiveClientReg import org.springframework.security.web.server.SecurityWebFilterChain; import org.springframework.security.web.server.ServerAuthenticationEntryPoint; import org.springframework.security.web.server.authentication.ServerAuthenticationEntryPointFailureHandler; -import org.springframework.security.web.server.authentication.ServerAuthenticationFailureHandler; import org.springframework.security.web.server.authentication.ServerAuthenticationSuccessHandler; import org.springframework.security.web.server.util.matcher.PathPatternParserServerWebExchangeMatcher; import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher; @@ -90,7 +92,7 @@ public class SecurityConfig { private ServerAuthenticationSuccessHandler authenticationSuccessHandler; @Autowired - private ServerAuthenticationFailureHandler authenticationFailureHandler; + private AuthenticationFailureHandler authenticationFailureHandler; @Autowired private ServerAuthenticationEntryPoint authenticationEntryPoint; @@ -119,21 +121,28 @@ public class SecurityConfig { @Autowired private CsrfConfig csrfConfig; + @Autowired + private MeterRegistry meterRegistry; + @Value("${appsmith.internal.password}") private String INTERNAL_PASSWORD; private static final String INTERNAL = "INTERNAL"; /** - * This routerFunction is required to map /public/** endpoints to the src/main/resources/public folder - * This is to allow static resources to be served by the server. Couldn't find an easier way to do this, + * This routerFunction is required to map /public/** endpoints to the + * src/main/resources/public folder + * This is to allow static resources to be served by the server. Couldn't find + * an easier way to do this, * hence using RouterFunctions to implement this feature. *

* Future folks: Please check out links: * - ... - * - ... + * - ... * - Class ResourceHandlerRegistry - * for details. If you figure out a cleaner approach, please modify this function + * for details. If you figure out a cleaner approach, please modify this + * function */ @Bean public RouterFunction publicRouter() { @@ -182,14 +191,17 @@ public class SecurityConfig { // Disabled because we use CSP's `frame-ancestors` instead. .frameOptions(options -> options.disable())) .anonymous(anonymousSpec -> anonymousSpec.principal(createAnonymousUser())) - // This returns 401 unauthorized for all requests that are not authenticated but authentication is + // This returns 401 unauthorized for all requests that are not authenticated but + // authentication is // required - // The client will redirect to the login page if we return 401 as Http status response + // The client will redirect to the login page if we return 401 as Http status + // response .exceptionHandling(exceptionHandlingSpec -> exceptionHandlingSpec .authenticationEntryPoint(authenticationEntryPoint) .accessDeniedHandler(accessDeniedHandler)) .authorizeExchange(authorizeExchangeSpec -> authorizeExchangeSpec - // The following endpoints are allowed to be accessed without authentication + // The following endpoints are allowed to be accessed without + // authentication .matchers( ServerWebExchangeMatchers.pathMatchers(HttpMethod.GET, Url.HEALTH_CHECK), ServerWebExchangeMatchers.pathMatchers(HttpMethod.POST, USER_URL), @@ -226,7 +238,10 @@ public class SecurityConfig { .authenticated()) // Add Pre Auth rate limit filter before authentication filter .addFilterBefore( - new ConditionalFilter(new LoginRateLimitFilter(rateLimitService), Url.LOGIN_URL), + new ConditionalFilter(new LoginMetricsFilter(meterRegistry), Url.LOGIN_URL), + SecurityWebFiltersOrder.FORM_LOGIN) + .addFilterBefore( + new ConditionalFilter(new LoginRateLimitFilter(rateLimitService, meterRegistry), Url.LOGIN_URL), SecurityWebFiltersOrder.FORM_LOGIN) .httpBasic(httpBasicSpec -> httpBasicSpec.authenticationFailureHandler(failureHandler)) .formLogin(formLoginSpec -> formLoginSpec @@ -264,7 +279,8 @@ public class SecurityConfig { } /** - * This bean configures the parameters that need to be set when a Cookie is created for a logged in user + * This bean configures the parameters that need to be set when a Cookie is + * created for a logged in user */ @Bean public WebSessionIdResolver webSessionIdResolver() { @@ -283,7 +299,8 @@ public class SecurityConfig { private Mono sanityCheckFilter(ServerWebExchange exchange, WebFilterChain chain) { final HttpHeaders headers = exchange.getRequest().getHeaders(); - // 1. Check if the content-type is valid at all. Mostly just checks if it contains a `/`. + // 1. Check if the content-type is valid at all. Mostly just checks if it + // contains a `/`. MediaType contentType; try { contentType = headers.getContentType(); @@ -299,7 +316,8 @@ public class SecurityConfig { return writeErrorResponse(exchange, chain, "Unsupported Content-Type"); } - // 3. Check Appsmith version, if present. Not making this a mandatory check for now, but reconsider later. + // 3. Check Appsmith version, if present. Not making this a mandatory check for + // now, but reconsider later. final String versionHeaderValue = headers.getFirst(CsrfConfig.VERSION_HEADER); final String serverVersion = projectProperties.getVersion(); if (versionHeaderValue != null && !serverVersion.equals(versionHeaderValue)) { diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/LoginMetricsFilter.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/LoginMetricsFilter.java new file mode 100644 index 0000000000..8ca2f8f332 --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/LoginMetricsFilter.java @@ -0,0 +1,49 @@ +package com.appsmith.server.filters; + +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.Timer; +import lombok.extern.slf4j.Slf4j; +import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.server.WebFilter; +import org.springframework.web.server.WebFilterChain; +import reactor.core.publisher.Mono; + +import static com.appsmith.external.constants.spans.LoginSpan.LOGIN_ATTEMPT; +import static com.appsmith.external.constants.spans.LoginSpan.LOGIN_FAILURE; + +@Slf4j +public class LoginMetricsFilter implements WebFilter { + + private final MeterRegistry meterRegistry; + + public LoginMetricsFilter(MeterRegistry meterRegistry) { + this.meterRegistry = meterRegistry; + } + + @Override + public Mono filter(ServerWebExchange exchange, WebFilterChain chain) { + return Mono.defer(() -> { + Timer.Sample sample = Timer.start(meterRegistry); + return chain.filter(exchange) + .doOnSuccess(aVoid -> { + sample.stop(Timer.builder(LOGIN_ATTEMPT).register(meterRegistry)); + }) + .doOnError(throwable -> { + sample.stop(Timer.builder(LOGIN_ATTEMPT) + .tag("message", throwable.getMessage()) + .register(meterRegistry)); + + meterRegistry + .counter( + LOGIN_FAILURE, + "source", + "form_login", + "errorCode", + "AuthenticationFailed", + "message", + throwable.getMessage()) + .increment(); + }); + }); + } +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/LoginRateLimitFilter.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/LoginRateLimitFilter.java index d64ca5d820..ce3746bb70 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/LoginRateLimitFilter.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/filters/LoginRateLimitFilter.java @@ -3,6 +3,7 @@ package com.appsmith.server.filters; import com.appsmith.server.constants.FieldName; import com.appsmith.server.constants.RateLimitConstants; import com.appsmith.server.ratelimiting.RateLimitService; +import io.micrometer.core.instrument.MeterRegistry; import lombok.extern.slf4j.Slf4j; import org.springframework.security.web.server.DefaultServerRedirectStrategy; import org.springframework.security.web.server.ServerRedirectStrategy; @@ -15,6 +16,7 @@ import java.net.URI; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; +import static com.appsmith.external.constants.spans.LoginSpan.LOGIN_FAILURE; import static java.lang.Boolean.FALSE; @Slf4j @@ -22,9 +24,11 @@ public class LoginRateLimitFilter implements WebFilter { private final ServerRedirectStrategy redirectStrategy = new DefaultServerRedirectStrategy(); private final RateLimitService rateLimitService; + private final MeterRegistry meterRegistry; - public LoginRateLimitFilter(RateLimitService rateLimitService) { + public LoginRateLimitFilter(RateLimitService rateLimitService, MeterRegistry meterRegistry) { this.rateLimitService = rateLimitService; + this.meterRegistry = meterRegistry; } @Override @@ -60,6 +64,18 @@ public class LoginRateLimitFilter implements WebFilter { // Set the error in the URL query parameter for rate limiting String url = "/user/login?error=true&message=" + URLEncoder.encode(RateLimitConstants.RATE_LIMIT_REACHED_ACCOUNT_SUSPENDED, StandardCharsets.UTF_8); + + meterRegistry + .counter( + LOGIN_FAILURE, + "source", + "rate_limit", + "errorCode", + "RateLimitExceeded", + "message", + RateLimitConstants.RATE_LIMIT_REACHED_ACCOUNT_SUSPENDED) + .increment(); + return redirectWithUrl(exchange, url); }