Skip to content

Commit

Permalink
Update golangci-lint version, linting fixes. (crewjam#511)
Browse files Browse the repository at this point in the history
(cherry picked from commit f9e6716)
  • Loading branch information
Michael Wilson authored and mgyongyosi committed Oct 25, 2023
1 parent 94154cb commit 4ec10ab
Show file tree
Hide file tree
Showing 37 changed files with 303 additions and 215 deletions.
34 changes: 14 additions & 20 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,41 +7,35 @@

linters:
enable:
- gofmt # Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification [fast: true, auto-fix: true]
- goimports # Goimports does everything that gofmt does. Additionally it checks unused imports [fast: true, auto-fix: true]
- gosec # Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases [fast: true, auto-fix: false]
- misspell # Finds commonly misspelled English words in comments [fast: true, auto-fix: true]
- deadcode # Finds unused code [fast: true, auto-fix: false]
- revive # Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes [fast: true, auto-fix: false]
- unconvert # Remove unnecessary type conversions [fast: true, auto-fix: false]

disable:
# TODO(ross): fix errors reported by these checkers and enable them
- bodyclose # checks whether HTTP response body is closed successfully [fast: false, auto-fix: false]
- depguard # Go linter that checks if package imports are in a list of acceptable packages [fast: true, auto-fix: false]
- dupl # Tool for code clone detection [fast: true, auto-fix: false]
- errcheck # Inspects source code for security problems [fast: true, auto-fix: false]
- gochecknoglobals # Checks that no globals are present in Go code [fast: true, auto-fix: false]
- gochecknoinits # Checks that no init functions are present in Go code [fast: true, auto-fix: false]
- goconst # Finds repeated strings that could be replaced by a constant [fast: true, auto-fix: false]
- gocritic # The most opinionated Go source code linter [fast: true, auto-fix: false]
- gocyclo # Computes and checks the cyclomatic complexity of functions [fast: true, auto-fix: false]
- gofmt # Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification [fast: true, auto-fix: true]
- goimports # Goimports does everything that gofmt does. Additionally it checks unused imports [fast: true, auto-fix: true]
- gosec # Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases [fast: true, auto-fix: false]
- gosimple # Linter for Go source code that specializes in simplifying a code [fast: false, auto-fix: false]
- govet # Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string [fast: false, auto-fix: false]
- ineffassign # Detects when assignments to existing variables are not used [fast: true, auto-fix: false]
- interfacer # Linter that suggests narrower interface types [fast: false, auto-fix: false]
- lll # Reports long lines [fast: true, auto-fix: false]
- maligned # Tool to detect Go structs that would take less memory if their fields were sorted [fast: true, auto-fix: false]
- misspell # Finds commonly misspelled English words in comments [fast: true, auto-fix: true]
- nakedret # Finds naked returns in functions greater than a specified function length [fast: true, auto-fix: false]
- prealloc # Finds slice declarations that could potentially be preallocated [fast: true, auto-fix: false]
- scopelint # Scopelint checks for unpinned variables in go programs [fast: true, auto-fix: false]
- revive # Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes [fast: true, auto-fix: false]
- staticcheck # Staticcheck is a go vet on steroids, applying a ton of static analysis checks [fast: false, auto-fix: false]
- structcheck # Finds unused struct fields [fast: true, auto-fix: false]
- stylecheck # Stylecheck is a replacement for golint [fast: false, auto-fix: false]
- typecheck # Like the front-end of a Go compiler, parses and type-checks Go code [fast: true, auto-fix: false]
- unconvert # Remove unnecessary type conversions [fast: true, auto-fix: false]
- unparam # Reports unused function parameters [fast: false, auto-fix: false]
- unused # Checks Go code for unused constants, variables, functions and types [fast: false, auto-fix: false]
- varcheck # Finds unused global variables and constants [fast: true, auto-fix: false]

disable:
# TODO(ross): fix errors reported by these checkers and enable them
- dupl # Tool for code clone detection [fast: true, auto-fix: false]
- gochecknoglobals # Checks that no globals are present in Go code [fast: true, auto-fix: false]
- gochecknoinits # Checks that no init functions are present in Go code [fast: true, auto-fix: false]
- goconst # Finds repeated strings that could be replaced by a constant [fast: true, auto-fix: false]
- lll # Reports long lines [fast: true, auto-fix: false]
linters-settings:
goimports:
local-prefixes: github.com/crewjam/saml
Expand Down
1 change: 1 addition & 0 deletions example/idp/idp.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Package main contains an example identity provider implementation.
package main

import (
Expand Down
22 changes: 15 additions & 7 deletions example/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type Link struct {
}

// CreateLink handles requests to create links
func CreateLink(c web.C, w http.ResponseWriter, r *http.Request) {
func CreateLink(_ web.C, w http.ResponseWriter, r *http.Request) {
account := r.Header.Get("X-Remote-User")
l := Link{
ShortLink: uniuri.New(),
Expand All @@ -42,22 +42,20 @@ func CreateLink(c web.C, w http.ResponseWriter, r *http.Request) {
links[l.ShortLink] = l

fmt.Fprintf(w, "%s\n", l.ShortLink)
return
}

// ServeLink handles requests to redirect to a link
func ServeLink(c web.C, w http.ResponseWriter, r *http.Request) {
func ServeLink(_ web.C, w http.ResponseWriter, r *http.Request) {
l, ok := links[strings.TrimPrefix(r.URL.Path, "/")]
if !ok {
http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound)
return
}
http.Redirect(w, r, l.Target, http.StatusFound)
return
}

// ListLinks returns a list of the current user's links
func ListLinks(c web.C, w http.ResponseWriter, r *http.Request) {
func ListLinks(_ web.C, w http.ResponseWriter, r *http.Request) {
account := r.Header.Get("X-Remote-User")
for _, l := range links {
if l.Owner == account {
Expand Down Expand Up @@ -145,14 +143,24 @@ func main() {

spURL := *idpMetadataURL
spURL.Path = "/services/sp"
http.Post(spURL.String(), "text/xml", bytes.NewReader(spMetadataBuf))
resp, err := http.Post(spURL.String(), "text/xml", bytes.NewReader(spMetadataBuf))

if err != nil {
panic(err)
}

if err := resp.Body.Close(); err != nil {
panic(err)
}

goji.Handle("/saml/*", samlSP)

authMux := web.New()
authMux.Use(samlSP.RequireAccount)
authMux.Get("/whoami", func(w http.ResponseWriter, r *http.Request) {
pretty.Fprintf(w, "%# v", r)
if _, err := pretty.Fprintf(w, "%# v", r); err != nil {
panic(err)
}
})
authMux.Post("/", CreateLink)
authMux.Get("/", ListLinks)
Expand Down
2 changes: 2 additions & 0 deletions example/trivial/trivial.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Package main contains an example service provider implementation.
package main

import (
Expand Down Expand Up @@ -70,6 +71,7 @@ func main() {
})
app := http.HandlerFunc(hello)
slo := http.HandlerFunc(logout)

http.Handle("/hello", samlMiddleware.RequireAccount(app))
http.Handle("/saml/", samlMiddleware)
http.Handle("/logout", slo)
Expand Down
11 changes: 6 additions & 5 deletions identity_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,13 @@ func (idp *IdentityProvider) Handler() http.Handler {
}

// ServeMetadata is an http.HandlerFunc that serves the IDP metadata
func (idp *IdentityProvider) ServeMetadata(w http.ResponseWriter, r *http.Request) {
func (idp *IdentityProvider) ServeMetadata(w http.ResponseWriter, _ *http.Request) {
buf, _ := xml.MarshalIndent(idp.Metadata(), "", " ")
w.Header().Set("Content-Type", "application/samlmetadata+xml")
w.Write(buf)
if _, err := w.Write(buf); err != nil {
idp.Logger.Printf("ERROR: %s", err)
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
}
}

// ServeSSO handles SAML auth requests.
Expand Down Expand Up @@ -718,9 +721,7 @@ func (DefaultAssertionMaker) MakeAssertion(req *IdpAuthnRequest, session *Sessio
})
}

for _, ca := range session.CustomAttributes {
attributes = append(attributes, ca)
}
attributes = append(attributes, session.CustomAttributes...)

if len(session.Groups) != 0 {
groupMemberAttributeValues := []AttributeValue{}
Expand Down
6 changes: 4 additions & 2 deletions identity_provider_go117_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,10 @@ func TestIDPHTTPCanHandleSSORequest(t *testing.T) {
d := bytes.Replace(c, []byte("<AuthnRequest"), []byte("<AuthnRequest ::foo=\"bar\">]]"), 1)
f := bytes.Buffer{}
e, _ := flate.NewWriter(&f, flate.DefaultCompression)
e.Write(d)
e.Close()
_, err := e.Write(d)
assert.Check(t, err)
err = e.Close()
assert.Check(t, err)
g := base64.StdEncoding.EncodeToString(f.Bytes())
invalidRequest := url.QueryEscape(g)

Expand Down
12 changes: 6 additions & 6 deletions identity_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -763,7 +763,7 @@ func TestIDPIDPInitiatedNewSession(t *testing.T) {
r, _ := http.NewRequest("GET", "https://idp.example.com/services/sp/whoami", nil)
test.IDP.ServeIDPInitiated(w, r, test.SP.MetadataURL.String(), "ThisIsTheRelayState")
assert.Check(t, is.Equal(200, w.Code))
assert.Check(t, is.Equal("RelayState: ThisIsTheRelayState", string(w.Body.Bytes())))
assert.Check(t, is.Equal("RelayState: ThisIsTheRelayState", w.Body.String()))
}

func TestIDPIDPInitiatedExistingSession(t *testing.T) {
Expand Down Expand Up @@ -1029,18 +1029,18 @@ func TestIDPRejectDecompressionBomb(t *testing.T) {
},
}

//w := httptest.NewRecorder()

data := bytes.Repeat([]byte("a"), 768*1024*1024)
var compressed bytes.Buffer
w, _ := flate.NewWriter(&compressed, flate.BestCompression)
w.Write(data)
w.Close()
_, err := w.Write(data)
assert.Check(t, err)
err = w.Close()
assert.Check(t, err)
encoded := base64.StdEncoding.EncodeToString(compressed.Bytes())

r, _ := http.NewRequest("GET", "/dontcare?"+url.Values{
"SAMLRequest": {encoded},
}.Encode(), nil)
_, err := NewIdpAuthnRequest(&test.IDP, r)
_, err = NewIdpAuthnRequest(&test.IDP, r)
assert.Error(t, err, "cannot decompress request: flate: uncompress limit exceeded (10485760 bytes)")
}
1 change: 1 addition & 0 deletions logger/logger.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Package logger provides a logging interface.
package logger

import (
Expand Down
2 changes: 1 addition & 1 deletion metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ type EntityDescriptor struct {
}

// MarshalXML implements xml.Marshaler
func (m EntityDescriptor) MarshalXML(e *xml.Encoder, start xml.StartElement) error {
func (m EntityDescriptor) MarshalXML(e *xml.Encoder, _ xml.StartElement) error {
type Alias EntityDescriptor
aux := &struct {
ValidUntil RelaxedTime `xml:"validUntil,attr,omitempty"`
Expand Down
4 changes: 2 additions & 2 deletions samlidp/samlidp.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ type Server struct {
// New returns a new Server
func New(opts Options) (*Server, error) {
metadataURL := opts.URL
metadataURL.Path = metadataURL.Path + "/metadata"
metadataURL.Path += "/metadata"
ssoURL := opts.URL
ssoURL.Path = ssoURL.Path + "/sso"
ssoURL.Path += "/sso"
logr := opts.Logger
if logr == nil {
logr = logger.DefaultLogger
Expand Down
8 changes: 4 additions & 4 deletions samlidp/samlidp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ func TestHTTPCanHandleMetadataRequest(t *testing.T) {
test.Server.ServeHTTP(w, r)
assert.Check(t, is.Equal(http.StatusOK, w.Code))
assert.Check(t,
strings.HasPrefix(string(w.Body.Bytes()), "<EntityDescriptor"),
string(w.Body.Bytes()))
strings.HasPrefix(w.Body.String(), "<EntityDescriptor"),
w.Body.String())
golden.Assert(t, w.Body.String(), "http_metadata_response.html")
}

Expand All @@ -139,7 +139,7 @@ func TestHTTPCanSSORequest(t *testing.T) {
test.Server.ServeHTTP(w, r)
assert.Check(t, is.Equal(http.StatusOK, w.Code))
assert.Check(t,
strings.HasPrefix(string(w.Body.Bytes()), "<html><p></p><form method=\"post\" action=\"https://idp.example.com/sso\">"),
string(w.Body.Bytes()))
strings.HasPrefix(w.Body.String(), "<html><p></p><form method=\"post\" action=\"https://idp.example.com/sso\">"),
w.Body.String())
golden.Assert(t, w.Body.String(), "http_sso_response.html")
}
20 changes: 14 additions & 6 deletions samlidp/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type Service struct {
// service provider ID, which is typically the service provider's
// metadata URL. If an appropriate service provider cannot be found then
// the returned error must be os.ErrNotExist.
func (s *Server) GetServiceProvider(r *http.Request, serviceProviderID string) (*saml.EntityDescriptor, error) {
func (s *Server) GetServiceProvider(_ *http.Request, serviceProviderID string) (*saml.EntityDescriptor, error) {
s.idpConfigMu.RLock()
defer s.idpConfigMu.RUnlock()
rv, ok := s.serviceProviders[serviceProviderID]
Expand All @@ -37,30 +37,38 @@ func (s *Server) GetServiceProvider(r *http.Request, serviceProviderID string) (

// HandleListServices handles the `GET /services/` request and responds with a JSON formatted list
// of service names.
func (s *Server) HandleListServices(c web.C, w http.ResponseWriter, r *http.Request) {
func (s *Server) HandleListServices(_ web.C, w http.ResponseWriter, _ *http.Request) {
services, err := s.Store.List("/services/")
if err != nil {
s.logger.Printf("ERROR: %s", err)
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
return
}

json.NewEncoder(w).Encode(struct {
err = json.NewEncoder(w).Encode(struct {
Services []string `json:"services"`
}{Services: services})
if err != nil {
s.logger.Printf("ERROR: %s", err)
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
}
}

// HandleGetService handles the `GET /services/:id` request and responds with the service
// metadata in XML format.
func (s *Server) HandleGetService(c web.C, w http.ResponseWriter, r *http.Request) {
func (s *Server) HandleGetService(c web.C, w http.ResponseWriter, _ *http.Request) {
service := Service{}
err := s.Store.Get(fmt.Sprintf("/services/%s", c.URLParams["id"]), &service)
if err != nil {
s.logger.Printf("ERROR: %s", err)
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
return
}
xml.NewEncoder(w).Encode(service.Metadata)
err = xml.NewEncoder(w).Encode(service.Metadata)
if err != nil {
s.logger.Printf("ERROR: %s", err)
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
}
}

// HandlePutService handles the `PUT /shortcuts/:id` request. It accepts the XML-formatted
Expand Down Expand Up @@ -92,7 +100,7 @@ func (s *Server) HandlePutService(c web.C, w http.ResponseWriter, r *http.Reques
}

// HandleDeleteService handles the `DELETE /services/:id` request.
func (s *Server) HandleDeleteService(c web.C, w http.ResponseWriter, r *http.Request) {
func (s *Server) HandleDeleteService(c web.C, w http.ResponseWriter, _ *http.Request) {
service := Service{}
err := s.Store.Get(fmt.Sprintf("/services/%s", c.URLParams["id"]), &service)
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions samlidp/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func TestServicesCrud(t *testing.T) {
r, _ := http.NewRequest("GET", "https://idp.example.com/services/", nil)
test.Server.ServeHTTP(w, r)
assert.Check(t, is.Equal(http.StatusOK, w.Code))
assert.Check(t, is.Equal("{\"services\":[]}\n", string(w.Body.Bytes())))
assert.Check(t, is.Equal("{\"services\":[]}\n", w.Body.String()))

w = httptest.NewRecorder()
r, _ = http.NewRequest("PUT", "https://idp.example.com/services/sp",
Expand All @@ -36,7 +36,7 @@ func TestServicesCrud(t *testing.T) {
r, _ = http.NewRequest("GET", "https://idp.example.com/services/", nil)
test.Server.ServeHTTP(w, r)
assert.Check(t, is.Equal(http.StatusOK, w.Code))
assert.Check(t, is.Equal("{\"services\":[\"sp\"]}\n", string(w.Body.Bytes())))
assert.Check(t, is.Equal("{\"services\":[\"sp\"]}\n", w.Body.String()))

assert.Check(t, is.Len(test.Server.serviceProviders, 2))

Expand All @@ -49,6 +49,6 @@ func TestServicesCrud(t *testing.T) {
r, _ = http.NewRequest("GET", "https://idp.example.com/services/", nil)
test.Server.ServeHTTP(w, r)
assert.Check(t, is.Equal(http.StatusOK, w.Code))
assert.Check(t, is.Equal("{\"services\":[]}\n", string(w.Body.Bytes())))
assert.Check(t, is.Equal("{\"services\":[]}\n", w.Body.String()))
assert.Check(t, is.Len(test.Server.serviceProviders, 1))
}
Loading

0 comments on commit 4ec10ab

Please sign in to comment.