Skip to content

Modernize module iframes while preserving dashboard flow (#914) - #920

Open
abhinav-phi wants to merge 1 commit into
OWASP:devfrom
abhinav-phi:dev#914
Open

abhinav-phi wants to merge 1 commit into
OWASP:devfrom
abhinav-phi:dev#914

Conversation

@abhinav-phi

@abhinav-phi abhinav-phi commented Sep 17, 2026

Copy link
Copy Markdown

Closes #914.

Scope

Issue #914 asks for replacing the iframes on the web application with a modern alternative. The module views are full documents with their own scripts and relative paths, while result submission stays in the dashboard. Converting every module page into dashboard-compatible fragments would be a full frontend rewrite, so this PR takes a deliberately bounded approach: it keeps same-origin frames for lesson and challenge content, but modernizes how those frames are created and secured.

Changes

In src/main/webapp/index.jsp, I added isSafeModuleSrc and createModuleFrame helpers and replaced the deprecated frameborder='no' string-injected iframes in all three module loaders (the open/tourney challenge loader, the open/tourney lesson loader, and the CTF-mode lesson loader) with DOM-created frames. The new frames use a title attribute, loading="lazy", referrerpolicy="same-origin", and a sandbox of allow-same-origin allow-scripts allow-forms allow-modals allow-popups allow-popups-to-escape-sandbox allow-downloads. The sandbox blocks top-level navigation so module content cannot break out of the dashboard, while preserving the scripts, forms, alerts, external target="_blank" links, and downloads the existing lessons rely on. The getModule response is validated as a same-origin relative address before loading, and the deprecated jQuery .load() calls were replaced with .on("load"), removing the duplicate appendTo after html().

In src/main/webapp/getStarted.jsp, I fixed the double html() handling that was clearing the welcome frame, and the readyToPlay.jsp frame is now created with the same modern attributes through DOM APIs. In src/main/webapp/css/theCss.css, I added border: 0 to .levelIframe to replace the deprecated frameborder attribute.

No Java, session, CSRF, or getModule/solutionSubmit logic was changed, and the training servlets under servlets/module/lesson and servlets/module/challenge are untouched.

Verification

I verified statically that zero frameborder iframe attributes and zero string-concatenated <iframe creations remain in code (one mention of the word survives only inside a code comment), and that the helper plus sandbox attributes are present in both index.jsp and getStarted.jsp. The JavaScript helper passes node --check, and its same-origin validation correctly accepts relative module paths while rejecting javascript:, data:, absolute, and protocol-relative URLs. The CSS change is confirmed present.

Maven is not installed in this environment, so mvn test and mvn spotless:check were not run. No Java files were changed, but a full build and Docker runtime verification are still pending.

Remaining work and limitations

This still needs maintainer confirmation that keeping sandboxed frames, rather than converting modules into dashboard fragments, matches the intended scope for #914. It also needs CI plus a local Docker run (login, open a lesson and a challenge, submit a result, and check the cheat-sheet toggle) before merge. No new automated tests were added because there is no existing JavaScript or JSP test harness covering these views, so manual regression is required.

Replace deprecated frameborder string injection with DOM-created sandboxed frames. Adds title, loading lazy, referrerpolicy same-origin, and same-origin sandbox that blocks top navigation but keeps scripts, forms, modals, popups and downloads for training content. Adds same-origin src validation and fixes getStarted double-html handling. No Java or session logic changed.
@abhinav-phi

abhinav-phi commented Sep 17, 2026

Copy link
Copy Markdown
Author

Hi, this update covers the bounded iframe work for #914.

I replaced the deprecated frameborder string injection in index.jsp and getStarted.jsp with DOM-created frames. Session, CSRF, and lesson flows are unchanged, and the training servlets are untouched.

Static checks and the Node validation for the new helper pass in my environment. Maven tests and full Docker runtime checks are still pending, so review plus local verification are needed before merge. Thanks.

Related issue: #914

@abhinav-phi

Copy link
Copy Markdown
Author

Hi SeanDuggan, this PR is ready for review whenever you have a moment.

It implements the bounded iframe modernization for #914: sandboxed same-origin frames created through DOM APIs with title, lazy loading, and a same-origin referrer policy, plus source validation and the getStarted frame fix. Session handling and lesson functionality are unchanged.

Two limitations to flag upfront: the Maven tests and a full Docker runtime check are still pending on my side, and I would appreciate your confirmation that keeping sandboxed frames matches the intended scope. Thanks.

@SeanDuggan

Copy link
Copy Markdown
Member

Hi Abinhav-Phi,

This one will take me a while to test. Once testing is done, I'll follow up with comments or close depending on results.

Thanks,
Seán

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

Replace iframes on Web Application

2 participants