Skip to content

Commit

Permalink
refactor: cleanup, mostly tests
Browse files Browse the repository at this point in the history
In preparation for more work, related to #112, #37 and #8.

- Migrated all tests to use testify's `Suite` instead of direct `assert`
  calls.
- Simplified how the credentials file location is determined, now it's
  defined in one simple function.

Changes I may revert: I hate string concatenation, it's just ugly in my
eyes, so I may just add that linter to disabled and revert to `Sprintf`. I
know it's more efficient, but not on this scale.

Closes #8

References:
- #8
- #37
- #112
  • Loading branch information
yitsushi committed Mar 27, 2024
1 parent 00f90ce commit 5ea9a9f
Show file tree
Hide file tree
Showing 19 changed files with 319 additions and 181 deletions.
30 changes: 23 additions & 7 deletions .github/workflows/quality-check.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
- name: Set up Go 1.x
uses: actions/setup-go@v4
with:
go-version: ^1.21
go-version: ^1.22

- name: Get dependencies
run: |
Expand Down Expand Up @@ -43,19 +43,35 @@ jobs:
- name: Set up Go 1.x
uses: actions/setup-go@v4
with:
go-version: ^1.21
go-version: ^1.22

- name: go vet
run: go vet ./...

lint:
name: go vet and lint
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Set up Go 1.x
uses: actions/setup-go@v4
with:
go-version: ^1.22

- name: Get dependencies
run: |
go get -v -t -d ./...
go install golang.org/x/lint/golint@latest
go install github.com/Antonboom/testifylint@latest
- name: go vet
run: go vet ./...

- name: go lint
- name: golint
run: golint -set_exit_status ./...

- name: testifylint
run: testifylint -v -enable-all ./...

golangci:
name: golangci lint check
runs-on: ubuntu-latest
Expand All @@ -66,4 +82,4 @@ jobs:
- name: golangci-lint
uses: golangci/golangci-lint-action@v3
with:
version: v1.54.2
version: v1.57.1
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ linters-settings:
stylecheck:
go: "1.18"
govet:
check-shadowing: true
shadow: true
misspell:
locale: US
cyclop:
Expand Down
17 changes: 17 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
ifeq (, $(shell which testifylint))
$(error "No 'testifylint' on PATH, consider doing: go install github.com/Antonboom/testifylint@latest")
endif

ifeq (, $(shell which golint))
$(error "No 'golint' on PATH, consider doing: go install golang.org/x/lint/golint@latest")
endif

ifeq (, $(shell which golangci-lint))
$(error "No 'golangci-lint' on PATH, consider following these instructions: https://golangci-lint.run/welcome/install/#local-installation")
endif

.PHONY: lint
lint:
golint -set_exit_status ./...
testifylint ./...
golangci-lint run
12 changes: 6 additions & 6 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
module github.com/yitsushi/totp-cli

go 1.21
go 1.22

require (
filippo.io/age v1.1.1
github.com/stretchr/testify v1.7.1
github.com/urfave/cli/v2 v2.27.1
golang.org/x/term v0.16.0
golang.org/x/term v0.18.0
gopkg.in/yaml.v3 v3.0.1
)

require (
github.com/cpuguy83/go-md2man/v2 v2.0.3 // indirect
github.com/cpuguy83/go-md2man/v2 v2.0.4 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/russross/blackfriday/v2 v2.1.0 // indirect
github.com/xrash/smetrics v0.0.0-20231213231151-1d8dd44e695e // indirect
golang.org/x/crypto v0.17.0 // indirect
golang.org/x/sys v0.16.0 // indirect
github.com/xrash/smetrics v0.0.0-20240312152122-5f08fbb34913 // indirect
golang.org/x/crypto v0.21.0 // indirect
golang.org/x/sys v0.18.0 // indirect
)
30 changes: 10 additions & 20 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
filippo.io/age v1.1.1 h1:pIpO7l151hCnQ4BdyBujnGP2YlUo0uj6sAVNHGBvXHg=
filippo.io/age v1.1.1/go.mod h1:l03SrzDUrBkdBx8+IILdnn2KZysqQdbEBUQ4p3sqEQE=
github.com/cpuguy83/go-md2man/v2 v2.0.2 h1:p1EgwI/C7NhT0JmVkwCD2ZBK8j4aeHQX2pMHHBfMQ6w=
github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
github.com/cpuguy83/go-md2man/v2 v2.0.3 h1:qMCsGGgs+MAzDFyp9LpAe1Lqy/fY/qCovCm0qnXZOBM=
github.com/cpuguy83/go-md2man/v2 v2.0.3/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
github.com/cpuguy83/go-md2man/v2 v2.0.4 h1:wfIWP927BUkWJb2NmU/kNDYIBTh/ziUX91+lVfRxZq4=
github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
Expand All @@ -14,24 +12,16 @@ github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQD
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.7.1 h1:5TQK59W5E3v0r2duFAb7P95B6hEeOyEnHRa8MjYSMTY=
github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/urfave/cli/v2 v2.25.7 h1:VAzn5oq403l5pHjc4OhD54+XGO9cdKVL/7lDjF+iKUs=
github.com/urfave/cli/v2 v2.25.7/go.mod h1:8qnjx1vcq5s2/wpsqoZFndg2CE5tNFyrTvS6SinrnYQ=
github.com/urfave/cli/v2 v2.27.1 h1:8xSQ6szndafKVRmfyeUMxkNUJQMjL1F2zmsZ+qHpfho=
github.com/urfave/cli/v2 v2.27.1/go.mod h1:8qnjx1vcq5s2/wpsqoZFndg2CE5tNFyrTvS6SinrnYQ=
github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 h1:bAn7/zixMGCfxrRTfdpNzjtPYqr8smhKouy9mxVdGPU=
github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673/go.mod h1:N3UwUGtsrSj3ccvlPHLoLsHnpR27oXr4ZE984MbSER8=
github.com/xrash/smetrics v0.0.0-20231213231151-1d8dd44e695e h1:+SOyEddqYF09QP7vr7CgJ1eti3pY9Fn3LHO1M1r/0sI=
github.com/xrash/smetrics v0.0.0-20231213231151-1d8dd44e695e/go.mod h1:N3UwUGtsrSj3ccvlPHLoLsHnpR27oXr4ZE984MbSER8=
golang.org/x/crypto v0.17.0 h1:r8bRNjWL3GshPW3gkd+RpvzWrZAwPS49OmTGZ/uhM4k=
golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4=
golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc=
golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/sys v0.16.0 h1:xWw16ngr6ZMtmxDyKyIgsE93KNKz5HKmMa3b8ALHidU=
golang.org/x/sys v0.16.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/term v0.15.0 h1:y/Oo/a/q3IXu26lQgl04j/gjuBDOBlx7X6Om1j2CPW4=
golang.org/x/term v0.15.0/go.mod h1:BDl952bC7+uMoWR75FIrCDx79TPU9oHkTZ9yRbYOrX0=
golang.org/x/term v0.16.0 h1:m+B6fahuftsE9qjo0VWp2FW0mB3MTJvR0BaMQrq0pmE=
golang.org/x/term v0.16.0/go.mod h1:yn7UURbUtPyrVJPGPq404EukNFxcm/foM+bV/bfcDsY=
github.com/xrash/smetrics v0.0.0-20240312152122-5f08fbb34913 h1:+qGGcbkzsfDQNPPe9UDgpxAWQrhbbBXOYJFQDq/dtJw=
github.com/xrash/smetrics v0.0.0-20240312152122-5f08fbb34913/go.mod h1:4aEEwZQutDLsQv2Deui4iYQ6DWTxR14g6m8Wv88+Xqk=
golang.org/x/crypto v0.21.0 h1:X31++rzVUdKhX5sWmSOFZxx8UW/ldWx55cbf08iNAMA=
golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs=
golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4=
golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/term v0.18.0 h1:FcHjZXDMxI8mM3nwhX9HlKop4C0YQvCVCdwYl2wOtE8=
golang.org/x/term v0.18.0/go.mod h1:ILwASektA3OnRv7amZ1xhE/KTR+u50pbXfZ03+6Nx58=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
Expand Down
10 changes: 6 additions & 4 deletions internal/cmd/error.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
package cmd

import "fmt"

// DownloadError is an error during downloading an update.
type DownloadError struct {
Message string
}

func (e DownloadError) Error() string {
return fmt.Sprintf("download error: %s", e.Message)
return "download error: %s" + e.Message
}

// CommandError is an error during downloading an update.
Expand All @@ -17,5 +15,9 @@ type CommandError struct {
}

func (e CommandError) Error() string {
return fmt.Sprintf("error: %s", e.Message)
return "error: %s" + e.Message
}

func resourceNotFoundError(name string) CommandError {
return CommandError{Message: name + " does not exist"}
}
6 changes: 3 additions & 3 deletions internal/cmd/rename.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func renameNamespaceCommand() *cli.Command {

namespace, err := storage.FindNamespace(nsName)
if err != nil {
return CommandError{Message: fmt.Sprintf("%s does not exist", nsName)}
return resourceNotFoundError(nsName)
}

namespace.Name = newName
Expand Down Expand Up @@ -91,12 +91,12 @@ func renameAccountCommand() *cli.Command {

namespace, err := storage.FindNamespace(nsName)
if err != nil {
return CommandError{Message: fmt.Sprintf("%s does not exist", nsName)}
return resourceNotFoundError(nsName)
}

account, err := namespace.FindAccount(accName)
if err != nil {
return CommandError{Message: fmt.Sprintf("%s/%s does not exist", namespace.Name, accName)}
return resourceNotFoundError(fmt.Sprintf("%s/%s", namespace.Name, accName))
}

account.Name = newName
Expand Down
4 changes: 2 additions & 2 deletions internal/cmd/set_length.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@ func SetLengthCommand() *cli.Command {

namespace, err = storage.FindNamespace(nsName)
if err != nil {
return CommandError{Message: fmt.Sprintf("%s does not exist", nsName)}
return resourceNotFoundError(nsName)
}

account, err = namespace.FindAccount(accName)
if err != nil {
return CommandError{Message: fmt.Sprintf("%s/%s does not exist", namespace.Name, accName)}
return resourceNotFoundError(fmt.Sprintf("%s/%s", namespace.Name, accName))
}

account.Length = length
Expand Down
4 changes: 2 additions & 2 deletions internal/cmd/set_prefix.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ func SetPrefixCommand() *cli.Command {

namespace, err = storage.FindNamespace(nsName)
if err != nil {
return CommandError{Message: fmt.Sprintf("%s does not exist", nsName)}
return resourceNotFoundError(nsName)
}

account, err = namespace.FindAccount(accName)
if err != nil {
return CommandError{Message: fmt.Sprintf("%s/%s does not exist", namespace.Name, accName)}
return resourceNotFoundError(fmt.Sprintf("%s/%s", namespace.Name, accName))
}

account.Prefix = prefix
Expand Down
4 changes: 1 addition & 3 deletions internal/security/error.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
package security

import "fmt"

// OTPError is an error describing an error during generation.
type OTPError struct {
Message string
}

func (e OTPError) Error() string {
return fmt.Sprintf("otp error: %s", e.Message)
return "otp error: " + e.Message
}
40 changes: 24 additions & 16 deletions internal/security/otp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,20 @@ import (

"github.com/yitsushi/totp-cli/internal/storage"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"

"github.com/yitsushi/totp-cli/internal/security"
)

func TestGenerateOTPCode(t *testing.T) {
func TestGenerateOTPCodeSuit(t *testing.T) {
suite.Run(t, &GenerateOTPCodeTestSuite{})
}

type GenerateOTPCodeTestSuite struct {
suite.Suite
}

func (suite *GenerateOTPCodeTestSuite) TestDefault() {
input := base32.StdEncoding.EncodeToString([]byte("82394783472398472348"))
table := map[time.Time]string{
time.Date(1970, 1, 1, 0, 0, 59, 0, time.UTC): "007459",
Expand All @@ -27,12 +35,12 @@ func TestGenerateOTPCode(t *testing.T) {
for when, expected := range table {
code, _, err := security.GenerateOTPCode(input, when, storage.DefaultTokenLength)

assert.NoError(t, err)
assert.Equal(t, expected, code, when.String())
suite.Require().NoError(err)
suite.Equal(expected, code, when.String())
}
}

func TestGenerateOTPCode_length8(t *testing.T) {
func (suite *GenerateOTPCodeTestSuite) TestDifferentLength() {
input := base32.StdEncoding.EncodeToString([]byte("82394783472398472348"))
table := map[time.Time]string{
time.Date(1970, 1, 1, 0, 0, 59, 0, time.UTC): "53007459",
Expand All @@ -47,12 +55,12 @@ func TestGenerateOTPCode_length8(t *testing.T) {
for when, expected := range table {
code, _, err := security.GenerateOTPCode(input, when, 8)

assert.NoError(t, err)
assert.Equal(t, expected, code, when.String())
suite.Require().NoError(err)
suite.Equal(expected, code, when.String())
}
}

func TestGenerateOTPCode_SpaceSeparatedToken(t *testing.T) {
func (suite *GenerateOTPCodeTestSuite) TestSpaceSeparatedToken() {
input := "37kh vdxt c5hj ttfp ujok cipy jy"
table := map[time.Time]string{
time.Date(1970, 1, 1, 0, 0, 59, 0, time.UTC): "066634",
Expand All @@ -67,12 +75,12 @@ func TestGenerateOTPCode_SpaceSeparatedToken(t *testing.T) {
for when, expected := range table {
code, _, err := security.GenerateOTPCode(input, when, storage.DefaultTokenLength)

assert.NoError(t, err)
assert.Equal(t, expected, code, when.String())
suite.Require().NoError(err)
suite.Equal(expected, code, when.String())
}
}

func TestGenerateOTPCode_NonPaddedHashes(t *testing.T) {
func (suite *GenerateOTPCodeTestSuite) TestNonPaddedHashes() {
input := "a6mryljlbufszudtjdt42nh5by"
table := map[time.Time]string{
time.Date(1970, 1, 1, 0, 0, 59, 0, time.UTC): "866149",
Expand All @@ -87,12 +95,12 @@ func TestGenerateOTPCode_NonPaddedHashes(t *testing.T) {
for when, expected := range table {
code, _, err := security.GenerateOTPCode(input, when, storage.DefaultTokenLength)

assert.NoError(t, err)
assert.Equal(t, expected, code, when.String())
suite.Require().NoError(err)
suite.Equal(expected, code, when.String())
}
}

func TestGenerateOTPCode_InvalidPadding(t *testing.T) {
func (suite *GenerateOTPCodeTestSuite) TestInvalidPadding() {
input := "a6mr*&^&*%*&ylj|'[lbufszudtjdt42nh5by"
table := map[time.Time]string{
time.Date(1970, 1, 1, 0, 0, 59, 0, time.UTC): "",
Expand All @@ -102,7 +110,7 @@ func TestGenerateOTPCode_InvalidPadding(t *testing.T) {
for when, expected := range table {
code, _, err := security.GenerateOTPCode(input, when, storage.DefaultTokenLength)

assert.Error(t, err)
assert.Equal(t, expected, code, when.String())
suite.Require().Error(err)
suite.Equal(expected, code, when.String())
}
}
4 changes: 2 additions & 2 deletions internal/security/unsecure.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package security

import (
"crypto/sha1" //nolint:gosec // It's hard to change now without breaking. Issue #41.
"fmt"
"encoding/hex"
)

// UnsecureSHA1 is not secure, but makes a fixed length password.
Expand All @@ -15,7 +15,7 @@ func UnsecureSHA1(text string) []byte {
hash := sha1.New() //nolint:gosec // yolo?
_, _ = hash.Write([]byte(text))
h := hash.Sum(nil)
text = fmt.Sprintf("%x", h)
text = hex.EncodeToString(h)

copy(result, text[0:passwordHashLength])

Expand Down
8 changes: 4 additions & 4 deletions internal/storage/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ const DefaultTokenLength = 6

// Account represents a TOTP account.
type Account struct {
Name string
Token string
Prefix string
Length uint
Name string `json:"name" yaml:"name"`
Token string `json:"token" yaml:"token"`
Prefix string `json:"prefix" yaml:"prefix"`
Length uint `json:"length" yaml:"length"`
}
2 changes: 1 addition & 1 deletion internal/storage/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@ type BackendError struct {
}

func (e BackendError) Error() string {
return fmt.Sprintf("storage error: %s", e.Message)
return "storage error: " + e.Message
}
Loading

0 comments on commit 5ea9a9f

Please sign in to comment.