Add secret-safe copy and reveal UX coverage
This commit is contained in:
+18
-2
@@ -2,7 +2,6 @@ package clipboard
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
|
||||
systemclipboard "github.com/atotto/clipboard"
|
||||
|
||||
@@ -10,6 +9,7 @@ import (
|
||||
)
|
||||
|
||||
var ErrUnsupportedTarget = errors.New("unsupported clipboard target")
|
||||
var ErrWriteFailed = errors.New("clipboard write failed")
|
||||
|
||||
type Target string
|
||||
|
||||
@@ -39,7 +39,7 @@ func (s Service) Copy(model vault.Model, entryID string, target Target) error {
|
||||
}
|
||||
|
||||
if err := s.writer().WriteText(content); err != nil {
|
||||
return fmt.Errorf("write clipboard text: %w", err)
|
||||
return writeError{err: err}
|
||||
}
|
||||
|
||||
return nil
|
||||
@@ -79,3 +79,19 @@ type systemWriter struct{}
|
||||
func (systemWriter) WriteText(text string) error {
|
||||
return systemclipboard.WriteAll(text)
|
||||
}
|
||||
|
||||
type writeError struct {
|
||||
err error
|
||||
}
|
||||
|
||||
func (e writeError) Error() string {
|
||||
return ErrWriteFailed.Error()
|
||||
}
|
||||
|
||||
func (e writeError) Unwrap() error {
|
||||
return e.err
|
||||
}
|
||||
|
||||
func (e writeError) Is(target error) bool {
|
||||
return target == ErrWriteFailed
|
||||
}
|
||||
|
||||
@@ -68,6 +68,25 @@ func TestServiceRejectsUnknownEntryAndUnsupportedTarget(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestServiceSanitizesClipboardWriteErrors(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
service := Service{Writer: failingWriter{err: errors.New("backend refused token-1")}}
|
||||
model := vault.Model{
|
||||
Entries: []vault.Entry{
|
||||
{ID: "git-server", Password: "token-1"},
|
||||
},
|
||||
}
|
||||
|
||||
err := service.Copy(model, "git-server", TargetPassword)
|
||||
if !errors.Is(err, ErrWriteFailed) {
|
||||
t.Fatalf("Copy() write error = %v, want ErrWriteFailed", err)
|
||||
}
|
||||
if err.Error() != ErrWriteFailed.Error() {
|
||||
t.Fatalf("Copy() write error string = %q, want %q", err.Error(), ErrWriteFailed.Error())
|
||||
}
|
||||
}
|
||||
|
||||
type memoryWriter struct {
|
||||
content string
|
||||
}
|
||||
@@ -76,3 +95,11 @@ func (w *memoryWriter) WriteText(text string) error {
|
||||
w.content = text
|
||||
return nil
|
||||
}
|
||||
|
||||
type failingWriter struct {
|
||||
err error
|
||||
}
|
||||
|
||||
func (w failingWriter) WriteText(string) error {
|
||||
return w.err
|
||||
}
|
||||
|
||||
@@ -86,7 +86,6 @@ type ui struct {
|
||||
copyUser widget.Clickable
|
||||
copyPass widget.Clickable
|
||||
copyURL widget.Clickable
|
||||
openURL widget.Clickable
|
||||
lockVault widget.Clickable
|
||||
unlockVault widget.Clickable
|
||||
createVault widget.Clickable
|
||||
@@ -135,6 +134,7 @@ type ui struct {
|
||||
eyeIcon *widget.Icon
|
||||
eyeOffIcon *widget.Icon
|
||||
copyIcon *widget.Icon
|
||||
clipboardWriter clipboard.Writer
|
||||
loadingMessage string
|
||||
statusMessage string
|
||||
errorMessage string
|
||||
@@ -975,10 +975,7 @@ func (u *ui) detailPanel(gtx layout.Context) layout.Dimensions {
|
||||
layout.Rigid(u.entryEditorPanel),
|
||||
)
|
||||
}
|
||||
password := strings.Repeat("•", max(8, len(item.Password)))
|
||||
if u.showPassword {
|
||||
password = item.Password
|
||||
}
|
||||
password := u.detailPasswordValue()
|
||||
titleSize := unit.Sp(26)
|
||||
titlePad := unit.Dp(10)
|
||||
sectionGap := unit.Dp(8)
|
||||
@@ -1016,7 +1013,7 @@ func (u *ui) detailPanel(gtx layout.Context) layout.Dimensions {
|
||||
}),
|
||||
layout.Rigid(layout.Spacer{Height: unit.Dp(8)}.Layout),
|
||||
layout.Rigid(func(gtx layout.Context) layout.Dimensions {
|
||||
return tonedButton(gtx, u.theme, &u.openURL, "Open URL")
|
||||
return tonedButton(gtx, u.theme, &u.copyURL, "Copy URL")
|
||||
}),
|
||||
)
|
||||
}
|
||||
@@ -1032,7 +1029,7 @@ func (u *ui) detailPanel(gtx layout.Context) layout.Dimensions {
|
||||
}),
|
||||
layout.Rigid(layout.Spacer{Width: unit.Dp(8)}.Layout),
|
||||
layout.Rigid(func(gtx layout.Context) layout.Dimensions {
|
||||
btn := material.Button(u.theme, &u.openURL, "Open URL")
|
||||
btn := material.Button(u.theme, &u.copyURL, "Copy URL")
|
||||
return btn.Layout(gtx)
|
||||
}),
|
||||
)
|
||||
@@ -1201,6 +1198,17 @@ func (u *ui) passwordLine(label, value string) layout.Widget {
|
||||
}
|
||||
}
|
||||
|
||||
func (u *ui) detailPasswordValue() string {
|
||||
item, ok := u.selectedEntry()
|
||||
if !ok {
|
||||
return ""
|
||||
}
|
||||
if u.showPassword {
|
||||
return item.Password
|
||||
}
|
||||
return strings.Repeat("•", max(8, len(item.Password)))
|
||||
}
|
||||
|
||||
func card(gtx layout.Context, w layout.Widget) layout.Dimensions {
|
||||
return layout.Background{}.Layout(gtx, fill(panelColor), func(gtx layout.Context) layout.Dimensions {
|
||||
return layout.UniformInset(unit.Dp(16)).Layout(gtx, w)
|
||||
|
||||
+143
@@ -8,6 +8,7 @@ import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"slices"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"git.julianfamily.org/keepassgo/clipboard"
|
||||
@@ -891,3 +892,145 @@ func TestUIUsesKeePassGOProductCopy(t *testing.T) {
|
||||
t.Fatalf("desktopSubtitle = %q, want updated product subtitle", desktopSubtitle)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUICopyActionsWriteExpectedClipboardContentsAndSanitizedFeedback(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
model := vault.Model{
|
||||
Entries: []vault.Entry{
|
||||
{
|
||||
ID: "git-server",
|
||||
Title: "Git Server",
|
||||
Username: "joejulian",
|
||||
Password: "token-1",
|
||||
URL: "https://git.julianfamily.org",
|
||||
Path: []string{"Root", "Internet"},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
target clipboard.Target
|
||||
label string
|
||||
want string
|
||||
}{
|
||||
{name: "username", target: clipboard.TargetUsername, label: "copy username", want: "joejulian"},
|
||||
{name: "password", target: clipboard.TargetPassword, label: "copy password", want: "token-1"},
|
||||
{name: "url", target: clipboard.TargetURL, label: "copy URL", want: "https://git.julianfamily.org"},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
u := newUIWithModel("desktop", model)
|
||||
writer := &memoryClipboardWriter{}
|
||||
u.clipboardWriter = writer
|
||||
u.showEntriesSection()
|
||||
u.currentPath = []string{"Root", "Internet"}
|
||||
u.filter()
|
||||
u.state.SelectedEntryID = "git-server"
|
||||
|
||||
u.runAction(tt.label, func() error { return u.copySelectedFieldAction(tt.target) })
|
||||
|
||||
if writer.content != tt.want {
|
||||
t.Fatalf("clipboard content = %q, want %q", writer.content, tt.want)
|
||||
}
|
||||
if u.statusMessage != tt.label+" complete" {
|
||||
t.Fatalf("statusMessage = %q, want %q", u.statusMessage, tt.label+" complete")
|
||||
}
|
||||
if u.errorMessage != "" {
|
||||
t.Fatalf("errorMessage = %q, want empty", u.errorMessage)
|
||||
}
|
||||
if strings.Contains(u.statusMessage, tt.want) {
|
||||
t.Fatalf("statusMessage = %q, must not contain copied secret or field value %q", u.statusMessage, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestUICopyActionSanitizesClipboardBackendErrors(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
u := newUIWithModel("desktop", vault.Model{
|
||||
Entries: []vault.Entry{
|
||||
{
|
||||
ID: "git-server",
|
||||
Title: "Git Server",
|
||||
Username: "joejulian",
|
||||
Password: "token-1",
|
||||
URL: "https://git.julianfamily.org",
|
||||
Path: []string{"Root", "Internet"},
|
||||
},
|
||||
},
|
||||
})
|
||||
u.clipboardWriter = failingClipboardWriter{err: os.ErrPermission}
|
||||
u.showEntriesSection()
|
||||
u.currentPath = []string{"Root", "Internet"}
|
||||
u.filter()
|
||||
u.state.SelectedEntryID = "git-server"
|
||||
|
||||
u.runAction("copy password", func() error { return u.copySelectedFieldAction(clipboard.TargetPassword) })
|
||||
|
||||
if u.errorMessage != clipboard.ErrWriteFailed.Error() {
|
||||
t.Fatalf("errorMessage = %q, want %q", u.errorMessage, clipboard.ErrWriteFailed.Error())
|
||||
}
|
||||
if strings.Contains(u.errorMessage, "token-1") {
|
||||
t.Fatalf("errorMessage = %q, must not contain copied password", u.errorMessage)
|
||||
}
|
||||
if u.statusMessage != "" {
|
||||
t.Fatalf("statusMessage = %q, want empty on copy failure", u.statusMessage)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUIPasswordRevealTogglesDisplayedPasswordAndLockResetsIt(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
u := newUIWithModel("desktop", vault.Model{
|
||||
Entries: []vault.Entry{
|
||||
{
|
||||
ID: "git-server",
|
||||
Title: "Git Server",
|
||||
Username: "joejulian",
|
||||
Password: "token-1",
|
||||
Path: []string{"Root", "Internet"},
|
||||
},
|
||||
},
|
||||
})
|
||||
u.showEntriesSection()
|
||||
u.currentPath = []string{"Root", "Internet"}
|
||||
u.filter()
|
||||
u.state.SelectedEntryID = "git-server"
|
||||
|
||||
if got := u.detailPasswordValue(); got != "••••••••" {
|
||||
t.Fatalf("detailPasswordValue() hidden = %q, want %q", got, "••••••••")
|
||||
}
|
||||
|
||||
u.showPassword = true
|
||||
if got := u.detailPasswordValue(); got != "token-1" {
|
||||
t.Fatalf("detailPasswordValue() revealed = %q, want %q", got, "token-1")
|
||||
}
|
||||
|
||||
if err := u.lockAction(); err != nil {
|
||||
t.Fatalf("lockAction() error = %v", err)
|
||||
}
|
||||
if u.showPassword {
|
||||
t.Fatal("showPassword = true after lockAction(), want false")
|
||||
}
|
||||
}
|
||||
|
||||
type memoryClipboardWriter struct {
|
||||
content string
|
||||
}
|
||||
|
||||
func (w *memoryClipboardWriter) WriteText(text string) error {
|
||||
w.content = text
|
||||
return nil
|
||||
}
|
||||
|
||||
type failingClipboardWriter struct {
|
||||
err error
|
||||
}
|
||||
|
||||
func (w failingClipboardWriter) WriteText(string) error {
|
||||
return w.err
|
||||
}
|
||||
|
||||
+1
-1
@@ -241,7 +241,7 @@ func (u *ui) copySelectedFieldAction(target clipboard.Target) error {
|
||||
return err
|
||||
}
|
||||
|
||||
service := clipboard.Service{}
|
||||
service := clipboard.Service{Writer: u.clipboardWriter}
|
||||
return service.Copy(model, u.state.SelectedEntryID, target)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user