Merge pull request #160 from uselagoon/safer-empty-groups

Treat empty groups response from Keycloak as an error

go.mod

@@ -3,7 +3,7 @@ module github.com/uselagoon/lagoon-opensearch-sync
 go 1.22.1
 
 require (
-	github.com/alecthomas/assert v1.0.0
+	github.com/alecthomas/assert/v2 v2.11.0
 	github.com/alecthomas/kong v1.6.1
 	github.com/coreos/go-oidc/v3 v3.12.0
 	github.com/davecgh/go-spew v1.1.1
@@ -15,12 +15,9 @@ require (
 
 require (
 	filippo.io/edwards25519 v1.1.0 // indirect
-	github.com/alecthomas/colour v0.1.0 // indirect
 	github.com/alecthomas/repr v0.4.0 // indirect
 	github.com/go-jose/go-jose/v4 v4.0.2 // indirect
-	github.com/mattn/go-isatty v0.0.14 // indirect
-	github.com/sergi/go-diff v1.2.0 // indirect
+	github.com/hexops/gotextdiff v1.0.3 // indirect
 	go.uber.org/multierr v1.10.0 // indirect
 	golang.org/x/crypto v0.31.0 // indirect
-	golang.org/x/sys v0.28.0 // indirect
 )

go.sum

@@ -1,18 +1,13 @@
 filippo.io/edwards25519 v1.1.0 h1:FNf4tywRC1HmFuKW5xopWpigGjJKiJSV0Cqo0cJWDaA=
 filippo.io/edwards25519 v1.1.0/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4=
-github.com/alecthomas/assert v1.0.0 h1:3XmGh/PSuLzDbK3W2gUbRXwgW5lqPkuqvRgeQ30FI5o=
-github.com/alecthomas/assert v1.0.0/go.mod h1:va/d2JC+M7F6s+80kl/R3G7FUiW6JzUO+hPhLyJ36ZY=
 github.com/alecthomas/assert/v2 v2.11.0 h1:2Q9r3ki8+JYXvGsDyBXwH3LcJ+WK5D0gc5E8vS6K3D0=
 github.com/alecthomas/assert/v2 v2.11.0/go.mod h1:Bze95FyfUr7x34QZrjL+XP+0qgp/zg8yS+TtBj1WA3k=
-github.com/alecthomas/colour v0.1.0 h1:nOE9rJm6dsZ66RGWYSFrXw461ZIt9A6+nHgL7FRrDUk=
-github.com/alecthomas/colour v0.1.0/go.mod h1:QO9JBoKquHd+jz9nshCh40fOfO+JzsoXy8qTHF68zU0=
 github.com/alecthomas/kong v1.6.1 h1:/7bVimARU3uxPD0hbryPE8qWrS3Oz3kPQoxA/H2NKG8=
 github.com/alecthomas/kong v1.6.1/go.mod h1:p2vqieVMeTAnaC83txKtXe8FLke2X07aruPWXyMPQrU=
 github.com/alecthomas/repr v0.4.0 h1:GhI2A8MACjfegCPVq9f1FLvIBS+DrQ2KQBFZP1iFzXc=
 github.com/alecthomas/repr v0.4.0/go.mod h1:Fr0507jx4eOXV7AlPV6AVZLYrLIuIeSOWtW57eE/O/4=
 github.com/coreos/go-oidc/v3 v3.12.0 h1:sJk+8G2qq94rDI6ehZ71Bol3oUHy63qNYmkiSjrc/Jo=
 github.com/coreos/go-oidc/v3 v3.12.0/go.mod h1:gE3LgjOgFoHi9a4ce4/tJczr0Ai2/BoDhf0r5lltWI0=
-github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/go-jose/go-jose/v4 v4.0.2 h1:R3l3kkBds16bO7ZFAEEcofK0MkrAJt3jlJznWZG0nvk=
@@ -25,21 +20,12 @@ github.com/hexops/gotextdiff v1.0.3 h1:gitA9+qJrrTCsiCl7+kh75nPqQt1cx4ZkudSTLoUq
 github.com/hexops/gotextdiff v1.0.3/go.mod h1:pSWU5MAI3yDq+fZBTazCSJysOMbxWL1BSow5/V2vxeg=
 github.com/jmoiron/sqlx v1.4.0 h1:1PLqN7S1UYp5t4SrVVnt4nUVNemrDAtxlulVe+Qgm3o=
 github.com/jmoiron/sqlx v1.4.0/go.mod h1:ZrZ7UsYB/weZdl2Bxg6jCRO9c3YHl8r3ahlKmRT4JLY=
-github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
-github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
-github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/lib/pq v1.10.9 h1:YXG7RB+JIjhP29X+OtkiDnYaXQwpS4JEWq7dtCCRUEw=
 github.com/lib/pq v1.10.9/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
-github.com/mattn/go-isatty v0.0.14 h1:yVuAays6BHfxijgZPzw+3Zlu5yQgKGP2/hcQbHb7S9Y=
-github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94=
 github.com/mattn/go-sqlite3 v1.14.22 h1:2gZY6PC6kBnID23Tichd1K+Z0oS6nE/XwU+Vz/5o4kU=
 github.com/mattn/go-sqlite3 v1.14.22/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxUwlHK0RXw+Y=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
-github.com/sergi/go-diff v1.2.0 h1:XU+rvMAioB0UC3q1MFrIQy4Vo5/4VsRDQQXHsEya6xQ=
-github.com/sergi/go-diff v1.2.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM=
-github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
-github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
 github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8=
 github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
 go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=
@@ -52,12 +38,5 @@ golang.org/x/crypto v0.31.0 h1:ihbySMvVjLAeSH1IbfcRTkD/iNscyz8rGzjF/E5hV6U=
 golang.org/x/crypto v0.31.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk=
 golang.org/x/oauth2 v0.25.0 h1:CY4y7XT9v0cRI9oupztF8AgiIu99L/ksR/Xp/6jrZ70=
 golang.org/x/oauth2 v0.25.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbhtI=
-golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA=
-golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
-gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
-gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
-gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
-gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

internal/keycloak/groups.go

@@ -3,6 +3,7 @@ package keycloak
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -53,5 +54,13 @@ func (c *Client) Groups(ctx context.Context) ([]Group, error) {
 		return nil, fmt.Errorf("couldn't get groups from Keycloak API: %v", err)
 	}
 	var groups []Group
+	if err = json.Unmarshal(data, &groups); err != nil {
+		return nil, fmt.Errorf("couldn't unmarshal groups from Keycloak API: %v", err)
+	}
+	if len(groups) == 0 {
+		// https://github.com/uselagoon/lagoon-opensearch-sync/issues/150
+		return nil,
+			errors.New("empty groups response from Keycloak. Permissions issue?")
+	}
 	return groups, json.Unmarshal(data, &groups)
 }

internal/keycloak/groups_test.go

@@ -1,18 +1,64 @@
 package keycloak_test
 
 import (
-	"encoding/json"
+	"bytes"
+	"context"
+	"io"
+	"net/http"
+	"net/http/httptest"
 	"os"
-	"reflect"
 	"testing"
 
+	"github.com/alecthomas/assert/v2"
 	"github.com/uselagoon/lagoon-opensearch-sync/internal/keycloak"
 )
 
-func TestGroupsUnmarshal(t *testing.T) {
+// newTestGroupsServer sets up a mock keycloak which responds with
+// appropriate group JSON data to exercise Groups.
+func newTestGroupsServer(tt *testing.T, testDataPath string) *httptest.Server {
+	// load the discovery JSON first, because the mux closure needs to
+	// reference its buffer
+	discoveryBuf, err := os.ReadFile("testdata/realm.oidc.discovery.json")
+	if err != nil {
+		tt.Fatal(err)
+		return nil
+	}
+	// configure router with the URLs that OIDC discovery and JWKS require
+	mux := http.NewServeMux()
+	mux.HandleFunc("/auth/realms/lagoon/.well-known/openid-configuration",
+		func(w http.ResponseWriter, r *http.Request) {
+			d := bytes.NewBuffer(discoveryBuf)
+			_, err = io.Copy(w, d)
+			if err != nil {
+				tt.Fatal(err)
+			}
+		})
+	// configure the "all groups" path
+	mux.HandleFunc("/auth/admin/realms/lagoon/groups",
+		func(w http.ResponseWriter, r *http.Request) {
+			f, err := os.Open(testDataPath)
+			if err != nil {
+				tt.Fatal(err)
+				return
+			}
+			_, err = io.Copy(w, f)
+			if err != nil {
+				tt.Fatal(err)
+			}
+		})
+	ts := httptest.NewServer(mux)
+	// now replace the example URL in the discovery JSON with the actual
+	// httptest server URL
+	discoveryBuf = bytes.ReplaceAll(discoveryBuf,
+		[]byte("https://keycloak.example.com"), []byte(ts.URL))
+	return ts
+}
+
+func TestGroups(t *testing.T) {
 	var testCases = map[string]struct {
-		input  string
-		expect []keycloak.Group
+		input       string
+		expect      []keycloak.Group
+		expectError bool
 	}{
 		"unmarshal groups": {
 			input: "testdata/groups.json",
@@ -110,20 +156,36 @@ func TestGroupsUnmarshal(t *testing.T) {
 				},
 			},
 		},
+		// https://github.com/uselagoon/lagoon-opensearch-sync/issues/150
+		"empty groups error response": {
+			input:       "testdata/groups.empty.json",
+			expectError: true,
+		},
 	}
 	for name, tc := range testCases {
 		t.Run(name, func(tt *testing.T) {
-			jb, err := os.ReadFile(tc.input)
+			ts := newTestGroupsServer(tt, tc.input)
+			defer ts.Close()
+			ctx := context.Background()
+			k, err := keycloak.NewClientCredentialsClient(
+				ctx,
+				ts.URL,
+				"test-client-id",
+				"test-client-secret",
+			)
 			if err != nil {
 				tt.Fatal(err)
 			}
-			var groups []keycloak.Group
-			if err = json.Unmarshal(jb, &groups); err != nil {
-				tt.Fatal(err)
-			}
-			if !reflect.DeepEqual(tc.expect, groups) {
-				tt.Fatalf("expected %v, got %v", tc.expect, groups)
+			// override internal client credentials HTTP client for testing
+			k.UseDefaultHTTPClient()
+			// execute test
+			groups, err := k.Groups(ctx)
+			if tc.expectError {
+				assert.Error(tt, err, name)
+			} else {
+				assert.NoError(tt, err, name)
 			}
+			assert.Equal(tt, tc.expect, groups, name)
 		})
 	}
 }

internal/keycloak/helper_test.go

@@ -0,0 +1,9 @@
+package keycloak
+
+import "net/http"
+
+// UseDefaultHTTPClient uses the default http client to avoid token refresh in
+// tests.
+func (c *Client) UseDefaultHTTPClient() {
+	c.httpClient = http.DefaultClient
+}

internal/keycloak/testdata/groups.empty.json

@@ -0,0 +1 @@
+[]