From 2fb48c1b1b37751f5be5fb705302a664c4892844 Mon Sep 17 00:00:00 2001 From: sriram veeraghanta Date: Wed, 8 Jul 2026 01:51:37 +0530 Subject: [PATCH 1/2] fix(security): block bot user logins (#9368) Bot service accounts (User.is_bot=True, e.g. the WORKSPACE_SEED bot) are internal identities meant to act only through API tokens. Nothing stopped one from being driven through the interactive login flow if its email was known, letting a human assume a service identity. Reject bot accounts at the shared login chokepoint, Adapter.complete_login_or_signup(), right beside the existing deactivated-account check. This covers every interactive provider in one place: email/password, magic code, and all OAuth providers (Google, GitHub, GitLab, Gitea) across both the app and space surfaces. Bot API-token access is left untouched, since that is how bots are meant to operate. Also add a defense-in-depth is_bot guard to InstanceAdminSignInEndpoint, which mints its own admin session outside the chokepoint (a bot is never an InstanceAdmin today, so this is not currently reachable, but it closes the path regardless). Surface the rejection with a new dedicated error code BOT_USER_LOGIN_FORBIDDEN (5017), plumbed into the app and space frontend error helpers as well as the shared @plane/constants and @plane/utils packages (message map + banner-alert list) so any consumer of the shared auth-error handler renders it correctly. The admin path reuses the existing ADMIN_AUTHENTICATION_FAILED code so it discloses no bot-specific error. Add contract regression tests: a bot blocked via password and via magic code, a bot blocked at the admin sign-in endpoint, and a non-bot control that still logs in. --- apps/api/plane/authentication/adapter/base.py | 13 ++ .../api/plane/authentication/adapter/error.py | 1 + apps/api/plane/license/api/views/admin.py | 19 +++ .../tests/contract/app/test_authentication.py | 127 ++++++++++++++++++ apps/space/helpers/authentication.helper.tsx | 6 + apps/web/helpers/authentication.helper.tsx | 6 + packages/constants/src/auth/index.ts | 1 + packages/utils/src/auth.ts | 5 + 8 files changed, 178 insertions(+) diff --git a/apps/api/plane/authentication/adapter/base.py b/apps/api/plane/authentication/adapter/base.py index 574fe9897ac..112570d56f3 100644 --- a/apps/api/plane/authentication/adapter/base.py +++ b/apps/api/plane/authentication/adapter/base.py @@ -330,6 +330,19 @@ def complete_login_or_signup(self): payload={"email": email}, ) + # Reject bot service accounts (BOT_USER_LOGIN_FORBIDDEN). Bots (is_bot=True, + # e.g. the WORKSPACE_SEED bot) are internal identities that act only through + # API tokens; they must never be assumable via the interactive login/signup + # flow (email/password, magic code, or any OAuth provider). A brand-new + # signup can never be a bot — bots are provisioned internally, never through + # this path — so guarding on an existing `user` record is sufficient. + if user and user.is_bot: + raise AuthenticationException( + error_code=AUTHENTICATION_ERROR_CODES["BOT_USER_LOGIN_FORBIDDEN"], + error_message="BOT_USER_LOGIN_FORBIDDEN", + payload={"email": email}, + ) + # True = new user (signup), False = returning user (login) is_signup = not bool(user) # If user is not present, create a new user diff --git a/apps/api/plane/authentication/adapter/error.py b/apps/api/plane/authentication/adapter/error.py index da4b88052bd..02b46adf2ad 100644 --- a/apps/api/plane/authentication/adapter/error.py +++ b/apps/api/plane/authentication/adapter/error.py @@ -9,6 +9,7 @@ "EMAIL_REQUIRED": 5010, "SIGNUP_DISABLED": 5015, "MAGIC_LINK_LOGIN_DISABLED": 5016, + "BOT_USER_LOGIN_FORBIDDEN": 5017, "PASSWORD_LOGIN_DISABLED": 5018, "USER_ACCOUNT_DEACTIVATED": 5019, # Password strength diff --git a/apps/api/plane/license/api/views/admin.py b/apps/api/plane/license/api/views/admin.py index 6217cc87fa4..6b9761ae77e 100644 --- a/apps/api/plane/license/api/views/admin.py +++ b/apps/api/plane/license/api/views/admin.py @@ -306,6 +306,25 @@ def post(self, request): ) return HttpResponseRedirect(url) + # Reject bot service accounts (defense-in-depth for the same intent as + # BOT_USER_LOGIN_FORBIDDEN on the app/space flow). Bots are internal + # identities that act only via API tokens and must never sign in to the + # admin console. A bot is never registered as an InstanceAdmin, so this + # is not reachable today, but the guard closes the path regardless. + # Reuse ADMIN_AUTHENTICATION_FAILED so no bot-specific admin error code + # is disclosed to the caller. + if user.is_bot: + exc = AuthenticationException( + error_code=AUTHENTICATION_ERROR_CODES["ADMIN_AUTHENTICATION_FAILED"], + error_message="ADMIN_AUTHENTICATION_FAILED", + payload={"email": email}, + ) + url = urljoin( + base_host(request=request, is_admin=True), + "?" + urlencode(exc.get_error_dict()), + ) + return HttpResponseRedirect(url) + # is_active if not user.is_active: exc = AuthenticationException( diff --git a/apps/api/plane/tests/contract/app/test_authentication.py b/apps/api/plane/tests/contract/app/test_authentication.py index 300d862b84b..87b871cace3 100644 --- a/apps/api/plane/tests/contract/app/test_authentication.py +++ b/apps/api/plane/tests/contract/app/test_authentication.py @@ -626,3 +626,130 @@ def test_magic_sign_up_throttled(self, django_client, setup_instance): response = django_client.post(url, {"email": "throttle-up@plane.so", "code": "000000"}, follow=False) assert "RATE_LIMIT_EXCEEDED" in response.url + + +@pytest.mark.contract +class TestBotUserLoginBlocked: + """Bot service accounts (is_bot=True) must never authenticate through the + interactive login flow. + + Bots are internal identities (e.g. the WORKSPACE_SEED bot) that act only via + API tokens. Every interactive provider funnels through + Adapter.complete_login_or_signup(), which rejects bots with + BOT_USER_LOGIN_FORBIDDEN (5017). These are regression guards for that block. + """ + + BOT_EMAIL = "bot-login@plane.so" + HUMAN_EMAIL = "human-login@plane.so" + PASSWORD = "Str0ng-Pass!42" + + @pytest.fixture(autouse=True) + def _clear_state(self): + """Reset throttle cache and the bot's magic-link redis state around each test.""" + cache.clear() + ri = redis_instance() + ri.delete(f"magic_{self.BOT_EMAIL}") + ri.delete(f"magic_{self.BOT_EMAIL}:verify_attempts") + yield + cache.clear() + ri.delete(f"magic_{self.BOT_EMAIL}") + ri.delete(f"magic_{self.BOT_EMAIL}:verify_attempts") + + @pytest.fixture + def bot_user(self, db): + """An active bot account with a known password so the credential check + passes and execution reaches the login chokepoint.""" + user = User.objects.create(email=self.BOT_EMAIL, is_bot=True, is_active=True) + user.set_password(self.PASSWORD) + user.save() + return user + + @pytest.fixture + def human_user(self, db): + """A normal (non-bot) account, identical apart from is_bot, as a control.""" + user = User.objects.create(email=self.HUMAN_EMAIL, is_active=True) + user.set_password(self.PASSWORD) + user.save() + return user + + @pytest.mark.django_db + def test_bot_password_sign_in_blocked(self, django_client, bot_user, setup_instance): + """Password sign-in with a bot's *correct* credentials is still rejected: + the block happens after credential verification, so no session is created.""" + url = reverse("sign-in") + response = django_client.post( + url, {"email": self.BOT_EMAIL, "password": self.PASSWORD}, follow=False + ) + assert response.status_code == 302 + assert "BOT_USER_LOGIN_FORBIDDEN" in response.url + # The block must prevent authentication. + assert "_auth_user_id" not in django_client.session + + @pytest.mark.django_db + @patch("plane.bgtasks.magic_link_code_task.magic_link.delay") + def test_bot_magic_sign_in_blocked( + self, mock_magic_link, django_client, api_client, bot_user, setup_instance + ): + """The same block applies via a second provider (magic code), proving the + guard sits at the shared chokepoint rather than in one provider.""" + token = _generate_magic_token(api_client, self.BOT_EMAIL) + url = reverse("magic-sign-in") + response = django_client.post(url, {"email": self.BOT_EMAIL, "code": token}, follow=False) + assert response.status_code == 302 + assert "BOT_USER_LOGIN_FORBIDDEN" in response.url + assert "_auth_user_id" not in django_client.session + + @pytest.mark.django_db + def test_human_password_sign_in_allowed(self, django_client, human_user, setup_instance): + """Control: a normal user with the identical setup still signs in — the + guard is scoped strictly to is_bot and does not regress human logins.""" + url = reverse("sign-in") + response = django_client.post( + url, {"email": self.HUMAN_EMAIL, "password": self.PASSWORD}, follow=False + ) + assert response.status_code == 302 + assert "BOT_USER_LOGIN_FORBIDDEN" not in response.url + assert "error_code" not in response.url + assert "_auth_user_id" in django_client.session + + +@pytest.mark.contract +class TestBotUserAdminSignInBlocked: + """A bot must not sign in to the instance-admin console either. + + InstanceAdminSignInEndpoint mints its own session via user_login() outside + Adapter.complete_login_or_signup(), so it carries an independent is_bot + guard that rejects bots with ADMIN_AUTHENTICATION_FAILED before the admin + membership check. (Uses the literal path because license/urls.py reuses the + name "instance-admin-sign-in" for both sign-in and sign-up.) + """ + + ADMIN_SIGN_IN_PATH = "/api/instances/admins/sign-in/" + BOT_EMAIL = "admin-bot@plane.so" + PASSWORD = "Str0ng-Pass!42" + + @pytest.fixture(autouse=True) + def _clear_state(self): + cache.clear() + yield + cache.clear() + + @pytest.fixture + def bot_user(self, db): + user = User.objects.create(email=self.BOT_EMAIL, is_bot=True, is_active=True) + user.set_password(self.PASSWORD) + user.save() + return user + + @pytest.mark.django_db + def test_bot_admin_sign_in_blocked(self, django_client, bot_user, setup_instance): + """A bot is rejected at the admin sign-in endpoint and no session is created, + even though it is active and the password is correct.""" + response = django_client.post( + self.ADMIN_SIGN_IN_PATH, + {"email": self.BOT_EMAIL, "password": self.PASSWORD}, + follow=False, + ) + assert response.status_code == 302 + assert "ADMIN_AUTHENTICATION_FAILED" in response.url + assert "_auth_user_id" not in django_client.session diff --git a/apps/space/helpers/authentication.helper.tsx b/apps/space/helpers/authentication.helper.tsx index 8104a1d8f7e..6e11d61bd02 100644 --- a/apps/space/helpers/authentication.helper.tsx +++ b/apps/space/helpers/authentication.helper.tsx @@ -42,6 +42,7 @@ export enum EAuthenticationErrorCodes { INVALID_EMAIL_MAGIC_SIGN_UP = "5050", MAGIC_SIGN_UP_EMAIL_CODE_REQUIRED = "5055", // Sign In + BOT_USER_LOGIN_FORBIDDEN = "5017", USER_ACCOUNT_DEACTIVATED = "5019", USER_DOES_NOT_EXIST = "5060", AUTHENTICATION_FAILED_SIGN_IN = "5065", @@ -160,6 +161,10 @@ const errorCodeMessages: { }, // sign in + [EAuthenticationErrorCodes.BOT_USER_LOGIN_FORBIDDEN]: { + title: `Sign in not allowed`, + message: () => `This account cannot be used to sign in. Please use a personal account.`, + }, [EAuthenticationErrorCodes.USER_ACCOUNT_DEACTIVATED]: { title: `User account deactivated`, message: () => `User account deactivated. Please contact ${SUPPORT_EMAIL ? SUPPORT_EMAIL : "administrator"}.`, @@ -384,6 +389,7 @@ export const authErrorHandler = (errorCode: EAuthenticationErrorCodes, email?: s EAuthenticationErrorCodes.ADMIN_AUTHENTICATION_FAILED, EAuthenticationErrorCodes.ADMIN_USER_ALREADY_EXIST, EAuthenticationErrorCodes.ADMIN_USER_DOES_NOT_EXIST, + EAuthenticationErrorCodes.BOT_USER_LOGIN_FORBIDDEN, EAuthenticationErrorCodes.USER_ACCOUNT_DEACTIVATED, ]; diff --git a/apps/web/helpers/authentication.helper.tsx b/apps/web/helpers/authentication.helper.tsx index 5db2e1aa9b3..977f0940625 100644 --- a/apps/web/helpers/authentication.helper.tsx +++ b/apps/web/helpers/authentication.helper.tsx @@ -43,6 +43,7 @@ export enum EAuthenticationErrorCodes { EMAIL_REQUIRED = "5010", SIGNUP_DISABLED = "5015", MAGIC_LINK_LOGIN_DISABLED = "5016", + BOT_USER_LOGIN_FORBIDDEN = "5017", PASSWORD_LOGIN_DISABLED = "5018", USER_ACCOUNT_DEACTIVATED = "5019", // Password strength @@ -141,6 +142,10 @@ const errorCodeMessages: { title: `User account deactivated`, message: () => `User account deactivated. Please contact ${SUPPORT_EMAIL ? SUPPORT_EMAIL : "administrator"}.`, }, + [EAuthenticationErrorCodes.BOT_USER_LOGIN_FORBIDDEN]: { + title: `Sign in not allowed`, + message: () => `This account cannot be used to sign in. Please use a personal account.`, + }, [EAuthenticationErrorCodes.INVALID_PASSWORD]: { title: `Invalid password`, message: () => `Invalid password. Please try again.`, @@ -380,6 +385,7 @@ export const authErrorHandler = (errorCode: EAuthenticationErrorCodes, email?: s EAuthenticationErrorCodes.SIGNUP_DISABLED, EAuthenticationErrorCodes.MAGIC_LINK_LOGIN_DISABLED, EAuthenticationErrorCodes.PASSWORD_LOGIN_DISABLED, + EAuthenticationErrorCodes.BOT_USER_LOGIN_FORBIDDEN, EAuthenticationErrorCodes.USER_ACCOUNT_DEACTIVATED, EAuthenticationErrorCodes.INVALID_PASSWORD, EAuthenticationErrorCodes.SMTP_NOT_CONFIGURED, diff --git a/packages/constants/src/auth/index.ts b/packages/constants/src/auth/index.ts index 32a7d5eee64..9d29181f0b6 100644 --- a/packages/constants/src/auth/index.ts +++ b/packages/constants/src/auth/index.ts @@ -110,6 +110,7 @@ export enum EAuthErrorCodes { EMAIL_REQUIRED = "5010", SIGNUP_DISABLED = "5015", MAGIC_LINK_LOGIN_DISABLED = "5016", + BOT_USER_LOGIN_FORBIDDEN = "5017", PASSWORD_LOGIN_DISABLED = "5018", USER_ACCOUNT_DEACTIVATED = "5019", // Password strength diff --git a/packages/utils/src/auth.ts b/packages/utils/src/auth.ts index fd8b80ad007..cd0646af012 100644 --- a/packages/utils/src/auth.ts +++ b/packages/utils/src/auth.ts @@ -142,6 +142,10 @@ const errorCodeMessages: { message: () => `Invalid email. Please try again.`, }, // sign in + [EAuthErrorCodes.BOT_USER_LOGIN_FORBIDDEN]: { + title: `Sign in not allowed`, + message: () => `This account cannot be used to sign in. Please use a personal account.`, + }, [EAuthErrorCodes.USER_ACCOUNT_DEACTIVATED]: { title: `User account deactivated`, message: () => `User account deactivated. Please contact administrator.`, @@ -349,6 +353,7 @@ export const authErrorHandler = (errorCode: EAuthErrorCodes, email?: string): TA EAuthErrorCodes.ADMIN_AUTHENTICATION_FAILED, EAuthErrorCodes.ADMIN_USER_ALREADY_EXIST, EAuthErrorCodes.ADMIN_USER_DOES_NOT_EXIST, + EAuthErrorCodes.BOT_USER_LOGIN_FORBIDDEN, EAuthErrorCodes.USER_ACCOUNT_DEACTIVATED, ]; From d076efdf53047cd21dfeb456da6c0e880d6f3aff Mon Sep 17 00:00:00 2001 From: Manish Gupta <59428681+mguptahub@users.noreply.github.com> Date: Thu, 9 Jul 2026 16:31:33 +0530 Subject: [PATCH 2/2] [WEB-7778] fix(security): reject unverified OAuth provider emails to prevent ATO (Cluster E) (#9289) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [WEB-7778] fix(security): reject unverified OAuth provider emails to prevent ATO An attacker controlling a self-hosted OAuth provider (Gitea, GitLab) could assert any email address in the OAuth response and be matched to an existing Plane account, bypassing authentication entirely. - Add OAUTH_PROVIDER_UNVERIFIED_EMAIL (5124) error code - GitHub: require both primary=True AND verified=True on email (was primary-only) - Google: check verified_email=False field in userinfo response - GitLab: check confirmed_at is non-null before accepting email - Gitea __get_email: remove unverified fallbacks (primary-unverified, any-unverified) - Gitea set_user_data: remove fast-path using .email from user object (no verification flag); always go through __get_email() which enforces verified Fixes GHSA-7j95-vh8g-f365 (critical ATO). Note: GHSA-cv9p-325g-wmv5 and GHSA-hx79-5pj5-qh42 (avatar SSRF) were already fixed in PR #9163. Co-authored-by: Plane AI * fix(security): add read:user scope to Gitea; fail-closed on absent Google verified_email Gitea's /api/v1/user/emails endpoint requires the read:user granular scope — openid+email+profile alone is insufficient and __get_email() would return a 401/403. Add read:user to the scope string. Google: change default from True to fail-closed (is not True) so a userinfo response that omits verified_email is rejected rather than trusted. The service-account justification was incorrect — service accounts do not go through the interactive OAuth2 callback flow. Co-authored-by: Plane AI --------- Co-authored-by: Plane AI --- .../authentication/provider/oauth/gitea.py | 22 ++++++++++--------- .../authentication/provider/oauth/github.py | 13 +++++++---- .../authentication/provider/oauth/gitlab.py | 7 ++++++ .../authentication/provider/oauth/google.py | 8 +++++++ 4 files changed, 36 insertions(+), 14 deletions(-) diff --git a/apps/api/plane/authentication/provider/oauth/gitea.py b/apps/api/plane/authentication/provider/oauth/gitea.py index 8c0c3a5db51..7a1392854d6 100644 --- a/apps/api/plane/authentication/provider/oauth/gitea.py +++ b/apps/api/plane/authentication/provider/oauth/gitea.py @@ -19,7 +19,7 @@ class GiteaOAuthProvider(OauthAdapter): provider = "gitea" - scope = "openid email profile" + scope = "openid email profile read:user" def __init__(self, request, code=None, state=None, callback=None): (GITEA_CLIENT_ID, GITEA_CLIENT_SECRET, GITEA_HOST) = get_configuration_value( @@ -130,15 +130,17 @@ def __get_email(self, headers): error_code=AUTHENTICATION_ERROR_CODES["GITEA_OAUTH_PROVIDER_ERROR"], error_message="GITEA_OAUTH_PROVIDER_ERROR: No emails found", ) - # Prefer primary+verified, then any verified, then primary, else first + # Prefer primary+verified, then any verified. Never fall back to an unverified + # email — an attacker with a self-hosted Gitea instance could assert any address + # to take over an existing account (GHSA-7j95-vh8g-f365). email = next((e.get("email") for e in emails_response if e.get("primary") and e.get("verified")), None) if not email: email = next((e.get("email") for e in emails_response if e.get("verified")), None) if not email: - email = next((e.get("email") for e in emails_response if e.get("primary")), None) - if not email and emails_response: - # If no primary email, use the first one - email = emails_response[0].get("email") + raise AuthenticationException( + error_code=AUTHENTICATION_ERROR_CODES["OAUTH_PROVIDER_UNVERIFIED_EMAIL"], + error_message="OAUTH_PROVIDER_UNVERIFIED_EMAIL", + ) return email except requests.RequestException: raise AuthenticationException( @@ -153,10 +155,10 @@ def set_user_data(self): "Accept": "application/json", } - # Get email if not provided in user info - email = user_info_response.get("email") - if not email: - email = self.__get_email(headers=headers) + # Always use __get_email() which enforces the verified-email requirement. + # The user object's .email field carries no verification flag, so it cannot + # be trusted directly (GHSA-7j95-vh8g-f365). + email = self.__get_email(headers=headers) super().set_user_data( { diff --git a/apps/api/plane/authentication/provider/oauth/github.py b/apps/api/plane/authentication/provider/oauth/github.py index 363cd722e5e..852d8d0f66f 100644 --- a/apps/api/plane/authentication/provider/oauth/github.py +++ b/apps/api/plane/authentication/provider/oauth/github.py @@ -117,12 +117,17 @@ def __get_email(self, headers): error_code=AUTHENTICATION_ERROR_CODES["GITHUB_OAUTH_PROVIDER_ERROR"], error_message="GITHUB_OAUTH_PROVIDER_ERROR", ) - email = next((email["email"] for email in emails_response if email["primary"]), None) + # Require both primary AND verified — an unverified primary email can be + # exploited to take over an existing account (GHSA-7j95-vh8g-f365). + email = next( + (e["email"] for e in emails_response if e.get("primary") and e.get("verified")), + None, + ) if not email: - self.logger.error("No primary email found for user") + self.logger.error("No primary verified email found for GitHub user") raise AuthenticationException( - error_code=AUTHENTICATION_ERROR_CODES["GITHUB_OAUTH_PROVIDER_ERROR"], - error_message="GITHUB_OAUTH_PROVIDER_ERROR", + error_code=AUTHENTICATION_ERROR_CODES["OAUTH_PROVIDER_UNVERIFIED_EMAIL"], + error_message="OAUTH_PROVIDER_UNVERIFIED_EMAIL", ) return email except requests.RequestException: diff --git a/apps/api/plane/authentication/provider/oauth/gitlab.py b/apps/api/plane/authentication/provider/oauth/gitlab.py index 088987c2379..7efb9ac94bf 100644 --- a/apps/api/plane/authentication/provider/oauth/gitlab.py +++ b/apps/api/plane/authentication/provider/oauth/gitlab.py @@ -108,6 +108,13 @@ def set_token_data(self): def set_user_data(self): user_info_response = self.get_user_response() + # confirmed_at is null/absent for unverified GitLab accounts. Reject them to + # prevent ATO via self-hosted GitLab with unverified emails (GHSA-7j95-vh8g-f365). + if not user_info_response.get("confirmed_at"): + raise AuthenticationException( + error_code=AUTHENTICATION_ERROR_CODES["OAUTH_PROVIDER_UNVERIFIED_EMAIL"], + error_message="OAUTH_PROVIDER_UNVERIFIED_EMAIL", + ) email = user_info_response.get("email") super().set_user_data( { diff --git a/apps/api/plane/authentication/provider/oauth/google.py b/apps/api/plane/authentication/provider/oauth/google.py index b02eda87de3..ce15a050651 100644 --- a/apps/api/plane/authentication/provider/oauth/google.py +++ b/apps/api/plane/authentication/provider/oauth/google.py @@ -102,6 +102,14 @@ def set_token_data(self): def set_user_data(self): user_info_response = self.get_user_response() + # Reject unverified emails — an attacker-controlled provider could otherwise assert + # any email to match an existing account (GHSA-7j95-vh8g-f365). Fail closed: treat + # an absent verified_email claim the same as verified_email=false. + if user_info_response.get("verified_email") is not True: + raise AuthenticationException( + error_code=AUTHENTICATION_ERROR_CODES["OAUTH_PROVIDER_UNVERIFIED_EMAIL"], + error_message="OAUTH_PROVIDER_UNVERIFIED_EMAIL", + ) user_data = { "email": user_info_response.get("email"), "user": {