Add Google reCAPTCHA for mobile app phone verification (#1373)
# What this PR does Adds reCAPTCHA validation to the get mobile verification code endpoint ## Which issue(s) this PR fixes ## Checklist - [x] Tests updated - [ ] Documentation added (N/A) - [x] `CHANGELOG.md` updated --------- Co-authored-by: Maxim <maxim.mordasov@grafana.com>
This commit is contained in:
parent
2cf83a9650
commit
c55a9010f7
13 changed files with 168 additions and 42 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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) => {
|
|||
/>
|
||||
</WithPermissionControl>
|
||||
</Field>
|
||||
|
||||
{!user.verified_phone_number && (
|
||||
<Input
|
||||
ref={codeInputRef}
|
||||
|
|
@ -211,7 +217,20 @@ const PhoneVerification = observer((props: PhoneVerificationProps) => {
|
|||
className={cx('phone__field')}
|
||||
/>
|
||||
)}
|
||||
|
||||
<HorizontalGroup spacing="xs">
|
||||
<Icon name="info-circle" />
|
||||
<Text type="secondary">
|
||||
This site is protected by reCAPTCHA and the Google{' '}
|
||||
<a target="_blank" rel="noreferrer" href="https://policies.google.com/privacy">
|
||||
<Text type="link">Privacy Policy</Text>
|
||||
</a>{' '}
|
||||
and{' '}
|
||||
<a target="_blank" rel="noreferrer" href="https://policies.google.com/terms">
|
||||
<Text type="link">Terms of Service </Text>
|
||||
</a>{' '}
|
||||
apply.
|
||||
</Text>
|
||||
</HorizontalGroup>
|
||||
{showToggle && (
|
||||
<div className={cx('switch')}>
|
||||
<div className={cx('switch__icon')}>
|
||||
|
|
@ -315,25 +334,25 @@ function PhoneVerificationButtonsGroup({
|
|||
)}
|
||||
|
||||
{user.verified_phone_number && (
|
||||
<WithPermissionControl userAction={action}>
|
||||
<Button
|
||||
disabled={!user?.verified_phone_number || !isTwilioConfigured || isTestCallInProgress}
|
||||
onClick={handleMakeTestCallClick}
|
||||
>
|
||||
{isTestCallInProgress ? 'Making Test Call...' : 'Make Test Call'}
|
||||
</Button>
|
||||
</WithPermissionControl>
|
||||
<>
|
||||
<WithPermissionControl userAction={action}>
|
||||
<Button
|
||||
disabled={!user?.verified_phone_number || !isTwilioConfigured || isTestCallInProgress}
|
||||
onClick={handleMakeTestCallClick}
|
||||
>
|
||||
{isTestCallInProgress ? 'Making Test Call...' : 'Make Test Call'}
|
||||
</Button>
|
||||
</WithPermissionControl>
|
||||
<Tooltip content={'Click "Make Test Call" to save a phone number and add it to DnD exceptions.'}>
|
||||
<Icon
|
||||
name="info-circle"
|
||||
style={{
|
||||
marginLeft: '10px',
|
||||
}}
|
||||
/>
|
||||
</Tooltip>
|
||||
</>
|
||||
)}
|
||||
|
||||
<Tooltip content={'Click "Make Test Call" to save a phone number and add it to DnD exceptions.'}>
|
||||
<Icon
|
||||
name="info-circle"
|
||||
style={{
|
||||
marginLeft: '10px',
|
||||
color: '#1890ff',
|
||||
}}
|
||||
/>
|
||||
</Tooltip>
|
||||
</HorizontalGroup>
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 },
|
||||
});
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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 <RT = any>(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 <RT = any>(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 <RT = any>(path: string, config: RequestConfig)
|
|||
params,
|
||||
data,
|
||||
validateStatus,
|
||||
headers,
|
||||
})
|
||||
.then((response) => {
|
||||
FaroHelper.faro?.api.pushEvent('Request completed', { url });
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -48,3 +48,7 @@
|
|||
padding-left: 4px;
|
||||
padding-right: 4px;
|
||||
}
|
||||
|
||||
.grecaptcha-badge {
|
||||
visibility: hidden;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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';
|
||||
|
|
|
|||
6
grafana-plugin/src/utils/loadJs.ts
Normal file
6
grafana-plugin/src/utils/loadJs.ts
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
export default function loadJs(url: string) {
|
||||
let script = document.createElement('script');
|
||||
script.src = url;
|
||||
|
||||
document.head.appendChild(script);
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue