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/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": { 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, ];