Skip to content

Commit

Permalink
fix(UVE): Unauthorized requests error due to Cookie Rejection on Fire…
Browse files Browse the repository at this point in the history
…fox (#31148)

This pull request includes changes to enhance the security and
functionality of the `edit-ema-editor` component. The most important
changes include adding a sandbox attribute to the iframe and modifying
the iframe URL generation to avoid caching issues.

Enhancements to security and functionality:

*
[`core-web/libs/portlets/edit-ema/portlet/src/lib/edit-ema-editor/edit-ema-editor.component.html`](diffhunk://#diff-7f93b3d5700edd531f6ee9f65ed695175ee686d7b1cfa63b46de3669e65fb1efR42):
Added the `sandbox` attribute to the iframe to enhance security by
restricting the actions the iframe can perform.
*
[`core-web/libs/portlets/edit-ema/portlet/src/lib/store/features/editor/withEditor.ts`](diffhunk://#diff-86e692578757ed7f4f6cba5d0aeb07641312f3b17885825d1a45987153ae87f0L53-R55):
Modified the iframe URL generation to avoid caching issues by returning
a new string reference, ensuring the iframe reloads correctly.



## Screenshot



https://github.com/user-attachments/assets/31f15b34-a547-48bf-a6d5-13721fc8541e
  • Loading branch information
zJaaal authored Jan 16, 2025
1 parent f07c0ce commit cb68f9e
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
pointerEvents: $editorProps().iframe.pointerEvents,
opacity: $editorProps().iframe.opacity
}"
sandbox="allow-scripts allow-same-origin allow-forms"
#iframe
data-testId="iframe"
width="100%"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2660,7 +2660,7 @@ describe('EditEmaEditorComponent', () => {
iframe.nativeElement.dispatchEvent(new Event('load'));
spectator.detectChanges();

expect(iframe.nativeElement.src).toContain('about:blank'); //When dont have src, the src is the same as the current page
expect(iframe.nativeElement.src).toContain('http://localhost/'); //When dont have src, the src is the same as the current page
expect(iframe.nativeElement.contentDocument.body.innerHTML).toContain(
'<div>New Content - Hello World</div>'
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -686,13 +686,22 @@ describe('withEditor', () => {
);
});

it('should contain `about:blanck` in src when the page is traditional', () => {
it('should be an instance of String in src when the page is traditional', () => {
patchState(store, {
pageAPIResponse: MOCK_RESPONSE_VTL,
isTraditionalPage: true
});

expect(store.$iframeURL()).toContain('about:blank');
expect(store.$iframeURL()).toBeInstanceOf(String);
});

it('should be an empty string in src when the page is traditional', () => {
patchState(store, {
pageAPIResponse: MOCK_RESPONSE_VTL,
isTraditionalPage: true
});

expect(store.$iframeURL().toString()).toBe('');
});

it('should contain the right url when the page is a vanity url ', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ import { withClient } from '../client/withClient';
const buildIframeURL = ({ pageURI, params, isTraditionalPage }) => {
if (isTraditionalPage) {
// Force iframe reload on every page load to avoid caching issues and window dirty state
return `about:blank?t=${Date.now()}`;
// We need a new reference to avoid the iframe to be cached
// More reference: https://github.com/dotCMS/core/issues/30981
return new String('');
}

const pageAPIQueryParams = createPageApiUrlWithQueryParams(pageURI, params);
Expand Down Expand Up @@ -198,7 +200,7 @@ export function withEditor() {
: null
};
}),
$iframeURL: computed<string>(() => {
$iframeURL: computed<string | InstanceType<typeof String>>(() => {
const page = store.pageAPIResponse().page;
const vanityURL = store.pageAPIResponse().vanityUrl?.url;

Expand Down
11 changes: 11 additions & 0 deletions core-web/libs/ui/src/lib/pipes/safe-url/safe-url.pipe.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,15 @@ describe('SafeUrlPipe', () => {
expect(transformedUrl).toBe(safeUrl);
expect(sanitizer.bypassSecurityTrustResourceUrl).toHaveBeenCalledWith(url);
});
it('should transform URL to a safe resource URL when the URL is an instance of String', () => {
const url = new String('http://example.com');
const safeUrl: SafeResourceUrl = 'safeUrl: http://example.com';

(sanitizer.bypassSecurityTrustResourceUrl as jasmine.Spy).and.returnValue(safeUrl);

const transformedUrl = pipe.transform(url);

expect(transformedUrl).toBe(safeUrl);
expect(sanitizer.bypassSecurityTrustResourceUrl).toHaveBeenCalledWith(url.toString());
});
});
4 changes: 2 additions & 2 deletions core-web/libs/ui/src/lib/pipes/safe-url/safe-url.pipe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { DomSanitizer } from '@angular/platform-browser';
export class SafeUrlPipe implements PipeTransform {
constructor(private sanitizer: DomSanitizer) {}

transform(url: string) {
return this.sanitizer.bypassSecurityTrustResourceUrl(url);
transform(url: string | InstanceType<typeof String>) {
return this.sanitizer.bypassSecurityTrustResourceUrl(url.toString());
}
}

0 comments on commit cb68f9e

Please sign in to comment.