-
Notifications
You must be signed in to change notification settings - Fork 4.4k
05/06 Daily Promotion #40872
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
05/06 Daily Promotion #40872
Conversation
…loud billing domains (#40855) ## Problem With the introduction of cloud billing and multi-organization support, users can now access Appsmith through organization-specific subdomains (e.g., `testorg.appsmith.com`). However, when users attempt to access non-existent domains (e.g., `domaindoesnotexist.appsmith.com`), the system fails to return a user object, causing the application to get stuck in an infinite loading state. ## 🔍 Root Cause The issue was in the `LandingScreen` component's conditional logic: ```tsx // Previous implementation ❌ if (props.user && window.location.pathname === BASE_URL) { if (props.user.email === ANONYMOUS_USERNAME) { return <Redirect to={AUTH_LOGIN_URL} />; } else { return <Redirect to={APPLICATIONS_URL} />; } } ``` **The problem:** When accessing a non-existent domain, `props.user` is `null`/`undefined`, causing the condition `props.user && window.location.pathname === BASE_URL` to evaluate to `false`. This prevented the redirect logic from executing, leaving users on a perpetual loading screen instead of being properly redirected to the login page. ## ✅ Solution Updated the conditional logic to handle cases where the user object is undefined: ```tsx // Fixed implementation ✅ if (window.location.pathname === BASE_URL) { if (!props.user || props.user.email === ANONYMOUS_USERNAME) { return <Redirect to={AUTH_LOGIN_URL} />; } else { return <Redirect to={APPLICATIONS_URL} />; } } ``` ### Key changes: 1. **Removed the `props.user &&` guard condition** - Now the redirect logic always executes when on the base URL 2. **Added explicit null/undefined check** - The condition `!props.user || props.user.email === ANONYMOUS_USERNAME` properly handles both scenarios: - When no user object exists (non-existent domains) - When the user is anonymous ## 📊 Impact | Scenario | Before | After | |----------|---------|-------| | Valid org domain + authenticated user | ✅ Redirects to applications | ✅ Redirects to applications | | Valid org domain + anonymous user | ✅ Redirects to login | ✅ Redirects to login | | **Non-existent org domain** | ❌ **Infinite loading screen** | ✅ **Redirects to login** | | Direct access to login URL | ✅ Works as expected | ✅ Works as expected | ## 🧪 Testing Scenarios This fix addresses the following scenarios in the cloud billing multi-org environment: - [x] Valid organization domain with authenticated user → Redirects to applications - [x] Valid organization domain with anonymous user → Redirects to login - [x] **Non-existent organization domain → Redirects to login (instead of infinite loading)** - [x] Direct access to login URL → Works as expected ## Automation /ok-to-test tags="@tag.Authentication, @tag.Sanity" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/15438685266> > Commit: afee6b1 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=15438685266&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Authentication, @tag.Sanity` > Spec: > <hr>Wed, 04 Jun 2025 10:09:08 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved user redirection on the landing screen to ensure users are correctly routed based on their authentication status. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Description Requires: #40642 Adds an environment variable activating nss_wrapper when the `securityPolicy.runAsUser` value is set so the UID can be set dynamically. This avoids the `I have no name!` in the prompt when doing a `kubectl exec` with that value set. I am also introducing [helm-unittest](https://github.com/helm-unittest/helm-unittest) for ensuring that changes to our defaults are made explicit. Fixes #38787 ## Automation /ok-to-test tags="" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!WARNING] > Tests have not run on the HEAD d30d87f yet > <hr>Mon, 19 May 2025 18:59:10 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added automated unit testing for Helm charts, including snapshot and security context tests. - Introduced documentation for running and understanding Helm chart unit tests. - Added a GitHub Actions workflow to run Helm chart unit tests on pull requests and manually. - **Bug Fixes** - Ensured the LD_PRELOAD environment variable is set when a specific security context is configured in deployments. - **Documentation** - Updated Helm chart README to reference new testing documentation and improve clarity. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Goutham Pratapa <[email protected]>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
…40873) Updated the chart version from 3.6.2 to 3.6.3 in the Helm test snapshots to reflect the latest changes and ensure consistency across deployments. ## 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._ 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="" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!CAUTION] > If you modify the content in this section, you are likely to disrupt the CI result for your PR. <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No
RakshaKShetty
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved
This pull request contains the daily promotion from the
releasebranch to themasterbranch.Warning
Tests have not run on the HEAD 9ae1eb9 yet
Thu, 05 Jun 2025 06:22:03 UTC