update for most recent three go versions (#537)

Author: crewjam

.github/workflows/lint.yml

@@ -8,11 +8,10 @@ on:
 
 jobs:
   golangci:
-    name: Run golangci-lint
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@v2
+        uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.52.2
+          version: v1.54.2

.github/workflows/test.yml

@@ -7,20 +7,13 @@ on:
     branches: [ 'main' ]
 jobs:
   tests:
-    name: Run tests
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        go: [ '1.17.x', '1.18.x', '1.19.x']
+        go: [ '1.19.x', '1.20.x', '1.21.x']
     steps:
-      - name: Check out code into the Go module directory
-        uses: actions/checkout@v2
-      - name: Set up Go ${{ matrix.go }}
-        uses: actions/setup-go@v2
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v4
         with:
           go-version: ${{ matrix.go }}
-      - name: Go version
-        run: go version
-      - name: Run Go tests
-        run: |
-          go test -v ./...
+      - run: go test -v ./...

.golangci.yml

@@ -8,7 +8,6 @@
 linters:
   enable:
     - 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]
     - 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]
@@ -36,6 +35,7 @@ linters:
     - 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

go.mod

@@ -1,6 +1,6 @@
 module github.com/crewjam/saml
 
-go 1.16
+go 1.19
 
 require (
 	github.com/beevik/etree v1.1.0
@@ -10,10 +10,19 @@ require (
 	github.com/google/go-cmp v0.5.9
 	github.com/kr/pretty v0.3.1
 	github.com/mattermost/xml-roundtrip-validator v0.1.0
-	github.com/pkg/errors v0.9.1 // indirect
 	github.com/russellhaering/goxmldsig v1.3.0
 	github.com/stretchr/testify v1.8.1
 	github.com/zenazn/goji v1.0.1
 	golang.org/x/crypto v0.0.0-20220128200615-198e4374d7ed
 	gotest.tools v2.2.0+incompatible
 )
+
+require (
+	github.com/davecgh/go-spew v1.1.1 // indirect
+	github.com/jonboulle/clockwork v0.2.2 // indirect
+	github.com/kr/text v0.2.0 // indirect
+	github.com/pkg/errors v0.9.1 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
+	github.com/rogpeppe/go-internal v1.9.0 // indirect
+	gopkg.in/yaml.v3 v3.0.1 // indirect
+)

go.sum

@@ -50,13 +50,6 @@ github.com/zenazn/goji v1.0.1 h1:4lbD8Mx2h7IvloP7r2C0D6ltZP6Ufip8Hn0wmSK5LR8=
 github.com/zenazn/goji v1.0.1/go.mod h1:7S9M489iMyHBNxwZnk9/EHS098H4/F6TATF2mIxtB1Q=
 golang.org/x/crypto v0.0.0-20220128200615-198e4374d7ed h1:YoWVYYAfvQ4ddHv3OKmIvX7NCAhFGTj62VP2l2kfBbA=
 golang.org/x/crypto v0.0.0-20220128200615-198e4374d7ed/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
-golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
-golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
-golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
-golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=

identity_provider.go

@@ -9,7 +9,6 @@ import (
 	"encoding/xml"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"net/http"
 	"net/url"
 	"os"
@@ -366,7 +365,7 @@ func NewIdpAuthnRequest(idp *IdentityProvider, r *http.Request) (*IdpAuthnReques
 		if err != nil {
 			return nil, fmt.Errorf("cannot decode request: %s", err)
 		}
-		req.RequestBuffer, err = ioutil.ReadAll(newSaferFlateReader(bytes.NewReader(compressedRequest)))
+		req.RequestBuffer, err = io.ReadAll(newSaferFlateReader(bytes.NewReader(compressedRequest)))
 		if err != nil {
 			return nil, fmt.Errorf("cannot decompress request: %s", err)
 		}

identity_provider_go116_test.go

@@ -10,6 +10,7 @@ import (
 	"encoding/pem"
 	"encoding/xml"
 	"fmt"
+	"io"
 	"math/rand"
 	"net/http"
 	"net/http/httptest"
@@ -1088,3 +1089,44 @@ func TestIDPRejectDecompressionBomb(t *testing.T) {
 	_, err = NewIdpAuthnRequest(&test.IDP, r)
 	assert.Error(t, err, "cannot decompress request: flate: uncompress limit exceeded (10485760 bytes)")
 }
+
+func TestIDPHTTPCanHandleSSORequest(t *testing.T) {
+	test := NewIdentityProviderTest(t, applyKey)
+	w := httptest.NewRecorder()
+
+	const validRequest = `lJJBayoxFIX%2FypC9JhnU5wszAz7lgWCLaNtFd5fMbQ1MkmnunVb%2FfUfbUqEgdhs%2BTr5zkmLW8S5s8KVD4mzvm0Cl6FIwEciRCeCRDFuznd2sTD5Upk2Ro42NyGZEmNjFMI%2BBOo9pi%2BnVWbzfrEqxY27JSEntEPfg2waHNnpJ4JtcgiWRLfoLXYBjwDfu6p%2B8JIoiWy5K4eqBUipXIzVRUwXKKtRK53qkJ3qqQVuNPUjU4TIQQ%2BBS5EqPBzofKH2ntBn%2FMervo8jWnyX%2BuVC78FwKkT1gopNKX1JUxSklXTMIfM0gsv8xeeDL%2BPGk7%2FF0Qg0GdnwQ1cW5PDLUwFDID6uquO1Dlot1bJw9%2FPLRmia%2BzRMCYyk4dSiq6205QSDXOxfy3KAq5Pkvqt4DAAD%2F%2Fw%3D%3D`
+
+	r, _ := http.NewRequest("GET", "https://idp.example.com/saml/sso?RelayState=ThisIsTheRelayState&"+
+		"SAMLRequest="+validRequest, nil)
+	test.IDP.Handler().ServeHTTP(w, r)
+	assert.Check(t, is.Equal(http.StatusOK, w.Code))
+
+	// rejects requests that are invalid
+	w = httptest.NewRecorder()
+	r, _ = http.NewRequest("GET", "https://idp.example.com/saml/sso?RelayState=ThisIsTheRelayState&"+
+		"SAMLRequest=PEF1dGhuUmVxdWVzdA%3D%3D", nil)
+	test.IDP.Handler().ServeHTTP(w, r)
+	assert.Check(t, is.Equal(http.StatusBadRequest, w.Code))
+
+	// rejects requests that contain malformed XML
+	{
+		a, _ := url.QueryUnescape(validRequest)
+		b, _ := base64.StdEncoding.DecodeString(a)
+		c, _ := io.ReadAll(flate.NewReader(bytes.NewReader(b)))
+		d := bytes.Replace(c, []byte("<AuthnRequest"), []byte("<AuthnRequest ::foo=\"bar\">]]"), 1)
+		f := bytes.Buffer{}
+		e, _ := flate.NewWriter(&f, flate.DefaultCompression)
+		_, 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)
+
+		w = httptest.NewRecorder()
+		r, _ = http.NewRequest("GET", "https://idp.example.com/saml/sso?RelayState=ThisIsTheRelayState&"+
+			"SAMLRequest="+invalidRequest, nil)
+		test.IDP.Handler().ServeHTTP(w, r)
+		assert.Check(t, is.Equal(http.StatusBadRequest, w.Code))
+	}
+}

identity_provider_go117_test.go

@@ -5,7 +5,6 @@ import (
 	"encoding/xml"
 	"errors"
 	"io"
-	"io/ioutil"
 
 	xrv "github.com/mattermost/xml-roundtrip-validator"
 
@@ -22,7 +21,7 @@ func randomBytes(n int) []byte {
 
 func getSPMetadata(r io.Reader) (spMetadata *saml.EntityDescriptor, err error) {
 	var data []byte
-	if data, err = ioutil.ReadAll(r); err != nil {
+	if data, err = io.ReadAll(r); err != nil {
 		return nil, err
 	}
 

identity_provider_test.go

@@ -8,12 +8,11 @@ import (
 	"testing"
 
 	"gotest.tools/assert"
-	is "gotest.tools/assert/cmp"
 )
 
 func TestGetSPMetadata(t *testing.T) {
 	good := "" +
-		"<EntityDescriptor xmlns=\"urn:oasis:names:tc:SAML:2.0:metadata\" ::attr=\"foo\" validUntil=\"2013-03-10T00:32:19.104Z\" cacheDuration=\"PT1H\" entityID=\"http://localhost:5000/e087a985171710fb9fb30f30f41384f9/saml2/metadata/\">\n" +
+		"<EntityDescriptor xmlns=\"urn:oasis:names:tc:SAML:2.0:metadata\" validUntil=\"2013-03-10T00:32:19.104Z\" cacheDuration=\"PT1H\" entityID=\"http://localhost:5000/e087a985171710fb9fb30f30f41384f9/saml2/metadata/\">\n" +
 		"</EntityDescriptor>"
 	_, err := getSPMetadata(strings.NewReader(good))
 	assert.Check(t, err)
@@ -22,5 +21,5 @@ func TestGetSPMetadata(t *testing.T) {
 		"<EntityDescriptor xmlns=\"urn:oasis:names:tc:SAML:2.0:metadata\" ::attr=\"foo\" validUntil=\"2013-03-10T00:32:19.104Z\" cacheDuration=\"PT1H\" entityID=\"http://localhost:5000/e087a985171710fb9fb30f30f41384f9/saml2/metadata/\">]]>\n" +
 		"</EntityDescriptor>"
 	_, err = getSPMetadata(strings.NewReader(bad))
-	assert.Check(t, is.Error(err, "XML syntax error on line 1: unescaped ]]> not in CDATA section"))
+	assert.Check(t, err != nil)
 }

samlidp/util.go

@@ -5,7 +5,7 @@ import (
 	"context"
 	"encoding/xml"
 	"errors"
-	"io/ioutil"
+	"io"
 	"net/http"
 	"net/url"
 
@@ -72,7 +72,7 @@ func FetchMetadata(ctx context.Context, httpClient *http.Client, metadataURL url
 		return nil, httperr.Response(*resp)
 	}
 
-	data, err := ioutil.ReadAll(resp.Body)
+	data, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return nil, err
 	}