From c9ddd9c76540e62d2b1695cf62447d200752a4d2 Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Wed, 27 Nov 2024 10:32:35 +0530 Subject: [PATCH] chore: Remove CS URL in client (#37665) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The server should be the source of truth and owner for the current CS URL, and the client having direct access to the CS URL is (almost) an abstraction leak. We're using it on client for one purpose only, to redirect to CS for Google sheets authorization. That's just as well achieved with another redirect via the server. This PR does that redirection and removes access to the CS URL to the client code. Not used anywhere else, and shouldn't be needed. ## Automation /test sanity datasource ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: 0a1937e7d5d492c7c6a98bde124a157663126ac1 > Cypress dashboard. > Tags: `@tag.Sanity, @tag.Datasource` > Spec: >
Tue, 26 Nov 2024 12:21:11 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No ## Summary by CodeRabbit ## Release Notes - **New Features** - Enhanced CI workflow with improved error handling and logging. - Added a new authorization redirection endpoint in the SaaS controller. - **Improvements** - Database URL validation step added to CI tests. - Artifact management for test results has been clarified and improved. - Removal of `cloudServicesBaseUrl` from various configurations to streamline cloud service integration. - **Bug Fixes** - Refined logic for rerunning tests based on previous results. These updates contribute to a more robust and efficient testing and configuration environment. --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- .github/workflows/ci-test-custom-script.yml | 12 +++---- .../workflows/ci-test-limited-with-count.yml | 12 +++---- .github/workflows/ci-test-limited.yml | 2 -- .../docker/templates/nginx-app.conf.template | 1 - app/client/public/index.html | 3 -- app/client/src/api/CloudServicesApi.ts | 6 +--- app/client/src/ce/configs/index.ts | 6 ---- app/client/src/ce/configs/types.ts | 2 -- .../server/controllers/SaasController.java | 5 +-- .../controllers/ce/SaasControllerCE.java | 31 ++++++++++++++++--- 10 files changed, 40 insertions(+), 40 deletions(-) diff --git a/.github/workflows/ci-test-custom-script.yml b/.github/workflows/ci-test-custom-script.yml index ffcaea802a..0cd2c50650 100644 --- a/.github/workflows/ci-test-custom-script.yml +++ b/.github/workflows/ci-test-custom-script.yml @@ -166,10 +166,8 @@ jobs: -e APPSMITH_JWT_SECRET=appsmith-cloud-services-jwt-secret-dummy-key \ -e APPSMITH_ENCRYPTION_SALT=encryption-salt \ -e APPSMITH_ENCRYPTION_PASSWORD=encryption-password \ - -e APPSMITH_CLOUD_SERVICES_URL=https://cs-dev.appsmith.com \ -e APPSMITH_CUSTOMER_PORTAL_URL=https://dev.appsmith.com \ -e APPSMITH_CLOUD_SERVICES_BASE_URL=https://cs-dev.appsmith.com \ - -e APPSMITH_CLOUD_SERVER_BASE_URL=https://release.app.appsmith.com \ -e AUTH0_ISSUER_URL=https://login.release-customer.appsmith.com/ \ -e AUTH0_CLIENT_ID=dummy-client-id \ -e AUTH0_CLIENT_SECRET=dummy-secret-id \ @@ -198,7 +196,7 @@ jobs: uses: actions/setup-node@v4 with: node-version-file: app/client/package.json - + - name: Check DB URL if: steps.run_result.outputs.run_result != 'success' run: | @@ -457,20 +455,20 @@ jobs: name: server-logs-${{ matrix.job }} path: app/server/server-logs.log overwrite: true - + - name: Collect docker log as file if: always() run: | docker logs appsmith >& app/server/docker-logs.log - - - name: Upload server docker logs bundle on failure + + - name: Upload server docker logs bundle on failure uses: actions/upload-artifact@v4 if: always() with: name: docker-logs-${{ matrix.job }} path: app/server/docker-logs.log overwrite: true - + # Set status = success - name: Save the status of the run run: | diff --git a/.github/workflows/ci-test-limited-with-count.yml b/.github/workflows/ci-test-limited-with-count.yml index b7786c07aa..23af3ad3e4 100644 --- a/.github/workflows/ci-test-limited-with-count.yml +++ b/.github/workflows/ci-test-limited-with-count.yml @@ -17,7 +17,7 @@ on: run_count: description: 'Number of times to repeat the test run' required: false - type: number + type: number default: 1 workflow_call: inputs: @@ -176,10 +176,8 @@ jobs: -e APPSMITH_JWT_SECRET=appsmith-cloud-services-jwt-secret-dummy-key \ -e APPSMITH_ENCRYPTION_SALT=encryption-salt \ -e APPSMITH_ENCRYPTION_PASSWORD=encryption-password \ - -e APPSMITH_CLOUD_SERVICES_URL=https://cs-dev.appsmith.com \ -e APPSMITH_CUSTOMER_PORTAL_URL=https://dev.appsmith.com \ -e APPSMITH_CLOUD_SERVICES_BASE_URL=https://cs-dev.appsmith.com \ - -e APPSMITH_CLOUD_SERVER_BASE_URL=https://release.app.appsmith.com \ -e AUTH0_ISSUER_URL=https://login.release-customer.appsmith.com/ \ -e AUTH0_CLIENT_ID=dummy-client-id \ -e AUTH0_CLIENT_SECRET=dummy-secret-id \ @@ -351,10 +349,10 @@ jobs: npx cypress-repeat-pro run -n ${{ inputs.run_count }} --force \ --spec ${{ env.specs_to_run }} \ --config-file "cypress_ci_custom.config.ts" - cat cy-repeat-summary.txt + cat cy-repeat-summary.txt # Define the path for the failure flag file FAILURE_FLAG_FILE="ci_test_status.txt" - + # Check for test results and store the status in the file if ! grep -q "Total Failed: 0" cy-repeat-summary.txt; then echo "ci_test_failed=true" > "$FAILURE_FLAG_FILE" @@ -362,7 +360,7 @@ jobs: echo "ci_test_failed=false" > "$FAILURE_FLAG_FILE" fi cat "$FAILURE_FLAG_FILE" - + - name: Trim number of cypress log files if: failure() run: | @@ -375,7 +373,7 @@ jobs: name: cypress-repeat-logs path: ${{ github.workspace }}/app/client/cy-repeat-summary.txt overwrite: true - + - name: Upload ci_test_status.txt artifact if: always() uses: actions/upload-artifact@v4 diff --git a/.github/workflows/ci-test-limited.yml b/.github/workflows/ci-test-limited.yml index b9cdeaf90f..0407cc6440 100644 --- a/.github/workflows/ci-test-limited.yml +++ b/.github/workflows/ci-test-limited.yml @@ -166,10 +166,8 @@ jobs: -e APPSMITH_JWT_SECRET=appsmith-cloud-services-jwt-secret-dummy-key \ -e APPSMITH_ENCRYPTION_SALT=encryption-salt \ -e APPSMITH_ENCRYPTION_PASSWORD=encryption-password \ - -e APPSMITH_CLOUD_SERVICES_URL=https://cs-dev.appsmith.com \ -e APPSMITH_CUSTOMER_PORTAL_URL=https://dev.appsmith.com \ -e APPSMITH_CLOUD_SERVICES_BASE_URL=https://cs-dev.appsmith.com \ - -e APPSMITH_CLOUD_SERVER_BASE_URL=https://release.app.appsmith.com \ -e AUTH0_ISSUER_URL=https://login.release-customer.appsmith.com/ \ -e AUTH0_CLIENT_ID=dummy-client-id \ -e AUTH0_CLIENT_SECRET=dummy-secret-id \ diff --git a/app/client/docker/templates/nginx-app.conf.template b/app/client/docker/templates/nginx-app.conf.template index e23bd86a16..aa71c4314c 100644 --- a/app/client/docker/templates/nginx-app.conf.template +++ b/app/client/docker/templates/nginx-app.conf.template @@ -37,7 +37,6 @@ server { sub_filter __APPSMITH_VERSION_RELEASE_DATE__ '${APPSMITH_VERSION_RELEASE_DATE}'; sub_filter __APPSMITH_INTERCOM_APP_ID__ '${APPSMITH_INTERCOM_APP_ID}'; sub_filter __APPSMITH_MAIL_ENABLED__ '${APPSMITH_MAIL_ENABLED}'; - sub_filter __APPSMITH_CLOUD_SERVICES_BASE_URL__ '${APPSMITH_CLOUD_SERVICES_BASE_URL}'; sub_filter __APPSMITH_RECAPTCHA_SITE_KEY__ '${APPSMITH_RECAPTCHA_SITE_KEY}'; sub_filter __APPSMITH_DISABLE_INTERCOM__ '${APPSMITH_DISABLE_INTERCOM}'; sub_filter __APPSMITH_ZIPY_SDK_KEY__ '${APPSMITH_ZIPY_SDK_KEY}'; diff --git a/app/client/public/index.html b/app/client/public/index.html index e8588fa4f8..5197059b17 100755 --- a/app/client/public/index.html +++ b/app/client/public/index.html @@ -263,9 +263,6 @@ }, intercomAppID: INTERCOM_APP_ID, mailEnabled: parseConfig('{{env "APPSMITH_MAIL_ENABLED"}}'), - cloudServicesBaseUrl: - parseConfig('{{env "APPSMITH_CLOUD_SERVICES_BASE_URL"}}') || - "https://cs.appsmith.com", googleRecaptchaSiteKey: parseConfig('{{env "APPSMITH_RECAPTCHA_SITE_KEY"}}'), hideWatermark: parseConfig('{{env "APPSMITH_HIDE_WATERMARK"}}'), disableIframeWidgetSandbox: parseConfig( diff --git a/app/client/src/api/CloudServicesApi.ts b/app/client/src/api/CloudServicesApi.ts index ec91c838a1..4817f08830 100644 --- a/app/client/src/api/CloudServicesApi.ts +++ b/app/client/src/api/CloudServicesApi.ts @@ -1,6 +1,2 @@ -import { getAppsmithConfigs } from "ee/configs"; - -const { cloudServicesBaseUrl: BASE_URL } = getAppsmithConfigs(); - export const authorizeDatasourceWithAppsmithToken = (appsmithToken: string) => - `${BASE_URL}/api/v1/integrations/oauth/authorize?appsmithToken=${appsmithToken}`; + `/api/v1/saas/authorize?appsmithToken=${appsmithToken}`; diff --git a/app/client/src/ce/configs/index.ts b/app/client/src/ce/configs/index.ts index b58345e488..ce15b1a076 100644 --- a/app/client/src/ce/configs/index.ts +++ b/app/client/src/ce/configs/index.ts @@ -43,7 +43,6 @@ export interface INJECTED_CONFIGS { }; intercomAppID: string; mailEnabled: boolean; - cloudServicesBaseUrl: string; googleRecaptchaSiteKey: string; supportEmail: string; disableIframeWidgetSandbox: boolean; @@ -116,7 +115,6 @@ export const getConfigsFromEnvVars = (): INJECTED_CONFIGS => { mailEnabled: process.env.REACT_APP_MAIL_ENABLED ? process.env.REACT_APP_MAIL_ENABLED.length > 0 : false, - cloudServicesBaseUrl: process.env.REACT_APP_CLOUD_SERVICES_BASE_URL || "", googleRecaptchaSiteKey: process.env.REACT_APP_GOOGLE_RECAPTCHA_SITE_KEY || "", supportEmail: process.env.APPSMITH_SUPPORT_EMAIL || "support@appsmith.com", @@ -287,10 +285,6 @@ export const getAppsmithConfigs = (): AppsmithUIConfigs => { ENV_CONFIG.intercomAppID || APPSMITH_FEATURE_CONFIGS?.intercomAppID || "", mailEnabled: ENV_CONFIG.mailEnabled || APPSMITH_FEATURE_CONFIGS?.mailEnabled || false, - cloudServicesBaseUrl: - ENV_CONFIG.cloudServicesBaseUrl || - APPSMITH_FEATURE_CONFIGS?.cloudServicesBaseUrl || - "", appsmithSupportEmail: ENV_CONFIG.supportEmail, disableIframeWidgetSandbox: ENV_CONFIG.disableIframeWidgetSandbox || diff --git a/app/client/src/ce/configs/types.ts b/app/client/src/ce/configs/types.ts index 57e089b18a..3c9a3aa282 100644 --- a/app/client/src/ce/configs/types.ts +++ b/app/client/src/ce/configs/types.ts @@ -56,8 +56,6 @@ export interface AppsmithUIConfigs { intercomAppID: string; mailEnabled: boolean; - cloudServicesBaseUrl: string; - googleRecaptchaSiteKey: { enabled: boolean; apiKey: string; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/SaasController.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/SaasController.java index c455a3dba1..54921ed26e 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/SaasController.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/SaasController.java @@ -1,5 +1,6 @@ package com.appsmith.server.controllers; +import com.appsmith.server.configurations.CloudServicesConfig; import com.appsmith.server.constants.Url; import com.appsmith.server.controllers.ce.SaasControllerCE; import com.appsmith.server.solutions.AuthenticationService; @@ -12,7 +13,7 @@ import org.springframework.web.bind.annotation.RestController; @RequestMapping(Url.SAAS_URL) public class SaasController extends SaasControllerCE { - public SaasController(AuthenticationService authenticationService) { - super(authenticationService); + public SaasController(AuthenticationService authenticationService, CloudServicesConfig cloudServicesConfig) { + super(authenticationService, cloudServicesConfig); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/SaasControllerCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/SaasControllerCE.java index 81028d4b4a..ce811e0050 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/SaasControllerCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/SaasControllerCE.java @@ -2,15 +2,18 @@ package com.appsmith.server.controllers.ce; import com.appsmith.external.models.OAuth2ResponseDTO; import com.appsmith.external.views.Views; +import com.appsmith.server.configurations.CloudServicesConfig; import com.appsmith.server.constants.FieldName; import com.appsmith.server.constants.Url; import com.appsmith.server.dtos.RequestAppsmithTokenDTO; import com.appsmith.server.dtos.ResponseDTO; import com.appsmith.server.solutions.AuthenticationService; import com.fasterxml.jackson.annotation.JsonView; +import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; +import org.springframework.http.server.reactive.ServerHttpResponse; +import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestBody; @@ -20,16 +23,17 @@ import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.server.ServerWebExchange; import reactor.core.publisher.Mono; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; + @Slf4j @RequestMapping(Url.SAAS_URL) +@RequiredArgsConstructor public class SaasControllerCE { private final AuthenticationService authenticationService; - @Autowired - public SaasControllerCE(AuthenticationService authenticationService) { - this.authenticationService = authenticationService; - } + private final CloudServicesConfig cloudServicesConfig; @JsonView(Views.Public.class) @PostMapping("/{datasourceId}/oauth") @@ -63,4 +67,21 @@ public class SaasControllerCE { .getAccessTokenFromCloud(datasourceId, environmentId, appsmithToken) .map(datasource -> new ResponseDTO<>(HttpStatus.OK.value(), datasource, null)); } + + @GetMapping("authorize") + public Mono redirectForAuthorize(ServerWebExchange exchange, @RequestParam String appsmithToken) { + if (appsmithToken == null || appsmithToken.isEmpty()) { + exchange.getResponse().setStatusCode(HttpStatus.BAD_REQUEST); + return exchange.getResponse().setComplete(); + } + + final String url = cloudServicesConfig.getBaseUrl() + "/api/v1/integrations/oauth/authorize?appsmithToken=" + + URLEncoder.encode(appsmithToken, StandardCharsets.UTF_8); + + ServerHttpResponse response = exchange.getResponse(); + response.setStatusCode(HttpStatus.TEMPORARY_REDIRECT); + response.getHeaders().set("Location", url); + + return response.setComplete(); + } }