From 360cb73da35fab34d6bc306b43e635d91e1a2ca7 Mon Sep 17 00:00:00 2001 From: Charlie Voiselle <464492+angrycub@users.noreply.github.com> Date: Fri, 13 Jun 2025 15:30:01 -0400 Subject: [PATCH 1/3] defensively handle nil maps and slices in marshaling --- coderd/idpsync/group.go | 11 +++++++++++ coderd/idpsync/organization.go | 11 +++++++++++ coderd/idpsync/role.go | 11 +++++++++++ enterprise/coderd/idpsync.go | 3 +++ 4 files changed, 36 insertions(+) diff --git a/coderd/idpsync/group.go b/coderd/idpsync/group.go index b85ce1b749e28..b5d8003165665 100644 --- a/coderd/idpsync/group.go +++ b/coderd/idpsync/group.go @@ -274,6 +274,17 @@ func (s *GroupSyncSettings) String() string { return runtimeconfig.JSONString(s) } +func (s *GroupSyncSettings) MarshalJSON() ([]byte, error) { + if s.Mapping == nil { + s.Mapping = make(map[string][]uuid.UUID) + } + + // Aliasing the struct to avoid infinite recursion when calling json.Marshal + // on the struct itself. + type Alias GroupSyncSettings + return json.Marshal(&struct{ *Alias }{Alias: (*Alias)(s)}) +} + type ExpectedGroup struct { OrganizationID uuid.UUID GroupID *uuid.UUID diff --git a/coderd/idpsync/organization.go b/coderd/idpsync/organization.go index f0736e1ea7559..cfc6e819d7ae5 100644 --- a/coderd/idpsync/organization.go +++ b/coderd/idpsync/organization.go @@ -234,6 +234,17 @@ func (s *OrganizationSyncSettings) String() string { return runtimeconfig.JSONString(s) } +func (s *OrganizationSyncSettings) MarshalJSON() ([]byte, error) { + if s.Mapping == nil { + s.Mapping = make(map[string][]uuid.UUID) + } + + // Aliasing the struct to avoid infinite recursion when calling json.Marshal + // on the struct itself. + type Alias OrganizationSyncSettings + return json.Marshal(&struct{ *Alias }{Alias: (*Alias)(s)}) +} + // ParseClaims will parse the claims and return the list of organizations the user // should sync to. func (s *OrganizationSyncSettings) ParseClaims(ctx context.Context, db database.Store, mergedClaims jwt.MapClaims) ([]uuid.UUID, error) { diff --git a/coderd/idpsync/role.go b/coderd/idpsync/role.go index c21e7c99c4614..b6f555dc1e1e8 100644 --- a/coderd/idpsync/role.go +++ b/coderd/idpsync/role.go @@ -291,3 +291,14 @@ func (s *RoleSyncSettings) String() string { } return runtimeconfig.JSONString(s) } + +func (s *RoleSyncSettings) MarshalJSON() ([]byte, error) { + if s.Mapping == nil { + s.Mapping = make(map[string][]string) + } + + // Aliasing the struct to avoid infinite recursion when calling json.Marshal + // on the struct itself. + type Alias RoleSyncSettings + return json.Marshal(&struct{ *Alias }{Alias: (*Alias)(s)}) +} diff --git a/enterprise/coderd/idpsync.go b/enterprise/coderd/idpsync.go index 2dcee572eb692..416acc7ee070f 100644 --- a/enterprise/coderd/idpsync.go +++ b/enterprise/coderd/idpsync.go @@ -836,6 +836,9 @@ func (api *API) idpSyncClaimFieldValues(orgID uuid.UUID, rw http.ResponseWriter, httpapi.InternalServerError(rw, err) return } + if fieldValues == nil { + fieldValues = []string{} + } httpapi.Write(ctx, rw, http.StatusOK, fieldValues) } From e126f78a74d7898980ed2ea5851d2bec45bcc550 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 17 Jun 2025 16:05:51 -0500 Subject: [PATCH 2/3] add unit test for marshal json --- coderd/idpsync/idpsync_test.go | 45 ++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/coderd/idpsync/idpsync_test.go b/coderd/idpsync/idpsync_test.go index 317122ddc6092..0f7f75db00bce 100644 --- a/coderd/idpsync/idpsync_test.go +++ b/coderd/idpsync/idpsync_test.go @@ -2,6 +2,7 @@ package idpsync_test import ( "encoding/json" + "regexp" "testing" "github.com/stretchr/testify/require" @@ -9,6 +10,50 @@ import ( "github.com/coder/coder/v2/coderd/idpsync" ) +// TestMarshalJSONEmpty ensures no empty maps are marshaled as `null` in JSON. +func TestMarshalJSONEmpty(t *testing.T) { + t.Parallel() + + t.Run("Group", func(t *testing.T) { + t.Parallel() + + output, err := json.Marshal(&idpsync.GroupSyncSettings{ + RegexFilter: regexp.MustCompile(".*"), + }) + require.NoError(t, err, "marshal empty group settings") + require.NotContains(t, string(output), "null") + + require.JSONEq(t, + `{"field":"","mapping":{},"regex_filter":".*","auto_create_missing_groups":false}`, + string(output)) + }) + + t.Run("Role", func(t *testing.T) { + t.Parallel() + + output, err := json.Marshal(&idpsync.RoleSyncSettings{}) + require.NoError(t, err, "marshal empty group settings") + require.NotContains(t, string(output), "null") + + require.JSONEq(t, + `{"field":"","mapping":{}}`, + string(output)) + }) + + t.Run("Organization", func(t *testing.T) { + t.Parallel() + + output, err := json.Marshal(&idpsync.OrganizationSyncSettings{}) + require.NoError(t, err, "marshal empty group settings") + require.NotContains(t, string(output), "null") + + require.JSONEq(t, + `{"field":"","mapping":{},"assign_default":false}`, + string(output)) + }) + +} + func TestParseStringSliceClaim(t *testing.T) { t.Parallel() From a10a96cdbae500787abb2bf5ee5a237da2c9444e Mon Sep 17 00:00:00 2001 From: Charlie Voiselle <464492+angrycub@users.noreply.github.com> Date: Tue, 17 Jun 2025 17:25:47 -0400 Subject: [PATCH 3/3] fix for linter --- coderd/idpsync/idpsync_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/coderd/idpsync/idpsync_test.go b/coderd/idpsync/idpsync_test.go index 0f7f75db00bce..0db5c66bed174 100644 --- a/coderd/idpsync/idpsync_test.go +++ b/coderd/idpsync/idpsync_test.go @@ -51,7 +51,6 @@ func TestMarshalJSONEmpty(t *testing.T) { `{"field":"","mapping":{},"assign_default":false}`, string(output)) }) - } func TestParseStringSliceClaim(t *testing.T) {