-
Notifications
You must be signed in to change notification settings - Fork 26
fix(auth): auto-submit filled keyboard-interactive prompts #214
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -190,10 +190,15 @@ async function findAllDefaultPrivateKeys(options = {}) { | |
| // If only simple auth methods and no fallback keys needed, use array-based handler | ||
| if (hasExplicitAuth && !hasFallbackOptions) { | ||
| const authMethods = []; | ||
| if (effectiveAgent) authMethods.push("agent"); | ||
| if (privateKey) authMethods.push("publickey"); | ||
| if (password) authMethods.push("password"); | ||
| authMethods.push("keyboard-interactive"); | ||
| if (isPasswordOnly) { | ||
| authMethods.push("keyboard-interactive"); | ||
| authMethods.push("password"); | ||
| } else { | ||
| if (effectiveAgent) authMethods.push("agent"); | ||
| if (privateKey) authMethods.push("publickey"); | ||
| if (password) authMethods.push("password"); | ||
| authMethods.push("keyboard-interactive"); | ||
| } | ||
|
|
||
| return { | ||
| authHandler: authMethods, | ||
|
|
@@ -205,17 +210,19 @@ async function findAllDefaultPrivateKeys(options = {}) { | |
|
|
||
| // Build comprehensive authMethods array with all auth options | ||
| // Order depends on what user explicitly configured: | ||
| // - Password-only: password -> agent -> default keys -> keyboard-interactive | ||
| // - Password-only: keyboard-interactive -> password -> agent -> default keys | ||
| // - Key-only: user key -> password -> agent -> default keys -> keyboard-interactive | ||
| // - Agent configured: agent -> user key -> password -> default keys -> keyboard-interactive | ||
| // - No explicit auth: agent -> default keys -> keyboard-interactive | ||
| const authMethods = []; | ||
|
|
||
| if (isPasswordOnly) { | ||
| // Password-only: password first, then fallbacks | ||
| // keyboard-interactive first: many bastion/2FA setups expect password + OTP | ||
| // via interactive prompts. Starting with "password" can trigger duplicate challenges. | ||
| authMethods.push({ type: "keyboard-interactive", id: "keyboard-interactive" }); | ||
| authMethods.push({ type: "password", id: "password" }); | ||
|
Comment on lines
+222
to
223
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In Useful? React with 👍 / 👎. |
||
|
|
||
| // Add agent and default keys AFTER password as fallback | ||
| // Add agent and default keys after password as fallback | ||
| if (sshAgentSocket) { | ||
| authMethods.push({ type: "agent", id: "agent" }); | ||
| } | ||
|
|
@@ -308,8 +315,10 @@ async function findAllDefaultPrivateKeys(options = {}) { | |
| }); | ||
| } | ||
|
|
||
| // Keyboard-interactive as last resort | ||
| authMethods.push({ type: "keyboard-interactive", id: "keyboard-interactive" }); | ||
| // Keyboard-interactive as last resort (if not already present) | ||
| if (!authMethods.some((m) => m.type === "keyboard-interactive")) { | ||
| authMethods.push({ type: "keyboard-interactive", id: "keyboard-interactive" }); | ||
| } | ||
|
|
||
| console.log(`${logPrefix} Auth methods configured`, { | ||
| isPasswordOnly, | ||
|
|
@@ -320,47 +329,50 @@ async function findAllDefaultPrivateKeys(options = {}) { | |
| methods: authMethods.map(m => m.id), | ||
| }); | ||
|
|
||
| // Use dynamic authHandler to try all keys | ||
| let authIndex = 0; | ||
| const attemptedMethodIds = new Set(); | ||
|
|
||
| const authHandler = (methodsLeft, partialSuccess, callback) => { | ||
| const availableMethods = methodsLeft || ["publickey", "password", "keyboard-interactive", "agent"]; | ||
|
|
||
| while (authIndex < authMethods.length) { | ||
| const method = authMethods[authIndex]; | ||
| authIndex++; | ||
|
|
||
| if (attemptedMethodIds.has(method.id)) continue; | ||
| attemptedMethodIds.add(method.id); | ||
|
|
||
| // Use dynamic authHandler to try all configured methods. | ||
| // Important: only mark a method as attempted when it is actually sent to ssh2. | ||
| // This avoids consuming methods that are unavailable in an earlier round but | ||
| // become available after partialSuccess (for example password -> keyboard-interactive MFA). | ||
| const attemptedMethodIds = new Set(); | ||
|
|
||
| const authHandler = (methodsLeft, partialSuccess, callback) => { | ||
| const availableMethods = methodsLeft || ["publickey", "password", "keyboard-interactive", "agent"]; | ||
|
|
||
| for (const method of authMethods) { | ||
| if (attemptedMethodIds.has(method.id)) continue; | ||
|
|
||
| if (method.type === "agent" && (availableMethods.includes("publickey") || availableMethods.includes("agent"))) { | ||
| attemptedMethodIds.add(method.id); | ||
| console.log(`${logPrefix} Trying agent auth`); | ||
| return callback("agent"); | ||
| } else if (method.type === "publickey" && availableMethods.includes("publickey")) { | ||
| console.log(`${logPrefix} Trying publickey auth:`, method.id); | ||
| const pubkeyAuth = { | ||
| type: "publickey", | ||
| username, | ||
| key: method.key, | ||
| }; | ||
| if (method.passphrase) { | ||
| pubkeyAuth.passphrase = method.passphrase; | ||
| } | ||
| return callback(pubkeyAuth); | ||
| attemptedMethodIds.add(method.id); | ||
| console.log(`${logPrefix} Trying publickey auth:`, method.id); | ||
| const pubkeyAuth = { | ||
| type: "publickey", | ||
| username, | ||
| key: method.key, | ||
| }; | ||
| if (method.passphrase) { | ||
| pubkeyAuth.passphrase = method.passphrase; | ||
| } | ||
| return callback(pubkeyAuth); | ||
| } else if (method.type === "password" && availableMethods.includes("password")) { | ||
| attemptedMethodIds.add(method.id); | ||
| console.log(`${logPrefix} Trying password auth`); | ||
| return callback({ | ||
| type: "password", | ||
| username, | ||
| password, | ||
| }); | ||
| } else if (method.type === "keyboard-interactive" && availableMethods.includes("keyboard-interactive")) { | ||
| return callback("keyboard-interactive"); | ||
| } | ||
| } | ||
| return callback(false); | ||
| }; | ||
| } else if (method.type === "keyboard-interactive" && availableMethods.includes("keyboard-interactive")) { | ||
| attemptedMethodIds.add(method.id); | ||
| return callback("keyboard-interactive"); | ||
| } | ||
| } | ||
|
|
||
| return callback(false); | ||
| }; | ||
|
|
||
| // Determine the agent to return - if authMethods includes agent, we need to provide the socket | ||
| // even if effectiveAgent is null (for fallback scenarios) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OTP guard regex misses common prompt text like
"One-time password:", sogetAutofillPasswordIndex()will classify it as a normal password prompt (because it containspassword) andbuildAutofilledResponses()will auto-fill and auto-submit the account password for single-prompt challenges. In password-only flows this can immediately consume the keyboard-interactive attempt with the wrong secret, and users never get a chance to enter their OTP on servers that rely on keyboard-interactive only.Useful? React with 👍 / 👎.