Skip to content

Commit

Permalink
Merge pull request #2182 from City-of-Helsinki/HL-891-csrf-token
Browse files Browse the repository at this point in the history
HL-891: Fix CSRF token issues
  • Loading branch information
mjturt authored Aug 22, 2023
2 parents 002d87a + cc4f2d7 commit 8ca325a
Show file tree
Hide file tree
Showing 17 changed files with 82 additions and 11 deletions.
1 change: 1 addition & 0 deletions backend/benefit/helsinkibenefit/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@
CSRF_TRUSTED_ORIGINS = env.list("CSRF_TRUSTED_ORIGINS")
CSRF_COOKIE_NAME = env.str("CSRF_COOKIE_NAME")
CSRF_COOKIE_SECURE = True
CSRF_USE_SESSIONS = True

# Audit logging
AUDIT_LOG_ORIGIN = env.str("AUDIT_LOG_ORIGIN")
Expand Down
2 changes: 1 addition & 1 deletion backend/benefit/helsinkibenefit/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
path("v1/company/", GetUsersOrganizationView.as_view()),
path("v1/company/search/<str:name>/", SearchOrganisationsView.as_view()),
path("v1/company/get/<str:business_id>/", GetOrganisationByIdView.as_view()),
path("v1/users/me/", CurrentUserView.as_view()),
path("v1/users/me/", CurrentUserView.as_view(), name="users-me"),
path("v1/users/options/", UserOptionsView.as_view()),
path(
"v1/handlerapplications/<str:application_id>/review/", ReviewStateView.as_view()
Expand Down
5 changes: 4 additions & 1 deletion backend/benefit/users/api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from django.contrib.auth.base_user import AbstractBaseUser
from django.core.exceptions import PermissionDenied
from django.db import DatabaseError, transaction
from django.middleware.csrf import get_token
from django.shortcuts import get_object_or_404
from drf_spectacular.utils import extend_schema
from helsinki_gdpr.views import DeletionNotAllowed, DryRunException, GDPRAPIView
Expand Down Expand Up @@ -30,7 +31,9 @@ def get(self, request):
serializer = UserSerializer(
self._get_current_user(request), context={"request": request}
)
return Response(serializer.data)
response = serializer.data
response["csrf_token"] = get_token(request)
return Response(response)

def _get_current_user(self, request):
if not request.user.is_authenticated:
Expand Down
13 changes: 13 additions & 0 deletions backend/benefit/users/tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,23 @@
import factory
import pytest
from rest_framework.test import APIClient

from applications.tests.factories import ApplicationFactory
from common.tests.conftest import * # noqa
from companies.tests.conftest import * # noqa
from helsinkibenefit.tests.conftest import * # noqa


@pytest.fixture
def gdpr_api_client():
return APIClient()


@pytest.fixture
def application(mock_get_organisation_roles_and_create_company):
# Application which belongs to logged in user company
with factory.Faker.override_default_locale("fi_FI"):
app = ApplicationFactory()
app.company = mock_get_organisation_roles_and_create_company
app.save()
return app
11 changes: 11 additions & 0 deletions backend/benefit/users/tests/test_user_api.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
from rest_framework.reverse import reverse

from common.tests.conftest import get_client_user


def test_applications_unauthorized(api_client, application):
response = api_client.get(reverse("users-me"))
user = get_client_user(api_client)
assert response.status_code == 200
assert response.data["id"] == str(user.id)
assert len(response.data["csrf_token"]) > 0
1 change: 1 addition & 0 deletions frontend/benefit/applicant/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,4 +113,5 @@ export const SUBMITTED_STATUSES = [

export enum LOCAL_STORAGE_KEYS {
IS_TERMS_OF_SERVICE_APPROVED = 'isTermsOfServiceApproved',
CSRF_TOKEN = 'csrfToken',
}
5 changes: 3 additions & 2 deletions frontend/benefit/applicant/src/hooks/useUserQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { useQuery, UseQueryResult } from 'react-query';
import showErrorToast from 'shared/components/toast/show-error-toast';
import useBackendAPI from 'shared/hooks/useBackendAPI';
import useLocale from 'shared/hooks/useLocale';
import { setLocalStorageItem } from 'shared/utils/localstorage.utils';

import { LOCAL_STORAGE_KEYS } from '../constants';

Expand Down Expand Up @@ -51,9 +52,9 @@ const useUserQuery = (
select: (data) => camelcaseKeys(data, { deep: true }),
onError: (error) => handleError(error),
onSuccess: (data) => {
setLocalStorageItem(LOCAL_STORAGE_KEYS.CSRF_TOKEN, data.csrfToken);
if (data.id && data.termsOfServiceApprovalNeeded)
// eslint-disable-next-line scanjs-rules/identifier_localStorage
localStorage.setItem(
setLocalStorageItem(
LOCAL_STORAGE_KEYS.IS_TERMS_OF_SERVICE_APPROVED,
'false'
);
Expand Down
1 change: 1 addition & 0 deletions frontend/benefit/applicant/src/pages/_app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ const App: React.FC<AppProps> = (appProps) => {
<BackendAPIProvider
baseURL={getBackendDomain()}
headers={getHeaders(locale)}
isLocalStorageCsrf
>
<QueryClientProvider client={queryClient}>
<AuthProvider>
Expand Down
8 changes: 4 additions & 4 deletions frontend/benefit/applicant/src/pages/login.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ import {
$GridCell,
} from 'shared/components/forms/section/FormSection.sc';
import getServerSideTranslations from 'shared/i18n/get-server-side-translations';
import { removeLocalStorageItem } from 'shared/utils/localstorage.utils';

import { IS_CLIENT, LOCAL_STORAGE_KEYS } from '../constants';
import { LOCAL_STORAGE_KEYS } from '../constants';

type NotificationProps =
| (Pick<HDSNotificationProps, 'type' | 'label'> & {
Expand Down Expand Up @@ -60,9 +61,8 @@ const Login: NextPage = () => {
}, [logout, queryClient]);

useEffect(() => {
if (IS_CLIENT)
// eslint-disable-next-line scanjs-rules/identifier_localStorage
localStorage.removeItem(LOCAL_STORAGE_KEYS.IS_TERMS_OF_SERVICE_APPROVED);
removeLocalStorageItem(LOCAL_STORAGE_KEYS.IS_TERMS_OF_SERVICE_APPROVED);
removeLocalStorageItem(LOCAL_STORAGE_KEYS.CSRF_TOKEN);
}, []);

return (
Expand Down
4 changes: 4 additions & 0 deletions frontend/benefit/handler/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,3 +193,7 @@ export const ALL_APPLICATION_STATUSES: APPLICATION_STATUSES[] = [
APPLICATION_STATUSES.DRAFT,
APPLICATION_STATUSES.REJECTED,
];

export enum LOCAL_STORAGE_KEYS {
CSRF_TOKEN = 'csrfToken',
}
10 changes: 8 additions & 2 deletions frontend/benefit/handler/src/hooks/useUserQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import { useQuery, UseQueryResult } from 'react-query';
import useBackendAPI from 'shared/hooks/useBackendAPI';
import useLocale from 'shared/hooks/useLocale';
import User from 'shared/types/user';
import { setLocalStorageItem } from 'shared/utils/localstorage.utils';

import { ROUTES } from '../constants';
import { LOCAL_STORAGE_KEYS, ROUTES } from '../constants';
import useLogout from './useLogout';

// check that authentication is still alive in every 5 minutes
Expand Down Expand Up @@ -46,6 +47,11 @@ const useUserQuery = <T extends User>(
}
};

const onSuccessHandler = (user: User): void => {
checkForStaffStatus(user);
setLocalStorageItem(LOCAL_STORAGE_KEYS.CSRF_TOKEN, user.csrf_token);
};

return useQuery(
`${BackendEndpoint.USER_ME}`,
() => handleResponse<User>(axios.get(BackendEndpoint.USER_ME)),
Expand All @@ -54,7 +60,7 @@ const useUserQuery = <T extends User>(
enabled: !logout,
retry: false,
select,
onSuccess: checkForStaffStatus,
onSuccess: onSuccessHandler,
onError: (error) => handleError(error),
}
);
Expand Down
1 change: 1 addition & 0 deletions frontend/benefit/handler/src/pages/_app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const App: React.FC<AppProps> = (appProps) => {
<BackendAPIProvider
baseURL={getBackendDomain()}
headers={getHeaders(locale)}
isLocalStorageCsrf
>
<AppContextProvider>
<QueryClientProvider client={queryClient}>
Expand Down
7 changes: 7 additions & 0 deletions frontend/benefit/handler/src/pages/login.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,11 @@ import React, { useEffect } from 'react';
import { useQueryClient } from 'react-query';
import Container from 'shared/components/container/Container';
import getServerSideTranslations from 'shared/i18n/get-server-side-translations';
import { removeLocalStorageItem } from 'shared/utils/localstorage.utils';
import { useTheme } from 'styled-components';

import { LOCAL_STORAGE_KEYS } from '../constants';

type NotificationProps = Pick<HDSNotificationProps, 'type' | 'label'> & {
content?: string;
};
Expand Down Expand Up @@ -61,6 +64,10 @@ const Login: NextPage = () => {
}
}, [router.query.logout, router.query.userStateError, queryClient]);

useEffect(() => {
removeLocalStorageItem(LOCAL_STORAGE_KEYS.CSRF_TOKEN);
}, []);

return (
<Container>
<Notification
Expand Down
2 changes: 2 additions & 0 deletions frontend/benefit/shared/src/types/application.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@ export type UserData = {
last_name: string;
terms_of_service_approval_needed: boolean;
terms_of_service_in_effect: TermsOfServiceInEffectData;
csrf_token: string;
};

export type ApplicantConsents = {
Expand All @@ -528,6 +529,7 @@ export type User = {
lastName: string;
termsOfServiceApprovalNeeded: boolean;
termsOfServiceInEffect: TermsOfServiceInEffect;
csrfToken: string;
};

export type ApproveTermsOfServiceResponseData = {
Expand Down
7 changes: 6 additions & 1 deletion frontend/shared/src/backend-api/BackendAPIProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,30 @@ import Axios from 'axios';
import React from 'react';
import { getLastCookieValue } from 'shared/cookies/get-last-cookie-value';
import { Headers } from 'shared/types/common';
import { getLocalStorageItem } from 'shared/utils/localstorage.utils';

import BackendAPIContext from './BackendAPIContext';

export interface BackendAPIProviderProps {
baseURL: string;
headers?: Headers;
isLocalStorageCsrf?: boolean;
}

const BackendAPIProvider: React.FC<BackendAPIProviderProps> = ({
baseURL,
headers,
isLocalStorageCsrf = false,
children,
}): JSX.Element => (
<BackendAPIContext.Provider
value={Axios.create({
baseURL,
headers: {
'Content-Type': 'application/json',
'X-CSRFToken': getLastCookieValue('yjdhcsrftoken'),
'X-CSRFToken': isLocalStorageCsrf
? getLocalStorageItem('csrfToken')
: getLastCookieValue('yjdhcsrftoken'),
...headers,
},
withCredentials: true,
Expand Down
1 change: 1 addition & 0 deletions frontend/shared/src/types/user.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@ type User = {
name: string;
organization_name?: string;
is_staff?: boolean;
csrf_token?: string;
};
export default User;
14 changes: 14 additions & 0 deletions frontend/shared/src/utils/localstorage.utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/* eslint-disable scanjs-rules/identifier_localStorage */

const IS_CLIENT = typeof window !== 'undefined';

export const getLocalStorageItem = (key: string): string =>
IS_CLIENT ? localStorage.getItem(key) || '' : '';

export const setLocalStorageItem = (key: string, value: string): void =>
IS_CLIENT && localStorage.setItem(key, value);

export const removeLocalStorageItem = (key: string): void =>
IS_CLIENT && localStorage.removeItem(key);

/* eslint-enable scanjs-rules/identifier_localStorage */

0 comments on commit 8ca325a

Please sign in to comment.