Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactored the control panel and workspace reload #489

Merged

Conversation

JensHouses
Copy link
Contributor

should fix the issues:
QoL improvement: submit/next/prev buttons on navbar #105
and
Better reconnection for nonvnc on challenge restart #478

  • I Used BroadcastChannel to trigger an event through browser tabs and/or windows if a challenge (re)started.
  • VSCode reloads the page.
  • noVNC changes the iframe src.
  • I added a description to the control panel and moved the notifications to the bottom of the control panel. I Limited the max-size to 50vh and made it scrollable. Therefor I changed the scrollbar to be more visible.
  • I added the description to the test of active_module.

dojo_plugin/pages/workspace.py Outdated Show resolved Hide resolved
dojo_theme/static/css/custom.css Outdated Show resolved Hide resolved
@JensHouses JensHouses requested a review from ConnorNelson August 8, 2024 13:14
Copy link
Member

@ConnorNelson ConnorNelson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks super awesome! Just 1 more small change: let's support async for all workspace services, not just the desktop. Then it will be ready to merge. Thanks for all your work!


@workspace.route("/workspace/<service>")
@authed_only
def view_workspace(service):
user = get_current_user()
active = bool(get_current_dojo_challenge())
iframe_src = "" if service=="desktop" else f"/workspace/{service}/"
loadUrlAsync = service == "desktop"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's get rid of the decision to loadUrlAsync. All /workspace/* should be loaded async; the desktop is not special :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code just seams to have the iframe src /workspace/code/
what other workspaces are there ? :)

@@ -45,6 +45,16 @@ <h1 class="inactive">No active challenge session; start a challenge!<h1>

{% block scripts %}
{{ super() }}
{% if active and loadUrlAsync %}
<script>
let response = fetch("/pwncollege_api/v1/workspace").then(response => response.json()).then(data => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somehow we should pass to the workspace endpoint what url we're looking for. You can pass this all the way through with a jina {{ service }}.

Tangentially, it might also make sense to rename this from iframe.html => workspace.html. If there is someone else using iframe.html besides a workspace-related feature (I don't think so?) then we'd want to make a copy.

@workspace_namespace.route("")
class view_desktop(Resource):
@authed_only
def get(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably add a service = request.args.get("service") or something like that.

@JensHouses
Copy link
Contributor Author

Final changes: 🤣

  • Added service parameter to view_desktop endpoint in workspace.py.
  • Updated iframe source handling based on service parameter in workspace.py.
  • Simplified view_workspace route in pages/workspace.py.
  • Removed async URL loading and share URLs script from iframe.html.
  • Created new workspace.html template for dynamic iframe loading.

@JensHouses JensHouses requested a review from ConnorNelson August 9, 2024 06:44
@ConnorNelson
Copy link
Member

Great! Looks like testcases aren't passing though?

@JensHouses
Copy link
Contributor Author

Spent 3 hours debugging an issue where Firefox refused to open WebSockets, which led me to believe there was a problem with my code. After exhausting all possible code-related issues, I finally tested it in Chrome, where it worked perfectly fine. Turns out, the problem was with Firefox all along, not my code. Once I realized this, the actual task—changing a '2' to a '3'—took just a few minutes. 3 hours of work to change a single number!

@zardus
Copy link
Contributor

zardus commented Aug 14, 2024

DEFCON's over, so we'll get back to getting this merged in shortly :-)

Looks like we'll need to squash, cause rebasing will have conflicts.

@JensHouses JensHouses force-pushed the feature/control-panel-and-desktop-reload branch from 07a99e6 to 8057559 Compare August 17, 2024 09:14
@JensHouses JensHouses marked this pull request as draft August 17, 2024 09:18
I Used BroadcastChannel to trigger an event through browser tabs and/or windows if a challenge (re)started.
VSCode reloads the page.
noVNC changes the iframe src.

I added a description to the control panel and moved the notifications to the bottom of the control panel. I Limited the max-size to 50vh and made it scrollable. Therefor I changed the scrollbar to be more visible.

I added the description to the test of active_module.
increased the wait time as we now fetch the iframe url async via api.

Refactor workspace API and update templates

- Added `service` parameter to `view_desktop` endpoint in `workspace.py`.
- Updated iframe source handling based on `service` parameter in `workspace.py`.
- Simplified `view_workspace` route in `pages/workspace.py`.
- Removed async URL loading and share URLs script from `iframe.html`.
- Created new `workspace.html` template for dynamic iframe loading.

Refactor workspace API and update templates

- Added `service` parameter to `view_desktop` endpoint in `workspace.py`.
- Updated iframe source handling based on `service` parameter in `workspace.py`.
- Simplified `view_workspace` route in `pages/workspace.py`.
- Removed async URL loading and share URLs script from `iframe.html`.
- Created new `workspace.html` template for dynamic iframe loading.

Improve reconnection on challenge (re)start

- Added API endpoint to retrieve the current iframe URL.
- Moved `container_password` to `utils`.
- Relocated `start_on_demand_service` to `utils/workspace`.
- Removed special route for workspace desktop.
- Added edge-case handling to the workspace `<service>` route.
- Updated `iframe.html` to support new functionality.
- Modified `navbar.js` to use the new API endpoint.
@JensHouses JensHouses force-pushed the feature/control-panel-and-desktop-reload branch from 8057559 to 3bbd272 Compare August 17, 2024 09:29
increased the wait time as we now fetch the iframe url async via api.

Refactor workspace API and update templates

- Added `service` parameter to `view_desktop` endpoint in `workspace.py`.
- Updated iframe source handling based on `service` parameter in `workspace.py`.
- Simplified `view_workspace` route in `pages/workspace.py`.
- Removed async URL loading and share URLs script from `iframe.html`.
- Created new `workspace.html` template for dynamic iframe loading.

Refactor workspace API and update templates

- Added `service` parameter to `view_desktop` endpoint in `workspace.py`.
- Updated iframe source handling based on `service` parameter in `workspace.py`.
- Simplified `view_workspace` route in `pages/workspace.py`.
- Removed async URL loading and share URLs script from `iframe.html`.
- Created new `workspace.html` template for dynamic iframe loading.

Improve reconnection on challenge (re)start

- Added API endpoint to retrieve the current iframe URL.
- Moved `container_password` to `utils`.
- Relocated `start_on_demand_service` to `utils/workspace`.
- Removed special route for workspace desktop.
- Added edge-case handling to the workspace `<service>` route.
- Updated `iframe.html` to support new functionality.
- Modified `navbar.js` to use the new API endpoint.
…p-reload' into feature/control-panel-and-desktop-reload

# Conflicts:
#	dojo_theme/static/js/dojo/navbar.js
@JensHouses JensHouses marked this pull request as ready for review August 17, 2024 09:54
@JensHouses
Copy link
Contributor Author

Finally... =)

@zardus
Copy link
Contributor

zardus commented Aug 18, 2024

Thanks, @JensHouses!! @ConnorNelson thoughts on the API/page split? If you're happy, then we yolo?

broadcast.onmessage = (event) => {
if (event.data.msg === 'New challenge started') {
if (window.location.pathname === '/workspace/code') {
window.location.reload();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason code uses a window reload, while the desktop gets/sets the iframe url?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i cant figure out why reloading the iframe does not work. as the iframe url seams to be the same.
reloading the page seam to solve the problem. But i currently don't know why.
Any advice would be welcome.

@zardus zardus merged commit 9009948 into pwncollege:master Aug 27, 2024
1 check passed
@zardus
Copy link
Contributor

zardus commented Aug 27, 2024

image

@zardus
Copy link
Contributor

zardus commented Aug 27, 2024

thanks!!!!

@JensHouses JensHouses deleted the feature/control-panel-and-desktop-reload branch August 30, 2024 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants