diff --git a/CHANGELOG.md b/CHANGELOG.md index 74dc54b6..eafc9c10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Added + +- Add reCAPTCHA validation for requesting a mobile verification code + ### Changed - Add ratelimits for phone verification diff --git a/engine/apps/api/serializers/user.py b/engine/apps/api/serializers/user.py index 804cdbfa..7ad0c444 100644 --- a/engine/apps/api/serializers/user.py +++ b/engine/apps/api/serializers/user.py @@ -3,6 +3,7 @@ import time import typing from django.conf import settings +from drf_recaptcha.fields import ReCaptchaV3Field from rest_framework import serializers from apps.api.permissions import DONT_USE_LEGACY_PERMISSION_MAPPING @@ -212,3 +213,7 @@ class FilterUserSerializer(EagerLoadingMixin, serializers.ModelSerializer): "pk", "username", ] + + +class MobileVerificationCodeRecaptchaSerializer(serializers.Serializer): + recaptcha = ReCaptchaV3Field(action="mobile_verification_code") diff --git a/engine/apps/api/tests/test_user.py b/engine/apps/api/tests/test_user.py index ad90e264..021562f3 100644 --- a/engine/apps/api/tests/test_user.py +++ b/engine/apps/api/tests/test_user.py @@ -3,6 +3,7 @@ from unittest.mock import Mock, patch import pytest from django.core.cache import cache from django.core.exceptions import ObjectDoesNotExist +from django.test import override_settings from django.urls import reverse from django.utils import timezone from rest_framework import status @@ -1590,3 +1591,38 @@ def test_phone_number_verification_flow_ratelimit_per_org( url = reverse("api-internal:user-verify-number", kwargs={"pk": second_user.public_primary_key}) response = client.put(f"{url}?token=12345", format="json", **make_user_auth_headers(second_user, token)) assert response.status_code == status.HTTP_429_TOO_MANY_REQUESTS + + +@patch("apps.twilioapp.phone_manager.PhoneManager.send_verification_code", return_value=True) +@pytest.mark.parametrize( + "recaptcha_testing_pass,expected_status", + [ + (True, status.HTTP_200_OK), + (False, status.HTTP_400_BAD_REQUEST), + ], +) +@pytest.mark.django_db +def test_phone_number_verification_recaptcha( + mock_verification_start, + make_organization_and_user_with_plugin_token, + make_user_auth_headers, + recaptcha_testing_pass, + expected_status, +): + _, user, token = make_organization_and_user_with_plugin_token() + + recaptcha_token = "asdasdfasdf" + client = APIClient() + request_headers = {"HTTP_X-OnCall-Recaptcha": recaptcha_token, **make_user_auth_headers(user, token)} + + url = reverse("api-internal:user-get-verification-code", kwargs={"pk": user.public_primary_key}) + + with override_settings(DRF_RECAPTCHA_TESTING_PASS=recaptcha_testing_pass): + response = client.get(url, format="json", **request_headers) + + assert response.status_code == expected_status + + if expected_status == status.HTTP_200_OK: + mock_verification_start.assert_called_once_with() + else: + mock_verification_start.assert_not_called() diff --git a/engine/apps/api/views/user.py b/engine/apps/api/views/user.py index 6fcd2123..7cf27705 100644 --- a/engine/apps/api/views/user.py +++ b/engine/apps/api/views/user.py @@ -23,7 +23,12 @@ from apps.api.permissions import ( user_is_authorized, ) from apps.api.serializers.team import TeamSerializer -from apps.api.serializers.user import FilterUserSerializer, UserHiddenFieldsSerializer, UserSerializer +from apps.api.serializers.user import ( + FilterUserSerializer, + MobileVerificationCodeRecaptchaSerializer, + UserHiddenFieldsSerializer, + UserSerializer, +) from apps.api.throttlers import ( GetPhoneVerificationCodeThrottlerPerOrg, GetPhoneVerificationCodeThrottlerPerUser, @@ -292,11 +297,29 @@ class UserView( throttle_classes=[GetPhoneVerificationCodeThrottlerPerUser, GetPhoneVerificationCodeThrottlerPerOrg], ) def get_verification_code(self, request, pk): + """ + See `DRF_RECAPTCHA_TESTING` in `settings/base.py` + and [here](https://github.com/llybin/drf-recaptcha#testing) to better understand + when the recaptcha checks are actually made + """ + logger.info("Validating reCAPTCHA code") + + serializer = MobileVerificationCodeRecaptchaSerializer( + data={"recaptcha": request.headers.get("X-OnCall-Recaptcha", "some-non-null-value")}, + context={"request": request}, + ) + + if not serializer.is_valid(): + logger.warning(f"Invalid reCAPTCHA validation: {serializer._errors}") + return Response(status=status.HTTP_400_BAD_REQUEST) + logger.info("reCAPTCHA code is valid") + user = self.get_object() phone_manager = PhoneManager(user) code_sent = phone_manager.send_verification_code() if not code_sent: + logger.warning(f"Mobile app verification code was not successfully sent") return Response(status=status.HTTP_400_BAD_REQUEST) return Response(status=status.HTTP_200_OK) diff --git a/engine/requirements.txt b/engine/requirements.txt index 3c86476e..b82fd31a 100644 --- a/engine/requirements.txt +++ b/engine/requirements.txt @@ -48,3 +48,4 @@ opentelemetry-instrumentation-wsgi==0.36b0 opentelemetry-exporter-otlp-proto-grpc==1.15.0 pyroscope-io==0.8.1 django-dbconn-retry==0.1.7 +drf-recaptcha==2.2.2 diff --git a/engine/settings/base.py b/engine/settings/base.py index 39c2ad3d..7ff98ec4 100644 --- a/engine/settings/base.py +++ b/engine/settings/base.py @@ -222,6 +222,7 @@ INSTALLED_APPS = [ "django_migration_linter", "fcm_django", "django_dbconn_retry", + "drf_recaptcha", ] REST_FRAMEWORK = { @@ -655,6 +656,18 @@ if OSS_INSTALLATION: "args": (), } # noqa +# google recaptcha is disabled by default +# +# without setting DRF_RECAPTCHA_TESTING, drf_recaptcha complains with +# AttributeError: 'Settings' object has no attribute 'DRF_RECAPTCHA_SECRET_KEY' +# +# Set DRF_RECAPTCHA_TESTING=True in settings, no request to Google, no warnings +# DRF_RECAPTCHA_SECRET_KEY is not required, set returning verification result in setting below. +DRF_RECAPTCHA_SECRET_KEY = os.environ.get("DRF_RECAPTCHA_SECRET_KEY", default=None) +DRF_RECAPTCHA_DEFAULT_V3_SCORE = 0.5 +DRF_RECAPTCHA_TESTING = True +DRF_RECAPTCHA_TESTING_PASS = True + MIGRATION_LINTER_OPTIONS = {"exclude_apps": ["social_django", "silk", "fcm_django"]} # Run migrations linter on each `python manage.py makemigrations` MIGRATION_LINTER_OVERRIDE_MAKEMIGRATIONS = True diff --git a/grafana-plugin/src/containers/UserSettings/parts/tabs/PhoneVerification/PhoneVerification.tsx b/grafana-plugin/src/containers/UserSettings/parts/tabs/PhoneVerification/PhoneVerification.tsx index 4063ba64..d9f2078b 100644 --- a/grafana-plugin/src/containers/UserSettings/parts/tabs/PhoneVerification/PhoneVerification.tsx +++ b/grafana-plugin/src/containers/UserSettings/parts/tabs/PhoneVerification/PhoneVerification.tsx @@ -12,6 +12,7 @@ import { AppFeature } from 'state/features'; import { useStore } from 'state/useStore'; import { openErrorNotification } from 'utils'; import { isUserActionAllowed, UserAction, UserActions } from 'utils/authorization'; +import { reCAPTCHA_site_key } from 'utils/consts'; import styles from './PhoneVerification.module.css'; @@ -99,26 +100,32 @@ const PhoneVerification = observer((props: PhoneVerificationProps) => { openErrorNotification(error.response.data); }); } else { - await userStore.updateUser({ - pk: userPk, - email: user.email, - unverified_phone_number: phone, + window.grecaptcha.ready(function () { + window.grecaptcha + .execute(reCAPTCHA_site_key, { action: 'mobile_verification_code' }) + .then(async function (token) { + await userStore.updateUser({ + pk: userPk, + email: user.email, + unverified_phone_number: phone, + }); + + userStore + .fetchVerificationCode(userPk, token) + .then(() => { + setState({ isCodeSent: true }); + + if (codeInputRef.current) { + codeInputRef.current.focus(); + } + }) + .catch(() => { + openErrorNotification( + 'Grafana OnCall is unable to verify your phone number due to incorrect number or verification service being unavailable.' + ); + }); + }); }); - - userStore - .fetchVerificationCode(userPk) - .then(() => { - setState({ isCodeSent: true }); - - if (codeInputRef.current) { - codeInputRef.current.focus(); - } - }) - .catch(() => { - openErrorNotification( - 'Grafana OnCall is unable to verify your phone number due to incorrect number or verification service being unavailable.' - ); - }); } }, [ code, @@ -200,7 +207,6 @@ const PhoneVerification = observer((props: PhoneVerificationProps) => { /> - {!user.verified_phone_number && ( { className={cx('phone__field')} /> )} - + + + + This site is protected by reCAPTCHA and the Google{' '} + + Privacy Policy + {' '} + and{' '} + + Terms of Service + {' '} + apply. + + {showToggle && (
@@ -315,25 +334,25 @@ function PhoneVerificationButtonsGroup({ )} {user.verified_phone_number && ( - - - + <> + + + + + + + )} - - - - ); } diff --git a/grafana-plugin/src/models/user/user.ts b/grafana-plugin/src/models/user/user.ts index fffa31c1..101a5f8b 100644 --- a/grafana-plugin/src/models/user/user.ts +++ b/grafana-plugin/src/models/user/user.ts @@ -233,9 +233,10 @@ export class UserStore extends BaseStore { } @action - async fetchVerificationCode(userPk: User['pk']) { + async fetchVerificationCode(userPk: User['pk'], recaptchaToken: string) { await makeRequest(`/users/${userPk}/get_verification_code/`, { method: 'GET', + headers: { 'X-OnCall-Recaptcha': recaptchaToken }, }); } diff --git a/grafana-plugin/src/network/index.ts b/grafana-plugin/src/network/index.ts index 53b1f579..175cda1f 100644 --- a/grafana-plugin/src/network/index.ts +++ b/grafana-plugin/src/network/index.ts @@ -32,12 +32,15 @@ interface RequestConfig { data?: any; withCredentials?: boolean; validateStatus?: (status: number) => boolean; + headers?: { + [key: string]: string | number; + }; } export const isNetworkError = axios.isAxiosError; export const makeRequest = async (path: string, config: RequestConfig) => { - const { method = 'GET', params, data, validateStatus } = config; + const { method = 'GET', params, data, validateStatus, headers } = config; const url = `${API_PROXY_PREFIX}${API_PATH_PREFIX}${path}`; const otel = FaroHelper.faro?.api?.getOTEL(); @@ -63,6 +66,7 @@ export const makeRequest = async (path: string, config: RequestConfig) params, data, validateStatus, + headers, }) .then((response) => { FaroHelper.faro.api.pushEvent('Request completed', { url }); @@ -86,6 +90,7 @@ export const makeRequest = async (path: string, config: RequestConfig) params, data, validateStatus, + headers, }) .then((response) => { FaroHelper.faro?.api.pushEvent('Request completed', { url }); diff --git a/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx b/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx index 9c0dbd3f..d71910c6 100644 --- a/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx +++ b/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx @@ -40,6 +40,8 @@ import 'interceptors'; import { rootStore } from 'state'; import { useStore } from 'state/useStore'; import { isUserActionAllowed } from 'utils/authorization'; +import { reCAPTCHA_site_key } from 'utils/consts'; +import loadJs from 'utils/loadJs'; dayjs.extend(utc); dayjs.extend(timezone); @@ -89,6 +91,10 @@ export const Root = observer((props: AppRootProps) => { }; }, []); + useEffect(() => { + loadJs(`https://www.google.com/recaptcha/api.js?render=${reCAPTCHA_site_key}`); + }, []); + const updateBasicData = async () => { await store.updateBasicData(); setDidFinishLoading(true); diff --git a/grafana-plugin/src/style/global.css b/grafana-plugin/src/style/global.css index 85cf4a32..5d47bdc4 100644 --- a/grafana-plugin/src/style/global.css +++ b/grafana-plugin/src/style/global.css @@ -48,3 +48,7 @@ padding-left: 4px; padding-right: 4px; } + +.grecaptcha-badge { + visibility: hidden; +} diff --git a/grafana-plugin/src/utils/consts.ts b/grafana-plugin/src/utils/consts.ts index 57e004ba..fdc0636f 100644 --- a/grafana-plugin/src/utils/consts.ts +++ b/grafana-plugin/src/utils/consts.ts @@ -15,6 +15,9 @@ export const DEFAULT_PAGE = 'incidents'; export const PLUGIN_ROOT = '/a/grafana-oncall-app'; +// https://developers.google.com/recaptcha/docs/v3 +export const reCAPTCHA_site_key = '6LeIPJ8kAAAAAJdUfjO3uUtQtVxsYf93y46mTec1'; + // Environment options list for onCallApiUrl export const ONCALL_PROD = 'https://oncall-prod-us-central-0.grafana.net/oncall'; export const ONCALL_OPS = 'https://oncall-ops-us-east-0.grafana.net/oncall'; diff --git a/grafana-plugin/src/utils/loadJs.ts b/grafana-plugin/src/utils/loadJs.ts new file mode 100644 index 00000000..83e7d4e6 --- /dev/null +++ b/grafana-plugin/src/utils/loadJs.ts @@ -0,0 +1,6 @@ +export default function loadJs(url: string) { + let script = document.createElement('script'); + script.src = url; + + document.head.appendChild(script); +}