From 933b40a02ad99c27717e602bf719056b61e39459 Mon Sep 17 00:00:00 2001 From: max furman Date: Fri, 8 Oct 2021 14:59:57 -0400 Subject: [PATCH] Introduce gocritic linter and address warnings --- .golangci.yml | 20 ++- acme/api/account.go | 2 +- acme/api/account_test.go | 4 +- acme/api/handler_test.go | 16 +- acme/api/middleware.go | 4 +- acme/api/middleware_test.go | 56 +++---- acme/api/order_test.go | 18 +-- acme/challenge.go | 10 +- acme/challenge_test.go | 2 +- acme/db/nosql/account_test.go | 70 ++++---- acme/db/nosql/authz_test.go | 66 ++++---- acme/db/nosql/certificate_test.go | 22 ++- acme/db/nosql/challenge_test.go | 56 +++---- acme/db/nosql/nonce.go | 2 +- acme/db/nosql/nonce_test.go | 8 +- acme/db/nosql/nosql.go | 2 +- acme/db/nosql/nosql_test.go | 4 +- acme/db/nosql/order_test.go | 89 +++++------ acme/order.go | 1 + api/api.go | 29 ++-- api/api_test.go | 36 ++--- api/errors.go | 10 +- api/ssh.go | 10 +- api/sshRekey.go | 2 +- api/sshRenew.go | 2 +- api/sshRevoke.go | 2 +- api/ssh_test.go | 2 +- authority/admin/api/middleware.go | 2 +- authority/admin/db/nosql/admin_test.go | 131 +++++++-------- authority/admin/db/nosql/nosql.go | 2 +- authority/admin/db/nosql/provisioner_test.go | 151 ++++++++---------- authority/administrator/collection.go | 4 +- authority/admins.go | 4 +- authority/authority.go | 6 +- authority/authorize.go | 12 +- authority/authorize_test.go | 2 +- authority/config/types.go | 2 +- authority/linkedca.go | 4 +- authority/options.go | 12 +- authority/provisioner/aws.go | 22 +-- authority/provisioner/aws_test.go | 8 +- authority/provisioner/azure.go | 14 +- authority/provisioner/azure_test.go | 8 +- authority/provisioner/collection.go | 15 +- authority/provisioner/gcp.go | 22 +-- authority/provisioner/gcp_test.go | 8 +- authority/provisioner/keystore.go | 2 +- authority/provisioner/noop.go | 2 +- authority/provisioner/oidc.go | 4 +- authority/provisioner/oidc_test.go | 46 +++--- authority/provisioner/options.go | 2 +- authority/provisioner/provisioner.go | 2 +- .../provisioner/sign_ssh_options_test.go | 16 +- authority/provisioner/sshpop_test.go | 8 +- authority/provisioner/utils_test.go | 4 +- authority/ssh.go | 6 +- authority/tls.go | 18 +-- authority/tls_test.go | 1 + ca/acmeClient.go | 2 +- ca/acmeClient_test.go | 1 + ca/adminClient.go | 18 ++- ca/bootstrap.go | 2 +- ca/ca.go | 52 +++--- ca/ca_test.go | 10 +- ca/client.go | 32 ++-- ca/client_test.go | 2 +- ca/identity/client_test.go | 7 +- ca/tls.go | 8 +- cas/cloudcas/cloudcas.go | 4 +- cas/cloudcas/cloudcas_test.go | 11 +- cas/softcas/softcas.go | 4 +- cas/softcas/softcas_test.go | 2 +- cas/stepcas/stepcas.go | 4 +- cas/stepcas/x5c_issuer.go | 4 +- cas/stepcas/x5c_issuer_test.go | 2 +- cmd/step-awskms-init/main.go | 14 +- cmd/step-ca/main.go | 8 +- cmd/step-cloudkms-init/main.go | 14 +- cmd/step-pkcs11-init/main.go | 8 +- cmd/step-yubikey-init/main.go | 8 +- commands/app.go | 10 +- commands/export.go | 12 +- commands/onboard.go | 34 ++-- db/db_test.go | 8 +- kms/sshagentkms/sshagentkms_test.go | 2 + pki/pki.go | 55 +++---- scep/api/api.go | 4 +- templates/templates.go | 14 +- 88 files changed, 699 insertions(+), 742 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 92af7723..cf389517 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -36,22 +36,30 @@ linters-settings: - performance - style - experimental + - diagnostic disabled-checks: - - wrapperFunc - - dupImport # https://github.com/go-critic/go-critic/issues/845 + - commentFormatting + - commentedOutCode + - evalOrder + - hugeParam + - octalLiteral + - rangeValCopy + - tooManyResultsChecker + - unnamedResult linters: disable-all: true enable: + - deadcode + - gocritic - gofmt - - revive + - gosimple - govet - - misspell - ineffassign - - deadcode + - misspell + - revive - staticcheck - unused - - gosimple run: skip-dirs: diff --git a/acme/api/account.go b/acme/api/account.go index b733c679..259cb2a2 100644 --- a/acme/api/account.go +++ b/acme/api/account.go @@ -19,7 +19,7 @@ type NewAccountRequest struct { func validateContacts(cs []string) error { for _, c := range cs { - if len(c) == 0 { + if c == "" { return acme.NewError(acme.ErrorMalformedType, "contact cannot be empty string") } } diff --git a/acme/api/account_test.go b/acme/api/account_test.go index c4d7a812..a45751a0 100644 --- a/acme/api/account_test.go +++ b/acme/api/account_test.go @@ -178,7 +178,7 @@ func TestHandler_GetOrdersByAccountID(t *testing.T) { provName := url.PathEscape(prov.GetName()) baseURL := &url.URL{Scheme: "https", Host: "test.ca.smallstep.com"} - url := fmt.Sprintf("http://ca.smallstep.com/acme/%s/account/%s/orders", provName, accID) + u := fmt.Sprintf("http://ca.smallstep.com/acme/%s/account/%s/orders", provName, accID) oids := []string{"foo", "bar"} oidURLs := []string{ @@ -255,7 +255,7 @@ func TestHandler_GetOrdersByAccountID(t *testing.T) { tc := run(t) t.Run(name, func(t *testing.T) { h := &Handler{db: tc.db, linker: NewLinker("dns", "acme")} - req := httptest.NewRequest("GET", url, nil) + req := httptest.NewRequest("GET", u, nil) req = req.WithContext(tc.ctx) w := httptest.NewRecorder() h.GetOrdersByAccountID(w, req) diff --git a/acme/api/handler_test.go b/acme/api/handler_test.go index f354bbac..8112ad4c 100644 --- a/acme/api/handler_test.go +++ b/acme/api/handler_test.go @@ -148,7 +148,7 @@ func TestHandler_GetAuthorization(t *testing.T) { // Request with chi context chiCtx := chi.NewRouteContext() chiCtx.URLParams.Add("authzID", az.ID) - url := fmt.Sprintf("%s/acme/%s/authz/%s", + u := fmt.Sprintf("%s/acme/%s/authz/%s", baseURL.String(), provName, az.ID) type test struct { @@ -280,7 +280,7 @@ func TestHandler_GetAuthorization(t *testing.T) { expB, err := json.Marshal(az) assert.FatalError(t, err) assert.Equals(t, bytes.TrimSpace(body), expB) - assert.Equals(t, res.Header["Location"], []string{url}) + assert.Equals(t, res.Header["Location"], []string{u}) assert.Equals(t, res.Header["Content-Type"], []string{"application/json"}) } }) @@ -314,7 +314,7 @@ func TestHandler_GetCertificate(t *testing.T) { // Request with chi context chiCtx := chi.NewRouteContext() chiCtx.URLParams.Add("certID", certID) - url := fmt.Sprintf("%s/acme/%s/certificate/%s", + u := fmt.Sprintf("%s/acme/%s/certificate/%s", baseURL.String(), provName, certID) type test struct { @@ -396,7 +396,7 @@ func TestHandler_GetCertificate(t *testing.T) { tc := run(t) t.Run(name, func(t *testing.T) { h := &Handler{db: tc.db} - req := httptest.NewRequest("GET", url, nil) + req := httptest.NewRequest("GET", u, nil) req = req.WithContext(tc.ctx) w := httptest.NewRecorder() h.GetCertificate(w, req) @@ -434,7 +434,7 @@ func TestHandler_GetChallenge(t *testing.T) { baseURL := &url.URL{Scheme: "https", Host: "test.ca.smallstep.com"} - url := fmt.Sprintf("%s/acme/%s/challenge/%s/%s", + u := fmt.Sprintf("%s/acme/%s/challenge/%s/%s", baseURL.String(), provName, "authzID", "chID") type test struct { @@ -635,7 +635,7 @@ func TestHandler_GetChallenge(t *testing.T) { AuthorizationID: "authzID", Type: acme.HTTP01, AccountID: "accID", - URL: url, + URL: u, Error: acme.NewError(acme.ErrorConnectionType, "force"), }, vco: &acme.ValidateChallengeOptions{ @@ -652,7 +652,7 @@ func TestHandler_GetChallenge(t *testing.T) { tc := run(t) t.Run(name, func(t *testing.T) { h := &Handler{db: tc.db, linker: NewLinker("dns", "acme"), validateChallengeOptions: tc.vco} - req := httptest.NewRequest("GET", url, nil) + req := httptest.NewRequest("GET", u, nil) req = req.WithContext(tc.ctx) w := httptest.NewRecorder() h.GetChallenge(w, req) @@ -678,7 +678,7 @@ func TestHandler_GetChallenge(t *testing.T) { assert.FatalError(t, err) assert.Equals(t, bytes.TrimSpace(body), expB) assert.Equals(t, res.Header["Link"], []string{fmt.Sprintf("<%s/acme/%s/authz/%s>;rel=\"up\"", baseURL, provName, "authzID")}) - assert.Equals(t, res.Header["Location"], []string{url}) + assert.Equals(t, res.Header["Location"], []string{u}) assert.Equals(t, res.Header["Content-Type"], []string{"application/json"}) } }) diff --git a/acme/api/middleware.go b/acme/api/middleware.go index b2244dd7..bc67dbc6 100644 --- a/acme/api/middleware.go +++ b/acme/api/middleware.go @@ -223,7 +223,7 @@ func (h *Handler) validateJWS(next nextHTTP) nextHTTP { api.WriteError(w, acme.NewError(acme.ErrorMalformedType, "jwk and kid are mutually exclusive")) return } - if hdr.JSONWebKey == nil && len(hdr.KeyID) == 0 { + if hdr.JSONWebKey == nil && hdr.KeyID == "" { api.WriteError(w, acme.NewError(acme.ErrorMalformedType, "either jwk or kid must be defined in jws protected header")) return } @@ -367,7 +367,7 @@ func (h *Handler) verifyAndExtractJWSPayload(next nextHTTP) nextHTTP { api.WriteError(w, err) return } - if len(jwk.Algorithm) != 0 && jwk.Algorithm != jws.Signatures[0].Protected.Algorithm { + if jwk.Algorithm != "" && jwk.Algorithm != jws.Signatures[0].Protected.Algorithm { api.WriteError(w, acme.NewError(acme.ErrorMalformedType, "verifier and signature algorithm do not match")) return } diff --git a/acme/api/middleware_test.go b/acme/api/middleware_test.go index 40090e83..e8d22d53 100644 --- a/acme/api/middleware_test.go +++ b/acme/api/middleware_test.go @@ -108,7 +108,7 @@ func TestHandler_baseURLFromRequest(t *testing.T) { } func TestHandler_addNonce(t *testing.T) { - url := "https://ca.smallstep.com/acme/new-nonce" + u := "https://ca.smallstep.com/acme/new-nonce" type test struct { db acme.DB err *acme.Error @@ -141,7 +141,7 @@ func TestHandler_addNonce(t *testing.T) { tc := run(t) t.Run(name, func(t *testing.T) { h := &Handler{db: tc.db} - req := httptest.NewRequest("GET", url, nil) + req := httptest.NewRequest("GET", u, nil) w := httptest.NewRecorder() h.addNonce(testNext)(w, req) res := w.Result() @@ -230,7 +230,7 @@ func TestHandler_verifyContentType(t *testing.T) { prov := newProv() escProvName := url.PathEscape(prov.GetName()) baseURL := &url.URL{Scheme: "https", Host: "test.ca.smallstep.com"} - url := fmt.Sprintf("%s/acme/%s/certificate/abc123", baseURL.String(), escProvName) + u := fmt.Sprintf("%s/acme/%s/certificate/abc123", baseURL.String(), escProvName) type test struct { h Handler ctx context.Context @@ -245,7 +245,7 @@ func TestHandler_verifyContentType(t *testing.T) { h: Handler{ linker: NewLinker("dns", "acme"), }, - url: url, + url: u, ctx: context.Background(), contentType: "foo", statusCode: 500, @@ -257,7 +257,7 @@ func TestHandler_verifyContentType(t *testing.T) { h: Handler{ linker: NewLinker("dns", "acme"), }, - url: url, + url: u, ctx: context.WithValue(context.Background(), provisionerContextKey, prov), contentType: "foo", statusCode: 400, @@ -319,11 +319,11 @@ func TestHandler_verifyContentType(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - _url := url + _u := u if tc.url != "" { - _url = tc.url + _u = tc.url } - req := httptest.NewRequest("GET", _url, nil) + req := httptest.NewRequest("GET", _u, nil) req = req.WithContext(tc.ctx) req.Header.Add("Content-Type", tc.contentType) w := httptest.NewRecorder() @@ -353,7 +353,7 @@ func TestHandler_verifyContentType(t *testing.T) { } func TestHandler_isPostAsGet(t *testing.T) { - url := "https://ca.smallstep.com/acme/new-account" + u := "https://ca.smallstep.com/acme/new-account" type test struct { ctx context.Context err *acme.Error @@ -392,7 +392,7 @@ func TestHandler_isPostAsGet(t *testing.T) { tc := run(t) t.Run(name, func(t *testing.T) { h := &Handler{} - req := httptest.NewRequest("GET", url, nil) + req := httptest.NewRequest("GET", u, nil) req = req.WithContext(tc.ctx) w := httptest.NewRecorder() h.isPostAsGet(testNext)(w, req) @@ -430,7 +430,7 @@ func (errReader) Close() error { } func TestHandler_parseJWS(t *testing.T) { - url := "https://ca.smallstep.com/acme/new-account" + u := "https://ca.smallstep.com/acme/new-account" type test struct { next nextHTTP body io.Reader @@ -483,7 +483,7 @@ func TestHandler_parseJWS(t *testing.T) { tc := run(t) t.Run(name, func(t *testing.T) { h := &Handler{} - req := httptest.NewRequest("GET", url, tc.body) + req := httptest.NewRequest("GET", u, tc.body) w := httptest.NewRecorder() h.parseJWS(tc.next)(w, req) res := w.Result() @@ -528,7 +528,7 @@ func TestHandler_verifyAndExtractJWSPayload(t *testing.T) { assert.FatalError(t, err) parsedJWS, err := jose.ParseJWS(raw) assert.FatalError(t, err) - url := "https://ca.smallstep.com/acme/account/1234" + u := "https://ca.smallstep.com/acme/account/1234" type test struct { ctx context.Context next func(http.ResponseWriter, *http.Request) @@ -681,7 +681,7 @@ func TestHandler_verifyAndExtractJWSPayload(t *testing.T) { tc := run(t) t.Run(name, func(t *testing.T) { h := &Handler{} - req := httptest.NewRequest("GET", url, nil) + req := httptest.NewRequest("GET", u, nil) req = req.WithContext(tc.ctx) w := httptest.NewRecorder() h.verifyAndExtractJWSPayload(tc.next)(w, req) @@ -713,7 +713,7 @@ func TestHandler_lookupJWK(t *testing.T) { prov := newProv() provName := url.PathEscape(prov.GetName()) baseURL := &url.URL{Scheme: "https", Host: "test.ca.smallstep.com"} - url := fmt.Sprintf("%s/acme/%s/account/1234", + u := fmt.Sprintf("%s/acme/%s/account/1234", baseURL, provName) jwk, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0) assert.FatalError(t, err) @@ -883,7 +883,7 @@ func TestHandler_lookupJWK(t *testing.T) { tc := run(t) t.Run(name, func(t *testing.T) { h := &Handler{db: tc.db, linker: tc.linker} - req := httptest.NewRequest("GET", url, nil) + req := httptest.NewRequest("GET", u, nil) req = req.WithContext(tc.ctx) w := httptest.NewRecorder() h.lookupJWK(tc.next)(w, req) @@ -934,7 +934,7 @@ func TestHandler_extractJWK(t *testing.T) { assert.FatalError(t, err) parsedJWS, err := jose.ParseJWS(raw) assert.FatalError(t, err) - url := fmt.Sprintf("https://ca.smallstep.com/acme/%s/account/1234", + u := fmt.Sprintf("https://ca.smallstep.com/acme/%s/account/1234", provName) type test struct { db acme.DB @@ -1079,7 +1079,7 @@ func TestHandler_extractJWK(t *testing.T) { tc := run(t) t.Run(name, func(t *testing.T) { h := &Handler{db: tc.db} - req := httptest.NewRequest("GET", url, nil) + req := httptest.NewRequest("GET", u, nil) req = req.WithContext(tc.ctx) w := httptest.NewRecorder() h.extractJWK(tc.next)(w, req) @@ -1108,7 +1108,7 @@ func TestHandler_extractJWK(t *testing.T) { } func TestHandler_validateJWS(t *testing.T) { - url := "https://ca.smallstep.com/acme/account/1234" + u := "https://ca.smallstep.com/acme/account/1234" type test struct { db acme.DB ctx context.Context @@ -1198,7 +1198,7 @@ func TestHandler_validateJWS(t *testing.T) { Algorithm: jose.RS256, JSONWebKey: &pub, ExtraHeaders: map[jose.HeaderKey]interface{}{ - "url": url, + "url": u, }, }, }, @@ -1226,7 +1226,7 @@ func TestHandler_validateJWS(t *testing.T) { Algorithm: jose.RS256, JSONWebKey: &pub, ExtraHeaders: map[jose.HeaderKey]interface{}{ - "url": url, + "url": u, }, }, }, @@ -1298,7 +1298,7 @@ func TestHandler_validateJWS(t *testing.T) { }, ctx: context.WithValue(context.Background(), jwsContextKey, jws), statusCode: 400, - err: acme.NewError(acme.ErrorMalformedType, "url header in JWS (foo) does not match request url (%s)", url), + err: acme.NewError(acme.ErrorMalformedType, "url header in JWS (foo) does not match request url (%s)", u), } }, "fail/both-jwk-kid": func(t *testing.T) test { @@ -1313,7 +1313,7 @@ func TestHandler_validateJWS(t *testing.T) { KeyID: "bar", JSONWebKey: &pub, ExtraHeaders: map[jose.HeaderKey]interface{}{ - "url": url, + "url": u, }, }, }, @@ -1337,7 +1337,7 @@ func TestHandler_validateJWS(t *testing.T) { Protected: jose.Header{ Algorithm: jose.ES256, ExtraHeaders: map[jose.HeaderKey]interface{}{ - "url": url, + "url": u, }, }, }, @@ -1362,7 +1362,7 @@ func TestHandler_validateJWS(t *testing.T) { Algorithm: jose.ES256, KeyID: "bar", ExtraHeaders: map[jose.HeaderKey]interface{}{ - "url": url, + "url": u, }, }, }, @@ -1392,7 +1392,7 @@ func TestHandler_validateJWS(t *testing.T) { Algorithm: jose.ES256, JSONWebKey: &pub, ExtraHeaders: map[jose.HeaderKey]interface{}{ - "url": url, + "url": u, }, }, }, @@ -1422,7 +1422,7 @@ func TestHandler_validateJWS(t *testing.T) { Algorithm: jose.RS256, JSONWebKey: &pub, ExtraHeaders: map[jose.HeaderKey]interface{}{ - "url": url, + "url": u, }, }, }, @@ -1446,7 +1446,7 @@ func TestHandler_validateJWS(t *testing.T) { tc := run(t) t.Run(name, func(t *testing.T) { h := &Handler{db: tc.db} - req := httptest.NewRequest("GET", url, nil) + req := httptest.NewRequest("GET", u, nil) req = req.WithContext(tc.ctx) w := httptest.NewRecorder() h.validateJWS(tc.next)(w, req) diff --git a/acme/api/order_test.go b/acme/api/order_test.go index afb23c3f..3c6d768f 100644 --- a/acme/api/order_test.go +++ b/acme/api/order_test.go @@ -264,7 +264,7 @@ func TestHandler_GetOrder(t *testing.T) { // Request with chi context chiCtx := chi.NewRouteContext() chiCtx.URLParams.Add("ordID", o.ID) - url := fmt.Sprintf("%s/acme/%s/order/%s", + u := fmt.Sprintf("%s/acme/%s/order/%s", baseURL.String(), escProvName, o.ID) type test struct { @@ -422,7 +422,7 @@ func TestHandler_GetOrder(t *testing.T) { tc := run(t) t.Run(name, func(t *testing.T) { h := &Handler{linker: NewLinker("dns", "acme"), db: tc.db} - req := httptest.NewRequest("GET", url, nil) + req := httptest.NewRequest("GET", u, nil) req = req.WithContext(tc.ctx) w := httptest.NewRecorder() h.GetOrder(w, req) @@ -448,7 +448,7 @@ func TestHandler_GetOrder(t *testing.T) { assert.FatalError(t, err) assert.Equals(t, bytes.TrimSpace(body), expB) - assert.Equals(t, res.Header["Location"], []string{url}) + assert.Equals(t, res.Header["Location"], []string{u}) assert.Equals(t, res.Header["Content-Type"], []string{"application/json"}) } }) @@ -663,7 +663,7 @@ func TestHandler_NewOrder(t *testing.T) { prov := newProv() escProvName := url.PathEscape(prov.GetName()) baseURL := &url.URL{Scheme: "https", Host: "test.ca.smallstep.com"} - url := fmt.Sprintf("%s/acme/%s/order/ordID", + u := fmt.Sprintf("%s/acme/%s/order/ordID", baseURL.String(), escProvName) type test struct { @@ -1335,7 +1335,7 @@ func TestHandler_NewOrder(t *testing.T) { tc := run(t) t.Run(name, func(t *testing.T) { h := &Handler{linker: NewLinker("dns", "acme"), db: tc.db} - req := httptest.NewRequest("GET", url, nil) + req := httptest.NewRequest("GET", u, nil) req = req.WithContext(tc.ctx) w := httptest.NewRecorder() h.NewOrder(w, req) @@ -1363,7 +1363,7 @@ func TestHandler_NewOrder(t *testing.T) { tc.vr(t, ro) } - assert.Equals(t, res.Header["Location"], []string{url}) + assert.Equals(t, res.Header["Location"], []string{u}) assert.Equals(t, res.Header["Content-Type"], []string{"application/json"}) } }) @@ -1406,7 +1406,7 @@ func TestHandler_FinalizeOrder(t *testing.T) { // Request with chi context chiCtx := chi.NewRouteContext() chiCtx.URLParams.Add("ordID", o.ID) - url := fmt.Sprintf("%s/acme/%s/order/%s", + u := fmt.Sprintf("%s/acme/%s/order/%s", baseURL.String(), escProvName, o.ID) _csr, err := pemutil.Read("../../authority/testdata/certs/foo.csr") @@ -1625,7 +1625,7 @@ func TestHandler_FinalizeOrder(t *testing.T) { tc := run(t) t.Run(name, func(t *testing.T) { h := &Handler{linker: NewLinker("dns", "acme"), db: tc.db} - req := httptest.NewRequest("GET", url, nil) + req := httptest.NewRequest("GET", u, nil) req = req.WithContext(tc.ctx) w := httptest.NewRecorder() h.FinalizeOrder(w, req) @@ -1654,7 +1654,7 @@ func TestHandler_FinalizeOrder(t *testing.T) { assert.FatalError(t, json.Unmarshal(body, ro)) assert.Equals(t, bytes.TrimSpace(body), expB) - assert.Equals(t, res.Header["Location"], []string{url}) + assert.Equals(t, res.Header["Location"], []string{u}) assert.Equals(t, res.Header["Content-Type"], []string{"application/json"}) } }) diff --git a/acme/challenge.go b/acme/challenge.go index 70c52578..b880708c 100644 --- a/acme/challenge.go +++ b/acme/challenge.go @@ -76,23 +76,23 @@ func (ch *Challenge) Validate(ctx context.Context, db DB, jwk *jose.JSONWebKey, } func http01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSONWebKey, vo *ValidateChallengeOptions) error { - url := &url.URL{Scheme: "http", Host: ch.Value, Path: fmt.Sprintf("/.well-known/acme-challenge/%s", ch.Token)} + u := &url.URL{Scheme: "http", Host: ch.Value, Path: fmt.Sprintf("/.well-known/acme-challenge/%s", ch.Token)} - resp, err := vo.HTTPGet(url.String()) + resp, err := vo.HTTPGet(u.String()) if err != nil { return storeError(ctx, db, ch, false, WrapError(ErrorConnectionType, err, - "error doing http GET for url %s", url)) + "error doing http GET for url %s", u)) } defer resp.Body.Close() if resp.StatusCode >= 400 { return storeError(ctx, db, ch, false, NewError(ErrorConnectionType, - "error doing http GET for url %s with status code %d", url, resp.StatusCode)) + "error doing http GET for url %s with status code %d", u, resp.StatusCode)) } body, err := ioutil.ReadAll(resp.Body) if err != nil { return WrapErrorISE(err, "error reading "+ - "response body for url %s", url) + "response body for url %s", u) } keyAuth := strings.TrimSpace(string(body)) diff --git a/acme/challenge_test.go b/acme/challenge_test.go index 97c5e4cd..a522790f 100644 --- a/acme/challenge_test.go +++ b/acme/challenge_test.go @@ -1276,7 +1276,7 @@ func newTLSALPNValidationCert(keyAuthHash []byte, obsoleteOID, critical bool, na oid = asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 30, 1} } - keyAuthHashEnc, _ := asn1.Marshal(keyAuthHash[:]) + keyAuthHashEnc, _ := asn1.Marshal(keyAuthHash) certTemplate.ExtraExtensions = []pkix.Extension{ { diff --git a/acme/db/nosql/account_test.go b/acme/db/nosql/account_test.go index 5ba99a73..a02e93dc 100644 --- a/acme/db/nosql/account_test.go +++ b/acme/db/nosql/account_test.go @@ -93,8 +93,8 @@ func TestDB_getDBAccount(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if dbacc, err := db.getDBAccount(context.Background(), accID); err != nil { + d := DB{db: tc.db} + if dbacc, err := d.getDBAccount(context.Background(), accID); err != nil { switch k := err.(type) { case *acme.Error: if assert.NotNil(t, tc.acmeErr) { @@ -109,15 +109,13 @@ func TestDB_getDBAccount(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) { - assert.Equals(t, dbacc.ID, tc.dbacc.ID) - assert.Equals(t, dbacc.Status, tc.dbacc.Status) - assert.Equals(t, dbacc.CreatedAt, tc.dbacc.CreatedAt) - assert.Equals(t, dbacc.DeactivatedAt, tc.dbacc.DeactivatedAt) - assert.Equals(t, dbacc.Contact, tc.dbacc.Contact) - assert.Equals(t, dbacc.Key.KeyID, tc.dbacc.Key.KeyID) - } + } else if assert.Nil(t, tc.err) { + assert.Equals(t, dbacc.ID, tc.dbacc.ID) + assert.Equals(t, dbacc.Status, tc.dbacc.Status) + assert.Equals(t, dbacc.CreatedAt, tc.dbacc.CreatedAt) + assert.Equals(t, dbacc.DeactivatedAt, tc.dbacc.DeactivatedAt) + assert.Equals(t, dbacc.Contact, tc.dbacc.Contact) + assert.Equals(t, dbacc.Key.KeyID, tc.dbacc.Key.KeyID) } }) } @@ -174,8 +172,8 @@ func TestDB_getAccountIDByKeyID(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if retAccID, err := db.getAccountIDByKeyID(context.Background(), kid); err != nil { + d := DB{db: tc.db} + if retAccID, err := d.getAccountIDByKeyID(context.Background(), kid); err != nil { switch k := err.(type) { case *acme.Error: if assert.NotNil(t, tc.acmeErr) { @@ -190,10 +188,8 @@ func TestDB_getAccountIDByKeyID(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) { - assert.Equals(t, retAccID, accID) - } + } else if assert.Nil(t, tc.err) { + assert.Equals(t, retAccID, accID) } }) } @@ -250,8 +246,8 @@ func TestDB_GetAccount(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if acc, err := db.GetAccount(context.Background(), accID); err != nil { + d := DB{db: tc.db} + if acc, err := d.GetAccount(context.Background(), accID); err != nil { switch k := err.(type) { case *acme.Error: if assert.NotNil(t, tc.acmeErr) { @@ -266,13 +262,11 @@ func TestDB_GetAccount(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) { - assert.Equals(t, acc.ID, tc.dbacc.ID) - assert.Equals(t, acc.Status, tc.dbacc.Status) - assert.Equals(t, acc.Contact, tc.dbacc.Contact) - assert.Equals(t, acc.Key.KeyID, tc.dbacc.Key.KeyID) - } + } else if assert.Nil(t, tc.err) { + assert.Equals(t, acc.ID, tc.dbacc.ID) + assert.Equals(t, acc.Status, tc.dbacc.Status) + assert.Equals(t, acc.Contact, tc.dbacc.Contact) + assert.Equals(t, acc.Key.KeyID, tc.dbacc.Key.KeyID) } }) } @@ -358,8 +352,8 @@ func TestDB_GetAccountByKeyID(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if acc, err := db.GetAccountByKeyID(context.Background(), kid); err != nil { + d := DB{db: tc.db} + if acc, err := d.GetAccountByKeyID(context.Background(), kid); err != nil { switch k := err.(type) { case *acme.Error: if assert.NotNil(t, tc.acmeErr) { @@ -374,13 +368,11 @@ func TestDB_GetAccountByKeyID(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) { - assert.Equals(t, acc.ID, tc.dbacc.ID) - assert.Equals(t, acc.Status, tc.dbacc.Status) - assert.Equals(t, acc.Contact, tc.dbacc.Contact) - assert.Equals(t, acc.Key.KeyID, tc.dbacc.Key.KeyID) - } + } else if assert.Nil(t, tc.err) { + assert.Equals(t, acc.ID, tc.dbacc.ID) + assert.Equals(t, acc.Status, tc.dbacc.Status) + assert.Equals(t, acc.Contact, tc.dbacc.Contact) + assert.Equals(t, acc.Key.KeyID, tc.dbacc.Key.KeyID) } }) } @@ -527,8 +519,8 @@ func TestDB_CreateAccount(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if err := db.CreateAccount(context.Background(), tc.acc); err != nil { + d := DB{db: tc.db} + if err := d.CreateAccount(context.Background(), tc.acc); err != nil { if assert.NotNil(t, tc.err) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } @@ -688,8 +680,8 @@ func TestDB_UpdateAccount(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if err := db.UpdateAccount(context.Background(), tc.acc); err != nil { + d := DB{db: tc.db} + if err := d.UpdateAccount(context.Background(), tc.acc); err != nil { if assert.NotNil(t, tc.err) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } diff --git a/acme/db/nosql/authz_test.go b/acme/db/nosql/authz_test.go index 0c2cec50..01c255dc 100644 --- a/acme/db/nosql/authz_test.go +++ b/acme/db/nosql/authz_test.go @@ -97,8 +97,8 @@ func TestDB_getDBAuthz(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if dbaz, err := db.getDBAuthz(context.Background(), azID); err != nil { + d := DB{db: tc.db} + if dbaz, err := d.getDBAuthz(context.Background(), azID); err != nil { switch k := err.(type) { case *acme.Error: if assert.NotNil(t, tc.acmeErr) { @@ -113,18 +113,16 @@ func TestDB_getDBAuthz(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) { - assert.Equals(t, dbaz.ID, tc.dbaz.ID) - assert.Equals(t, dbaz.AccountID, tc.dbaz.AccountID) - assert.Equals(t, dbaz.Identifier, tc.dbaz.Identifier) - assert.Equals(t, dbaz.Status, tc.dbaz.Status) - assert.Equals(t, dbaz.Token, tc.dbaz.Token) - assert.Equals(t, dbaz.CreatedAt, tc.dbaz.CreatedAt) - assert.Equals(t, dbaz.ExpiresAt, tc.dbaz.ExpiresAt) - assert.Equals(t, dbaz.Error.Error(), tc.dbaz.Error.Error()) - assert.Equals(t, dbaz.Wildcard, tc.dbaz.Wildcard) - } + } else if assert.Nil(t, tc.err) { + assert.Equals(t, dbaz.ID, tc.dbaz.ID) + assert.Equals(t, dbaz.AccountID, tc.dbaz.AccountID) + assert.Equals(t, dbaz.Identifier, tc.dbaz.Identifier) + assert.Equals(t, dbaz.Status, tc.dbaz.Status) + assert.Equals(t, dbaz.Token, tc.dbaz.Token) + assert.Equals(t, dbaz.CreatedAt, tc.dbaz.CreatedAt) + assert.Equals(t, dbaz.ExpiresAt, tc.dbaz.ExpiresAt) + assert.Equals(t, dbaz.Error.Error(), tc.dbaz.Error.Error()) + assert.Equals(t, dbaz.Wildcard, tc.dbaz.Wildcard) } }) } @@ -293,8 +291,8 @@ func TestDB_GetAuthorization(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if az, err := db.GetAuthorization(context.Background(), azID); err != nil { + d := DB{db: tc.db} + if az, err := d.GetAuthorization(context.Background(), azID); err != nil { switch k := err.(type) { case *acme.Error: if assert.NotNil(t, tc.acmeErr) { @@ -309,21 +307,19 @@ func TestDB_GetAuthorization(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) { - assert.Equals(t, az.ID, tc.dbaz.ID) - assert.Equals(t, az.AccountID, tc.dbaz.AccountID) - assert.Equals(t, az.Identifier, tc.dbaz.Identifier) - assert.Equals(t, az.Status, tc.dbaz.Status) - assert.Equals(t, az.Token, tc.dbaz.Token) - assert.Equals(t, az.Wildcard, tc.dbaz.Wildcard) - assert.Equals(t, az.ExpiresAt, tc.dbaz.ExpiresAt) - assert.Equals(t, az.Challenges, []*acme.Challenge{ - {ID: "foo"}, - {ID: "bar"}, - }) - assert.Equals(t, az.Error.Error(), tc.dbaz.Error.Error()) - } + } else if assert.Nil(t, tc.err) { + assert.Equals(t, az.ID, tc.dbaz.ID) + assert.Equals(t, az.AccountID, tc.dbaz.AccountID) + assert.Equals(t, az.Identifier, tc.dbaz.Identifier) + assert.Equals(t, az.Status, tc.dbaz.Status) + assert.Equals(t, az.Token, tc.dbaz.Token) + assert.Equals(t, az.Wildcard, tc.dbaz.Wildcard) + assert.Equals(t, az.ExpiresAt, tc.dbaz.ExpiresAt) + assert.Equals(t, az.Challenges, []*acme.Challenge{ + {ID: "foo"}, + {ID: "bar"}, + }) + assert.Equals(t, az.Error.Error(), tc.dbaz.Error.Error()) } }) } @@ -445,8 +441,8 @@ func TestDB_CreateAuthorization(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if err := db.CreateAuthorization(context.Background(), tc.az); err != nil { + d := DB{db: tc.db} + if err := d.CreateAuthorization(context.Background(), tc.az); err != nil { if assert.NotNil(t, tc.err) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } @@ -594,8 +590,8 @@ func TestDB_UpdateAuthorization(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if err := db.UpdateAuthorization(context.Background(), tc.az); err != nil { + d := DB{db: tc.db} + if err := d.UpdateAuthorization(context.Background(), tc.az); err != nil { if assert.NotNil(t, tc.err) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } diff --git a/acme/db/nosql/certificate_test.go b/acme/db/nosql/certificate_test.go index 4ec4589e..37a61352 100644 --- a/acme/db/nosql/certificate_test.go +++ b/acme/db/nosql/certificate_test.go @@ -98,8 +98,8 @@ func TestDB_CreateCertificate(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if err := db.CreateCertificate(context.Background(), tc.cert); err != nil { + d := DB{db: tc.db} + if err := d.CreateCertificate(context.Background(), tc.cert); err != nil { if assert.NotNil(t, tc.err) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } @@ -228,8 +228,8 @@ func TestDB_GetCertificate(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - cert, err := db.GetCertificate(context.Background(), certID) + d := DB{db: tc.db} + cert, err := d.GetCertificate(context.Background(), certID) if err != nil { switch k := err.(type) { case *acme.Error: @@ -245,14 +245,12 @@ func TestDB_GetCertificate(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) { - assert.Equals(t, cert.ID, certID) - assert.Equals(t, cert.AccountID, "accountID") - assert.Equals(t, cert.OrderID, "orderID") - assert.Equals(t, cert.Leaf, leaf) - assert.Equals(t, cert.Intermediates, []*x509.Certificate{inter, root}) - } + } else if assert.Nil(t, tc.err) { + assert.Equals(t, cert.ID, certID) + assert.Equals(t, cert.AccountID, "accountID") + assert.Equals(t, cert.OrderID, "orderID") + assert.Equals(t, cert.Leaf, leaf) + assert.Equals(t, cert.Intermediates, []*x509.Certificate{inter, root}) } }) } diff --git a/acme/db/nosql/challenge_test.go b/acme/db/nosql/challenge_test.go index b39395e8..4da5679b 100644 --- a/acme/db/nosql/challenge_test.go +++ b/acme/db/nosql/challenge_test.go @@ -92,8 +92,8 @@ func TestDB_getDBChallenge(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if ch, err := db.getDBChallenge(context.Background(), chID); err != nil { + d := DB{db: tc.db} + if ch, err := d.getDBChallenge(context.Background(), chID); err != nil { switch k := err.(type) { case *acme.Error: if assert.NotNil(t, tc.acmeErr) { @@ -108,17 +108,15 @@ func TestDB_getDBChallenge(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) { - assert.Equals(t, ch.ID, tc.dbc.ID) - assert.Equals(t, ch.AccountID, tc.dbc.AccountID) - assert.Equals(t, ch.Type, tc.dbc.Type) - assert.Equals(t, ch.Status, tc.dbc.Status) - assert.Equals(t, ch.Token, tc.dbc.Token) - assert.Equals(t, ch.Value, tc.dbc.Value) - assert.Equals(t, ch.ValidatedAt, tc.dbc.ValidatedAt) - assert.Equals(t, ch.Error.Error(), tc.dbc.Error.Error()) - } + } else if assert.Nil(t, tc.err) { + assert.Equals(t, ch.ID, tc.dbc.ID) + assert.Equals(t, ch.AccountID, tc.dbc.AccountID) + assert.Equals(t, ch.Type, tc.dbc.Type) + assert.Equals(t, ch.Status, tc.dbc.Status) + assert.Equals(t, ch.Token, tc.dbc.Token) + assert.Equals(t, ch.Value, tc.dbc.Value) + assert.Equals(t, ch.ValidatedAt, tc.dbc.ValidatedAt) + assert.Equals(t, ch.Error.Error(), tc.dbc.Error.Error()) } }) } @@ -206,8 +204,8 @@ func TestDB_CreateChallenge(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if err := db.CreateChallenge(context.Background(), tc.ch); err != nil { + d := DB{db: tc.db} + if err := d.CreateChallenge(context.Background(), tc.ch); err != nil { if assert.NotNil(t, tc.err) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } @@ -286,8 +284,8 @@ func TestDB_GetChallenge(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if ch, err := db.GetChallenge(context.Background(), chID, azID); err != nil { + d := DB{db: tc.db} + if ch, err := d.GetChallenge(context.Background(), chID, azID); err != nil { switch k := err.(type) { case *acme.Error: if assert.NotNil(t, tc.acmeErr) { @@ -302,17 +300,15 @@ func TestDB_GetChallenge(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) { - assert.Equals(t, ch.ID, tc.dbc.ID) - assert.Equals(t, ch.AccountID, tc.dbc.AccountID) - assert.Equals(t, ch.Type, tc.dbc.Type) - assert.Equals(t, ch.Status, tc.dbc.Status) - assert.Equals(t, ch.Token, tc.dbc.Token) - assert.Equals(t, ch.Value, tc.dbc.Value) - assert.Equals(t, ch.ValidatedAt, tc.dbc.ValidatedAt) - assert.Equals(t, ch.Error.Error(), tc.dbc.Error.Error()) - } + } else if assert.Nil(t, tc.err) { + assert.Equals(t, ch.ID, tc.dbc.ID) + assert.Equals(t, ch.AccountID, tc.dbc.AccountID) + assert.Equals(t, ch.Type, tc.dbc.Type) + assert.Equals(t, ch.Status, tc.dbc.Status) + assert.Equals(t, ch.Token, tc.dbc.Token) + assert.Equals(t, ch.Value, tc.dbc.Value) + assert.Equals(t, ch.ValidatedAt, tc.dbc.ValidatedAt) + assert.Equals(t, ch.Error.Error(), tc.dbc.Error.Error()) } }) } @@ -442,8 +438,8 @@ func TestDB_UpdateChallenge(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if err := db.UpdateChallenge(context.Background(), tc.ch); err != nil { + d := DB{db: tc.db} + if err := d.UpdateChallenge(context.Background(), tc.ch); err != nil { if assert.NotNil(t, tc.err) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } diff --git a/acme/db/nosql/nonce.go b/acme/db/nosql/nonce.go index 9badae87..e438c9ed 100644 --- a/acme/db/nosql/nonce.go +++ b/acme/db/nosql/nonce.go @@ -31,7 +31,7 @@ func (db *DB) CreateNonce(ctx context.Context) (acme.Nonce, error) { ID: id, CreatedAt: clock.Now(), } - if err = db.save(ctx, id, n, nil, "nonce", nonceTable); err != nil { + if err := db.save(ctx, id, n, nil, "nonce", nonceTable); err != nil { return "", err } return acme.Nonce(id), nil diff --git a/acme/db/nosql/nonce_test.go b/acme/db/nosql/nonce_test.go index 05d73d52..7dc5cc91 100644 --- a/acme/db/nosql/nonce_test.go +++ b/acme/db/nosql/nonce_test.go @@ -67,8 +67,8 @@ func TestDB_CreateNonce(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if n, err := db.CreateNonce(context.Background()); err != nil { + d := DB{db: tc.db} + if n, err := d.CreateNonce(context.Background()); err != nil { if assert.NotNil(t, tc.err) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } @@ -144,8 +144,8 @@ func TestDB_DeleteNonce(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if err := db.DeleteNonce(context.Background(), acme.Nonce(nonceID)); err != nil { + d := DB{db: tc.db} + if err := d.DeleteNonce(context.Background(), acme.Nonce(nonceID)); err != nil { switch k := err.(type) { case *acme.Error: if assert.NotNil(t, tc.acmeErr) { diff --git a/acme/db/nosql/nosql.go b/acme/db/nosql/nosql.go index 052f5729..b1547373 100644 --- a/acme/db/nosql/nosql.go +++ b/acme/db/nosql/nosql.go @@ -41,7 +41,7 @@ func New(db nosqlDB.DB) (*DB, error) { // save writes the new data to the database, overwriting the old data if it // existed. -func (db *DB) save(ctx context.Context, id string, nu interface{}, old interface{}, typ string, table []byte) error { +func (db *DB) save(ctx context.Context, id string, nu, old interface{}, typ string, table []byte) error { var ( err error newB []byte diff --git a/acme/db/nosql/nosql_test.go b/acme/db/nosql/nosql_test.go index 4396acc8..d9c0b484 100644 --- a/acme/db/nosql/nosql_test.go +++ b/acme/db/nosql/nosql_test.go @@ -126,8 +126,8 @@ func TestDB_save(t *testing.T) { } for name, tc := range tests { t.Run(name, func(t *testing.T) { - db := &DB{db: tc.db} - if err := db.save(context.Background(), "id", tc.nu, tc.old, "challenge", challengeTable); err != nil { + d := &DB{db: tc.db} + if err := d.save(context.Background(), "id", tc.nu, tc.old, "challenge", challengeTable); err != nil { if assert.NotNil(t, tc.err) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } diff --git a/acme/db/nosql/order_test.go b/acme/db/nosql/order_test.go index 8882fd82..e92eb684 100644 --- a/acme/db/nosql/order_test.go +++ b/acme/db/nosql/order_test.go @@ -13,7 +13,6 @@ import ( "github.com/smallstep/certificates/db" "github.com/smallstep/nosql" "github.com/smallstep/nosql/database" - nosqldb "github.com/smallstep/nosql/database" ) func TestDB_getDBOrder(t *testing.T) { @@ -32,7 +31,7 @@ func TestDB_getDBOrder(t *testing.T) { assert.Equals(t, bucket, orderTable) assert.Equals(t, string(key), orderID) - return nil, nosqldb.ErrNotFound + return nil, database.ErrNotFound }, }, acmeErr: acme.NewError(acme.ErrorMalformedType, "order orderID not found"), @@ -101,8 +100,8 @@ func TestDB_getDBOrder(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if dbo, err := db.getDBOrder(context.Background(), orderID); err != nil { + d := DB{db: tc.db} + if dbo, err := d.getDBOrder(context.Background(), orderID); err != nil { switch k := err.(type) { case *acme.Error: if assert.NotNil(t, tc.acmeErr) { @@ -117,20 +116,18 @@ func TestDB_getDBOrder(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) { - assert.Equals(t, dbo.ID, tc.dbo.ID) - assert.Equals(t, dbo.ProvisionerID, tc.dbo.ProvisionerID) - assert.Equals(t, dbo.CertificateID, tc.dbo.CertificateID) - assert.Equals(t, dbo.Status, tc.dbo.Status) - assert.Equals(t, dbo.CreatedAt, tc.dbo.CreatedAt) - assert.Equals(t, dbo.ExpiresAt, tc.dbo.ExpiresAt) - assert.Equals(t, dbo.NotBefore, tc.dbo.NotBefore) - assert.Equals(t, dbo.NotAfter, tc.dbo.NotAfter) - assert.Equals(t, dbo.Identifiers, tc.dbo.Identifiers) - assert.Equals(t, dbo.AuthorizationIDs, tc.dbo.AuthorizationIDs) - assert.Equals(t, dbo.Error.Error(), tc.dbo.Error.Error()) - } + } else if assert.Nil(t, tc.err) { + assert.Equals(t, dbo.ID, tc.dbo.ID) + assert.Equals(t, dbo.ProvisionerID, tc.dbo.ProvisionerID) + assert.Equals(t, dbo.CertificateID, tc.dbo.CertificateID) + assert.Equals(t, dbo.Status, tc.dbo.Status) + assert.Equals(t, dbo.CreatedAt, tc.dbo.CreatedAt) + assert.Equals(t, dbo.ExpiresAt, tc.dbo.ExpiresAt) + assert.Equals(t, dbo.NotBefore, tc.dbo.NotBefore) + assert.Equals(t, dbo.NotAfter, tc.dbo.NotAfter) + assert.Equals(t, dbo.Identifiers, tc.dbo.Identifiers) + assert.Equals(t, dbo.AuthorizationIDs, tc.dbo.AuthorizationIDs) + assert.Equals(t, dbo.Error.Error(), tc.dbo.Error.Error()) } }) } @@ -165,7 +162,7 @@ func TestDB_GetOrder(t *testing.T) { assert.Equals(t, bucket, orderTable) assert.Equals(t, string(key), orderID) - return nil, nosqldb.ErrNotFound + return nil, database.ErrNotFound }, }, acmeErr: acme.NewError(acme.ErrorMalformedType, "order orderID not found"), @@ -207,8 +204,8 @@ func TestDB_GetOrder(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if o, err := db.GetOrder(context.Background(), orderID); err != nil { + d := DB{db: tc.db} + if o, err := d.GetOrder(context.Background(), orderID); err != nil { switch k := err.(type) { case *acme.Error: if assert.NotNil(t, tc.acmeErr) { @@ -223,20 +220,18 @@ func TestDB_GetOrder(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) { - assert.Equals(t, o.ID, tc.dbo.ID) - assert.Equals(t, o.AccountID, tc.dbo.AccountID) - assert.Equals(t, o.ProvisionerID, tc.dbo.ProvisionerID) - assert.Equals(t, o.CertificateID, tc.dbo.CertificateID) - assert.Equals(t, o.Status, tc.dbo.Status) - assert.Equals(t, o.ExpiresAt, tc.dbo.ExpiresAt) - assert.Equals(t, o.NotBefore, tc.dbo.NotBefore) - assert.Equals(t, o.NotAfter, tc.dbo.NotAfter) - assert.Equals(t, o.Identifiers, tc.dbo.Identifiers) - assert.Equals(t, o.AuthorizationIDs, tc.dbo.AuthorizationIDs) - assert.Equals(t, o.Error.Error(), tc.dbo.Error.Error()) - } + } else if assert.Nil(t, tc.err) { + assert.Equals(t, o.ID, tc.dbo.ID) + assert.Equals(t, o.AccountID, tc.dbo.AccountID) + assert.Equals(t, o.ProvisionerID, tc.dbo.ProvisionerID) + assert.Equals(t, o.CertificateID, tc.dbo.CertificateID) + assert.Equals(t, o.Status, tc.dbo.Status) + assert.Equals(t, o.ExpiresAt, tc.dbo.ExpiresAt) + assert.Equals(t, o.NotBefore, tc.dbo.NotBefore) + assert.Equals(t, o.NotAfter, tc.dbo.NotAfter) + assert.Equals(t, o.Identifiers, tc.dbo.Identifiers) + assert.Equals(t, o.AuthorizationIDs, tc.dbo.AuthorizationIDs) + assert.Equals(t, o.Error.Error(), tc.dbo.Error.Error()) } }) } @@ -367,8 +362,8 @@ func TestDB_UpdateOrder(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if err := db.UpdateOrder(context.Background(), tc.o); err != nil { + d := DB{db: tc.db} + if err := d.UpdateOrder(context.Background(), tc.o); err != nil { if assert.NotNil(t, tc.err) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } @@ -512,7 +507,7 @@ func TestDB_CreateOrder(t *testing.T) { MGet: func(bucket, key []byte) ([]byte, error) { assert.Equals(t, string(bucket), string(ordersByAccountIDTable)) assert.Equals(t, string(key), o.AccountID) - return nil, nosqldb.ErrNotFound + return nil, database.ErrNotFound }, MCmpAndSwap: func(bucket, key, old, nu []byte) ([]byte, bool, error) { switch string(bucket) { @@ -558,8 +553,8 @@ func TestDB_CreateOrder(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if err := db.CreateOrder(context.Background(), tc.o); err != nil { + d := DB{db: tc.db} + if err := d.CreateOrder(context.Background(), tc.o); err != nil { if assert.NotNil(t, tc.err) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } @@ -681,7 +676,7 @@ func TestDB_updateAddOrderIDs(t *testing.T) { MGet: func(bucket, key []byte) ([]byte, error) { assert.Equals(t, bucket, ordersByAccountIDTable) assert.Equals(t, key, []byte(accID)) - return nil, nosqldb.ErrNotFound + return nil, database.ErrNotFound }, MCmpAndSwap: func(bucket, key, old, nu []byte) ([]byte, bool, error) { assert.Equals(t, bucket, ordersByAccountIDTable) @@ -996,15 +991,15 @@ func TestDB_updateAddOrderIDs(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} + d := DB{db: tc.db} var ( res []string err error ) if tc.addOids == nil { - res, err = db.updateAddOrderIDs(context.Background(), accID) + res, err = d.updateAddOrderIDs(context.Background(), accID) } else { - res, err = db.updateAddOrderIDs(context.Background(), accID, tc.addOids...) + res, err = d.updateAddOrderIDs(context.Background(), accID, tc.addOids...) } if err != nil { @@ -1022,10 +1017,8 @@ func TestDB_updateAddOrderIDs(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) { - assert.True(t, reflect.DeepEqual(res, tc.res)) - } + } else if assert.Nil(t, tc.err) { + assert.True(t, reflect.DeepEqual(res, tc.res)) } }) } diff --git a/acme/order.go b/acme/order.go index fd8956f7..237c6979 100644 --- a/acme/order.go +++ b/acme/order.go @@ -289,6 +289,7 @@ func canonicalize(csr *x509.CertificateRequest) (canonicalized *x509.Certificate // name or in an extensionRequest attribute [RFC2985] requesting a // subjectAltName extension, or both. if csr.Subject.CommonName != "" { + // nolint:gocritic canonicalized.DNSNames = append(csr.DNSNames, csr.Subject.CommonName) } canonicalized.DNSNames = uniqueSortedLowerNames(csr.DNSNames) diff --git a/api/api.go b/api/api.go index 5be9ecc1..30ba03f9 100644 --- a/api/api.go +++ b/api/api.go @@ -240,9 +240,9 @@ type caHandler struct { } // New creates a new RouterHandler with the CA endpoints. -func New(authority Authority) RouterHandler { +func New(auth Authority) RouterHandler { return &caHandler{ - Authority: authority, + Authority: auth, } } @@ -295,7 +295,7 @@ func (h *caHandler) Health(w http.ResponseWriter, r *http.Request) { // certificate for the given SHA256. func (h *caHandler) Root(w http.ResponseWriter, r *http.Request) { sha := chi.URLParam(r, "sha") - sum := strings.ToLower(strings.Replace(sha, "-", "", -1)) + sum := strings.ToLower(strings.ReplaceAll(sha, "-", "")) // Load root certificate with the cert, err := h.Authority.Root(sum) if err != nil { @@ -409,19 +409,20 @@ func LogCertificate(w http.ResponseWriter, cert *x509.Certificate) { "certificate": base64.StdEncoding.EncodeToString(cert.Raw), } for _, ext := range cert.Extensions { - if ext.Id.Equal(oidStepProvisioner) { - val := &stepProvisioner{} - rest, err := asn1.Unmarshal(ext.Value, val) - if err != nil || len(rest) > 0 { - break - } - if len(val.CredentialID) > 0 { - m["provisioner"] = fmt.Sprintf("%s (%s)", val.Name, val.CredentialID) - } else { - m["provisioner"] = string(val.Name) - } + if !ext.Id.Equal(oidStepProvisioner) { + continue + } + val := &stepProvisioner{} + rest, err := asn1.Unmarshal(ext.Value, val) + if err != nil || len(rest) > 0 { break } + if len(val.CredentialID) > 0 { + m["provisioner"] = fmt.Sprintf("%s (%s)", val.Name, val.CredentialID) + } else { + m["provisioner"] = string(val.Name) + } + break } rl.WithFields(m) } diff --git a/api/api_test.go b/api/api_test.go index 33d2bae7..89596165 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -186,8 +186,8 @@ func TestCertificate_MarshalJSON(t *testing.T) { }{ {"nil", fields{Certificate: nil}, []byte("null"), false}, {"empty", fields{Certificate: &x509.Certificate{Raw: nil}}, []byte(`"-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----\n"`), false}, - {"root", fields{Certificate: parseCertificate(rootPEM)}, []byte(`"` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n"`), false}, - {"cert", fields{Certificate: parseCertificate(certPEM)}, []byte(`"` + strings.Replace(certPEM, "\n", `\n`, -1) + `\n"`), false}, + {"root", fields{Certificate: parseCertificate(rootPEM)}, []byte(`"` + strings.ReplaceAll(rootPEM, "\n", `\n`) + `\n"`), false}, + {"cert", fields{Certificate: parseCertificate(certPEM)}, []byte(`"` + strings.ReplaceAll(certPEM, "\n", `\n`) + `\n"`), false}, } for _, tt := range tests { @@ -219,11 +219,11 @@ func TestCertificate_UnmarshalJSON(t *testing.T) { {"invalid string", []byte(`"foobar"`), false, true}, {"invalid bytes 0", []byte{}, false, true}, {"invalid bytes 1", []byte{1}, false, true}, {"empty csr", []byte(`"-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE----\n"`), false, true}, - {"invalid type", []byte(`"` + strings.Replace(csrPEM, "\n", `\n`, -1) + `"`), false, true}, + {"invalid type", []byte(`"` + strings.ReplaceAll(csrPEM, "\n", `\n`) + `"`), false, true}, {"empty string", []byte(`""`), false, false}, {"json null", []byte(`null`), false, false}, - {"valid root", []byte(`"` + strings.Replace(rootPEM, "\n", `\n`, -1) + `"`), true, false}, - {"valid cert", []byte(`"` + strings.Replace(certPEM, "\n", `\n`, -1) + `"`), true, false}, + {"valid root", []byte(`"` + strings.ReplaceAll(rootPEM, "\n", `\n`) + `"`), true, false}, + {"valid cert", []byte(`"` + strings.ReplaceAll(certPEM, "\n", `\n`) + `"`), true, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -251,7 +251,7 @@ func TestCertificate_UnmarshalJSON_json(t *testing.T) { {"empty crt (null)", `{"crt":null}`, false, false}, {"empty crt (string)", `{"crt":""}`, false, false}, {"empty crt", `{"crt":"-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE----\n"}`, false, true}, - {"valid crt", `{"crt":"` + strings.Replace(certPEM, "\n", `\n`, -1) + `"}`, true, false}, + {"valid crt", `{"crt":"` + strings.ReplaceAll(certPEM, "\n", `\n`) + `"}`, true, false}, } type request struct { @@ -297,7 +297,7 @@ func TestCertificateRequest_MarshalJSON(t *testing.T) { }{ {"nil", fields{CertificateRequest: nil}, []byte("null"), false}, {"empty", fields{CertificateRequest: &x509.CertificateRequest{}}, []byte(`"-----BEGIN CERTIFICATE REQUEST-----\n-----END CERTIFICATE REQUEST-----\n"`), false}, - {"csr", fields{CertificateRequest: parseCertificateRequest(csrPEM)}, []byte(`"` + strings.Replace(csrPEM, "\n", `\n`, -1) + `\n"`), false}, + {"csr", fields{CertificateRequest: parseCertificateRequest(csrPEM)}, []byte(`"` + strings.ReplaceAll(csrPEM, "\n", `\n`) + `\n"`), false}, } for _, tt := range tests { @@ -329,10 +329,10 @@ func TestCertificateRequest_UnmarshalJSON(t *testing.T) { {"invalid string", []byte(`"foobar"`), false, true}, {"invalid bytes 0", []byte{}, false, true}, {"invalid bytes 1", []byte{1}, false, true}, {"empty csr", []byte(`"-----BEGIN CERTIFICATE REQUEST-----\n-----END CERTIFICATE REQUEST----\n"`), false, true}, - {"invalid type", []byte(`"` + strings.Replace(rootPEM, "\n", `\n`, -1) + `"`), false, true}, + {"invalid type", []byte(`"` + strings.ReplaceAll(rootPEM, "\n", `\n`) + `"`), false, true}, {"empty string", []byte(`""`), false, false}, {"json null", []byte(`null`), false, false}, - {"valid csr", []byte(`"` + strings.Replace(csrPEM, "\n", `\n`, -1) + `"`), true, false}, + {"valid csr", []byte(`"` + strings.ReplaceAll(csrPEM, "\n", `\n`) + `"`), true, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -360,7 +360,7 @@ func TestCertificateRequest_UnmarshalJSON_json(t *testing.T) { {"empty csr (null)", `{"csr":null}`, false, false}, {"empty csr (string)", `{"csr":""}`, false, false}, {"empty csr", `{"csr":"-----BEGIN CERTIFICATE REQUEST-----\n-----END CERTIFICATE REQUEST----\n"}`, false, true}, - {"valid csr", `{"csr":"` + strings.Replace(csrPEM, "\n", `\n`, -1) + `"}`, true, false}, + {"valid csr", `{"csr":"` + strings.ReplaceAll(csrPEM, "\n", `\n`) + `"}`, true, false}, } type request struct { @@ -739,7 +739,7 @@ func (m *mockAuthority) CheckSSHHost(ctx context.Context, principal, token strin return m.ret1.(bool), m.err } -func (m *mockAuthority) GetSSHBastion(ctx context.Context, user string, hostname string) (*authority.Bastion, error) { +func (m *mockAuthority) GetSSHBastion(ctx context.Context, user, hostname string) (*authority.Bastion, error) { if m.getSSHBastion != nil { return m.getSSHBastion(ctx, user, hostname) } @@ -816,7 +816,7 @@ func Test_caHandler_Root(t *testing.T) { req := httptest.NewRequest("GET", "http://example.com/root/efc7d6b475a56fe587650bcdb999a4a308f815ba44db4bf0371ea68a786ccd36", nil) req = req.WithContext(context.WithValue(context.Background(), chi.RouteCtxKey, chiCtx)) - expected := []byte(`{"ca":"` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n"}`) + expected := []byte(`{"ca":"` + strings.ReplaceAll(rootPEM, "\n", `\n`) + `\n"}`) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -860,8 +860,8 @@ func Test_caHandler_Sign(t *testing.T) { t.Fatal(err) } - expected1 := []byte(`{"crt":"` + strings.Replace(certPEM, "\n", `\n`, -1) + `\n","ca":"` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n","certChain":["` + strings.Replace(certPEM, "\n", `\n`, -1) + `\n","` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n"]}`) - expected2 := []byte(`{"crt":"` + strings.Replace(stepCertPEM, "\n", `\n`, -1) + `\n","ca":"` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n","certChain":["` + strings.Replace(stepCertPEM, "\n", `\n`, -1) + `\n","` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n"]}`) + expected1 := []byte(`{"crt":"` + strings.ReplaceAll(certPEM, "\n", `\n`) + `\n","ca":"` + strings.ReplaceAll(rootPEM, "\n", `\n`) + `\n","certChain":["` + strings.ReplaceAll(certPEM, "\n", `\n`) + `\n","` + strings.ReplaceAll(rootPEM, "\n", `\n`) + `\n"]}`) + expected2 := []byte(`{"crt":"` + strings.ReplaceAll(stepCertPEM, "\n", `\n`) + `\n","ca":"` + strings.ReplaceAll(rootPEM, "\n", `\n`) + `\n","certChain":["` + strings.ReplaceAll(stepCertPEM, "\n", `\n`) + `\n","` + strings.ReplaceAll(rootPEM, "\n", `\n`) + `\n"]}`) tests := []struct { name string @@ -934,7 +934,7 @@ func Test_caHandler_Renew(t *testing.T) { {"renew error", cs, nil, nil, errs.Forbidden("an error"), http.StatusForbidden}, } - expected := []byte(`{"crt":"` + strings.Replace(certPEM, "\n", `\n`, -1) + `\n","ca":"` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n","certChain":["` + strings.Replace(certPEM, "\n", `\n`, -1) + `\n","` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n"]}`) + expected := []byte(`{"crt":"` + strings.ReplaceAll(certPEM, "\n", `\n`) + `\n","ca":"` + strings.ReplaceAll(rootPEM, "\n", `\n`) + `\n","certChain":["` + strings.ReplaceAll(certPEM, "\n", `\n`) + `\n","` + strings.ReplaceAll(rootPEM, "\n", `\n`) + `\n"]}`) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -995,7 +995,7 @@ func Test_caHandler_Rekey(t *testing.T) { {"json read error", "{", cs, nil, nil, nil, http.StatusBadRequest}, } - expected := []byte(`{"crt":"` + strings.Replace(certPEM, "\n", `\n`, -1) + `\n","ca":"` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n","certChain":["` + strings.Replace(certPEM, "\n", `\n`, -1) + `\n","` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n"]}`) + expected := []byte(`{"crt":"` + strings.ReplaceAll(certPEM, "\n", `\n`) + `\n","ca":"` + strings.ReplaceAll(rootPEM, "\n", `\n`) + `\n","certChain":["` + strings.ReplaceAll(certPEM, "\n", `\n`) + `\n","` + strings.ReplaceAll(rootPEM, "\n", `\n`) + `\n"]}`) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1210,7 +1210,7 @@ func Test_caHandler_Roots(t *testing.T) { {"fail", cs, nil, nil, fmt.Errorf("an error"), http.StatusForbidden}, } - expected := []byte(`{"crts":["` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n"]}`) + expected := []byte(`{"crts":["` + strings.ReplaceAll(rootPEM, "\n", `\n`) + `\n"]}`) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1256,7 +1256,7 @@ func Test_caHandler_Federation(t *testing.T) { {"fail", cs, nil, nil, fmt.Errorf("an error"), http.StatusForbidden}, } - expected := []byte(`{"crts":["` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n"]}`) + expected := []byte(`{"crts":["` + strings.ReplaceAll(rootPEM, "\n", `\n`) + `\n"]}`) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/api/errors.go b/api/errors.go index db3bc3e2..bff46b55 100644 --- a/api/errors.go +++ b/api/errors.go @@ -50,12 +50,10 @@ func WriteError(w http.ResponseWriter, err error) { rl.WithFields(map[string]interface{}{ "stack-trace": fmt.Sprintf("%+v", e), }) - } else { - if e, ok := cause.(errs.StackTracer); ok { - rl.WithFields(map[string]interface{}{ - "stack-trace": fmt.Sprintf("%+v", e), - }) - } + } else if e, ok := cause.(errs.StackTracer); ok { + rl.WithFields(map[string]interface{}{ + "stack-trace": fmt.Sprintf("%+v", e), + }) } } } diff --git a/api/ssh.go b/api/ssh.go index 8c0c1aa3..7c7a5acd 100644 --- a/api/ssh.go +++ b/api/ssh.go @@ -52,7 +52,7 @@ func (s *SSHSignRequest) Validate() error { return errors.Errorf("unknown certType %s", s.CertType) case len(s.PublicKey) == 0: return errors.New("missing or empty publicKey") - case len(s.OTT) == 0: + case s.OTT == "": return errors.New("missing or empty ott") default: // Validate identity signature if provided @@ -408,18 +408,18 @@ func (h *caHandler) SSHConfig(w http.ResponseWriter, r *http.Request) { return } - var config SSHConfigResponse + var cfg SSHConfigResponse switch body.Type { case provisioner.SSHUserCert: - config.UserTemplates = ts + cfg.UserTemplates = ts case provisioner.SSHHostCert: - config.HostTemplates = ts + cfg.HostTemplates = ts default: WriteError(w, errs.InternalServer("it should hot get here")) return } - JSON(w, config) + JSON(w, cfg) } // SSHCheckHost is the HTTP handler that returns if a hosts certificate exists or not. diff --git a/api/sshRekey.go b/api/sshRekey.go index 3d8e7c47..9d9e17cf 100644 --- a/api/sshRekey.go +++ b/api/sshRekey.go @@ -19,7 +19,7 @@ type SSHRekeyRequest struct { // Validate validates the SSHSignRekey. func (s *SSHRekeyRequest) Validate() error { switch { - case len(s.OTT) == 0: + case s.OTT == "": return errors.New("missing or empty ott") case len(s.PublicKey) == 0: return errors.New("missing or empty public key") diff --git a/api/sshRenew.go b/api/sshRenew.go index cb6ec5fd..d0633ecf 100644 --- a/api/sshRenew.go +++ b/api/sshRenew.go @@ -18,7 +18,7 @@ type SSHRenewRequest struct { // Validate validates the SSHSignRequest. func (s *SSHRenewRequest) Validate() error { switch { - case len(s.OTT) == 0: + case s.OTT == "": return errors.New("missing or empty ott") default: return nil diff --git a/api/sshRevoke.go b/api/sshRevoke.go index 5a1c858c..c6ebe99d 100644 --- a/api/sshRevoke.go +++ b/api/sshRevoke.go @@ -36,7 +36,7 @@ func (r *SSHRevokeRequest) Validate() (err error) { if !r.Passive { return errs.NotImplemented("non-passive revocation not implemented") } - if len(r.OTT) == 0 { + if r.OTT == "" { return errs.BadRequest("missing ott") } return diff --git a/api/ssh_test.go b/api/ssh_test.go index 1873a96d..a2e8748f 100644 --- a/api/ssh_test.go +++ b/api/ssh_test.go @@ -284,7 +284,7 @@ func Test_caHandler_SSHSign(t *testing.T) { identityCerts := []*x509.Certificate{ parseCertificate(certPEM), } - identityCertsPEM := []byte(`"` + strings.Replace(certPEM, "\n", `\n`, -1) + `\n"`) + identityCertsPEM := []byte(`"` + strings.ReplaceAll(certPEM, "\n", `\n`) + `\n"`) tests := []struct { name string diff --git a/authority/admin/api/middleware.go b/authority/admin/api/middleware.go index 90289f85..19025a9d 100644 --- a/authority/admin/api/middleware.go +++ b/authority/admin/api/middleware.go @@ -27,7 +27,7 @@ func (h *Handler) requireAPIEnabled(next nextHTTP) nextHTTP { func (h *Handler) extractAuthorizeTokenAdmin(next nextHTTP) nextHTTP { return func(w http.ResponseWriter, r *http.Request) { tok := r.Header.Get("Authorization") - if len(tok) == 0 { + if tok == "" { api.WriteError(w, admin.NewError(admin.ErrorUnauthorizedType, "missing authorization header token")) return diff --git a/authority/admin/db/nosql/admin_test.go b/authority/admin/db/nosql/admin_test.go index 092d72db..4234d526 100644 --- a/authority/admin/db/nosql/admin_test.go +++ b/authority/admin/db/nosql/admin_test.go @@ -12,7 +12,6 @@ import ( "github.com/smallstep/certificates/db" "github.com/smallstep/nosql" "github.com/smallstep/nosql/database" - nosqldb "github.com/smallstep/nosql/database" "go.step.sm/linkedca" "google.golang.org/protobuf/types/known/timestamppb" ) @@ -32,7 +31,7 @@ func TestDB_getDBAdminBytes(t *testing.T) { assert.Equals(t, bucket, adminsTable) assert.Equals(t, string(key), adminID) - return nil, nosqldb.ErrNotFound + return nil, database.ErrNotFound }, }, adminErr: admin.NewError(admin.ErrorNotFoundType, "admin adminID not found"), @@ -67,8 +66,8 @@ func TestDB_getDBAdminBytes(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if b, err := db.getDBAdminBytes(context.Background(), adminID); err != nil { + d := DB{db: tc.db} + if b, err := d.getDBAdminBytes(context.Background(), adminID); err != nil { switch k := err.(type) { case *admin.Error: if assert.NotNil(t, tc.adminErr) { @@ -83,10 +82,8 @@ func TestDB_getDBAdminBytes(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) { - assert.Equals(t, string(b), "foo") - } + } else if assert.Nil(t, tc.err) { + assert.Equals(t, string(b), "foo") } }) } @@ -108,7 +105,7 @@ func TestDB_getDBAdmin(t *testing.T) { assert.Equals(t, bucket, adminsTable) assert.Equals(t, string(key), adminID) - return nil, nosqldb.ErrNotFound + return nil, database.ErrNotFound }, }, adminErr: admin.NewError(admin.ErrorNotFoundType, "admin adminID not found"), @@ -193,8 +190,8 @@ func TestDB_getDBAdmin(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} - if dba, err := db.getDBAdmin(context.Background(), adminID); err != nil { + d := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} + if dba, err := d.getDBAdmin(context.Background(), adminID); err != nil { switch k := err.(type) { case *admin.Error: if assert.NotNil(t, tc.adminErr) { @@ -209,16 +206,14 @@ func TestDB_getDBAdmin(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { - assert.Equals(t, dba.ID, adminID) - assert.Equals(t, dba.AuthorityID, tc.dba.AuthorityID) - assert.Equals(t, dba.ProvisionerID, tc.dba.ProvisionerID) - assert.Equals(t, dba.Subject, tc.dba.Subject) - assert.Equals(t, dba.Type, tc.dba.Type) - assert.Equals(t, dba.CreatedAt, tc.dba.CreatedAt) - assert.Fatal(t, dba.DeletedAt.IsZero()) - } + } else if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { + assert.Equals(t, dba.ID, adminID) + assert.Equals(t, dba.AuthorityID, tc.dba.AuthorityID) + assert.Equals(t, dba.ProvisionerID, tc.dba.ProvisionerID) + assert.Equals(t, dba.Subject, tc.dba.Subject) + assert.Equals(t, dba.Type, tc.dba.Type) + assert.Equals(t, dba.CreatedAt, tc.dba.CreatedAt) + assert.Fatal(t, dba.DeletedAt.IsZero()) } }) } @@ -283,8 +278,8 @@ func TestDB_unmarshalDBAdmin(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{authorityID: admin.DefaultAuthorityID} - if dba, err := db.unmarshalDBAdmin(tc.in, adminID); err != nil { + d := DB{authorityID: admin.DefaultAuthorityID} + if dba, err := d.unmarshalDBAdmin(tc.in, adminID); err != nil { switch k := err.(type) { case *admin.Error: if assert.NotNil(t, tc.adminErr) { @@ -299,16 +294,14 @@ func TestDB_unmarshalDBAdmin(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { - assert.Equals(t, dba.ID, adminID) - assert.Equals(t, dba.AuthorityID, tc.dba.AuthorityID) - assert.Equals(t, dba.ProvisionerID, tc.dba.ProvisionerID) - assert.Equals(t, dba.Subject, tc.dba.Subject) - assert.Equals(t, dba.Type, tc.dba.Type) - assert.Equals(t, dba.CreatedAt, tc.dba.CreatedAt) - assert.Fatal(t, dba.DeletedAt.IsZero()) - } + } else if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { + assert.Equals(t, dba.ID, adminID) + assert.Equals(t, dba.AuthorityID, tc.dba.AuthorityID) + assert.Equals(t, dba.ProvisionerID, tc.dba.ProvisionerID) + assert.Equals(t, dba.Subject, tc.dba.Subject) + assert.Equals(t, dba.Type, tc.dba.Type) + assert.Equals(t, dba.CreatedAt, tc.dba.CreatedAt) + assert.Fatal(t, dba.DeletedAt.IsZero()) } }) } @@ -360,8 +353,8 @@ func TestDB_unmarshalAdmin(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{authorityID: admin.DefaultAuthorityID} - if adm, err := db.unmarshalAdmin(tc.in, adminID); err != nil { + d := DB{authorityID: admin.DefaultAuthorityID} + if adm, err := d.unmarshalAdmin(tc.in, adminID); err != nil { switch k := err.(type) { case *admin.Error: if assert.NotNil(t, tc.adminErr) { @@ -376,16 +369,14 @@ func TestDB_unmarshalAdmin(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { - assert.Equals(t, adm.Id, adminID) - assert.Equals(t, adm.AuthorityId, tc.dba.AuthorityID) - assert.Equals(t, adm.ProvisionerId, tc.dba.ProvisionerID) - assert.Equals(t, adm.Subject, tc.dba.Subject) - assert.Equals(t, adm.Type, tc.dba.Type) - assert.Equals(t, adm.CreatedAt, timestamppb.New(tc.dba.CreatedAt)) - assert.Equals(t, adm.DeletedAt, timestamppb.New(tc.dba.DeletedAt)) - } + } else if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { + assert.Equals(t, adm.Id, adminID) + assert.Equals(t, adm.AuthorityId, tc.dba.AuthorityID) + assert.Equals(t, adm.ProvisionerId, tc.dba.ProvisionerID) + assert.Equals(t, adm.Subject, tc.dba.Subject) + assert.Equals(t, adm.Type, tc.dba.Type) + assert.Equals(t, adm.CreatedAt, timestamppb.New(tc.dba.CreatedAt)) + assert.Equals(t, adm.DeletedAt, timestamppb.New(tc.dba.DeletedAt)) } }) } @@ -407,7 +398,7 @@ func TestDB_GetAdmin(t *testing.T) { assert.Equals(t, bucket, adminsTable) assert.Equals(t, string(key), adminID) - return nil, nosqldb.ErrNotFound + return nil, database.ErrNotFound }, }, adminErr: admin.NewError(admin.ErrorNotFoundType, "admin adminID not found"), @@ -516,8 +507,8 @@ func TestDB_GetAdmin(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} - if adm, err := db.GetAdmin(context.Background(), adminID); err != nil { + d := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} + if adm, err := d.GetAdmin(context.Background(), adminID); err != nil { switch k := err.(type) { case *admin.Error: if assert.NotNil(t, tc.adminErr) { @@ -532,16 +523,14 @@ func TestDB_GetAdmin(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { - assert.Equals(t, adm.Id, adminID) - assert.Equals(t, adm.AuthorityId, tc.dba.AuthorityID) - assert.Equals(t, adm.ProvisionerId, tc.dba.ProvisionerID) - assert.Equals(t, adm.Subject, tc.dba.Subject) - assert.Equals(t, adm.Type, tc.dba.Type) - assert.Equals(t, adm.CreatedAt, timestamppb.New(tc.dba.CreatedAt)) - assert.Equals(t, adm.DeletedAt, timestamppb.New(tc.dba.DeletedAt)) - } + } else if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { + assert.Equals(t, adm.Id, adminID) + assert.Equals(t, adm.AuthorityId, tc.dba.AuthorityID) + assert.Equals(t, adm.ProvisionerId, tc.dba.ProvisionerID) + assert.Equals(t, adm.Subject, tc.dba.Subject) + assert.Equals(t, adm.Type, tc.dba.Type) + assert.Equals(t, adm.CreatedAt, timestamppb.New(tc.dba.CreatedAt)) + assert.Equals(t, adm.DeletedAt, timestamppb.New(tc.dba.DeletedAt)) } }) } @@ -562,7 +551,7 @@ func TestDB_DeleteAdmin(t *testing.T) { assert.Equals(t, bucket, adminsTable) assert.Equals(t, string(key), adminID) - return nil, nosqldb.ErrNotFound + return nil, database.ErrNotFound }, }, adminErr: admin.NewError(admin.ErrorNotFoundType, "admin adminID not found"), @@ -670,8 +659,8 @@ func TestDB_DeleteAdmin(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} - if err := db.DeleteAdmin(context.Background(), adminID); err != nil { + d := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} + if err := d.DeleteAdmin(context.Background(), adminID); err != nil { switch k := err.(type) { case *admin.Error: if assert.NotNil(t, tc.adminErr) { @@ -708,7 +697,7 @@ func TestDB_UpdateAdmin(t *testing.T) { assert.Equals(t, bucket, adminsTable) assert.Equals(t, string(key), adminID) - return nil, nosqldb.ErrNotFound + return nil, database.ErrNotFound }, }, adminErr: admin.NewError(admin.ErrorNotFoundType, "admin adminID not found"), @@ -821,8 +810,8 @@ func TestDB_UpdateAdmin(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} - if err := db.UpdateAdmin(context.Background(), tc.adm); err != nil { + d := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} + if err := d.UpdateAdmin(context.Background(), tc.adm); err != nil { switch k := err.(type) { case *admin.Error: if assert.NotNil(t, tc.adminErr) { @@ -919,8 +908,8 @@ func TestDB_CreateAdmin(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} - if err := db.CreateAdmin(context.Background(), tc.adm); err != nil { + d := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} + if err := d.CreateAdmin(context.Background(), tc.adm); err != nil { switch k := err.(type) { case *admin.Error: if assert.NotNil(t, tc.adminErr) { @@ -1095,8 +1084,8 @@ func TestDB_GetAdmins(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} - if admins, err := db.GetAdmins(context.Background()); err != nil { + d := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} + if admins, err := d.GetAdmins(context.Background()); err != nil { switch k := err.(type) { case *admin.Error: if assert.NotNil(t, tc.adminErr) { @@ -1111,10 +1100,8 @@ func TestDB_GetAdmins(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { - tc.verify(t, admins) - } + } else if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { + tc.verify(t, admins) } }) } diff --git a/authority/admin/db/nosql/nosql.go b/authority/admin/db/nosql/nosql.go index 18599b02..22b049f5 100644 --- a/authority/admin/db/nosql/nosql.go +++ b/authority/admin/db/nosql/nosql.go @@ -35,7 +35,7 @@ func New(db nosqlDB.DB, authorityID string) (*DB, error) { // save writes the new data to the database, overwriting the old data if it // existed. -func (db *DB) save(ctx context.Context, id string, nu interface{}, old interface{}, typ string, table []byte) error { +func (db *DB) save(ctx context.Context, id string, nu, old interface{}, typ string, table []byte) error { var ( err error newB []byte diff --git a/authority/admin/db/nosql/provisioner_test.go b/authority/admin/db/nosql/provisioner_test.go index 95811f26..e599ea04 100644 --- a/authority/admin/db/nosql/provisioner_test.go +++ b/authority/admin/db/nosql/provisioner_test.go @@ -12,7 +12,6 @@ import ( "github.com/smallstep/certificates/db" "github.com/smallstep/nosql" "github.com/smallstep/nosql/database" - nosqldb "github.com/smallstep/nosql/database" "go.step.sm/linkedca" ) @@ -31,7 +30,7 @@ func TestDB_getDBProvisionerBytes(t *testing.T) { assert.Equals(t, bucket, provisionersTable) assert.Equals(t, string(key), provID) - return nil, nosqldb.ErrNotFound + return nil, database.ErrNotFound }, }, adminErr: admin.NewError(admin.ErrorNotFoundType, "provisioner provID not found"), @@ -66,8 +65,8 @@ func TestDB_getDBProvisionerBytes(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db} - if b, err := db.getDBProvisionerBytes(context.Background(), provID); err != nil { + d := DB{db: tc.db} + if b, err := d.getDBProvisionerBytes(context.Background(), provID); err != nil { switch k := err.(type) { case *admin.Error: if assert.NotNil(t, tc.adminErr) { @@ -82,10 +81,8 @@ func TestDB_getDBProvisionerBytes(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { - assert.Equals(t, string(b), "foo") - } + } else if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { + assert.Equals(t, string(b), "foo") } }) } @@ -107,7 +104,7 @@ func TestDB_getDBProvisioner(t *testing.T) { assert.Equals(t, bucket, provisionersTable) assert.Equals(t, string(key), provID) - return nil, nosqldb.ErrNotFound + return nil, database.ErrNotFound }, }, adminErr: admin.NewError(admin.ErrorNotFoundType, "provisioner provID not found"), @@ -190,8 +187,8 @@ func TestDB_getDBProvisioner(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} - if dbp, err := db.getDBProvisioner(context.Background(), provID); err != nil { + d := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} + if dbp, err := d.getDBProvisioner(context.Background(), provID); err != nil { switch k := err.(type) { case *admin.Error: if assert.NotNil(t, tc.adminErr) { @@ -206,15 +203,13 @@ func TestDB_getDBProvisioner(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { - assert.Equals(t, dbp.ID, provID) - assert.Equals(t, dbp.AuthorityID, tc.dbp.AuthorityID) - assert.Equals(t, dbp.Type, tc.dbp.Type) - assert.Equals(t, dbp.Name, tc.dbp.Name) - assert.Equals(t, dbp.CreatedAt, tc.dbp.CreatedAt) - assert.Fatal(t, dbp.DeletedAt.IsZero()) - } + } else if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { + assert.Equals(t, dbp.ID, provID) + assert.Equals(t, dbp.AuthorityID, tc.dbp.AuthorityID) + assert.Equals(t, dbp.Type, tc.dbp.Type) + assert.Equals(t, dbp.Name, tc.dbp.Name) + assert.Equals(t, dbp.CreatedAt, tc.dbp.CreatedAt) + assert.Fatal(t, dbp.DeletedAt.IsZero()) } }) } @@ -278,8 +273,8 @@ func TestDB_unmarshalDBProvisioner(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{authorityID: admin.DefaultAuthorityID} - if dbp, err := db.unmarshalDBProvisioner(tc.in, provID); err != nil { + d := DB{authorityID: admin.DefaultAuthorityID} + if dbp, err := d.unmarshalDBProvisioner(tc.in, provID); err != nil { switch k := err.(type) { case *admin.Error: if assert.NotNil(t, tc.adminErr) { @@ -294,19 +289,17 @@ func TestDB_unmarshalDBProvisioner(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { - assert.Equals(t, dbp.ID, provID) - assert.Equals(t, dbp.AuthorityID, tc.dbp.AuthorityID) - assert.Equals(t, dbp.Type, tc.dbp.Type) - assert.Equals(t, dbp.Name, tc.dbp.Name) - assert.Equals(t, dbp.Details, tc.dbp.Details) - assert.Equals(t, dbp.Claims, tc.dbp.Claims) - assert.Equals(t, dbp.X509Template, tc.dbp.X509Template) - assert.Equals(t, dbp.SSHTemplate, tc.dbp.SSHTemplate) - assert.Equals(t, dbp.CreatedAt, tc.dbp.CreatedAt) - assert.Fatal(t, dbp.DeletedAt.IsZero()) - } + } else if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { + assert.Equals(t, dbp.ID, provID) + assert.Equals(t, dbp.AuthorityID, tc.dbp.AuthorityID) + assert.Equals(t, dbp.Type, tc.dbp.Type) + assert.Equals(t, dbp.Name, tc.dbp.Name) + assert.Equals(t, dbp.Details, tc.dbp.Details) + assert.Equals(t, dbp.Claims, tc.dbp.Claims) + assert.Equals(t, dbp.X509Template, tc.dbp.X509Template) + assert.Equals(t, dbp.SSHTemplate, tc.dbp.SSHTemplate) + assert.Equals(t, dbp.CreatedAt, tc.dbp.CreatedAt) + assert.Fatal(t, dbp.DeletedAt.IsZero()) } }) } @@ -402,8 +395,8 @@ func TestDB_unmarshalProvisioner(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{authorityID: admin.DefaultAuthorityID} - if prov, err := db.unmarshalProvisioner(tc.in, provID); err != nil { + d := DB{authorityID: admin.DefaultAuthorityID} + if prov, err := d.unmarshalProvisioner(tc.in, provID); err != nil { switch k := err.(type) { case *admin.Error: if assert.NotNil(t, tc.adminErr) { @@ -418,20 +411,18 @@ func TestDB_unmarshalProvisioner(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { - assert.Equals(t, prov.Id, provID) - assert.Equals(t, prov.AuthorityId, tc.dbp.AuthorityID) - assert.Equals(t, prov.Type, tc.dbp.Type) - assert.Equals(t, prov.Name, tc.dbp.Name) - assert.Equals(t, prov.Claims, tc.dbp.Claims) - assert.Equals(t, prov.X509Template, tc.dbp.X509Template) - assert.Equals(t, prov.SshTemplate, tc.dbp.SSHTemplate) - - retDetailsBytes, err := json.Marshal(prov.Details.GetData()) - assert.FatalError(t, err) - assert.Equals(t, retDetailsBytes, tc.dbp.Details) - } + } else if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { + assert.Equals(t, prov.Id, provID) + assert.Equals(t, prov.AuthorityId, tc.dbp.AuthorityID) + assert.Equals(t, prov.Type, tc.dbp.Type) + assert.Equals(t, prov.Name, tc.dbp.Name) + assert.Equals(t, prov.Claims, tc.dbp.Claims) + assert.Equals(t, prov.X509Template, tc.dbp.X509Template) + assert.Equals(t, prov.SshTemplate, tc.dbp.SSHTemplate) + + retDetailsBytes, err := json.Marshal(prov.Details.GetData()) + assert.FatalError(t, err) + assert.Equals(t, retDetailsBytes, tc.dbp.Details) } }) } @@ -453,7 +444,7 @@ func TestDB_GetProvisioner(t *testing.T) { assert.Equals(t, bucket, provisionersTable) assert.Equals(t, string(key), provID) - return nil, nosqldb.ErrNotFound + return nil, database.ErrNotFound }, }, adminErr: admin.NewError(admin.ErrorNotFoundType, "provisioner provID not found"), @@ -542,8 +533,8 @@ func TestDB_GetProvisioner(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} - if prov, err := db.GetProvisioner(context.Background(), provID); err != nil { + d := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} + if prov, err := d.GetProvisioner(context.Background(), provID); err != nil { switch k := err.(type) { case *admin.Error: if assert.NotNil(t, tc.adminErr) { @@ -558,20 +549,18 @@ func TestDB_GetProvisioner(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { - assert.Equals(t, prov.Id, provID) - assert.Equals(t, prov.AuthorityId, tc.dbp.AuthorityID) - assert.Equals(t, prov.Type, tc.dbp.Type) - assert.Equals(t, prov.Name, tc.dbp.Name) - assert.Equals(t, prov.Claims, tc.dbp.Claims) - assert.Equals(t, prov.X509Template, tc.dbp.X509Template) - assert.Equals(t, prov.SshTemplate, tc.dbp.SSHTemplate) - - retDetailsBytes, err := json.Marshal(prov.Details.GetData()) - assert.FatalError(t, err) - assert.Equals(t, retDetailsBytes, tc.dbp.Details) - } + } else if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { + assert.Equals(t, prov.Id, provID) + assert.Equals(t, prov.AuthorityId, tc.dbp.AuthorityID) + assert.Equals(t, prov.Type, tc.dbp.Type) + assert.Equals(t, prov.Name, tc.dbp.Name) + assert.Equals(t, prov.Claims, tc.dbp.Claims) + assert.Equals(t, prov.X509Template, tc.dbp.X509Template) + assert.Equals(t, prov.SshTemplate, tc.dbp.SSHTemplate) + + retDetailsBytes, err := json.Marshal(prov.Details.GetData()) + assert.FatalError(t, err) + assert.Equals(t, retDetailsBytes, tc.dbp.Details) } }) } @@ -592,7 +581,7 @@ func TestDB_DeleteProvisioner(t *testing.T) { assert.Equals(t, bucket, provisionersTable) assert.Equals(t, string(key), provID) - return nil, nosqldb.ErrNotFound + return nil, database.ErrNotFound }, }, adminErr: admin.NewError(admin.ErrorNotFoundType, "provisioner provID not found"), @@ -692,8 +681,8 @@ func TestDB_DeleteProvisioner(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} - if err := db.DeleteProvisioner(context.Background(), provID); err != nil { + d := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} + if err := d.DeleteProvisioner(context.Background(), provID); err != nil { switch k := err.(type) { case *admin.Error: if assert.NotNil(t, tc.adminErr) { @@ -853,8 +842,8 @@ func TestDB_GetProvisioners(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} - if provs, err := db.GetProvisioners(context.Background()); err != nil { + d := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} + if provs, err := d.GetProvisioners(context.Background()); err != nil { switch k := err.(type) { case *admin.Error: if assert.NotNil(t, tc.adminErr) { @@ -869,10 +858,8 @@ func TestDB_GetProvisioners(t *testing.T) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } - } else { - if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { - tc.verify(t, provs) - } + } else if assert.Nil(t, tc.err) && assert.Nil(t, tc.adminErr) { + tc.verify(t, provs) } }) } @@ -963,8 +950,8 @@ func TestDB_CreateProvisioner(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} - if err := db.CreateProvisioner(context.Background(), tc.prov); err != nil { + d := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} + if err := d.CreateProvisioner(context.Background(), tc.prov); err != nil { switch k := err.(type) { case *admin.Error: if assert.NotNil(t, tc.adminErr) { @@ -1001,7 +988,7 @@ func TestDB_UpdateProvisioner(t *testing.T) { assert.Equals(t, bucket, provisionersTable) assert.Equals(t, string(key), provID) - return nil, nosqldb.ErrNotFound + return nil, database.ErrNotFound }, }, adminErr: admin.NewError(admin.ErrorNotFoundType, "provisioner provID not found"), @@ -1199,8 +1186,8 @@ func TestDB_UpdateProvisioner(t *testing.T) { for name, run := range tests { tc := run(t) t.Run(name, func(t *testing.T) { - db := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} - if err := db.UpdateProvisioner(context.Background(), tc.prov); err != nil { + d := DB{db: tc.db, authorityID: admin.DefaultAuthorityID} + if err := d.UpdateProvisioner(context.Background(), tc.prov); err != nil { switch k := err.(type) { case *admin.Error: if assert.NotNil(t, tc.adminErr) { diff --git a/authority/administrator/collection.go b/authority/administrator/collection.go index ff04a41f..88d7bb2c 100644 --- a/authority/administrator/collection.go +++ b/authority/administrator/collection.go @@ -55,8 +55,8 @@ type subProv struct { provisioner string } -func newSubProv(subject, provisioner string) subProv { - return subProv{subject, provisioner} +func newSubProv(subject, prov string) subProv { + return subProv{subject, prov} } // LoadBySubProv a admin by the subject and provisioner name. diff --git a/authority/admins.go b/authority/admins.go index dcaf9b49..b975297a 100644 --- a/authority/admins.go +++ b/authority/admins.go @@ -16,10 +16,10 @@ func (a *Authority) LoadAdminByID(id string) (*linkedca.Admin, bool) { } // LoadAdminBySubProv returns an *linkedca.Admin with the given ID. -func (a *Authority) LoadAdminBySubProv(subject, provisioner string) (*linkedca.Admin, bool) { +func (a *Authority) LoadAdminBySubProv(subject, prov string) (*linkedca.Admin, bool) { a.adminMutex.RLock() defer a.adminMutex.RUnlock() - return a.admins.LoadBySubProv(subject, provisioner) + return a.admins.LoadBySubProv(subject, prov) } // GetAdmins returns a map listing each provisioner and the JWK Key Set diff --git a/authority/authority.go b/authority/authority.go index 3f97ceab..aa8698d7 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -78,14 +78,14 @@ type Authority struct { } // New creates and initiates a new Authority type. -func New(config *config.Config, opts ...Option) (*Authority, error) { - err := config.Validate() +func New(cfg *config.Config, opts ...Option) (*Authority, error) { + err := cfg.Validate() if err != nil { return nil, err } var a = &Authority{ - config: config, + config: cfg, certificates: new(sync.Map), } diff --git a/authority/authorize.go b/authority/authorize.go index 816699f7..a4e7e591 100644 --- a/authority/authorize.go +++ b/authority/authorize.go @@ -54,7 +54,7 @@ func (a *Authority) authorizeToken(ctx context.Context, token string) (provision // key in order to verify the claims and we need the issuer from the claims // before we can look up the provisioner. var claims Claims - if err = tok.UnsafeClaimsWithoutVerification(&claims); err != nil { + if err := tok.UnsafeClaimsWithoutVerification(&claims); err != nil { return nil, errs.Wrap(http.StatusUnauthorized, err, "authority.authorizeToken") } @@ -77,7 +77,7 @@ func (a *Authority) authorizeToken(ctx context.Context, token string) (provision // Store the token to protect against reuse unless it's skipped. // If we cannot get a token id from the provisioner, just hash the token. if !SkipTokenReuseFromContext(ctx) { - if err = a.UseToken(token, p); err != nil { + if err := a.UseToken(token, p); err != nil { return nil, err } } @@ -112,7 +112,7 @@ func (a *Authority) AuthorizeAdminToken(r *http.Request, token string) (*linkedc // to the public certificate in the `x5c` header of the token. // 2. Asserts that the claims are valid - have not been tampered with. var claims jose.Claims - if err = jwt.Claims(leaf.PublicKey, &claims); err != nil { + if err := jwt.Claims(leaf.PublicKey, &claims); err != nil { return nil, admin.WrapError(admin.ErrorUnauthorizedType, err, "adminHandler.authorizeToken; error parsing x5c claims") } @@ -122,13 +122,13 @@ func (a *Authority) AuthorizeAdminToken(r *http.Request, token string) (*linkedc } // Check that the token has not been used. - if err = a.UseToken(token, prov); err != nil { + if err := a.UseToken(token, prov); err != nil { return nil, admin.WrapError(admin.ErrorUnauthorizedType, err, "adminHandler.authorizeToken; error with reuse token") } // According to "rfc7519 JSON Web Token" acceptable skew should be no // more than a few minutes. - if err = claims.ValidateWithLeeway(jose.Expected{ + if err := claims.ValidateWithLeeway(jose.Expected{ Issuer: prov.GetName(), Time: time.Now().UTC(), }, time.Minute); err != nil { @@ -262,7 +262,7 @@ func (a *Authority) authorizeRevoke(ctx context.Context, token string) error { if err != nil { return errs.Wrap(http.StatusInternalServerError, err, "authority.authorizeRevoke") } - if err = p.AuthorizeRevoke(ctx, token); err != nil { + if err := p.AuthorizeRevoke(ctx, token); err != nil { return errs.Wrap(http.StatusInternalServerError, err, "authority.authorizeRevoke") } return nil diff --git a/authority/authorize_test.go b/authority/authorize_test.go index f308ec28..6d524a25 100644 --- a/authority/authorize_test.go +++ b/authority/authorize_test.go @@ -917,7 +917,7 @@ func createSSHCert(cert *ssh.Certificate, signer ssh.Signer) (*ssh.Certificate, if err != nil { return nil, nil, err } - if err = cert.SignCert(rand.Reader, signer); err != nil { + if err := cert.SignCert(rand.Reader, signer); err != nil { return nil, nil, err } return cert, jwk, nil diff --git a/authority/config/types.go b/authority/config/types.go index 6d7b9389..5ca3b15f 100644 --- a/authority/config/types.go +++ b/authority/config/types.go @@ -25,7 +25,7 @@ func (s multiString) HasEmpties() bool { return true } for _, ss := range s { - if len(ss) == 0 { + if ss == "" { return true } } diff --git a/authority/linkedca.go b/authority/linkedca.go index 9c816e1e..b568dcbb 100644 --- a/authority/linkedca.go +++ b/authority/linkedca.go @@ -272,12 +272,12 @@ func (c *linkedCaClient) Revoke(crt *x509.Certificate, rci *db.RevokedCertificat return errors.Wrap(err, "error revoking certificate") } -func (c *linkedCaClient) RevokeSSH(ssh *ssh.Certificate, rci *db.RevokedCertificateInfo) error { +func (c *linkedCaClient) RevokeSSH(cert *ssh.Certificate, rci *db.RevokedCertificateInfo) error { ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() _, err := c.client.RevokeSSHCertificate(ctx, &linkedca.RevokeSSHCertificateRequest{ Serial: rci.Serial, - Certificate: serializeSSHCertificate(ssh), + Certificate: serializeSSHCertificate(cert), Reason: rci.Reason, ReasonCode: linkedca.RevocationReasonCode(rci.ReasonCode), Passive: true, diff --git a/authority/options.go b/authority/options.go index 5c8a6e66..0f80cbbf 100644 --- a/authority/options.go +++ b/authority/options.go @@ -22,9 +22,9 @@ type Option func(*Authority) error // WithConfig replaces the current config with the given one. No validation is // performed in the given value. -func WithConfig(config *config.Config) Option { +func WithConfig(cfg *config.Config) Option { return func(a *Authority) error { - a.config = config + a.config = cfg return nil } } @@ -76,9 +76,9 @@ func WithIssuerPassword(password []byte) Option { // WithDatabase sets an already initialized authority database to a new // authority. This option is intended to be use on graceful reloads. -func WithDatabase(db db.AuthDB) Option { +func WithDatabase(d db.AuthDB) Option { return func(a *Authority) error { - a.db = db + a.db = d return nil } } @@ -225,9 +225,9 @@ func WithX509FederatedBundle(pemCerts []byte) Option { } // WithAdminDB is an option to set the database backing the admin APIs. -func WithAdminDB(db admin.DB) Option { +func WithAdminDB(d admin.DB) Option { return func(a *Authority) error { - a.adminDB = db + a.adminDB = d return nil } } diff --git a/authority/provisioner/aws.go b/authority/provisioner/aws.go index cdd06f00..cd129b7b 100644 --- a/authority/provisioner/aws.go +++ b/authority/provisioner/aws.go @@ -312,7 +312,7 @@ func (p *AWS) GetType() Type { } // GetEncryptedKey is not available in an AWS provisioner. -func (p *AWS) GetEncryptedKey() (kid string, key string, ok bool) { +func (p *AWS) GetEncryptedKey() (kid, key string, ok bool) { return "", "", false } @@ -449,13 +449,15 @@ func (p *AWS) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er // There's no way to trust them other than TOFU. var so []SignOption if p.DisableCustomSANs { - dnsName := fmt.Sprintf("ip-%s.%s.compute.internal", strings.Replace(doc.PrivateIP, ".", "-", -1), doc.Region) - so = append(so, dnsNamesValidator([]string{dnsName})) - so = append(so, ipAddressesValidator([]net.IP{ - net.ParseIP(doc.PrivateIP), - })) - so = append(so, emailAddressesValidator(nil)) - so = append(so, urisValidator(nil)) + dnsName := fmt.Sprintf("ip-%s.%s.compute.internal", strings.ReplaceAll(doc.PrivateIP, ".", "-"), doc.Region) + so = append(so, + dnsNamesValidator([]string{dnsName}), + ipAddressesValidator([]net.IP{ + net.ParseIP(doc.PrivateIP), + }), + emailAddressesValidator(nil), + urisValidator(nil), + ) // Template options data.SetSANs([]string{dnsName, doc.PrivateIP}) @@ -669,7 +671,7 @@ func (p *AWS) authorizeToken(token string) (*awsPayload, error) { if p.DisableCustomSANs { if payload.Subject != doc.InstanceID && payload.Subject != doc.PrivateIP && - payload.Subject != fmt.Sprintf("ip-%s.%s.compute.internal", strings.Replace(doc.PrivateIP, ".", "-", -1), doc.Region) { + payload.Subject != fmt.Sprintf("ip-%s.%s.compute.internal", strings.ReplaceAll(doc.PrivateIP, ".", "-"), doc.Region) { return nil, errs.Unauthorized("aws.authorizeToken; invalid token - invalid subject claim (sub)") } } @@ -720,7 +722,7 @@ func (p *AWS) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption, // Validated principals. principals := []string{ doc.PrivateIP, - fmt.Sprintf("ip-%s.%s.compute.internal", strings.Replace(doc.PrivateIP, ".", "-", -1), doc.Region), + fmt.Sprintf("ip-%s.%s.compute.internal", strings.ReplaceAll(doc.PrivateIP, ".", "-"), doc.Region), } // Only enforce known principals if disable custom sans is true. diff --git a/authority/provisioner/aws_test.go b/authority/provisioner/aws_test.go index aff0aecb..0d2786db 100644 --- a/authority/provisioner/aws_test.go +++ b/authority/provisioner/aws_test.go @@ -663,15 +663,15 @@ func TestAWS_AuthorizeSign(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctx := NewContextWithMethod(context.Background(), SignMethod) - got, err := tt.aws.AuthorizeSign(ctx, tt.args.token) - if (err != nil) != tt.wantErr { + switch got, err := tt.aws.AuthorizeSign(ctx, tt.args.token); { + case (err != nil) != tt.wantErr: t.Errorf("AWS.AuthorizeSign() error = %v, wantErr %v", err, tt.wantErr) return - } else if err != nil { + case err != nil: sc, ok := err.(errs.StatusCoder) assert.Fatal(t, ok, "error does not implement StatusCoder interface") assert.Equals(t, sc.StatusCode(), tt.code) - } else { + default: assert.Len(t, tt.wantLen, got) for _, o := range got { switch v := o.(type) { diff --git a/authority/provisioner/azure.go b/authority/provisioner/azure.go index fee50658..a90d1728 100644 --- a/authority/provisioner/azure.go +++ b/authority/provisioner/azure.go @@ -152,7 +152,7 @@ func (p *Azure) GetType() Type { } // GetEncryptedKey is not available in an Azure provisioner. -func (p *Azure) GetEncryptedKey() (kid string, key string, ok bool) { +func (p *Azure) GetEncryptedKey() (kid, key string, ok bool) { return "", "", false } @@ -303,11 +303,13 @@ func (p *Azure) AuthorizeSign(ctx context.Context, token string) ([]SignOption, var so []SignOption if p.DisableCustomSANs { // name will work only inside the virtual network - so = append(so, commonNameValidator(name)) - so = append(so, dnsNamesValidator([]string{name})) - so = append(so, ipAddressesValidator(nil)) - so = append(so, emailAddressesValidator(nil)) - so = append(so, urisValidator(nil)) + so = append(so, + commonNameValidator(name), + dnsNamesValidator([]string{name}), + ipAddressesValidator(nil), + emailAddressesValidator(nil), + urisValidator(nil), + ) // Enforce SANs in the template. data.SetSANs([]string{name}) diff --git a/authority/provisioner/azure_test.go b/authority/provisioner/azure_test.go index 8033d345..b7c321a6 100644 --- a/authority/provisioner/azure_test.go +++ b/authority/provisioner/azure_test.go @@ -446,15 +446,15 @@ func TestAzure_AuthorizeSign(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctx := NewContextWithMethod(context.Background(), SignMethod) - got, err := tt.azure.AuthorizeSign(ctx, tt.args.token) - if (err != nil) != tt.wantErr { + switch got, err := tt.azure.AuthorizeSign(ctx, tt.args.token); { + case (err != nil) != tt.wantErr: t.Errorf("Azure.AuthorizeSign() error = %v, wantErr %v", err, tt.wantErr) return - } else if err != nil { + case err != nil: sc, ok := err.(errs.StatusCoder) assert.Fatal(t, ok, "error does not implement StatusCoder interface") assert.Equals(t, sc.StatusCode(), tt.code) - } else { + default: assert.Len(t, tt.wantLen, got) for _, o := range got { switch v := o.(type) { diff --git a/authority/provisioner/collection.go b/authority/provisioner/collection.go index caf46ca9..1bec8689 100644 --- a/authority/provisioner/collection.go +++ b/authority/provisioner/collection.go @@ -229,14 +229,15 @@ func (c *Collection) Remove(id string) error { var found bool for i, elem := range c.sorted { - if elem.provisioner.GetID() == id { - // Remove index in sorted list - copy(c.sorted[i:], c.sorted[i+1:]) // Shift a[i+1:] left one index. - c.sorted[len(c.sorted)-1] = uidProvisioner{} // Erase last element (write zero value). - c.sorted = c.sorted[:len(c.sorted)-1] // Truncate slice. - found = true - break + if elem.provisioner.GetID() != id { + continue } + // Remove index in sorted list + copy(c.sorted[i:], c.sorted[i+1:]) // Shift a[i+1:] left one index. + c.sorted[len(c.sorted)-1] = uidProvisioner{} // Erase last element (write zero value). + c.sorted = c.sorted[:len(c.sorted)-1] // Truncate slice. + found = true + break } if !found { return admin.NewError(admin.ErrorNotFoundType, "provisioner %s not found in sorted list", prov.GetName()) diff --git a/authority/provisioner/gcp.go b/authority/provisioner/gcp.go index 1b599fb3..98d776d1 100644 --- a/authority/provisioner/gcp.go +++ b/authority/provisioner/gcp.go @@ -150,7 +150,7 @@ func (p *GCP) GetType() Type { } // GetEncryptedKey is not available in a GCP provisioner. -func (p *GCP) GetEncryptedKey() (kid string, key string, ok bool) { +func (p *GCP) GetEncryptedKey() (kid, key string, ok bool) { return "", "", false } @@ -244,15 +244,17 @@ func (p *GCP) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er if p.DisableCustomSANs { dnsName1 := fmt.Sprintf("%s.c.%s.internal", ce.InstanceName, ce.ProjectID) dnsName2 := fmt.Sprintf("%s.%s.c.%s.internal", ce.InstanceName, ce.Zone, ce.ProjectID) - so = append(so, commonNameSliceValidator([]string{ - ce.InstanceName, ce.InstanceID, dnsName1, dnsName2, - })) - so = append(so, dnsNamesValidator([]string{ - dnsName1, dnsName2, - })) - so = append(so, ipAddressesValidator(nil)) - so = append(so, emailAddressesValidator(nil)) - so = append(so, urisValidator(nil)) + so = append(so, + commonNameSliceValidator([]string{ + ce.InstanceName, ce.InstanceID, dnsName1, dnsName2, + }), + dnsNamesValidator([]string{ + dnsName1, dnsName2, + }), + ipAddressesValidator(nil), + emailAddressesValidator(nil), + urisValidator(nil), + ) // Template SANs data.SetSANs([]string{dnsName1, dnsName2}) diff --git a/authority/provisioner/gcp_test.go b/authority/provisioner/gcp_test.go index d6c4054c..5f6f9bc7 100644 --- a/authority/provisioner/gcp_test.go +++ b/authority/provisioner/gcp_test.go @@ -535,15 +535,15 @@ func TestGCP_AuthorizeSign(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctx := NewContextWithMethod(context.Background(), SignMethod) - got, err := tt.gcp.AuthorizeSign(ctx, tt.args.token) - if (err != nil) != tt.wantErr { + switch got, err := tt.gcp.AuthorizeSign(ctx, tt.args.token); { + case (err != nil) != tt.wantErr: t.Errorf("GCP.AuthorizeSign() error = %v, wantErr %v", err, tt.wantErr) return - } else if err != nil { + case err != nil: sc, ok := err.(errs.StatusCoder) assert.Fatal(t, ok, "error does not implement StatusCoder interface") assert.Equals(t, sc.StatusCode(), tt.code) - } else { + default: assert.Len(t, tt.wantLen, got) for _, o := range got { switch v := o.(type) { diff --git a/authority/provisioner/keystore.go b/authority/provisioner/keystore.go index f775e150..d1811fab 100644 --- a/authority/provisioner/keystore.go +++ b/authority/provisioner/keystore.go @@ -18,7 +18,7 @@ const ( defaultCacheJitter = 1 * time.Hour ) -var maxAgeRegex = regexp.MustCompile("max-age=([0-9]+)") +var maxAgeRegex = regexp.MustCompile(`max-age=(\d+)`) type keyStore struct { sync.RWMutex diff --git a/authority/provisioner/noop.go b/authority/provisioner/noop.go index 18a38331..1709fbca 100644 --- a/authority/provisioner/noop.go +++ b/authority/provisioner/noop.go @@ -29,7 +29,7 @@ func (p *noop) GetType() Type { return noopType } -func (p *noop) GetEncryptedKey() (kid string, key string, ok bool) { +func (p *noop) GetEncryptedKey() (kid, key string, ok bool) { return "", "", false } diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go index 3786f54b..ac1f2a25 100644 --- a/authority/provisioner/oidc.go +++ b/authority/provisioner/oidc.go @@ -148,7 +148,7 @@ func (o *OIDC) GetType() Type { } // GetEncryptedKey is not available in an OIDC provisioner. -func (o *OIDC) GetEncryptedKey() (kid string, key string, ok bool) { +func (o *OIDC) GetEncryptedKey() (kid, key string, ok bool) { return "", "", false } @@ -193,7 +193,7 @@ func (o *OIDC) Init(config Config) (err error) { } // Replace {tenantid} with the configured one if o.TenantID != "" { - o.configuration.Issuer = strings.Replace(o.configuration.Issuer, "{tenantid}", o.TenantID, -1) + o.configuration.Issuer = strings.ReplaceAll(o.configuration.Issuer, "{tenantid}", o.TenantID) } // Get JWK key set o.keyStore, err = newKeyStore(o.configuration.JWKSetURI) diff --git a/authority/provisioner/oidc_test.go b/authority/provisioner/oidc_test.go index 532bd2e0..7bf6ad7a 100644 --- a/authority/provisioner/oidc_test.go +++ b/authority/provisioner/oidc_test.go @@ -321,32 +321,26 @@ func TestOIDC_AuthorizeSign(t *testing.T) { assert.Fatal(t, ok, "error does not implement StatusCoder interface") assert.Equals(t, sc.StatusCode(), tt.code) assert.Nil(t, got) - } else { - if assert.NotNil(t, got) { - if tt.name == "admin" { - assert.Len(t, 5, got) - } else { - assert.Len(t, 5, got) - } - for _, o := range got { - switch v := o.(type) { - case certificateOptionsFunc: - case *provisionerExtensionOption: - assert.Equals(t, v.Type, int(TypeOIDC)) - assert.Equals(t, v.Name, tt.prov.GetName()) - assert.Equals(t, v.CredentialID, tt.prov.ClientID) - assert.Len(t, 0, v.KeyValuePairs) - case profileDefaultDuration: - assert.Equals(t, time.Duration(v), tt.prov.claimer.DefaultTLSCertDuration()) - case defaultPublicKeyValidator: - case *validityValidator: - assert.Equals(t, v.min, tt.prov.claimer.MinTLSCertDuration()) - assert.Equals(t, v.max, tt.prov.claimer.MaxTLSCertDuration()) - case emailOnlyIdentity: - assert.Equals(t, string(v), "name@smallstep.com") - default: - assert.FatalError(t, errors.Errorf("unexpected sign option of type %T", v)) - } + } else if assert.NotNil(t, got) { + assert.Len(t, 5, got) + for _, o := range got { + switch v := o.(type) { + case certificateOptionsFunc: + case *provisionerExtensionOption: + assert.Equals(t, v.Type, int(TypeOIDC)) + assert.Equals(t, v.Name, tt.prov.GetName()) + assert.Equals(t, v.CredentialID, tt.prov.ClientID) + assert.Len(t, 0, v.KeyValuePairs) + case profileDefaultDuration: + assert.Equals(t, time.Duration(v), tt.prov.claimer.DefaultTLSCertDuration()) + case defaultPublicKeyValidator: + case *validityValidator: + assert.Equals(t, v.min, tt.prov.claimer.MinTLSCertDuration()) + assert.Equals(t, v.max, tt.prov.claimer.MaxTLSCertDuration()) + case emailOnlyIdentity: + assert.Equals(t, string(v), "name@smallstep.com") + default: + assert.FatalError(t, errors.Errorf("unexpected sign option of type %T", v)) } } } diff --git a/authority/provisioner/options.go b/authority/provisioner/options.go index 100aa588..f86c4863 100644 --- a/authority/provisioner/options.go +++ b/authority/provisioner/options.go @@ -138,7 +138,7 @@ func unsafeParseSigned(s string) (map[string]interface{}, error) { return nil, err } claims := make(map[string]interface{}) - if err = token.UnsafeClaimsWithoutVerification(&claims); err != nil { + if err := token.UnsafeClaimsWithoutVerification(&claims); err != nil { return nil, err } return claims, nil diff --git a/authority/provisioner/provisioner.go b/authority/provisioner/provisioner.go index 652cb888..5d6b2f80 100644 --- a/authority/provisioner/provisioner.go +++ b/authority/provisioner/provisioner.go @@ -123,7 +123,7 @@ func (a Audiences) WithFragment(fragment string) Audiences { // generateSignAudience generates a sign audience with the format // https:///1.0/sign#provisionerID -func generateSignAudience(caURL string, provisionerID string) (string, error) { +func generateSignAudience(caURL, provisionerID string) (string, error) { u, err := url.Parse(caURL) if err != nil { return "", errors.Wrapf(err, "error parsing %s", caURL) diff --git a/authority/provisioner/sign_ssh_options_test.go b/authority/provisioner/sign_ssh_options_test.go index 693690f6..3a1ff324 100644 --- a/authority/provisioner/sign_ssh_options_test.go +++ b/authority/provisioner/sign_ssh_options_test.go @@ -44,7 +44,7 @@ func TestSSHOptions_Modify(t *testing.T) { valid func(*ssh.Certificate) err error } - tests := map[string](func() test){ + tests := map[string]func() test{ "fail/unexpected-cert-type": func() test { return test{ so: SignSSHOptions{CertType: "foo"}, @@ -117,7 +117,7 @@ func TestSSHOptions_Match(t *testing.T) { cmp SignSSHOptions err error } - tests := map[string](func() test){ + tests := map[string]func() test{ "fail/cert-type": func() test { return test{ so: SignSSHOptions{CertType: "foo"}, @@ -208,7 +208,7 @@ func Test_sshCertPrincipalsModifier_Modify(t *testing.T) { cert *ssh.Certificate expected []string } - tests := map[string](func() test){ + tests := map[string]func() test{ "ok": func() test { a := []string{"foo", "bar"} return test{ @@ -234,7 +234,7 @@ func Test_sshCertKeyIDModifier_Modify(t *testing.T) { cert *ssh.Certificate expected string } - tests := map[string](func() test){ + tests := map[string]func() test{ "ok": func() test { a := "foo" return test{ @@ -260,7 +260,7 @@ func Test_sshCertTypeModifier_Modify(t *testing.T) { cert *ssh.Certificate expected uint32 } - tests := map[string](func() test){ + tests := map[string]func() test{ "ok/user": func() test { return test{ modifier: sshCertTypeModifier("user"), @@ -299,7 +299,7 @@ func Test_sshCertValidAfterModifier_Modify(t *testing.T) { cert *ssh.Certificate expected uint64 } - tests := map[string](func() test){ + tests := map[string]func() test{ "ok": func() test { return test{ modifier: sshCertValidAfterModifier(15), @@ -324,7 +324,7 @@ func Test_sshCertDefaultsModifier_Modify(t *testing.T) { cert *ssh.Certificate valid func(*ssh.Certificate) } - tests := map[string](func() test){ + tests := map[string]func() test{ "ok/changes": func() test { n := time.Now() va := NewTimeDuration(n.Add(1 * time.Minute)) @@ -388,7 +388,7 @@ func Test_sshDefaultExtensionModifier_Modify(t *testing.T) { valid func(*ssh.Certificate) err error } - tests := map[string](func() test){ + tests := map[string]func() test{ "fail/unexpected-cert-type": func() test { cert := &ssh.Certificate{CertType: 3} return test{ diff --git a/authority/provisioner/sshpop_test.go b/authority/provisioner/sshpop_test.go index 79d82e00..3d343967 100644 --- a/authority/provisioner/sshpop_test.go +++ b/authority/provisioner/sshpop_test.go @@ -46,7 +46,7 @@ func createSSHCert(cert *ssh.Certificate, signer ssh.Signer) (*ssh.Certificate, if err != nil { return nil, nil, err } - if err = cert.SignCert(rand.Reader, signer); err != nil { + if err := cert.SignCert(rand.Reader, signer); err != nil { return nil, nil, err } return cert, jwk, nil @@ -214,10 +214,8 @@ func TestSSHPOP_authorizeToken(t *testing.T) { if assert.NotNil(t, tc.err) { assert.HasPrefix(t, err.Error(), tc.err.Error()) } - } else { - if assert.Nil(t, tc.err) { - assert.NotNil(t, claims) - } + } else if assert.Nil(t, tc.err) { + assert.NotNil(t, claims) } }) } diff --git a/authority/provisioner/utils_test.go b/authority/provisioner/utils_test.go index 534e83cf..e39efbcf 100644 --- a/authority/provisioner/utils_test.go +++ b/authority/provisioner/utils_test.go @@ -732,7 +732,7 @@ func withSSHPOPFile(cert *ssh.Certificate) tokOption { } } -func generateToken(sub, iss, aud string, email string, sans []string, iat time.Time, jwk *jose.JSONWebKey, tokOpts ...tokOption) (string, error) { +func generateToken(sub, iss, aud, email string, sans []string, iat time.Time, jwk *jose.JSONWebKey, tokOpts ...tokOption) (string, error) { so := new(jose.SignerOptions) so.WithType("JWT") so.WithHeader("kid", jwk.KeyID) @@ -773,7 +773,7 @@ func generateToken(sub, iss, aud string, email string, sans []string, iat time.T return jose.Signed(sig).Claims(claims).CompactSerialize() } -func generateOIDCToken(sub, iss, aud string, email string, preferredUsername string, iat time.Time, jwk *jose.JSONWebKey, tokOpts ...tokOption) (string, error) { +func generateOIDCToken(sub, iss, aud, email, preferredUsername string, iat time.Time, jwk *jose.JSONWebKey, tokOpts ...tokOption) (string, error) { so := new(jose.SignerOptions) so.WithType("JWT") so.WithHeader("kid", jwk.KeyID) diff --git a/authority/ssh.go b/authority/ssh.go index 1c873279..762319ae 100644 --- a/authority/ssh.go +++ b/authority/ssh.go @@ -108,7 +108,7 @@ func (a *Authority) GetSSHConfig(ctx context.Context, typ string, data map[strin // GetSSHBastion returns the bastion configuration, for the given pair user, // hostname. -func (a *Authority) GetSSHBastion(ctx context.Context, user string, hostname string) (*config.Bastion, error) { +func (a *Authority) GetSSHBastion(ctx context.Context, user, hostname string) (*config.Bastion, error) { if a.sshBastionFunc != nil { bs, err := a.sshBastionFunc(ctx, user, hostname) return bs, errs.Wrap(http.StatusInternalServerError, err, "authority.GetSSHBastion") @@ -477,7 +477,7 @@ func (a *Authority) SignSSHAddUser(ctx context.Context, key ssh.PublicKey, subje } // CheckSSHHost checks the given principal has been registered before. -func (a *Authority) CheckSSHHost(ctx context.Context, principal string, token string) (bool, error) { +func (a *Authority) CheckSSHHost(ctx context.Context, principal, token string) (bool, error) { if a.sshCheckHostFunc != nil { exists, err := a.sshCheckHostFunc(ctx, principal, token, a.GetRootCertificates()) if err != nil { @@ -531,5 +531,5 @@ func (a *Authority) getAddUserCommand(principal string) string { } else { cmd = a.config.SSH.AddUserCommand } - return strings.Replace(cmd, "", principal, -1) + return strings.ReplaceAll(cmd, "", principal) } diff --git a/authority/tls.go b/authority/tls.go index b434be55..839866a2 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -55,10 +55,10 @@ func withDefaultASN1DN(def *config.ASN1DN) provisioner.CertificateModifierFunc { if len(crt.Subject.StreetAddress) == 0 && def.StreetAddress != "" { crt.Subject.StreetAddress = append(crt.Subject.StreetAddress, def.StreetAddress) } - if len(crt.Subject.SerialNumber) == 0 && def.SerialNumber != "" { + if crt.Subject.SerialNumber == "" && def.SerialNumber != "" { crt.Subject.SerialNumber = def.SerialNumber } - if len(crt.Subject.CommonName) == 0 && def.CommonName != "" { + if crt.Subject.CommonName == "" && def.CommonName != "" { crt.Subject.CommonName = def.CommonName } return nil @@ -387,14 +387,14 @@ func (a *Authority) Revoke(ctx context.Context, revokeOpts *RevokeOptions) error return errs.Wrap(http.StatusInternalServerError, err, "authority.Revoke; could not get ID for token") } - opts = append(opts, errs.WithKeyVal("provisionerID", rci.ProvisionerID)) - opts = append(opts, errs.WithKeyVal("tokenID", rci.TokenID)) - } else { + opts = append(opts, + errs.WithKeyVal("provisionerID", rci.ProvisionerID), + errs.WithKeyVal("tokenID", rci.TokenID), + ) + } else if p, err = a.LoadProvisionerByCertificate(revokeOpts.Crt); err == nil { // Load the Certificate provisioner if one exists. - if p, err = a.LoadProvisionerByCertificate(revokeOpts.Crt); err == nil { - rci.ProvisionerID = p.GetID() - opts = append(opts, errs.WithKeyVal("provisionerID", rci.ProvisionerID)) - } + rci.ProvisionerID = p.GetID() + opts = append(opts, errs.WithKeyVal("provisionerID", rci.ProvisionerID)) } if provisioner.MethodFromContext(ctx) == provisioner.SSHRevokeMethod { diff --git a/authority/tls_test.go b/authority/tls_test.go index cdd4c59a..f1d1748d 100644 --- a/authority/tls_test.go +++ b/authority/tls_test.go @@ -426,6 +426,7 @@ ZYtQ9Ot36qc= {Id: stepOIDProvisioner, Value: []byte("foo")}, {Id: []int{1, 1, 1}, Value: []byte("bar")}})) now := time.Now().UTC() + // nolint:gocritic enforcedExtraOptions := append(extraOpts, &certificateDurationEnforcer{ NotBefore: now, NotAfter: now.Add(365 * 24 * time.Hour), diff --git a/ca/acmeClient.go b/ca/acmeClient.go index 5633dac5..d1f40f32 100644 --- a/ca/acmeClient.go +++ b/ca/acmeClient.go @@ -345,7 +345,7 @@ func readACMEError(r io.ReadCloser) error { ae := new(acme.Error) err = json.Unmarshal(b, &ae) // If we successfully marshaled to an ACMEError then return the ACMEError. - if err != nil || len(ae.Error()) == 0 { + if err != nil || ae.Error() == "" { fmt.Printf("b = %s\n", b) // Throw up our hands. return errors.Errorf("%s", b) diff --git a/ca/acmeClient_test.go b/ca/acmeClient_test.go index f5963de4..656a82cf 100644 --- a/ca/acmeClient_test.go +++ b/ca/acmeClient_test.go @@ -1247,6 +1247,7 @@ func TestACMEClient_GetCertificate(t *testing.T) { Type: "Certificate", Bytes: leaf.Raw, }) + // nolint:gocritic certBytes := append(leafb, leafb...) certBytes = append(certBytes, leafb...) ac := &ACMEClient{ diff --git a/ca/adminClient.go b/ca/adminClient.go index 2f3d4b5d..6022f677 100644 --- a/ca/adminClient.go +++ b/ca/adminClient.go @@ -70,7 +70,7 @@ func NewAdminClient(endpoint string, opts ...ClientOption) (*AdminClient, error) }, nil } -func (c *AdminClient) generateAdminToken(path string) (string, error) { +func (c *AdminClient) generateAdminToken(urlPath string) (string, error) { // A random jwt id will be used to identify duplicated tokens jwtID, err := randutil.Hex(64) // 256 bits if err != nil { @@ -82,7 +82,7 @@ func (c *AdminClient) generateAdminToken(path string) (string, error) { token.WithJWTID(jwtID), token.WithKid(c.x5cJWK.KeyID), token.WithIssuer(c.x5cIssuer), - token.WithAudience(path), + token.WithAudience(urlPath), token.WithValidity(now, now.Add(token.DefaultValidity)), token.WithX5CCerts(c.x5cCertStrs), } @@ -348,14 +348,15 @@ func (c *AdminClient) GetProvisioner(opts ...ProvisionerOption) (*linkedca.Provi return nil, err } var u *url.URL - if len(o.id) > 0 { + switch { + case len(o.id) > 0: u = c.endpoint.ResolveReference(&url.URL{ Path: "/admin/provisioners/id", RawQuery: o.rawQuery(), }) - } else if len(o.name) > 0 { + case len(o.name) > 0: u = c.endpoint.ResolveReference(&url.URL{Path: path.Join(adminURLPrefix, "provisioners", o.name)}) - } else { + default: return nil, errors.New("must set either name or id in method options") } tok, err := c.generateAdminToken(u.Path) @@ -456,14 +457,15 @@ func (c *AdminClient) RemoveProvisioner(opts ...ProvisionerOption) error { return err } - if len(o.id) > 0 { + switch { + case len(o.id) > 0: u = c.endpoint.ResolveReference(&url.URL{ Path: path.Join(adminURLPrefix, "provisioners/id"), RawQuery: o.rawQuery(), }) - } else if len(o.name) > 0 { + case len(o.name) > 0: u = c.endpoint.ResolveReference(&url.URL{Path: path.Join(adminURLPrefix, "provisioners", o.name)}) - } else { + default: return errors.New("must set either name or id in method options") } tok, err := c.generateAdminToken(u.Path) diff --git a/ca/bootstrap.go b/ca/bootstrap.go index 5f06e986..42087985 100644 --- a/ca/bootstrap.go +++ b/ca/bootstrap.go @@ -30,7 +30,7 @@ func Bootstrap(token string) (*Client, error) { // Validate bootstrap token switch { - case len(claims.SHA) == 0: + case claims.SHA == "": return nil, errors.New("invalid bootstrap token: sha claim is not present") case !strings.HasPrefix(strings.ToLower(claims.Audience[0]), "http"): return nil, errors.New("invalid bootstrap token: aud claim is not a url") diff --git a/ca/ca.go b/ca/ca.go index 00a5970a..c76e8c0a 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -88,9 +88,9 @@ func WithIssuerPassword(password []byte) Option { } // WithDatabase sets the given authority database to the CA options. -func WithDatabase(db db.AuthDB) Option { +func WithDatabase(d db.AuthDB) Option { return func(o *options) { - o.database = db + o.database = d } } @@ -113,17 +113,17 @@ type CA struct { } // New creates and initializes the CA with the given configuration and options. -func New(config *config.Config, opts ...Option) (*CA, error) { +func New(cfg *config.Config, opts ...Option) (*CA, error) { ca := &CA{ - config: config, + config: cfg, opts: new(options), } ca.opts.apply(opts) - return ca.Init(config) + return ca.Init(cfg) } // Init initializes the CA with the given configuration. -func (ca *CA) Init(config *config.Config) (*CA, error) { +func (ca *CA) Init(cfg *config.Config) (*CA, error) { // Set password, it's ok to set nil password, the ca will prompt for them if // they are required. opts := []authority.Option{ @@ -140,7 +140,7 @@ func (ca *CA) Init(config *config.Config) (*CA, error) { opts = append(opts, authority.WithDatabase(ca.opts.database)) } - auth, err := authority.New(config, opts...) + auth, err := authority.New(cfg, opts...) if err != nil { return nil, err } @@ -166,8 +166,8 @@ func (ca *CA) Init(config *config.Config) (*CA, error) { }) //Add ACME api endpoints in /acme and /1.0/acme - dns := config.DNSNames[0] - u, err := url.Parse("https://" + config.Address) + dns := cfg.DNSNames[0] + u, err := url.Parse("https://" + cfg.Address) if err != nil { return nil, err } @@ -179,7 +179,7 @@ func (ca *CA) Init(config *config.Config) (*CA, error) { // ACME Router prefix := "acme" var acmeDB acme.DB - if config.DB == nil { + if cfg.DB == nil { acmeDB = nil } else { acmeDB, err = acmeNoSQL.New(auth.GetDatabase().(nosql.DB)) @@ -188,7 +188,7 @@ func (ca *CA) Init(config *config.Config) (*CA, error) { } } acmeHandler := acmeAPI.NewHandler(acmeAPI.HandlerOptions{ - Backdate: *config.AuthorityConfig.Backdate, + Backdate: *cfg.AuthorityConfig.Backdate, DB: acmeDB, DNS: dns, Prefix: prefix, @@ -204,7 +204,7 @@ func (ca *CA) Init(config *config.Config) (*CA, error) { }) // Admin API Router - if config.AuthorityConfig.EnableAdmin { + if cfg.AuthorityConfig.EnableAdmin { adminDB := auth.GetAdminDatabase() if adminDB != nil { adminHandler := adminAPI.NewHandler(auth) @@ -248,8 +248,8 @@ func (ca *CA) Init(config *config.Config) (*CA, error) { //dumpRoutes(mux) // Add monitoring if configured - if len(config.Monitoring) > 0 { - m, err := monitoring.New(config.Monitoring) + if len(cfg.Monitoring) > 0 { + m, err := monitoring.New(cfg.Monitoring) if err != nil { return nil, err } @@ -258,8 +258,8 @@ func (ca *CA) Init(config *config.Config) (*CA, error) { } // Add logger if configured - if len(config.Logger) > 0 { - logger, err := logging.New("ca", config.Logger) + if len(cfg.Logger) > 0 { + logger, err := logging.New("ca", cfg.Logger) if err != nil { return nil, err } @@ -267,16 +267,16 @@ func (ca *CA) Init(config *config.Config) (*CA, error) { insecureHandler = logger.Middleware(insecureHandler) } - ca.srv = server.New(config.Address, handler, tlsConfig) + ca.srv = server.New(cfg.Address, handler, tlsConfig) // only start the insecure server if the insecure address is configured // and, currently, also only when it should serve SCEP endpoints. - if ca.shouldServeSCEPEndpoints() && config.InsecureAddress != "" { + if ca.shouldServeSCEPEndpoints() && cfg.InsecureAddress != "" { // TODO: instead opt for having a single server.Server but two // http.Servers handling the HTTP and HTTPS handler? The latter // will probably introduce more complexity in terms of graceful // reload. - ca.insecureSrv = server.New(config.InsecureAddress, insecureHandler, nil) + ca.insecureSrv = server.New(cfg.InsecureAddress, insecureHandler, nil) } return ca, nil @@ -285,24 +285,24 @@ func (ca *CA) Init(config *config.Config) (*CA, error) { // Run starts the CA calling to the server ListenAndServe method. func (ca *CA) Run() error { var wg sync.WaitGroup - errors := make(chan error, 1) + errs := make(chan error, 1) if ca.insecureSrv != nil { wg.Add(1) go func() { defer wg.Done() - errors <- ca.insecureSrv.ListenAndServe() + errs <- ca.insecureSrv.ListenAndServe() }() } wg.Add(1) go func() { defer wg.Done() - errors <- ca.srv.ListenAndServe() + errs <- ca.srv.ListenAndServe() }() // wait till error occurs; ensures the servers keep listening - err := <-errors + err := <-errs wg.Wait() @@ -331,7 +331,7 @@ func (ca *CA) Stop() error { // Reload reloads the configuration of the CA and calls to the server Reload // method. func (ca *CA) Reload() error { - config, err := config.LoadConfiguration(ca.opts.configFile) + cfg, err := config.LoadConfiguration(ca.opts.configFile) if err != nil { return errors.Wrap(err, "error reloading ca configuration") } @@ -343,12 +343,12 @@ func (ca *CA) Reload() error { } // Do not allow reload if the database configuration has changed. - if !reflect.DeepEqual(ca.config.DB, config.DB) { + if !reflect.DeepEqual(ca.config.DB, cfg.DB) { logContinue("Reload failed because the database configuration has changed.") return errors.New("error reloading ca: database configuration cannot change") } - newCA, err := New(config, + newCA, err := New(cfg, WithPassword(ca.opts.password), WithSSHHostPassword(ca.opts.sshHostPassword), WithSSHUserPassword(ca.opts.sshUserPassword), diff --git a/ca/ca_test.go b/ca/ca_test.go index 6e297733..ff264db7 100644 --- a/ca/ca_test.go +++ b/ca/ca_test.go @@ -322,7 +322,7 @@ ZEp7knvU2psWRw== assert.Equals(t, intermediate, realIntermediate) } else { err := readError(body) - if len(tc.errMsg) == 0 { + if tc.errMsg == "" { assert.FatalError(t, errors.New("must validate response error")) } assert.HasPrefix(t, err.Error(), tc.errMsg) @@ -375,7 +375,7 @@ func TestCAProvisioners(t *testing.T) { assert.Equals(t, a, b) } else { err := readError(body) - if len(tc.errMsg) == 0 { + if tc.errMsg == "" { assert.FatalError(t, errors.New("must validate response error")) } assert.HasPrefix(t, err.Error(), tc.errMsg) @@ -436,7 +436,7 @@ func TestCAProvisionerEncryptedKey(t *testing.T) { assert.Equals(t, ek.Key, tc.expectedKey) } else { err := readError(body) - if len(tc.errMsg) == 0 { + if tc.errMsg == "" { assert.FatalError(t, errors.New("must validate response error")) } assert.HasPrefix(t, err.Error(), tc.errMsg) @@ -497,7 +497,7 @@ func TestCARoot(t *testing.T) { assert.Equals(t, root.RootPEM.Certificate, rootCrt) } else { err := readError(body) - if len(tc.errMsg) == 0 { + if tc.errMsg == "" { assert.FatalError(t, errors.New("must validate response error")) } assert.HasPrefix(t, err.Error(), tc.errMsg) @@ -665,7 +665,7 @@ func TestCARenew(t *testing.T) { assert.Equals(t, *sign.TLSOptions, authority.DefaultTLSOptions) } else { err := readError(body) - if len(tc.errMsg) == 0 { + if tc.errMsg == "" { assert.FatalError(t, errors.New("must validate response error")) } assert.HasPrefix(t, err.Error(), tc.errMsg) diff --git a/ca/client.go b/ca/client.go index 8997fbd5..cfeddba0 100644 --- a/ca/client.go +++ b/ca/client.go @@ -74,17 +74,17 @@ func (c *uaClient) SetTransport(tr http.RoundTripper) { c.Client.Transport = tr } -func (c *uaClient) Get(url string) (*http.Response, error) { - req, err := http.NewRequest("GET", url, nil) +func (c *uaClient) Get(u string) (*http.Response, error) { + req, err := http.NewRequest("GET", u, nil) if err != nil { - return nil, errors.Wrapf(err, "new request GET %s failed", url) + return nil, errors.Wrapf(err, "new request GET %s failed", u) } req.Header.Set("User-Agent", UserAgent) return c.Client.Do(req) } -func (c *uaClient) Post(url, contentType string, body io.Reader) (*http.Response, error) { - req, err := http.NewRequest("POST", url, body) +func (c *uaClient) Post(u, contentType string, body io.Reader) (*http.Response, error) { + req, err := http.NewRequest("POST", u, body) if err != nil { return nil, err } @@ -305,7 +305,7 @@ func WithAdminX5C(certs []*x509.Certificate, key interface{}, passwordFile strin err error opts []jose.Option ) - if len(passwordFile) != 0 { + if passwordFile != "" { opts = append(opts, jose.WithPasswordFile(passwordFile)) } blk, err := pemutil.Serialize(key) @@ -326,14 +326,14 @@ func WithAdminX5C(certs []*x509.Certificate, key interface{}, passwordFile strin for _, e := range o.x5cCert.Extensions { if e.Id.Equal(stepOIDProvisioner) { - var provisioner stepProvisionerASN1 - if _, err := asn1.Unmarshal(e.Value, &provisioner); err != nil { + var prov stepProvisionerASN1 + if _, err := asn1.Unmarshal(e.Value, &prov); err != nil { return errors.Wrap(err, "error unmarshaling provisioner OID from certificate") } - o.x5cIssuer = string(provisioner.Name) + o.x5cIssuer = string(prov.Name) } } - if len(o.x5cIssuer) == 0 { + if o.x5cIssuer == "" { return errors.New("provisioner extension not found in certificate") } @@ -631,7 +631,7 @@ retry: // do not match. func (c *Client) Root(sha256Sum string) (*api.RootResponse, error) { var retried bool - sha256Sum = strings.ToLower(strings.Replace(sha256Sum, "-", "", -1)) + sha256Sum = strings.ToLower(strings.ReplaceAll(sha256Sum, "-", "")) u := c.endpoint.ResolveReference(&url.URL{Path: "/root/" + sha256Sum}) retry: resp, err := newInsecureClient().Get(u.String()) @@ -651,7 +651,7 @@ retry: } // verify the sha256 sum := sha256.Sum256(root.RootPEM.Raw) - if sha256Sum != strings.ToLower(hex.EncodeToString(sum[:])) { + if !strings.EqualFold(sha256Sum, strings.ToLower(hex.EncodeToString(sum[:]))) { return nil, errs.BadRequest("client.Root; root certificate SHA256 fingerprint do not match") } return &root, nil @@ -1066,16 +1066,16 @@ retry: } return nil, readError(resp.Body) } - var config api.SSHConfigResponse - if err := readJSON(resp.Body, &config); err != nil { + var cfg api.SSHConfigResponse + if err := readJSON(resp.Body, &cfg); err != nil { return nil, errors.Wrapf(err, "error reading %s", u) } - return &config, nil + return &cfg, nil } // SSHCheckHost performs the POST /ssh/check-host request to the CA with the // given principal. -func (c *Client) SSHCheckHost(principal string, token string) (*api.SSHCheckPrincipalResponse, error) { +func (c *Client) SSHCheckHost(principal, token string) (*api.SSHCheckPrincipalResponse, error) { var retried bool body, err := json.Marshal(&api.SSHCheckPrincipalRequest{ Type: provisioner.SSHHostCert, diff --git a/ca/client_test.go b/ca/client_test.go index 30669e6e..187066f0 100644 --- a/ca/client_test.go +++ b/ca/client_test.go @@ -135,7 +135,7 @@ func parseCertificateRequest(data string) *x509.CertificateRequest { return csr } -func equalJSON(t *testing.T, a interface{}, b interface{}) bool { +func equalJSON(t *testing.T, a, b interface{}) bool { if reflect.DeepEqual(a, b) { return true } diff --git a/ca/identity/client_test.go b/ca/identity/client_test.go index c792a6dc..402ec7b8 100644 --- a/ca/identity/client_test.go +++ b/ca/identity/client_test.go @@ -187,11 +187,12 @@ func TestLoadClient(t *testing.T) { } else { gotTransport := got.Client.Transport.(*http.Transport) wantTransport := tt.want.Client.Transport.(*http.Transport) - if gotTransport.TLSClientConfig.GetClientCertificate == nil { + switch { + case gotTransport.TLSClientConfig.GetClientCertificate == nil: t.Error("LoadClient() transport does not define GetClientCertificate") - } else if !reflect.DeepEqual(got.CaURL, tt.want.CaURL) || !reflect.DeepEqual(gotTransport.TLSClientConfig.RootCAs.Subjects(), wantTransport.TLSClientConfig.RootCAs.Subjects()) { + case !reflect.DeepEqual(got.CaURL, tt.want.CaURL) || !reflect.DeepEqual(gotTransport.TLSClientConfig.RootCAs.Subjects(), wantTransport.TLSClientConfig.RootCAs.Subjects()): t.Errorf("LoadClient() = %#v, want %#v", got, tt.want) - } else { + default: crt, err := gotTransport.TLSClientConfig.GetClientCertificate(nil) if err != nil { t.Errorf("LoadClient() GetClientCertificate error = %v", err) diff --git a/ca/tls.go b/ca/tls.go index cb9f4707..3a3b6766 100644 --- a/ca/tls.go +++ b/ca/tls.go @@ -105,7 +105,7 @@ func (c *Client) getClientTLSConfig(ctx context.Context, sign *api.SignResponse, tr := getDefaultTransport(tlsConfig) // Use mutable tls.Config on renew - tr.DialTLS = c.buildDialTLS(tlsCtx) // nolint:staticcheck + tr.DialTLS = c.buildDialTLS(tlsCtx) // nolint:staticcheck,gocritic // tr.DialTLSContext = c.buildDialTLSContext(tlsCtx) renewer.RenewCertificate = getRenewFunc(tlsCtx, c, tr, pk) @@ -154,7 +154,7 @@ func (c *Client) GetServerTLSConfig(ctx context.Context, sign *api.SignResponse, // Update renew function with transport tr := getDefaultTransport(tlsConfig) // Use mutable tls.Config on renew - tr.DialTLS = c.buildDialTLS(tlsCtx) // nolint:staticcheck + tr.DialTLS = c.buildDialTLS(tlsCtx) // nolint:staticcheck,gocritic // tr.DialTLSContext = c.buildDialTLSContext(tlsCtx) renewer.RenewCertificate = getRenewFunc(tlsCtx, c, tr, pk) @@ -195,7 +195,7 @@ func (c *Client) buildDialTLS(ctx *TLSOptionCtx) func(network, addr string) (net } // buildDialTLSContext returns an implementation of DialTLSContext callback in http.Transport. -// nolint:unused +// nolint:unused,gocritic func (c *Client) buildDialTLSContext(tlsCtx *TLSOptionCtx) func(ctx context.Context, network, addr string) (net.Conn, error) { return func(ctx context.Context, network, addr string) (net.Conn, error) { d := getDefaultDialer() @@ -253,6 +253,8 @@ func TLSCertificate(sign *api.SignResponse, pk crypto.PrivateKey) (*tls.Certific return nil, err } + // nolint:gocritic + // using a new variable for clarity chain := append(certPEM, caPEM...) cert, err := tls.X509KeyPair(chain, keyPEM) if err != nil { diff --git a/cas/cloudcas/cloudcas.go b/cas/cloudcas/cloudcas.go index 2e9da260..e3e956a9 100644 --- a/cas/cloudcas/cloudcas.go +++ b/cas/cloudcas/cloudcas.go @@ -29,9 +29,7 @@ func init() { }) } -var now = func() time.Time { - return time.Now() -} +var now = time.Now // The actual regular expression that matches a certificate authority is: // ^projects/[a-z][a-z0-9-]{4,28}[a-z0-9]/locations/[a-z0-9-]+/caPools/[a-zA-Z0-9-_]+/certificateAuthorities/[a-zA-Z0-9-_]+$ diff --git a/cas/cloudcas/cloudcas_test.go b/cas/cloudcas/cloudcas_test.go index 0561000c..7f996c15 100644 --- a/cas/cloudcas/cloudcas_test.go +++ b/cas/cloudcas/cloudcas_test.go @@ -12,7 +12,6 @@ import ( "encoding/pem" "fmt" "io" - "log" "net" "os" "reflect" @@ -103,7 +102,7 @@ MHcCAQEEIN51Rgg6YcQVLeCRzumdw4pjM3VWqFIdCbnsV3Up1e/goAoGCCqGSM49 AwEHoUQDQgAEjJIcDhvvxi7gu4aFkiW/8+E3BfPhmhXU5RlDQusre+MHXc7XYMtk Lm6PXPeTF1DNdS21Ju1G/j1yUykGJOmxkg== -----END EC PRIVATE KEY-----` - // nolint:unused,deadcode + // nolint:unused,deadcode,gocritic testIntermediateKey = `-----BEGIN EC PRIVATE KEY----- MHcCAQEEIMMX/XkXGnRDD4fYu7Z4rHACdJn/iyOy2UTwsv+oZ0C+oAoGCCqGSM49 AwEHoUQDQgAE8u6rGAFj5CZpdzzMogLwUyCMnp0X9wtv4OKDRcpzkYf9PU5GuGA6 @@ -190,7 +189,7 @@ func (b *badSigner) Public() crypto.PublicKey { return b.pub } -func (b *badSigner) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { +func (b *badSigner) Sign(rnd io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { return nil, fmt.Errorf("💥") } @@ -730,7 +729,7 @@ func TestCloudCAS_RevokeCertificate(t *testing.T) { func Test_createCertificateID(t *testing.T) { buf := new(bytes.Buffer) setTeeReader(t, buf) - uuid, err := uuid.NewRandomFromReader(rand.Reader) + id, err := uuid.NewRandomFromReader(rand.Reader) if err != nil { t.Fatal(err) } @@ -741,7 +740,7 @@ func Test_createCertificateID(t *testing.T) { want string wantErr bool }{ - {"ok", uuid.String(), false}, + {"ok", id.String(), false}, {"fail", "", true}, } for _, tt := range tests { @@ -858,7 +857,7 @@ func TestCloudCAS_CreateCertificateAuthority(t *testing.T) { return lis.Dial() })) if err != nil { - log.Fatal(err) + t.Fatal(err) } client, err := lroauto.NewOperationsClient(context.Background(), option.WithGRPCConn(conn)) diff --git a/cas/softcas/softcas.go b/cas/softcas/softcas.go index 23dac91b..87dfa5c5 100644 --- a/cas/softcas/softcas.go +++ b/cas/softcas/softcas.go @@ -19,9 +19,7 @@ func init() { }) } -var now = func() time.Time { - return time.Now() -} +var now = time.Now // SoftCAS implements a Certificate Authority Service using Golang or KMS // crypto. This is the default CAS used in step-ca. diff --git a/cas/softcas/softcas_test.go b/cas/softcas/softcas_test.go index c8e1a8e9..bd13f310 100644 --- a/cas/softcas/softcas_test.go +++ b/cas/softcas/softcas_test.go @@ -133,7 +133,7 @@ func (b *badSigner) Public() crypto.PublicKey { return testSigner.Public() } -func (b *badSigner) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { +func (b *badSigner) Sign(_ io.Reader, _ []byte, _ crypto.SignerOpts) ([]byte, error) { return nil, fmt.Errorf("💥") } diff --git a/cas/stepcas/stepcas.go b/cas/stepcas/stepcas.go index a124b4ae..9fcbd36c 100644 --- a/cas/stepcas/stepcas.go +++ b/cas/stepcas/stepcas.go @@ -90,9 +90,9 @@ func (s *StepCAS) RenewCertificate(req *apiv1.RenewCertificateRequest) (*apiv1.R return nil, apiv1.ErrNotImplemented{Message: "stepCAS does not support mTLS renewals"} } +// RevokeCertificate revokes a certificate. func (s *StepCAS) RevokeCertificate(req *apiv1.RevokeCertificateRequest) (*apiv1.RevokeCertificateResponse, error) { - switch { - case req.SerialNumber == "" && req.Certificate == nil: + if req.SerialNumber == "" && req.Certificate == nil { return nil, errors.New("revokeCertificateRequest `serialNumber` or `certificate` are required") } diff --git a/cas/stepcas/x5c_issuer.go b/cas/stepcas/x5c_issuer.go index 636d22f9..76ed9c3c 100644 --- a/cas/stepcas/x5c_issuer.go +++ b/cas/stepcas/x5c_issuer.go @@ -19,9 +19,7 @@ const defaultValidity = 5 * time.Minute // timeNow returns the current time. // This method is used for unit testing purposes. -var timeNow = func() time.Time { - return time.Now() -} +var timeNow = time.Now type x5cIssuer struct { caURL *url.URL diff --git a/cas/stepcas/x5c_issuer_test.go b/cas/stepcas/x5c_issuer_test.go index a3190255..b1bc653d 100644 --- a/cas/stepcas/x5c_issuer_test.go +++ b/cas/stepcas/x5c_issuer_test.go @@ -22,7 +22,7 @@ func (b noneSigner) Public() crypto.PublicKey { return []byte(b) } -func (b noneSigner) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) (signature []byte, err error) { +func (b noneSigner) Sign(rnd io.Reader, digest []byte, opts crypto.SignerOpts) (signature []byte, err error) { return digest, nil } diff --git a/cmd/step-awskms-init/main.go b/cmd/step-awskms-init/main.go index 0678ef39..8e30745f 100644 --- a/cmd/step-awskms-init/main.go +++ b/cmd/step-awskms-init/main.go @@ -24,10 +24,10 @@ import ( func main() { var credentialsFile, region string - var ssh bool + var enableSSH bool flag.StringVar(&credentialsFile, "credentials-file", "", "Path to the `file` containing the AWS KMS credentials.") flag.StringVar(®ion, "region", "", "AWS KMS region name.") - flag.BoolVar(&ssh, "ssh", false, "Create SSH keys.") + flag.BoolVar(&enableSSH, "ssh", false, "Create SSH keys.") flag.Usage = usage flag.Parse() @@ -47,7 +47,7 @@ func main() { fatal(err) } - if ssh { + if enableSSH { ui.Println() if err := createSSH(c); err != nil { fatal(err) @@ -120,7 +120,7 @@ func createX509(c *awskms.KMS) error { return err } - if err = fileutil.WriteFile("root_ca.crt", pem.EncodeToMemory(&pem.Block{ + if err := fileutil.WriteFile("root_ca.crt", pem.EncodeToMemory(&pem.Block{ Type: "CERTIFICATE", Bytes: b, }), 0600); err != nil { @@ -163,7 +163,7 @@ func createX509(c *awskms.KMS) error { return err } - if err = fileutil.WriteFile("intermediate_ca.crt", pem.EncodeToMemory(&pem.Block{ + if err := fileutil.WriteFile("intermediate_ca.crt", pem.EncodeToMemory(&pem.Block{ Type: "CERTIFICATE", Bytes: b, }), 0600); err != nil { @@ -193,7 +193,7 @@ func createSSH(c *awskms.KMS) error { return err } - if err = fileutil.WriteFile("ssh_user_ca_key.pub", ssh.MarshalAuthorizedKey(key), 0600); err != nil { + if err := fileutil.WriteFile("ssh_user_ca_key.pub", ssh.MarshalAuthorizedKey(key), 0600); err != nil { return err } @@ -214,7 +214,7 @@ func createSSH(c *awskms.KMS) error { return err } - if err = fileutil.WriteFile("ssh_host_ca_key.pub", ssh.MarshalAuthorizedKey(key), 0600); err != nil { + if err := fileutil.WriteFile("ssh_host_ca_key.pub", ssh.MarshalAuthorizedKey(key), 0600); err != nil { return err } diff --git a/cmd/step-ca/main.go b/cmd/step-ca/main.go index e0123678..bed1c14a 100644 --- a/cmd/step-ca/main.go +++ b/cmd/step-ca/main.go @@ -116,7 +116,7 @@ func main() { app.HelpName = "step-ca" app.Version = config.Version() app.Usage = "an online certificate authority for secure automated certificate management" - app.UsageText = `**step-ca** [**--password-file**=] + app.UsageText = `**step-ca** [**--password-file**=] [**--ssh-host-password-file**=] [**--ssh-user-password-file**=] [**--issuer-password-file**=] [**--resolver**=] [**--help**] [**--version**]` app.Description = `**step-ca** runs the Step Online Certificate Authority @@ -191,8 +191,8 @@ var placeholderString = regexp.MustCompile(`<.*?>`) func stringifyFlag(f cli.Flag) string { fv := flagValue(f) - usage := fv.FieldByName("Usage").String() - placeholder := placeholderString.FindString(usage) + usg := fv.FieldByName("Usage").String() + placeholder := placeholderString.FindString(usg) if placeholder == "" { switch f.(type) { case cli.BoolFlag, cli.BoolTFlag: @@ -200,5 +200,5 @@ func stringifyFlag(f cli.Flag) string { placeholder = "" } } - return cli.FlagNamePrefixer(fv.FieldByName("Name").String(), placeholder) + "\t" + usage + return cli.FlagNamePrefixer(fv.FieldByName("Name").String(), placeholder) + "\t" + usg } diff --git a/cmd/step-cloudkms-init/main.go b/cmd/step-cloudkms-init/main.go index 14bf50f1..27dc82ad 100644 --- a/cmd/step-cloudkms-init/main.go +++ b/cmd/step-cloudkms-init/main.go @@ -27,13 +27,13 @@ func main() { var credentialsFile string var project, location, ring string var protectionLevelName string - var ssh bool + var enableSSH bool flag.StringVar(&credentialsFile, "credentials-file", "", "Path to the `file` containing the Google's Cloud KMS credentials.") flag.StringVar(&project, "project", "", "Google Cloud Project ID.") flag.StringVar(&location, "location", "global", "Cloud KMS location name.") flag.StringVar(&ring, "ring", "pki", "Cloud KMS ring name.") flag.StringVar(&protectionLevelName, "protection-level", "SOFTWARE", "Protection level to use, SOFTWARE or HSM.") - flag.BoolVar(&ssh, "ssh", false, "Create SSH keys.") + flag.BoolVar(&enableSSH, "ssh", false, "Create SSH keys.") flag.Usage = usage flag.Parse() @@ -77,7 +77,7 @@ func main() { fatal(err) } - if ssh { + if enableSSH { ui.Println() if err := createSSH(c, project, location, ring, protectionLevel); err != nil { fatal(err) @@ -153,7 +153,7 @@ func createPKI(c *cloudkms.CloudKMS, project, location, keyRing string, protecti return err } - if err = fileutil.WriteFile("root_ca.crt", pem.EncodeToMemory(&pem.Block{ + if err := fileutil.WriteFile("root_ca.crt", pem.EncodeToMemory(&pem.Block{ Type: "CERTIFICATE", Bytes: b, }), 0600); err != nil { @@ -197,7 +197,7 @@ func createPKI(c *cloudkms.CloudKMS, project, location, keyRing string, protecti return err } - if err = fileutil.WriteFile("intermediate_ca.crt", pem.EncodeToMemory(&pem.Block{ + if err := fileutil.WriteFile("intermediate_ca.crt", pem.EncodeToMemory(&pem.Block{ Type: "CERTIFICATE", Bytes: b, }), 0600); err != nil { @@ -230,7 +230,7 @@ func createSSH(c *cloudkms.CloudKMS, project, location, keyRing string, protecti return err } - if err = fileutil.WriteFile("ssh_user_ca_key.pub", ssh.MarshalAuthorizedKey(key), 0600); err != nil { + if err := fileutil.WriteFile("ssh_user_ca_key.pub", ssh.MarshalAuthorizedKey(key), 0600); err != nil { return err } @@ -252,7 +252,7 @@ func createSSH(c *cloudkms.CloudKMS, project, location, keyRing string, protecti return err } - if err = fileutil.WriteFile("ssh_host_ca_key.pub", ssh.MarshalAuthorizedKey(key), 0600); err != nil { + if err := fileutil.WriteFile("ssh_host_ca_key.pub", ssh.MarshalAuthorizedKey(key), 0600); err != nil { return err } diff --git a/cmd/step-pkcs11-init/main.go b/cmd/step-pkcs11-init/main.go index 78c531c6..8e7bc075 100644 --- a/cmd/step-pkcs11-init/main.go +++ b/cmd/step-pkcs11-init/main.go @@ -329,7 +329,7 @@ func createPKI(k kms.KeyManager, c Config) error { } if cm, ok := k.(kms.CertificateManager); ok && !c.NoCerts { - if err = cm.StoreCertificate(&apiv1.StoreCertificateRequest{ + if err := cm.StoreCertificate(&apiv1.StoreCertificateRequest{ Name: c.RootObject, Certificate: root, }); err != nil { @@ -337,7 +337,7 @@ func createPKI(k kms.KeyManager, c Config) error { } } - if err = fileutil.WriteFile(c.RootPath, pem.EncodeToMemory(&pem.Block{ + if err := fileutil.WriteFile(c.RootPath, pem.EncodeToMemory(&pem.Block{ Type: "CERTIFICATE", Bytes: b, }), 0600); err != nil { @@ -406,7 +406,7 @@ func createPKI(k kms.KeyManager, c Config) error { } if cm, ok := k.(kms.CertificateManager); ok && !c.NoCerts { - if err = cm.StoreCertificate(&apiv1.StoreCertificateRequest{ + if err := cm.StoreCertificate(&apiv1.StoreCertificateRequest{ Name: c.CrtObject, Certificate: intermediate, }); err != nil { @@ -414,7 +414,7 @@ func createPKI(k kms.KeyManager, c Config) error { } } - if err = fileutil.WriteFile(c.CrtPath, pem.EncodeToMemory(&pem.Block{ + if err := fileutil.WriteFile(c.CrtPath, pem.EncodeToMemory(&pem.Block{ Type: "CERTIFICATE", Bytes: b, }), 0600); err != nil { diff --git a/cmd/step-yubikey-init/main.go b/cmd/step-yubikey-init/main.go index 163d0fcb..8b0ffab5 100644 --- a/cmd/step-yubikey-init/main.go +++ b/cmd/step-yubikey-init/main.go @@ -228,7 +228,7 @@ func createPKI(k kms.KeyManager, c Config) error { } if cm, ok := k.(kms.CertificateManager); ok { - if err = cm.StoreCertificate(&apiv1.StoreCertificateRequest{ + if err := cm.StoreCertificate(&apiv1.StoreCertificateRequest{ Name: c.RootSlot, Certificate: root, }); err != nil { @@ -236,7 +236,7 @@ func createPKI(k kms.KeyManager, c Config) error { } } - if err = fileutil.WriteFile("root_ca.crt", pem.EncodeToMemory(&pem.Block{ + if err := fileutil.WriteFile("root_ca.crt", pem.EncodeToMemory(&pem.Block{ Type: "CERTIFICATE", Bytes: b, }), 0600); err != nil { @@ -305,7 +305,7 @@ func createPKI(k kms.KeyManager, c Config) error { } if cm, ok := k.(kms.CertificateManager); ok { - if err = cm.StoreCertificate(&apiv1.StoreCertificateRequest{ + if err := cm.StoreCertificate(&apiv1.StoreCertificateRequest{ Name: c.CrtSlot, Certificate: intermediate, }); err != nil { @@ -313,7 +313,7 @@ func createPKI(k kms.KeyManager, c Config) error { } } - if err = fileutil.WriteFile("intermediate_ca.crt", pem.EncodeToMemory(&pem.Block{ + if err := fileutil.WriteFile("intermediate_ca.crt", pem.EncodeToMemory(&pem.Block{ Type: "CERTIFICATE", Bytes: b, }), 0600); err != nil { diff --git a/commands/app.go b/commands/app.go index 3aaee0f5..84232a6c 100644 --- a/commands/app.go +++ b/commands/app.go @@ -24,7 +24,7 @@ var AppCommand = cli.Command{ Name: "start", Action: appAction, UsageText: `**step-ca** [**--password-file**=] -[**--ssh-host-password-file**=] [**--ssh-user-password-file**=] +[**--ssh-host-password-file**=] [**--ssh-user-password-file**=] [**--issuer-password-file**=] [**--resolver**=]`, Flags: []cli.Flag{ cli.StringFlag{ @@ -79,13 +79,13 @@ func appAction(ctx *cli.Context) error { } configFile := ctx.Args().Get(0) - config, err := config.LoadConfiguration(configFile) + cfg, err := config.LoadConfiguration(configFile) if err != nil { fatal(err) } - if config.AuthorityConfig != nil { - if token == "" && strings.EqualFold(config.AuthorityConfig.DeploymentType, pki.LinkedDeployment.String()) { + if cfg.AuthorityConfig != nil { + if token == "" && strings.EqualFold(cfg.AuthorityConfig.DeploymentType, pki.LinkedDeployment.String()) { return errors.New(`'step-ca' requires the '--token' flag for linked deploy type. To get a linked authority token: @@ -136,7 +136,7 @@ To get a linked authority token: } } - srv, err := ca.New(config, + srv, err := ca.New(cfg, ca.WithConfigFile(configFile), ca.WithPassword(password), ca.WithSSHHostPassword(sshHostPassword), diff --git a/commands/export.go b/commands/export.go index be6d88e5..5586f576 100644 --- a/commands/export.go +++ b/commands/export.go @@ -63,11 +63,11 @@ func exportAction(ctx *cli.Context) error { passwordFile := ctx.String("password-file") issuerPasswordFile := ctx.String("issuer-password-file") - config, err := config.LoadConfiguration(configFile) + cfg, err := config.LoadConfiguration(configFile) if err != nil { return err } - if err := config.Validate(); err != nil { + if err := cfg.Validate(); err != nil { return err } @@ -76,19 +76,19 @@ func exportAction(ctx *cli.Context) error { if err != nil { return errors.Wrapf(err, "error reading %s", passwordFile) } - config.Password = string(bytes.TrimRightFunc(b, unicode.IsSpace)) + cfg.Password = string(bytes.TrimRightFunc(b, unicode.IsSpace)) } if issuerPasswordFile != "" { b, err := ioutil.ReadFile(issuerPasswordFile) if err != nil { return errors.Wrapf(err, "error reading %s", issuerPasswordFile) } - if config.AuthorityConfig.CertificateIssuer != nil { - config.AuthorityConfig.CertificateIssuer.Password = string(bytes.TrimRightFunc(b, unicode.IsSpace)) + if cfg.AuthorityConfig.CertificateIssuer != nil { + cfg.AuthorityConfig.CertificateIssuer.Password = string(bytes.TrimRightFunc(b, unicode.IsSpace)) } } - auth, err := authority.New(config) + auth, err := authority.New(cfg) if err != nil { return err } diff --git a/commands/onboard.go b/commands/onboard.go index eb8285aa..ebd468f5 100644 --- a/commands/onboard.go +++ b/commands/onboard.go @@ -103,8 +103,8 @@ func onboardAction(ctx *cli.Context) error { return errors.Wrap(msg, "error receiving onboarding guide") } - var config onboardingConfiguration - if err := readJSON(res.Body, &config); err != nil { + var cfg onboardingConfiguration + if err := readJSON(res.Body, &cfg); err != nil { return errors.Wrap(err, "error unmarshaling response") } @@ -112,16 +112,16 @@ func onboardAction(ctx *cli.Context) error { if err != nil { return err } - config.password = []byte(password) + cfg.password = []byte(password) ui.Println("Initializing step-ca with the following configuration:") - ui.PrintSelected("Name", config.Name) - ui.PrintSelected("DNS", config.DNS) - ui.PrintSelected("Address", config.Address) + ui.PrintSelected("Name", cfg.Name) + ui.PrintSelected("DNS", cfg.DNS) + ui.PrintSelected("Address", cfg.Address) ui.PrintSelected("Password", password) ui.Println() - caConfig, fp, err := onboardPKI(config) + caConfig, fp, err := onboardPKI(cfg) if err != nil { return err } @@ -149,23 +149,23 @@ func onboardAction(ctx *cli.Context) error { ui.Println("Initialized!") ui.Println("Step CA is starting. Please return to the onboarding guide in your browser to continue.") - srv, err := ca.New(caConfig, ca.WithPassword(config.password)) + srv, err := ca.New(caConfig, ca.WithPassword(cfg.password)) if err != nil { fatal(err) } go ca.StopReloaderHandler(srv) - if err = srv.Run(); err != nil && err != http.ErrServerClosed { + if err := srv.Run(); err != nil && err != http.ErrServerClosed { fatal(err) } return nil } -func onboardPKI(config onboardingConfiguration) (*config.Config, string, error) { +func onboardPKI(cfg onboardingConfiguration) (*config.Config, string, error) { var opts = []pki.Option{ - pki.WithAddress(config.Address), - pki.WithDNSNames([]string{config.DNS}), + pki.WithAddress(cfg.Address), + pki.WithDNSNames([]string{cfg.DNS}), pki.WithProvisioner("admin"), } @@ -179,25 +179,25 @@ func onboardPKI(config onboardingConfiguration) (*config.Config, string, error) // Generate pki ui.Println("Generating root certificate...") - root, err := p.GenerateRootCertificate(config.Name, config.Name, config.Name, config.password) + root, err := p.GenerateRootCertificate(cfg.Name, cfg.Name, cfg.Name, cfg.password) if err != nil { return nil, "", err } ui.Println("Generating intermediate certificate...") - err = p.GenerateIntermediateCertificate(config.Name, config.Name, config.Name, root, config.password) + err = p.GenerateIntermediateCertificate(cfg.Name, cfg.Name, cfg.Name, root, cfg.password) if err != nil { return nil, "", err } // Write files to disk - if err = p.WriteFiles(); err != nil { + if err := p.WriteFiles(); err != nil { return nil, "", err } // Generate provisioner ui.Println("Generating admin provisioner...") - if err = p.GenerateKeyPairs(config.password); err != nil { + if err := p.GenerateKeyPairs(cfg.password); err != nil { return nil, "", err } @@ -211,7 +211,7 @@ func onboardPKI(config onboardingConfiguration) (*config.Config, string, error) if err != nil { return nil, "", errors.Wrapf(err, "error marshaling %s", p.GetCAConfigPath()) } - if err = fileutil.WriteFile(p.GetCAConfigPath(), b, 0666); err != nil { + if err := fileutil.WriteFile(p.GetCAConfigPath(), b, 0666); err != nil { return nil, "", errs.FileError(err, p.GetCAConfigPath()) } diff --git a/db/db_test.go b/db/db_test.go index 7efc623e..40f59215 100644 --- a/db/db_test.go +++ b/db/db_test.go @@ -144,15 +144,15 @@ func TestUseToken(t *testing.T) { } for name, tc := range tests { t.Run(name, func(t *testing.T) { - ok, err := tc.db.UseToken(tc.id, tc.tok) - if err != nil { + switch ok, err := tc.db.UseToken(tc.id, tc.tok); { + case err != nil: if assert.NotNil(t, tc.want.err) { assert.HasPrefix(t, err.Error(), tc.want.err.Error()) } assert.False(t, ok) - } else if ok { + case ok: assert.True(t, tc.want.ok) - } else { + default: assert.False(t, tc.want.ok) } }) diff --git a/kms/sshagentkms/sshagentkms_test.go b/kms/sshagentkms/sshagentkms_test.go index 30edd5d1..d3a9e9f5 100644 --- a/kms/sshagentkms/sshagentkms_test.go +++ b/kms/sshagentkms/sshagentkms_test.go @@ -378,6 +378,7 @@ func TestSSHAgentKMS_CreateSigner(t *testing.T) { t.Errorf("SSHAgentKMS.CreateSigner() error = %v, wantErr %v", err, tt.wantErr) return } + // nolint:gocritic switch s := got.(type) { case *WrappedSSHSigner: gotPkS := s.Sshsigner.PublicKey().(*agent.Key).String() + "\n" @@ -562,6 +563,7 @@ func TestSSHAgentKMS_GetPublicKey(t *testing.T) { t.Errorf("SSHAgentKMS.GetPublicKey() error = %v, wantErr %v", err, tt.wantErr) return } + // nolint:gocritic switch tt.want.(type) { case ssh.PublicKey: // If we want a ssh.PublicKey, protote got to a diff --git a/pki/pki.go b/pki/pki.go index 12e71e47..18cd0dda 100644 --- a/pki/pki.go +++ b/pki/pki.go @@ -128,7 +128,7 @@ func GetTemplatesPath() string { // GetProvisioners returns the map of provisioners on the given CA. func GetProvisioners(caURL, rootFile string) (provisioner.List, error) { - if len(rootFile) == 0 { + if rootFile == "" { rootFile = GetRootCAPath() } client, err := ca.NewClient(caURL, ca.WithRootFile(rootFile)) @@ -153,7 +153,7 @@ func GetProvisioners(caURL, rootFile string) (provisioner.List, error) { // GetProvisionerKey returns the encrypted provisioner key with the for the // given kid. func GetProvisionerKey(caURL, rootFile, kid string) (string, error) { - if len(rootFile) == 0 { + if rootFile == "" { rootFile = GetRootCAPath() } client, err := ca.NewClient(caURL, ca.WithRootFile(rootFile)) @@ -315,17 +315,17 @@ func New(o apiv1.Options, opts ...Option) (*PKI, error) { // Use /home/step as the step path in helm configurations. // Use the current step path when creating pki in files. - var public, private, config string + var public, private, cfg string if p.options.isHelm { public = "/home/step/certs" private = "/home/step/secrets" - config = "/home/step/config" + cfg = "/home/step/config" } else { public = GetPublicPath() private = GetSecretsPath() - config = GetConfigPath() + cfg = GetConfigPath() // Create directories - dirs := []string{public, private, config, GetTemplatesPath()} + dirs := []string{public, private, cfg, GetTemplatesPath()} for _, name := range dirs { if _, err := os.Stat(name); os.IsNotExist(err) { if err = os.MkdirAll(name, 0700); err != nil { @@ -380,10 +380,10 @@ func New(o apiv1.Options, opts ...Option) (*PKI, error) { if p.Ssh.UserKey, err = getPath(private, "ssh_user_ca_key"); err != nil { return nil, err } - if p.defaults, err = getPath(config, "defaults.json"); err != nil { + if p.defaults, err = getPath(cfg, "defaults.json"); err != nil { return nil, err } - if p.config, err = getPath(config, "ca.json"); err != nil { + if p.config, err = getPath(cfg, "ca.json"); err != nil { return nil, err } p.Defaults.CaConfig = p.config @@ -620,16 +620,17 @@ func (p *PKI) askFeedback() { func (p *PKI) tellPKI() { ui.Println() - if p.casOptions.Is(apiv1.SoftCAS) { + switch { + case p.casOptions.Is(apiv1.SoftCAS): ui.PrintSelected("Root certificate", p.Root[0]) ui.PrintSelected("Root private key", p.RootKey[0]) ui.PrintSelected("Root fingerprint", p.Defaults.Fingerprint) ui.PrintSelected("Intermediate certificate", p.Intermediate) ui.PrintSelected("Intermediate private key", p.IntermediateKey) - } else if p.Defaults.Fingerprint != "" { + case p.Defaults.Fingerprint != "": ui.PrintSelected("Root certificate", p.Root[0]) ui.PrintSelected("Root fingerprint", p.Defaults.Fingerprint) - } else { + default: ui.Printf(`{{ "%s" | red }} {{ "Root certificate:" | bold }} failed to retrieve it from RA`+"\n", ui.IconBad) } if p.options.enableSSH { @@ -657,7 +658,7 @@ func (p *PKI) GenerateConfig(opt ...ConfigOption) (*authconfig.Config, error) { authorityOptions = &p.casOptions } - config := &authconfig.Config{ + cfg := &authconfig.Config{ Root: p.Root, FederatedRoots: p.FederatedRoots, IntermediateCert: p.Intermediate, @@ -681,7 +682,7 @@ func (p *PKI) GenerateConfig(opt ...ConfigOption) (*authconfig.Config, error) { // Add linked as a deployment type to detect it on start and provide a // message if the token is not given. if p.options.deploymentType == LinkedDeployment { - config.AuthorityConfig.DeploymentType = LinkedDeployment.String() + cfg.AuthorityConfig.DeploymentType = LinkedDeployment.String() } // On standalone deployments add the provisioners to either the ca.json or @@ -711,7 +712,7 @@ func (p *PKI) GenerateConfig(opt ...ConfigOption) (*authconfig.Config, error) { if p.options.enableSSH { enableSSHCA := true - config.SSH = &authconfig.SSHConfig{ + cfg.SSH = &authconfig.SSHConfig{ HostKey: p.Ssh.HostKey, UserKey: p.Ssh.UserKey, } @@ -733,19 +734,19 @@ func (p *PKI) GenerateConfig(opt ...ConfigOption) (*authconfig.Config, error) { // Apply configuration modifiers for _, o := range opt { - if err := o(config); err != nil { + if err := o(cfg); err != nil { return nil, err } } // Set authority.enableAdmin to true if p.options.enableAdmin { - config.AuthorityConfig.EnableAdmin = true + cfg.AuthorityConfig.EnableAdmin = true } if p.options.deploymentType == StandaloneDeployment { - if !config.AuthorityConfig.EnableAdmin { - config.AuthorityConfig.Provisioners = provisioners + if !cfg.AuthorityConfig.EnableAdmin { + cfg.AuthorityConfig.Provisioners = provisioners } else { // At this moment this code path is never used because `step ca // init` will always set enableAdmin to false for a standalone @@ -754,11 +755,11 @@ func (p *PKI) GenerateConfig(opt ...ConfigOption) (*authconfig.Config, error) { // // Note that we might want to be able to define the database as a // flag in `step ca init` so we can write to the proper place. - db, err := db.New(config.DB) + _db, err := db.New(cfg.DB) if err != nil { return nil, err } - adminDB, err := admindb.New(db.(nosql.DB), admin.DefaultAuthorityID) + adminDB, err := admindb.New(_db.(nosql.DB), admin.DefaultAuthorityID) if err != nil { return nil, err } @@ -788,7 +789,7 @@ func (p *PKI) GenerateConfig(opt ...ConfigOption) (*authconfig.Config, error) { } } - return config, nil + return cfg, nil } // Save stores the pki on a json file that will be used as the certificate @@ -804,12 +805,12 @@ func (p *PKI) Save(opt ...ConfigOption) error { // Generate and write ca.json if !p.options.pkiOnly { - config, err := p.GenerateConfig(opt...) + cfg, err := p.GenerateConfig(opt...) if err != nil { return err } - b, err := json.MarshalIndent(config, "", "\t") + b, err := json.MarshalIndent(cfg, "", "\t") if err != nil { return errors.Wrapf(err, "error marshaling %s", p.config) } @@ -833,14 +834,14 @@ func (p *PKI) Save(opt ...ConfigOption) error { } // Generate and write templates - if err := generateTemplates(config.Templates); err != nil { + if err := generateTemplates(cfg.Templates); err != nil { return err } - if config.DB != nil { - ui.PrintSelected("Database folder", config.DB.DataSource) + if cfg.DB != nil { + ui.PrintSelected("Database folder", cfg.DB.DataSource) } - if config.Templates != nil { + if cfg.Templates != nil { ui.PrintSelected("Templates folder", GetTemplatesPath()) } diff --git a/scep/api/api.go b/scep/api/api.go index e64eef83..4e02d4a1 100644 --- a/scep/api/api.go +++ b/scep/api/api.go @@ -198,14 +198,14 @@ func (h *Handler) lookupProvisioner(next nextHTTP) nextHTTP { return } - provisioner, ok := p.(*provisioner.SCEP) + prov, ok := p.(*provisioner.SCEP) if !ok { api.WriteError(w, errors.New("provisioner must be of type SCEP")) return } ctx := r.Context() - ctx = context.WithValue(ctx, scep.ProvisionerContextKey, scep.Provisioner(provisioner)) + ctx = context.WithValue(ctx, scep.ProvisionerContextKey, scep.Provisioner(prov)) next(w, r.WithContext(ctx)) } } diff --git a/templates/templates.go b/templates/templates.go index f98fb866..16e891d9 100644 --- a/templates/templates.go +++ b/templates/templates.go @@ -5,6 +5,7 @@ import ( "io/ioutil" "os" "path/filepath" + "strings" "text/template" "github.com/Masterminds/sprig/v3" @@ -226,14 +227,11 @@ func (t *Template) Output(data interface{}) (Output, error) { // backfill updates old templates with the required data. func (t *Template) backfill(b []byte) { - switch t.Name { - case "sshd_config.tpl": - if len(t.RequiredData) == 0 { - a := bytes.TrimSpace(b) - b := bytes.TrimSpace([]byte(DefaultSSHTemplateData[t.Name])) - if bytes.Equal(a, b) { - t.RequiredData = []string{"Certificate", "Key"} - } + if strings.EqualFold(t.Name, "sshd_config.tpl") && len(t.RequiredData) == 0 { + a := bytes.TrimSpace(b) + b := bytes.TrimSpace([]byte(DefaultSSHTemplateData[t.Name])) + if bytes.Equal(a, b) { + t.RequiredData = []string{"Certificate", "Key"} } } }