Skip to content

Commit

Permalink
improve: add tests for verify server cert
Browse files Browse the repository at this point in the history
  • Loading branch information
dnephin committed Jun 3, 2022
1 parent f47cc59 commit 585291b
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 1 deletion.
1 change: 1 addition & 0 deletions internal/cmd/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,7 @@ func promptLoginOptions(cli *CLI, client *api.Client) (loginMethod loginMethod,
func promptVerifyTLSCert(cli *CLI, cert *x509.Certificate) error {
// TODO: improve this message
// TODO: use color/bold to highlight important parts
// TODO: test format with golden
fmt.Fprintf(cli.Stderr, `The certificate presented by the server could not be automatically verified.
Subject: %[1]s
Expand Down
90 changes: 89 additions & 1 deletion internal/cmd/login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ package cmd

import (
"context"
"encoding/pem"
"os"
"path/filepath"
"testing"
"time"

"github.com/AlecAivazis/survey/v2/terminal"
"github.com/Netflix/go-expect"
"github.com/creack/pty"
"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -44,6 +46,30 @@ func TestLoginCmd(t *testing.T) {
assert.Check(t, srv.Run(ctx))
}()

runStep(t, "reject server certificate", func(t *testing.T) {
ctx, cancel := context.WithCancel(ctx)
t.Cleanup(cancel)

console := newConsole(t)
ctx = PatchCLIWithPTY(ctx, console.Tty())

g, ctx := errgroup.WithContext(ctx)
g.Go(func() error {
// TODO: why isn't this working without --non-interactive=false? the other test works
return Run(ctx, "login", "--non-interactive=false", srv.Addrs.HTTPS.String())
})
exp := expector{console: console}
exp.ExpectString(t, "verify the certificate can be trusted")
exp.Send(t, "I do not trust this certificate\n")

assert.ErrorIs(t, g.Wait(), terminal.InterruptErr)

// Check we haven't persisted any certificates
cfg, err := readConfig()
assert.NilError(t, err)
assert.Equal(t, len(cfg.Hosts), 0)
})

runStep(t, "first login prompts for setup", func(t *testing.T) {
ctx, cancel := context.WithCancel(ctx)
t.Cleanup(cancel)
Expand Down Expand Up @@ -95,6 +121,67 @@ func TestLoginCmd(t *testing.T) {
expected := clientcmdapi.Config{}
assert.DeepEqual(t, expected, kubeCfg, cmpopts.EquateEmpty())
})

runStep(t, "trust server certificate", func(t *testing.T) {
ctx, cancel := context.WithCancel(ctx)
t.Cleanup(cancel)

// Create an access key for login
cfg, err := readConfig()
assert.NilError(t, err)
assert.Equal(t, len(cfg.Hosts), 1)
userID, _ := cfg.Hosts[0].PolymorphicID.ID()
client, err := defaultAPIClient()
assert.NilError(t, err)

resp, err := client.CreateAccessKey(&api.CreateAccessKeyRequest{
UserID: userID,
TTL: api.Duration(opts.SessionDuration + time.Minute),
ExtensionDeadline: api.Duration(time.Minute),
})
assert.NilError(t, err)
accessKey := resp.AccessKey

// Erase local data for previous login session
assert.NilError(t, writeConfig(&ClientConfig{}))

console := newConsole(t)
ctx = PatchCLIWithPTY(ctx, console.Tty())

g, ctx := errgroup.WithContext(ctx)
g.Go(func() error {
// TODO: why isn't this working without --non-interactive=false? the other test works
return Run(ctx, "login", "--non-interactive=false", "--key", accessKey, srv.Addrs.HTTPS.String())
})
exp := expector{console: console}
exp.ExpectString(t, "verify the certificate can be trusted")
exp.Send(t, "Trust and save the certificate\n")

assert.NilError(t, g.Wait())

cert, err := os.ReadFile("testdata/pki/localhost.crt")
assert.NilError(t, err)

// TODO: remove once stored as PEM encoded
block, _ := pem.Decode(cert)

// Check the client config
cfg, err = readConfig()
assert.NilError(t, err)
expected := []ClientHostConfig{
{
Name: "admin",
AccessKey: "any-access-key",
PolymorphicID: "any-id",
Host: srv.Addrs.HTTPS.String(),
Expires: api.Time(time.Now().UTC().Add(opts.SessionDuration)),
Current: true,
TrustedCertificate: block.Bytes,
},
}
// TODO: where is the extra entry coming from?
assert.DeepEqual(t, cfg.Hosts, expected, cmpClientHostConfig)
})
}

var cmpClientHostConfig = cmp.Options{
Expand Down Expand Up @@ -140,14 +227,15 @@ func newConsole(t *testing.T) *expect.Console {
pseudoTY, tty, err := pty.Open()
assert.NilError(t, err, "failed to open pseudo tty")

timeout := time.Second
timeout := time.Hour // TODO: time.Second
if os.Getenv("CI") != "" {
// CI takes much longer than local dev, use a much longer timeout
timeout = 20 * time.Second
}

term := vt10x.New(vt10x.WithWriter(tty))
console, err := expect.NewConsole(
// expect.WithLogger(log.New(os.Stderr, "", 0)),
expect.WithDefaultTimeout(timeout),
expect.WithStdout(os.Stdout),
expect.WithStdin(pseudoTY),
Expand Down

0 comments on commit 585291b

Please sign in to comment.