Skip to content

SSH: account creation and key upload - #1093

Open
zrayaz1 wants to merge 11 commits into
pwncollege:masterfrom
zrayaz1:ssh_tui
Open

SSH: account creation and key upload #1093
zrayaz1 wants to merge 11 commits into
pwncollege:masterfrom
zrayaz1:ssh_tui

Conversation

@zrayaz1

@zrayaz1 zrayaz1 commented May 12, 2026

Copy link
Copy Markdown
Contributor

wip

@codecov

codecov Bot commented May 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 69.49153% with 18 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
dojo_plugin/api/v1/auth.py 58.33% 15 Missing ⚠️
dojo_plugin/api/v1/user.py 80.00% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@zardus

zardus commented May 16, 2026

Copy link
Copy Markdown
Member

Wow, this is so much more elegant than the other attempt. What's left before we merge?

@zrayaz1

zrayaz1 commented May 23, 2026

Copy link
Copy Markdown
Contributor Author

Wow, this is so much more elegant than the other attempt. What's left before we merge?

Right now I'm having it still require users to give a username and email, and just setting their password to a random sequence so if they wanted to later use the website it would require going through the password reset process. I'm not sure if that's the best way to do it or if we would prefer something else.

The other part of this is an easier way of adding ssh keys, when doing ssh with a key that is not already affiliated with an account the user is prompted to either create an account or add that key to an existing account and then it generates a link which when followed would add the key. Also not sure if this is a good idea and might have some funny concerns about getting someone to click a link.

@zrayaz1
zrayaz1 marked this pull request as ready for review May 23, 2026 08:58

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 135b68102d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread dojo_plugin/pages/ssh_key.py Outdated
Comment on lines +16 to +18
@ssh_key.route("/ssh/link/<token>")
@authed_only
def link_ssh_key(token):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Make SSH key linking non-GET and CSRF-protected

This route performs account mutation (ssh_keys insert + link consumption) as soon as a logged-in user visits the URL, but it is exposed as a plain GET endpoint. Because browsers automatically send session cookies on top-level cross-site navigations, an attacker can generate a valid link token from SSH onboarding and trick any logged-in victim into opening /ssh/link/<token>, which silently links the attacker's key to the victim's account (account takeover). Use a non-GET confirmation flow (e.g., POST with CSRF token) before persisting the link.

Useful? React with 👍 / 👎.

@zardus

zardus commented Jul 15, 2026

Copy link
Copy Markdown
Member

Is this a stale PR?

@zrayaz1

zrayaz1 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Is this a stale PR?

Not anymore! Still needs review. I stripped it down to just what I think is needed for account creation to work.
Also looking for ui feedback for this

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.

2 participants