Public Access
Let the recovery code be copied, and give the phone a clipboard to copy to
Both screens had made the code selectable and both said why: a person who cannot get it out of the box photographs the screen, and a screenshot is a worse home for it than a clipboard. This finishes that argument. Selecting 64 characters of letter-spaced monospace with a thumb is the version of "possible" people give up on halfway — and on the phone the screen blocks screenshots, so the honest remaining options were retyping it or losing it. It is the one secret this application deliberately offers to a clipboard, and the contrast with the keychain's copy is the whole argument rather than an inconsistency. There, copying the private half is refused outright, because installing a key means pasting the public one and the private one has no business leaving the vault. Here there is no better route: the code exists for one screen, is stored nowhere, and has to reach a password manager. The clipboard is the intended destination rather than a way round the design. The sentence afterwards matters as much as the copy, and is asserted: a clipboard is a staging post, this screen is the only place the code exists, and the next thing copied replaces it. Somebody who copies and does nothing has not saved it. The phone had no clipboard delegate at all — the desktop passed one and this head passed null — so COPY PUBLIC KEY on the keychain answered "this machine has no clipboard" on a device that plainly has one. Nothing about that was platform shaped: Android has a clipboard and Avalonia surfaces it through the same TopLevel. Wiring it fixes that copy too. The test fixture built its shell without a clipboard, which modelled the bug rather than the product, so it has one now and the public-key test asserts what lands there instead of the refusal. The refusal keeps its own test, on a shell built without one, because the view model reads the delegate's absence rather than an empty result — and because a button that silently does nothing on this screen is worse than one that refuses.
This commit is contained in:
@@ -195,7 +195,10 @@ dotnet run --project src/DodoSSH.Client.App
|
||||
|
||||
In the app, enter `http://localhost:5233` as the server. Your browser opens for sign-in — the realm ships
|
||||
`alice` / `alice` — then choose a vault passphrase and **write down the recovery code**, which cannot be
|
||||
skipped and cannot be recovered from the server. You can then add a host and open a shell on it — double-click
|
||||
skipped and cannot be recovered from the server. **COPY CODE** puts it on the clipboard so it can go
|
||||
straight into a password manager: it is the one secret this application deliberately offers to a clipboard,
|
||||
because there is nowhere better for it to go and the alternative people actually reach for is a photograph
|
||||
of the screen. You can then add a host and open a shell on it — double-click
|
||||
its card, or select it and press **CONNECT** in the drawer that opens beside the grid, which is the same
|
||||
command with the password box above it. Keycloak's admin console is at `http://localhost:18080`
|
||||
(`admin` / `admin`).
|
||||
|
||||
+19
-3
@@ -1188,7 +1188,23 @@ directly instead of `Classes="... secret"`. `PasswordChar` is what the screen dr
|
||||
what the keyboard is told, and only the second one keeps a passphrase out of the IME's learning
|
||||
dictionary. A box showing dots with a suggestion strip over it is the worst case, not a cosmetic one.
|
||||
|
||||
### 10.2 The keyboard does not cover the box being typed into
|
||||
### 10.2 COPY CODE actually copies, on both heads
|
||||
|
||||
Enrol a fresh profile. On the recovery-code screen, press **COPY CODE**, then paste somewhere — another
|
||||
app on the phone, a text editor on the desktop.
|
||||
|
||||
**Pass:** the whole code arrives, and the line under the button says to put it in a password manager now.
|
||||
|
||||
**Failure means:** if the phone says *this machine has no clipboard*, the delegate is not being passed to
|
||||
`MainWindowViewModel` again — the phone shipped for a while with none, so every copy on that head said
|
||||
exactly that on a device that plainly has one. If nothing is said at all, the button silently no-opped,
|
||||
which on this screen is worse than refusing: somebody who believes the code is on their clipboard will not
|
||||
write it down, and it is shown once.
|
||||
|
||||
Worth pasting somewhere you can see all of it. The code is the only thing standing between a forgotten
|
||||
passphrase and an unrecoverable vault, and a truncated copy fails silently months later.
|
||||
|
||||
### 10.3 The keyboard does not cover the box being typed into
|
||||
|
||||
The same box: with the keyboard up, the passphrase box and the UNLOCK button under it are both visible.
|
||||
Repeat on each of the five boxes that take a secret — lock screen, both enrollment boxes, the connect
|
||||
@@ -1201,7 +1217,7 @@ password on HOSTS, and the connect password on FILES.
|
||||
`WindowSoftInputMode` has been dropped from the activity and the platform is panning the window instead of
|
||||
resizing it — which, for a window whose whole content is one native view, pans by nothing useful.
|
||||
|
||||
### 10.3 Nothing is stranded when the keyboard closes
|
||||
### 10.4 Nothing is stranded when the keyboard closes
|
||||
|
||||
Dismiss the keyboard with back or the down-chevron from each of those screens.
|
||||
|
||||
@@ -1211,7 +1227,7 @@ bottom and no scroll position left part way down.
|
||||
**Failure means:** the inset is being applied but not cleared — the closed state is not being read from the
|
||||
event, or the margin is only ever added to.
|
||||
|
||||
### 10.4 Rotating with the keyboard up
|
||||
### 10.5 Rotating with the keyboard up
|
||||
|
||||
Focus a passphrase box, then turn the phone sideways.
|
||||
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
using Avalonia;
|
||||
using Avalonia.Controls;
|
||||
using Avalonia.Controls.ApplicationLifetimes;
|
||||
using Avalonia.Input.Platform;
|
||||
using Avalonia.Markup.Xaml;
|
||||
|
||||
using DodoSSH.Client.Android.Platform;
|
||||
@@ -79,7 +81,14 @@ public sealed partial class DodoSshApp : Avalonia.Application
|
||||
// RESUME has something to read — and whatever a process death interrupted. See DocumentStaging.
|
||||
DocumentStaging.Sweep();
|
||||
|
||||
var viewModel = ComposeShell(paths, caches, workspace, knownHosts, connections);
|
||||
// Built before the view model, because the clipboard is reached through it — see ClipboardWriter,
|
||||
// which takes the surface and looks the TopLevel up on each call rather than now.
|
||||
var shell = new PhoneShell();
|
||||
|
||||
var viewModel = ComposeShell(
|
||||
paths, caches, workspace, knownHosts, connections, ClipboardWriter(shell));
|
||||
|
||||
shell.DataContext = viewModel;
|
||||
|
||||
// Difference 2: the foreground service, which is what makes TerminalWorkspace's promise — that a
|
||||
// shell outlives a vault lock — true on a platform that stops backgrounded processes.
|
||||
@@ -104,9 +113,40 @@ public sealed partial class DodoSshApp : Avalonia.Application
|
||||
|
||||
keepAlive.Refresh();
|
||||
|
||||
return new PhoneShell { DataContext = viewModel };
|
||||
return shell;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Writing to this phone's clipboard.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// ◆ <b>This head had none, and every control that wanted one said so out loud.</b> COPY PUBLIC KEY on
|
||||
/// the keychain answered "This machine has no clipboard" on a device that plainly has one, because the
|
||||
/// delegate was simply never wired here — the desktop passed one and the phone passed null. Android has
|
||||
/// a clipboard and Avalonia surfaces it through the same <c>TopLevel</c> the desktop reaches, so there
|
||||
/// was nothing platform-shaped about the gap.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Looked up per call rather than captured, exactly as the desktop's is: at composition there is no
|
||||
/// <c>TopLevel</c> yet, because this is the method building the view it will be attached to. A machine
|
||||
/// that somehow has none falls through silently and the view model decides what to say, which is what
|
||||
/// keeps "no clipboard here" and "copied" different answers.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// A delegate rather than an <c>IClipboard</c>, so nothing in the view models needs a visual and every
|
||||
/// test that drives them stays window-free.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
private static Func<string, Task> ClipboardWriter(Visual surface) =>
|
||||
async text =>
|
||||
{
|
||||
if (TopLevel.GetTopLevel(surface) is { Clipboard: { } clipboard })
|
||||
{
|
||||
await clipboard.SetTextAsync(text).ConfigureAwait(false);
|
||||
}
|
||||
};
|
||||
|
||||
/// <remarks>
|
||||
/// Split from <see cref="Compose"/> only for length. The division is a real one though: above this is
|
||||
/// the platform graph, and below it is the shell every head shares.
|
||||
@@ -116,7 +156,8 @@ public sealed partial class DodoSshApp : Avalonia.Application
|
||||
ClientCacheFactory caches,
|
||||
TerminalWorkspace workspace,
|
||||
VaultKnownHostStore knownHosts,
|
||||
SshNetConnectionFactory connections)
|
||||
SshNetConnectionFactory connections,
|
||||
Func<string, Task> copyToClipboard)
|
||||
{
|
||||
// Difference 3: the Android keystore, with a fingerprint or the device credential releasing the
|
||||
// key. A straight implementation of the interface the session layer has always taken.
|
||||
@@ -165,6 +206,8 @@ public sealed partial class DodoSshApp : Avalonia.Application
|
||||
// would name the same machine. See PhoneEnvironment.DeviceName.
|
||||
deviceName: PhoneEnvironment.DeviceName,
|
||||
|
||||
copyToClipboard: copyToClipboard,
|
||||
|
||||
updates: updates);
|
||||
|
||||
// Started rather than awaited: framework initialisation must not block on a schema migration. The
|
||||
|
||||
@@ -44,6 +44,26 @@
|
||||
LetterSpacing="1.5" LineHeight="26" TextWrapping="Wrap" />
|
||||
</Border>
|
||||
|
||||
<!--
|
||||
◆ COPY, and on this head it is the difference between selectable and reachable. The comment above
|
||||
says a user who cannot get the code out of the box photographs the screen; selecting a monospaced,
|
||||
letter-spaced string with a thumb is the version of "can" that people give up on halfway, and this
|
||||
screen blocks screenshots — so without a button the honest options were retyping it or losing it.
|
||||
|
||||
Full width and above the tick rather than beside the code, because it is a step in the sequence
|
||||
this screen walks somebody through: read it, take it, confirm you have it. A small icon in the
|
||||
corner of the box would be the same control drawn as an afterthought.
|
||||
|
||||
What it puts on the clipboard is the one secret this application deliberately offers there — see
|
||||
MainWindowViewModel.CopyRecoveryCode, which sets out why that is right here and wrong for a private
|
||||
key, and what the status line has to say afterwards.
|
||||
-->
|
||||
<Button Classes="secondary" Content="COPY CODE" Margin="0,10,0,0" Height="44"
|
||||
Command="{Binding CopyRecoveryCodeCommand}" />
|
||||
|
||||
<TextBlock Classes="detail" Margin="0,8,0,0" TextWrapping="Wrap"
|
||||
Text="{Binding StatusMessage}" />
|
||||
|
||||
<CheckBox IsChecked="{Binding RecoveryCodeWrittenDown}" Margin="0,20,0,0" MinHeight="44">
|
||||
<TextBlock Classes="body" Foreground="{StaticResource Text}"
|
||||
Text="I have written this code down. I understand it will never be shown again." />
|
||||
|
||||
@@ -297,8 +297,24 @@
|
||||
FontSize="17" Foreground="{StaticResource Accent}"
|
||||
TextWrapping="Wrap" />
|
||||
</Border>
|
||||
<CheckBox IsChecked="{Binding RecoveryCodeWrittenDown}"
|
||||
Content="I have written it down somewhere safe" />
|
||||
<!--
|
||||
◆ COPY, beside the tick rather than inside the box. The code is selectable and has been
|
||||
since this card existed, on the argument that refusing to let somebody take it only pushes
|
||||
them to a screenshot — this is that argument finished, because "select 64 characters of
|
||||
letter-spaced monospace without missing one" is a thing people get wrong silently.
|
||||
|
||||
It is the one secret this application offers to the clipboard on purpose; see
|
||||
MainWindowViewModel.CopyRecoveryCode for why that is right here and refused for a private
|
||||
key, and for what the status line below says afterwards.
|
||||
-->
|
||||
<StackPanel Orientation="Horizontal" Spacing="10">
|
||||
<Button Classes="ghost" Content="COPY CODE"
|
||||
Command="{Binding CopyRecoveryCodeCommand}"
|
||||
ToolTip.Tip="Puts the code on the clipboard. Paste it into your password manager now — it is shown once and the next thing you copy replaces it." />
|
||||
<CheckBox IsChecked="{Binding RecoveryCodeWrittenDown}"
|
||||
Content="I have written it down somewhere safe"
|
||||
VerticalAlignment="Center" />
|
||||
</StackPanel>
|
||||
<Button Classes="accent" Content="CONTINUE" Command="{Binding ConfirmRecoveryCodeCommand}"
|
||||
HorizontalAlignment="Left" />
|
||||
<TextBlock Classes="hint" Text="{Binding StatusMessage}" />
|
||||
|
||||
@@ -1848,6 +1848,52 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp
|
||||
}).ConfigureAwait(true);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Puts the recovery code on the clipboard.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// ◆ <b>The one secret in this application that is deliberately offered to the clipboard, and the
|
||||
/// contrast with <c>VaultViewModel.CopyPublicKeyAsync</c> is the whole argument.</b> There, copying the
|
||||
/// <em>private</em> key is refused outright, because installing a key means pasting the public half and
|
||||
/// the private one has no business leaving the vault. Here there is no better route: the code exists for
|
||||
/// one screen, is stored nowhere, and has to reach a password manager — so the clipboard is the intended
|
||||
/// destination rather than a way around the design.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Both screens already made the code selectable, and both said why: a person who cannot get it out of
|
||||
/// the box photographs the screen, and a screenshot is a far worse home for it than a clipboard. This is
|
||||
/// that argument finished. Selecting a monospaced, letter-spaced string with a thumb is the version of
|
||||
/// "possible" that people give up on.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// It says what it did, including the case where there is nothing to say it to — a machine with no
|
||||
/// clipboard has to be told so rather than left with a button that appears to do nothing, which is the
|
||||
/// same rule the keychain's copy already follows. And the sentence names what has to happen next,
|
||||
/// because a clipboard is not somewhere a recovery code may stay: this screen is the only moment it
|
||||
/// exists, and the next thing copied replaces it.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
[RelayCommand]
|
||||
private async Task CopyRecoveryCodeAsync()
|
||||
{
|
||||
if (RecoveryCode is not { Length: > 0 } code)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
if (copyToClipboard is null)
|
||||
{
|
||||
StatusMessage = "This machine has no clipboard. Select the code and copy it by hand.";
|
||||
return;
|
||||
}
|
||||
|
||||
await copyToClipboard(code).ConfigureAwait(true);
|
||||
|
||||
StatusMessage = "Copied. Paste it into your password manager now — this screen is the only place "
|
||||
+ "it exists, and the next thing you copy replaces it.";
|
||||
}
|
||||
|
||||
/// <summary>Leaves the recovery-code screen, once the user says they have it.</summary>
|
||||
[RelayCommand]
|
||||
private void ConfirmRecoveryCode()
|
||||
|
||||
@@ -27,6 +27,9 @@ public sealed class ShellFlowTests : IAsyncLifetime
|
||||
{
|
||||
private const string Passphrase = "a sufficiently long passphrase";
|
||||
|
||||
/// <summary>Everything this shell has copied, newest last.</summary>
|
||||
private readonly List<string> clipboard = [];
|
||||
|
||||
/// <remarks>
|
||||
/// Far below the shipped profile, for the same reason as everywhere else: these tests are about the
|
||||
/// state machine, not about how expensive the passphrase is to attack.
|
||||
@@ -127,7 +130,18 @@ public sealed class ShellFlowTests : IAsyncLifetime
|
||||
TimeProvider.System,
|
||||
ssh,
|
||||
CheapProfile,
|
||||
ResumeAsync);
|
||||
ResumeAsync,
|
||||
|
||||
// ◆ A clipboard, where this fixture used to pass none. Both heads wire one now — the phone's
|
||||
// was simply never passed, which made every copy on that head answer "this machine has no
|
||||
// clipboard" on a device that plainly has one. A fixture without one modelled the bug rather
|
||||
// than the product. The branch for a machine that really has none is still covered, by a shell
|
||||
// built without one where it is the thing under test.
|
||||
copyToClipboard: text =>
|
||||
{
|
||||
clipboard.Add(text);
|
||||
return Task.CompletedTask;
|
||||
});
|
||||
|
||||
return ValueTask.CompletedTask;
|
||||
}
|
||||
@@ -3397,11 +3411,12 @@ public sealed class ShellFlowTests : IAsyncLifetime
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// A machine with no clipboard reports itself rather than appearing to have copied. This shell is built
|
||||
/// without one, which is what makes the case reachable at all.
|
||||
/// The public half and only the public half. There is deliberately no command for the other one — a
|
||||
/// private key on a clipboard is a private key in every application on the machine — so what this pins
|
||||
/// is that the one thing installing a key needs does reach the clipboard.
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public async Task CopyingAPublicKey_WithNoClipboard_SaysSo()
|
||||
public async Task CopyingAPublicKey_PutsThePublicHalfOnTheClipboard()
|
||||
{
|
||||
await UnlockedAsync();
|
||||
var vault = shell.Vault!;
|
||||
@@ -3416,7 +3431,68 @@ public sealed class ShellFlowTests : IAsyncLifetime
|
||||
|
||||
await vault.CopyPublicKeyCommand.ExecuteAsync(null);
|
||||
|
||||
vault.Status.ShouldContain("no clipboard", Case.Insensitive);
|
||||
var copied = clipboard.ShouldHaveSingleItem();
|
||||
|
||||
copied.ShouldStartWith("ssh-");
|
||||
copied.ShouldNotContain("PRIVATE KEY");
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// ◆ <b>The one secret this application deliberately offers to the clipboard.</b> The recovery code
|
||||
/// exists for one screen, is stored nowhere and has to reach a password manager, so the clipboard is
|
||||
/// where it is going whatever the interface does — the only question is whether the interface helps or
|
||||
/// leaves somebody transcribing it, or photographing a screen that blocks screenshots.
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public async Task CopyingTheRecoveryCode_PutsItOnTheClipboardAndSaysWhereToPutIt()
|
||||
{
|
||||
await EnrolledAsync();
|
||||
|
||||
shell.State.ShouldBe(ShellState.ShowingRecoveryCode);
|
||||
|
||||
var code = shell.RecoveryCode.ShouldNotBeNull();
|
||||
|
||||
await shell.CopyRecoveryCodeCommand.ExecuteAsync(null);
|
||||
|
||||
clipboard.ShouldHaveSingleItem().ShouldBe(code);
|
||||
|
||||
// The sentence matters as much as the copy. A clipboard is a staging post rather than a home, and
|
||||
// this screen is the only place the code exists — somebody who copies it and does nothing has not
|
||||
// saved it.
|
||||
shell.StatusMessage.ShouldContain("password manager");
|
||||
shell.StatusMessage.ShouldContain("replaces it");
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// The other branch, and it needs a shell built without a clipboard because that is precisely the
|
||||
/// condition — the view model reads the delegate's absence, not an empty result. It must say so rather
|
||||
/// than leaving a button that appears to have worked: a recovery code somebody believes is on their
|
||||
/// clipboard is a recovery code they will not write down.
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public async Task CopyingTheRecoveryCode_WithNoClipboard_SaysSoRatherThanSeemingToWork()
|
||||
{
|
||||
var bare = new MainWindowViewModel(
|
||||
paths,
|
||||
caches,
|
||||
workspace,
|
||||
knownHosts,
|
||||
deviceKeys,
|
||||
SignInAsync,
|
||||
TimeProvider.System,
|
||||
ssh,
|
||||
CheapProfile,
|
||||
ResumeAsync);
|
||||
|
||||
await using (bare.ConfigureAwait(false))
|
||||
{
|
||||
bare.RecoveryCode = "correct horse battery staple";
|
||||
|
||||
await bare.CopyRecoveryCodeCommand.ExecuteAsync(null);
|
||||
|
||||
bare.StatusMessage.ShouldContain("no clipboard", Case.Insensitive);
|
||||
clipboard.ShouldBeEmpty("the other shell's clipboard must not have been written to either");
|
||||
}
|
||||
}
|
||||
|
||||
// ---- Importing ssh_config ----
|
||||
|
||||
Reference in New Issue
Block a user