Modernize module iframes while preserving dashboard flow (#914)

[abhinav-phi]

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.

[abhinav-phi]

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]

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]

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

[abhinav-phi]

@SeanDuggan Friendly ping - bounded iframe modernization is ready for testing on your side when you have a cycle. Let me know if you hit anything. Thanks!

[abhinav-phi]

hi @SeanDuggan, gentle nudge.. ci is green on the latest commit, so its ready for your local testing whenever you get a cycle.

[SeanDuggan]

I've tested these changes locally, loading="lazy" on the new iframes broke all challenge/lesson loading. These iframes sit inside a div that starts hidden (display:none), and lazy-loading never triggers for hidden elements. The iframe never loads, and the UI (which waits for the iframe's load event to reveal itself) gets stuck in a blank state.

Fix: removed loading="lazy" from the iframes in index.jsp and getStarted.jsp. Verified challenges/lessons and the welcome screen now load correctly.

Verified both fixes end-to-end locally: challenge/lesson content now loads and reveals correctly.

In src/main/webapp/index.jsp, in createModuleFrame():

frame.setAttribute("title", title);
**frame.setAttribute("loading", "lazy");   // remove this line**
frame.setAttribute("referrerpolicy", "same-origin");

Same fix in src/main/webapp/getStarted.jsp, around line 211:

startFrame.setAttribute("title", "Security Shepherd getting started");
**startFrame.setAttribute("loading", "lazy");   // remove this line**
startFrame.setAttribute("referrerpolicy", "same-origin");

[abhinav-phi]

@SeanDuggan thank you for actually running it, that is a catch i could not see from reading the diff. fixed in 79680c1.

the attribute is gone from both places, createModuleFrame() in index.jsp and the readyToPlay frame in getStarted.jsp, and I left a short note at each site saying why lazy loading must not come back there, so the next person does not add it again as a performance win. sandbox, referrerpolicy, the explicit titles and the class based sizing all stay as they were. also rebased onto current dev, the branch was behind.

to be straight about verification, I do not have this JSP stack running here, so your local pass is what proved it broken and your local pass is what proves it fixed. i am happy to take anything else it turns up when you get a cycle, and if the welcome screen and a couple of lessons look right then this is ready for the label.

[SeanDuggan]

Hi @abhinav-phi, I've remove the comments are they are not necessary in this case + while reviewing the two files that were changed, I noticed hard coded English strings. The hard coded strings are not in line with our translation standards so I also used this PR to update those strings.

@ismisepaul Can you review this?