Skip to content

Commit

Permalink
feature #2251 [LiveComponent] Remove CSRF tokens - rely on same-origi…
Browse files Browse the repository at this point in the history
…n/CORS instead (nicolas-grekas)

This PR was merged into the 2.x branch.

Discussion
----------

[LiveComponent] Remove CSRF tokens - rely on same-origin/CORS instead

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | could be
| Issues        | Fixes #2177
| License       | MIT

This PR replaces token-based CSRF-protection by same-origin/CORS policies.
It replaces #2250

Commits
-------

e3c0ef5 [LiveComponent] Remove CSRF tokens - rely on same-origin/CORS instead
  • Loading branch information
kbond committed Oct 23, 2024
2 parents d27bb10 + e3c0ef5 commit 45f6504
Show file tree
Hide file tree
Showing 39 changed files with 107 additions and 414 deletions.
6 changes: 6 additions & 0 deletions src/LiveComponent/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# CHANGELOG

## 2.22.0

- Remove CSRF tokens - rely on same-origin/CORS instead

## 2.19.0

- Add `submitForm()` to `TestLiveComponent`.
- Add `live_action` Twig function

Expand Down
4 changes: 1 addition & 3 deletions src/LiveComponent/assets/dist/Backend/Backend.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,19 @@ export interface BackendInterface {
}, files: {
[key: string]: FileList;
}): BackendRequest;
updateCsrfToken(csrfToken: string): void;
}
export interface BackendAction {
name: string;
args: Record<string, string>;
}
export default class implements BackendInterface {
private readonly requestBuilder;
constructor(url: string, method?: 'get' | 'post', csrfToken?: string | null);
constructor(url: string, method?: 'get' | 'post');
makeRequest(props: any, actions: BackendAction[], updated: {
[key: string]: any;
}, children: ChildrenFingerprints, updatedPropsFromParent: {
[key: string]: any;
}, files: {
[key: string]: FileList;
}): BackendRequest;
updateCsrfToken(csrfToken: string): void;
}
4 changes: 1 addition & 3 deletions src/LiveComponent/assets/dist/Backend/RequestBuilder.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ import type { BackendAction, ChildrenFingerprints } from './Backend';
export default class {
private url;
private method;
private csrfToken;
constructor(url: string, method?: 'get' | 'post', csrfToken?: string | null);
constructor(url: string, method?: 'get' | 'post');
buildRequest(props: any, actions: BackendAction[], updated: {
[key: string]: any;
}, children: ChildrenFingerprints, updatedPropsFromParent: {
Expand All @@ -15,5 +14,4 @@ export default class {
fetchOptions: RequestInit;
};
private willDataFitInUrl;
updateCsrfToken(csrfToken: string): void;
}
2 changes: 0 additions & 2 deletions src/LiveComponent/assets/dist/live_controller.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ export default class LiveControllerDefault extends Controller<HTMLElement> imple
type: ObjectConstructor;
default: {};
};
csrf: StringConstructor;
listeners: {
type: ArrayConstructor;
default: never[];
Expand Down Expand Up @@ -59,7 +58,6 @@ export default class LiveControllerDefault extends Controller<HTMLElement> imple
readonly urlValue: string;
readonly propsValue: any;
propsUpdatedFromParentValue: any;
readonly csrfValue: string;
readonly listenersValue: Array<{
event: string;
action: string;
Expand Down
22 changes: 4 additions & 18 deletions src/LiveComponent/assets/dist/live_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -2052,9 +2052,6 @@ class Component {
return response;
}
this.processRerender(html, backendResponse);
if (this.element.dataset.liveCsrfValue) {
this.backend.updateCsrfToken(this.element.dataset.liveCsrfValue);
}
this.backendRequest = null;
thisPromiseResolve(backendResponse);
if (this.isRequestPending) {
Expand Down Expand Up @@ -2255,10 +2252,9 @@ class BackendRequest {
}

class RequestBuilder {
constructor(url, method = 'post', csrfToken = null) {
constructor(url, method = 'post') {
this.url = url;
this.method = method;
this.csrfToken = csrfToken;
}
buildRequest(props, actions, updated, children, updatedPropsFromParent, files) {
const splitUrl = this.url.split('?');
Expand Down Expand Up @@ -2295,9 +2291,6 @@ class RequestBuilder {
if (hasFingerprints) {
requestData.children = children;
}
if (this.csrfToken && (actions.length || totalFiles)) {
fetchOptions.headers['X-CSRF-TOKEN'] = this.csrfToken;
}
if (actions.length > 0) {
if (actions.length === 1) {
requestData.args = actions[0].args;
Expand Down Expand Up @@ -2328,22 +2321,16 @@ class RequestBuilder {
const urlEncodedJsonData = new URLSearchParams(propsJson + updatedJson + childrenJson + propsFromParentJson).toString();
return (urlEncodedJsonData + params.toString()).length < 1500;
}
updateCsrfToken(csrfToken) {
this.csrfToken = csrfToken;
}
}

class Backend {
constructor(url, method = 'post', csrfToken = null) {
this.requestBuilder = new RequestBuilder(url, method, csrfToken);
constructor(url, method = 'post') {
this.requestBuilder = new RequestBuilder(url, method);
}
makeRequest(props, actions, updated, children, updatedPropsFromParent, files) {
const { url, fetchOptions } = this.requestBuilder.buildRequest(props, actions, updated, children, updatedPropsFromParent, files);
return new BackendRequest(fetch(url, fetchOptions), actions.map((backendAction) => backendAction.name), Object.keys(updated));
}
updateCsrfToken(csrfToken) {
this.requestBuilder.updateCsrfToken(csrfToken);
}
}

class StimulusElementDriver {
Expand Down Expand Up @@ -3203,7 +3190,6 @@ LiveControllerDefault.values = {
url: String,
props: { type: Object, default: {} },
propsUpdatedFromParent: { type: Object, default: {} },
csrf: String,
listeners: { type: Array, default: [] },
eventsToEmit: { type: Array, default: [] },
eventsToDispatch: { type: Array, default: [] },
Expand All @@ -3212,6 +3198,6 @@ LiveControllerDefault.values = {
requestMethod: { type: String, default: 'post' },
queryMapping: { type: Object, default: {} },
};
LiveControllerDefault.backendFactory = (controller) => new Backend(controller.urlValue, controller.requestMethodValue, controller.csrfValue);
LiveControllerDefault.backendFactory = (controller) => new Backend(controller.urlValue, controller.requestMethodValue);

export { Component, LiveControllerDefault as default, getComponent };
9 changes: 2 additions & 7 deletions src/LiveComponent/assets/src/Backend/Backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ export interface BackendInterface {
updatedPropsFromParent: { [key: string]: any },
files: { [key: string]: FileList }
): BackendRequest;
updateCsrfToken(csrfToken: string): void;
}

export interface BackendAction {
Expand All @@ -26,8 +25,8 @@ export interface BackendAction {
export default class implements BackendInterface {
private readonly requestBuilder: RequestBuilder;

constructor(url: string, method: 'get' | 'post' = 'post', csrfToken: string | null = null) {
this.requestBuilder = new RequestBuilder(url, method, csrfToken);
constructor(url: string, method: 'get' | 'post' = 'post') {
this.requestBuilder = new RequestBuilder(url, method);
}

makeRequest(
Expand All @@ -53,8 +52,4 @@ export default class implements BackendInterface {
Object.keys(updated)
);
}

updateCsrfToken(csrfToken: string) {
this.requestBuilder.updateCsrfToken(csrfToken);
}
}
12 changes: 1 addition & 11 deletions src/LiveComponent/assets/src/Backend/RequestBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,10 @@ import type { BackendAction, ChildrenFingerprints } from './Backend';
export default class {
private url: string;
private method: 'get' | 'post';
private csrfToken: string | null;

constructor(url: string, method: 'get' | 'post' = 'post', csrfToken: string | null = null) {
constructor(url: string, method: 'get' | 'post' = 'post') {
this.url = url;
this.method = method;
this.csrfToken = csrfToken;
}

buildRequest(
Expand Down Expand Up @@ -64,10 +62,6 @@ export default class {
requestData.children = children;
}

if (this.csrfToken && (actions.length || totalFiles)) {
fetchOptions.headers['X-CSRF-TOKEN'] = this.csrfToken;
}

if (actions.length > 0) {
// one or more ACTIONs

Expand Down Expand Up @@ -117,8 +111,4 @@ export default class {
// if the URL gets remotely close to 2000 chars, it may not fit
return (urlEncodedJsonData + params.toString()).length < 1500;
}

updateCsrfToken(csrfToken: string) {
this.csrfToken = csrfToken;
}
}
5 changes: 0 additions & 5 deletions src/LiveComponent/assets/src/Component/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,11 +329,6 @@ export default class Component {

this.processRerender(html, backendResponse);

// Store updated csrf token
if (this.element.dataset.liveCsrfValue) {
this.backend.updateCsrfToken(this.element.dataset.liveCsrfValue);
}

// finally resolve this promise
this.backendRequest = null;
thisPromiseResolve(backendResponse);
Expand Down
4 changes: 1 addition & 3 deletions src/LiveComponent/assets/src/live_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ export default class LiveControllerDefault extends Controller<HTMLElement> imple
url: String,
props: { type: Object, default: {} },
propsUpdatedFromParent: { type: Object, default: {} },
csrf: String,
listeners: { type: Array, default: [] },
eventsToEmit: { type: Array, default: [] },
eventsToDispatch: { type: Array, default: [] },
Expand All @@ -50,7 +49,6 @@ export default class LiveControllerDefault extends Controller<HTMLElement> imple
declare readonly urlValue: string;
declare readonly propsValue: any;
declare propsUpdatedFromParentValue: any;
declare readonly csrfValue: string;
declare readonly listenersValue: Array<{ event: string; action: string }>;
declare readonly eventsToEmitValue: Array<{
event: string;
Expand Down Expand Up @@ -79,7 +77,7 @@ export default class LiveControllerDefault extends Controller<HTMLElement> imple
private pendingFiles: { [key: string]: HTMLInputElement } = {};

static backendFactory: (controller: LiveControllerDefault) => BackendInterface = (controller) =>
new Backend(controller.urlValue, controller.requestMethodValue, controller.csrfValue);
new Backend(controller.urlValue, controller.requestMethodValue);

initialize() {
this.mutationObserver = new MutationObserver(this.onMutations.bind(this));
Expand Down
19 changes: 8 additions & 11 deletions src/LiveComponent/assets/test/Backend/RequestBuilder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import RequestBuilder from '../../src/Backend/RequestBuilder';

describe('buildRequest', () => {
it('sets basic data on GET request', () => {
const builder = new RequestBuilder('/_components?existing_param=1', 'get', '_the_csrf_token');
const builder = new RequestBuilder('/_components?existing_param=1', 'get');
const { url, fetchOptions } = builder.buildRequest(
{ firstName: 'Ryan' },
[],
Expand All @@ -23,7 +23,7 @@ describe('buildRequest', () => {
});

it('sets basic data on POST request', () => {
const builder = new RequestBuilder('/_components', 'post', '_the_csrf_token');
const builder = new RequestBuilder('/_components', 'post');
const { url, fetchOptions } = builder.buildRequest(
{ firstName: 'Ryan' },
[
Expand All @@ -42,7 +42,6 @@ describe('buildRequest', () => {
expect(fetchOptions.method).toEqual('POST');
expect(fetchOptions.headers).toEqual({
Accept: 'application/vnd.live-component+html',
'X-CSRF-TOKEN': '_the_csrf_token',
'X-Requested-With': 'XMLHttpRequest',
});
const body = <FormData>fetchOptions.body;
Expand All @@ -58,7 +57,7 @@ describe('buildRequest', () => {
});

it('sets basic data on POST request with batch actions', () => {
const builder = new RequestBuilder('/_components', 'post', '_the_csrf_token');
const builder = new RequestBuilder('/_components', 'post');
const { url, fetchOptions } = builder.buildRequest(
{ firstName: 'Ryan' },
[
Expand Down Expand Up @@ -101,7 +100,7 @@ describe('buildRequest', () => {

// when data is too long it makes a post request
it('makes a POST request when data is too long', () => {
const builder = new RequestBuilder('/_components', 'get', '_the_csrf_token');
const builder = new RequestBuilder('/_components', 'get');
const { url, fetchOptions } = builder.buildRequest(
{ firstName: 'Ryan'.repeat(1000) },
[],
Expand Down Expand Up @@ -129,7 +128,7 @@ describe('buildRequest', () => {
});

it('makes a POST request when method is post', () => {
const builder = new RequestBuilder('/_components', 'post', '_the_csrf_token');
const builder = new RequestBuilder('/_components', 'post');
const { url, fetchOptions } = builder.buildRequest(
{
firstName: 'Ryan',
Expand Down Expand Up @@ -161,7 +160,7 @@ describe('buildRequest', () => {
});

it('sends propsFromParent when specified', () => {
const builder = new RequestBuilder('/_components?existing_param=1', 'get', '_the_csrf_token');
const builder = new RequestBuilder('/_components?existing_param=1', 'get');
const { url } = builder.buildRequest({ firstName: 'Ryan' }, [], { firstName: 'Kevin' }, {}, { count: 5 }, {});

expect(url).toEqual(
Expand Down Expand Up @@ -216,7 +215,7 @@ describe('buildRequest', () => {
};

it('Sends file with request', () => {
const builder = new RequestBuilder('/_components', 'post', '_the_csrf_token');
const builder = new RequestBuilder('/_components', 'post');

const { url, fetchOptions } = builder.buildRequest(
{ firstName: 'Ryan' },
Expand All @@ -231,7 +230,6 @@ describe('buildRequest', () => {
expect(fetchOptions.method).toEqual('POST');
expect(fetchOptions.headers).toEqual({
Accept: 'application/vnd.live-component+html',
'X-CSRF-TOKEN': '_the_csrf_token',
'X-Requested-With': 'XMLHttpRequest',
});
const body = <FormData>fetchOptions.body;
Expand All @@ -241,7 +239,7 @@ describe('buildRequest', () => {
});

it('Sends multiple files with request', () => {
const builder = new RequestBuilder('/_components', 'post', '_the_csrf_token');
const builder = new RequestBuilder('/_components', 'post');

const { url, fetchOptions } = builder.buildRequest(
{ firstName: 'Ryan' },
Expand All @@ -256,7 +254,6 @@ describe('buildRequest', () => {
expect(fetchOptions.method).toEqual('POST');
expect(fetchOptions.headers).toEqual({
Accept: 'application/vnd.live-component+html',
'X-CSRF-TOKEN': '_the_csrf_token',
'X-Requested-With': 'XMLHttpRequest',
});
const body = <FormData>fetchOptions.body;
Expand Down
19 changes: 0 additions & 19 deletions src/LiveComponent/assets/test/controller/render.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -630,23 +630,4 @@ describe('LiveController rendering Tests', () => {
// verify the selectedIndex of the select option 2 is 0
expect(selectOption2.selectedIndex).toBe(0);
});

it('backend will have a new csrf token', async () => {
const test = await createTest(
{},
(data: any) => `
<div ${initComponent(data)} data-live-csrf-value="${data.csrf}">
</div>
`
);

test.expectsAjaxCall().serverWillChangeProps((data: any) => {
// change csrf token
data.csrf = 'Hello';
});

await test.component.render();

expect(test.mockedBackend.csrfToken).toEqual('Hello');
});
});
7 changes: 0 additions & 7 deletions src/LiveComponent/assets/test/tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,6 @@ class FunctionalTest {
class MockedBackend implements BackendInterface {
private expectedMockedAjaxCalls: Array<MockedAjaxCall> = [];

public csrfToken: string | null = null;

addMockedAjaxCall(mock: MockedAjaxCall) {
this.expectedMockedAjaxCalls.push(mock);
}
Expand Down Expand Up @@ -141,10 +139,6 @@ class MockedBackend implements BackendInterface {
return matchedMock.createBackendRequest();
}

updateCsrfToken(csrfToken: string) {
this.csrfToken = csrfToken;
}

getExpectedMockedAjaxCalls(): Array<MockedAjaxCall> {
return this.expectedMockedAjaxCalls;
}
Expand Down Expand Up @@ -469,7 +463,6 @@ export function initComponent(props: any = {}, controllerValues: any = {}) {
data-live-url-value="http://localhost/components/_test_component_${Math.round(Math.random() * 1000)}"
data-live-props-value="${dataToJsonAttribute(props)}"
${controllerValues.debounce ? `data-live-debounce-value="${controllerValues.debounce}"` : ''}
${controllerValues.csrf ? `data-live-csrf-value="${controllerValues.csrf}"` : ''}
${controllerValues.id ? `id="${controllerValues.id}"` : ''}
${controllerValues.fingerprint ? `data-live-fingerprint-value="${controllerValues.fingerprint}"` : ''}
${controllerValues.listeners ? `data-live-listeners-value="${dataToJsonAttribute(controllerValues.listeners)}"` : ''}
Expand Down
1 change: 1 addition & 0 deletions src/LiveComponent/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
},
"require": {
"php": ">=8.1",
"symfony/deprecation-contracts": "^2.5|^3.0",
"symfony/property-access": "^5.4.5|^6.0|^7.0",
"symfony/stimulus-bundle": "^2.9",
"symfony/ux-twig-component": "^2.8",
Expand Down
Loading

0 comments on commit 45f6504

Please sign in to comment.