Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
- name: golangci-lint
uses: golangci/golangci-lint-action@55c2c1448f86e01eaae002a5a3a9624417608d84 # v6
- uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0
with:
version: v1.54.2
go-version: stable
- uses: golangci/golangci-lint-action@9fae48acfc02a90574d7c304a1758ef9895495fa # v7
with:
version: v2.0
134 changes: 66 additions & 68 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,72 +1,70 @@
# Configuration file for golangci-lint
#
# https://github.com/golangci/golangci-lint
#
# fighting with false positives?
# https://github.com/golangci/golangci-lint#nolint

version: "2"
linters:
enable:
- bodyclose # checks whether HTTP response body is closed successfully [fast: false, auto-fix: false]
- errcheck # Inspects source code for security problems [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]
- 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]
- 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]
- 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]

- bodyclose
- gocritic
- gocyclo
- gosec
- misspell
- nakedret
- prealloc
- revive
- staticcheck
- unconvert
- unparam
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]
- depguard # Go linter that checks if package imports are in a list of acceptable packages [fast: true, auto-fix: false]
linters-settings:
goimports:
local-prefixes: github.com/crewjam/saml
govet:
disable:
- shadow
enable:
- asmdecl
- assign
- atomic
- bools
- buildtag
- cgocall
- composites
- copylocks
- errorsas
- httpresponse
- loopclosure
- lostcancel
- nilfunc
- printf
- shift
- stdmethods
- structtag
- tests
- unmarshal
- unreachable
- unsafeptr
- unusedresult
issues:
exclude-use-default: false
exclude:
- G104 # 'Errors unhandled. (gosec)

- depguard
- dupl
- gochecknoglobals
- gochecknoinits
- goconst
- lll
settings:
govet:
enable:
- asmdecl
- assign
- atomic
- bools
- buildtag
- cgocall
- composites
- copylocks
- errorsas
- httpresponse
- loopclosure
- lostcancel
- nilfunc
- printf
- shift
- stdmethods
- structtag
- tests
- unmarshal
- unreachable
- unsafeptr
- unusedresult
disable:
- shadow
exclusions:
generated: lax
rules:
- path: (.+)\.go$
text: G104 # 'Errors unhandled. (gosec)
paths:
- example/.*\.go$
formatters:
enable:
- gofmt
- goimports
settings:
goimports:
local-prefixes:
- github.com/crewjam/saml
exclusions:
generated: lax
paths:
- third_party$
- builtin$
- examples$
2 changes: 1 addition & 1 deletion identity_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func (idp *IdentityProvider) Metadata() *EntityDescriptor {
}

if idp.LogoutURL.String() != "" {
ed.IDPSSODescriptors[0].SSODescriptor.SingleLogoutServices = []Endpoint{
ed.IDPSSODescriptors[0].SingleLogoutServices = []Endpoint{
{
Binding: HTTPRedirectBinding,
Location: idp.LogoutURL.String(),
Expand Down
37 changes: 20 additions & 17 deletions identity_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,15 +126,15 @@ func NewIdentityProviderTest(t *testing.T, opts ...idpTestOpts) *IdentityProvide
MetadataURL: mustParseURL("https://idp.example.com/saml/metadata"),
SSOURL: mustParseURL("https://idp.example.com/saml/sso"),
ServiceProviderProvider: &mockServiceProviderProvider{
GetServiceProviderFunc: func(r *http.Request, serviceProviderID string) (*EntityDescriptor, error) {
GetServiceProviderFunc: func(_ *http.Request, serviceProviderID string) (*EntityDescriptor, error) {
if serviceProviderID == test.SP.MetadataURL.String() {
return test.SP.Metadata(), nil
}
return nil, os.ErrNotExist
},
},
SessionProvider: &mockSessionProvider{
GetSessionFunc: func(w http.ResponseWriter, r *http.Request, req *IdpAuthnRequest) *Session {
GetSessionFunc: func(_ http.ResponseWriter, _ *http.Request, _ *IdpAuthnRequest) *Session {
return nil
},
},
Expand Down Expand Up @@ -241,9 +241,10 @@ func TestIDPHTTPCanHandleMetadataRequest(t *testing.T) {
func TestIDPCanHandleRequestWithNewSession(t *testing.T) {
test := NewIdentityProviderTest(t, applyKey)
test.IDP.SessionProvider = &mockSessionProvider{
GetSessionFunc: func(w http.ResponseWriter, r *http.Request, req *IdpAuthnRequest) *Session {
fmt.Fprintf(w, "RelayState: %s\nSAMLRequest: %s",
GetSessionFunc: func(w http.ResponseWriter, _ *http.Request, req *IdpAuthnRequest) *Session {
_, err := fmt.Fprintf(w, "RelayState: %s\nSAMLRequest: %s",
req.RelayState, req.RequestBuffer)
assert.NilError(t, err)
return nil
},
}
Expand All @@ -267,7 +268,7 @@ func TestIDPCanHandleRequestWithNewSession(t *testing.T) {
func TestIDPCanHandleRequestWithExistingSession(t *testing.T) {
test := NewIdentityProviderTest(t, applyKey)
test.IDP.SessionProvider = &mockSessionProvider{
GetSessionFunc: func(w http.ResponseWriter, r *http.Request, req *IdpAuthnRequest) *Session {
GetSessionFunc: func(_ http.ResponseWriter, _ *http.Request, _ *IdpAuthnRequest) *Session {
return &Session{
ID: "f00df00df00d",
UserName: "alice",
Expand All @@ -292,7 +293,7 @@ func TestIDPCanHandleRequestWithExistingSession(t *testing.T) {
func TestIDPCanHandlePostRequestWithExistingSession(t *testing.T) {
test := NewIdentityProviderTest(t, applyKey)
test.IDP.SessionProvider = &mockSessionProvider{
GetSessionFunc: func(w http.ResponseWriter, r *http.Request, req *IdpAuthnRequest) *Session {
GetSessionFunc: func(_ http.ResponseWriter, _ *http.Request, _ *IdpAuthnRequest) *Session {
return &Session{
ID: "f00df00df00d",
UserName: "alice",
Expand Down Expand Up @@ -321,7 +322,7 @@ func TestIDPCanHandlePostRequestWithExistingSession(t *testing.T) {
func TestIDPRejectsInvalidRequest(t *testing.T) {
test := NewIdentityProviderTest(t, applyKey)
test.IDP.SessionProvider = &mockSessionProvider{
GetSessionFunc: func(w http.ResponseWriter, r *http.Request, req *IdpAuthnRequest) *Session {
GetSessionFunc: func(_ http.ResponseWriter, _ *http.Request, _ *IdpAuthnRequest) *Session {
panic("not reached")
},
}
Expand Down Expand Up @@ -798,8 +799,9 @@ func TestIDPWriteResponse(t *testing.T) {
func TestIDPIDPInitiatedNewSession(t *testing.T) {
test := NewIdentityProviderTest(t, applyKey)
test.IDP.SessionProvider = &mockSessionProvider{
GetSessionFunc: func(w http.ResponseWriter, r *http.Request, req *IdpAuthnRequest) *Session {
fmt.Fprintf(w, "RelayState: %s", req.RelayState)
GetSessionFunc: func(w http.ResponseWriter, _ *http.Request, req *IdpAuthnRequest) *Session {
_, err := fmt.Fprintf(w, "RelayState: %s", req.RelayState)
assert.NilError(t, err)
return nil
},
}
Expand All @@ -814,7 +816,7 @@ func TestIDPIDPInitiatedNewSession(t *testing.T) {
func TestIDPIDPInitiatedExistingSession(t *testing.T) {
test := NewIdentityProviderTest(t, applyKey)
test.IDP.SessionProvider = &mockSessionProvider{
GetSessionFunc: func(w http.ResponseWriter, r *http.Request, req *IdpAuthnRequest) *Session {
GetSessionFunc: func(_ http.ResponseWriter, _ *http.Request, _ *IdpAuthnRequest) *Session {
return &Session{
ID: "f00df00df00d",
UserName: "alice",
Expand All @@ -832,7 +834,7 @@ func TestIDPIDPInitiatedExistingSession(t *testing.T) {
func TestIDPIDPInitiatedBadServiceProvider(t *testing.T) {
test := NewIdentityProviderTest(t, applyKey)
test.IDP.SessionProvider = &mockSessionProvider{
GetSessionFunc: func(w http.ResponseWriter, r *http.Request, req *IdpAuthnRequest) *Session {
GetSessionFunc: func(_ http.ResponseWriter, _ *http.Request, _ *IdpAuthnRequest) *Session {
return &Session{
ID: "f00df00df00d",
UserName: "alice",
Expand All @@ -849,7 +851,7 @@ func TestIDPIDPInitiatedBadServiceProvider(t *testing.T) {
func TestIDPCanHandleUnencryptedResponse(t *testing.T) {
test := NewIdentityProviderTest(t, applyKey)
test.IDP.SessionProvider = &mockSessionProvider{
GetSessionFunc: func(w http.ResponseWriter, r *http.Request, req *IdpAuthnRequest) *Session {
GetSessionFunc: func(_ http.ResponseWriter, _ *http.Request, _ *IdpAuthnRequest) *Session {
return &Session{ID: "f00df00df00d", UserName: "alice"}
},
}
Expand All @@ -860,7 +862,7 @@ func TestIDPCanHandleUnencryptedResponse(t *testing.T) {
&metadata)
assert.Check(t, err)
test.IDP.ServiceProviderProvider = &mockServiceProviderProvider{
GetServiceProviderFunc: func(r *http.Request, serviceProviderID string) (*EntityDescriptor, error) {
GetServiceProviderFunc: func(_ *http.Request, serviceProviderID string) (*EntityDescriptor, error) {
if serviceProviderID == "https://gitlab.example.com/users/saml/metadata" {
return &metadata, nil
}
Expand Down Expand Up @@ -1027,7 +1029,7 @@ func TestIDPRequestedAttributes(t *testing.T) {
func TestIDPNoDestination(t *testing.T) {
test := NewIdentityProviderTest(t, applyKey)
test.IDP.SessionProvider = &mockSessionProvider{
GetSessionFunc: func(w http.ResponseWriter, r *http.Request, req *IdpAuthnRequest) *Session {
GetSessionFunc: func(_ http.ResponseWriter, _ *http.Request, _ *IdpAuthnRequest) *Session {
return &Session{ID: "f00df00df00d", UserName: "alice"}
},
}
Expand All @@ -1036,7 +1038,7 @@ func TestIDPNoDestination(t *testing.T) {
err := xml.Unmarshal(golden.Get(t, "TestIDPNoDestination_idp_metadata.xml"), &metadata)
assert.Check(t, err)
test.IDP.ServiceProviderProvider = &mockServiceProviderProvider{
GetServiceProviderFunc: func(r *http.Request, serviceProviderID string) (*EntityDescriptor, error) {
GetServiceProviderFunc: func(_ *http.Request, serviceProviderID string) (*EntityDescriptor, error) {
if serviceProviderID == "https://gitlab.example.com/users/saml/metadata" {
return &metadata, nil
}
Expand Down Expand Up @@ -1067,9 +1069,10 @@ func TestIDPNoDestination(t *testing.T) {
func TestIDPRejectDecompressionBomb(t *testing.T) {
test := NewIdentityProviderTest(t)
test.IDP.SessionProvider = &mockSessionProvider{
GetSessionFunc: func(w http.ResponseWriter, r *http.Request, req *IdpAuthnRequest) *Session {
fmt.Fprintf(w, "RelayState: %s\nSAMLRequest: %s",
GetSessionFunc: func(w http.ResponseWriter, _ *http.Request, req *IdpAuthnRequest) *Session {
_, err := fmt.Fprintf(w, "RelayState: %s\nSAMLRequest: %s",
req.RelayState, req.RequestBuffer)
assert.NilError(t, err)
return nil
},
}
Expand Down
6 changes: 3 additions & 3 deletions samlidp/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,10 @@ func (s *Server) HandlePutUser(c web.C, w http.ResponseWriter, r *http.Request)
} else {
existingUser := User{}
err := s.Store.Get(fmt.Sprintf("/users/%s", c.URLParams["id"]), &existingUser)
switch {
case err == nil:
switch err {
case nil:
user.HashedPassword = existingUser.HashedPassword
case err == ErrNotFound:
case ErrNotFound:
// nop
default:
s.logger.Printf("ERROR: %s", err)
Expand Down
Loading
Loading