From 926fc007f6d215af552550715f38073a8db1e8f3 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 16 Apr 2025 11:44:22 -0500 Subject: [PATCH 1/4] chore: 'assign_default' value should be loaded from legacy value If this value was set before v2.19.0, then assign_default was in a json field that would not match. And it would default to `false`. This corrects that. --- coderd/idpsync/organization.go | 11 ++++ coderd/idpsync/organizations_test.go | 77 ++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/coderd/idpsync/organization.go b/coderd/idpsync/organization.go index be65daba369df..afadef1bed36b 100644 --- a/coderd/idpsync/organization.go +++ b/coderd/idpsync/organization.go @@ -213,6 +213,17 @@ type OrganizationSyncSettings struct { } func (s *OrganizationSyncSettings) Set(v string) error { + legacyCheck := make(map[string]any) + err := json.Unmarshal([]byte(v), &legacyCheck) + if assign, ok := legacyCheck["AssignDefault"]; err == nil && ok { + // the legacy json was 'AssignDefault' instead of 'assign_default' + // Set the default value from the legacy if it exists. + isBool, ok := assign.(bool) + if ok { + s.AssignDefault = isBool + } + } + return json.Unmarshal([]byte(v), s) } diff --git a/coderd/idpsync/organizations_test.go b/coderd/idpsync/organizations_test.go index 3a00499bdbced..3d24389baef50 100644 --- a/coderd/idpsync/organizations_test.go +++ b/coderd/idpsync/organizations_test.go @@ -19,6 +19,83 @@ import ( "github.com/coder/coder/v2/testutil" ) +func TestFromLegacySettings(t *testing.T) { + t.Run("AssignDefault,True", func(t *testing.T) { + legacy := `{ + "Field":"groups", + "Mapping":{ + "engineering":[ + "10b2bd19-f5ca-4905-919f-bf02e95e3b6a" + ] + }, + "AssignDefault":true +}` + + var settings idpsync.OrganizationSyncSettings + settings.AssignDefault = true + err := settings.Set(legacy) + require.NoError(t, err) + + require.Equal(t, settings.Field, "groups", "field") + require.Equal(t, settings.Mapping, map[string][]uuid.UUID{ + "engineering": { + uuid.MustParse("10b2bd19-f5ca-4905-919f-bf02e95e3b6a"), + }, + }, "mapping") + require.True(t, settings.AssignDefault, "assign default") + }) + + t.Run("AssignDefault,False", func(t *testing.T) { + legacy := `{ + "Field":"groups", + "Mapping":{ + "engineering":[ + "10b2bd19-f5ca-4905-919f-bf02e95e3b6a" + ] + }, + "AssignDefault":false +}` + + var settings idpsync.OrganizationSyncSettings + settings.AssignDefault = true + err := settings.Set(legacy) + require.NoError(t, err) + + require.Equal(t, settings.Field, "groups", "field") + require.Equal(t, settings.Mapping, map[string][]uuid.UUID{ + "engineering": { + uuid.MustParse("10b2bd19-f5ca-4905-919f-bf02e95e3b6a"), + }, + }, "mapping") + require.False(t, settings.AssignDefault, "assign default") + }) + + t.Run("CorrectAssign", func(t *testing.T) { + legacy := `{ + "Field":"groups", + "Mapping":{ + "engineering":[ + "10b2bd19-f5ca-4905-919f-bf02e95e3b6a" + ] + }, + "AssignDefault":false +}` + + var settings idpsync.OrganizationSyncSettings + settings.AssignDefault = true + err := settings.Set(legacy) + require.NoError(t, err) + + require.Equal(t, settings.Field, "groups", "field") + require.Equal(t, settings.Mapping, map[string][]uuid.UUID{ + "engineering": { + uuid.MustParse("10b2bd19-f5ca-4905-919f-bf02e95e3b6a"), + }, + }, "mapping") + require.False(t, settings.AssignDefault, "assign default") + }) +} + func TestParseOrganizationClaims(t *testing.T) { t.Parallel() From b11533b6d7d269ece01db936ca5f9157cae4da49 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 16 Apr 2025 17:05:54 +0000 Subject: [PATCH 2/4] parallelize test --- coderd/idpsync/organizations_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/coderd/idpsync/organizations_test.go b/coderd/idpsync/organizations_test.go index 3d24389baef50..8a395c0ba8afc 100644 --- a/coderd/idpsync/organizations_test.go +++ b/coderd/idpsync/organizations_test.go @@ -20,7 +20,10 @@ import ( ) func TestFromLegacySettings(t *testing.T) { + t.Parallel() + t.Run("AssignDefault,True", func(t *testing.T) { + t.Parallel() legacy := `{ "Field":"groups", "Mapping":{ @@ -46,6 +49,7 @@ func TestFromLegacySettings(t *testing.T) { }) t.Run("AssignDefault,False", func(t *testing.T) { + t.Parallel() legacy := `{ "Field":"groups", "Mapping":{ @@ -71,6 +75,7 @@ func TestFromLegacySettings(t *testing.T) { }) t.Run("CorrectAssign", func(t *testing.T) { + t.Parallel() legacy := `{ "Field":"groups", "Mapping":{ From 5a53f6849bc57dffd994eba3949f4835e47f0200 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 16 Apr 2025 15:41:21 -0500 Subject: [PATCH 3/4] Update coderd/idpsync/organization.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: ケイラ --- coderd/idpsync/organization.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/idpsync/organization.go b/coderd/idpsync/organization.go index afadef1bed36b..e7f5866bbf275 100644 --- a/coderd/idpsync/organization.go +++ b/coderd/idpsync/organization.go @@ -216,7 +216,7 @@ func (s *OrganizationSyncSettings) Set(v string) error { legacyCheck := make(map[string]any) err := json.Unmarshal([]byte(v), &legacyCheck) if assign, ok := legacyCheck["AssignDefault"]; err == nil && ok { - // the legacy json was 'AssignDefault' instead of 'assign_default' + // The legacy JSON key was 'AssignDefault' instead of 'assign_default' // Set the default value from the legacy if it exists. isBool, ok := assign.(bool) if ok { From b7ebb06074dc9eaa658bf81afb36c65644900378 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 16 Apr 2025 15:44:31 -0500 Subject: [PATCH 4/4] fix indent --- coderd/idpsync/organizations_test.go | 46 ++++++++++------------------ 1 file changed, 16 insertions(+), 30 deletions(-) diff --git a/coderd/idpsync/organizations_test.go b/coderd/idpsync/organizations_test.go index 8a395c0ba8afc..c3c0a052f7100 100644 --- a/coderd/idpsync/organizations_test.go +++ b/coderd/idpsync/organizations_test.go @@ -2,6 +2,7 @@ package idpsync_test import ( "database/sql" + "fmt" "testing" "github.com/golang-jwt/jwt/v4" @@ -22,21 +23,24 @@ import ( func TestFromLegacySettings(t *testing.T) { t.Parallel() + legacy := func(assignDefault bool) string { + return fmt.Sprintf(`{ + "Field":"groups", + "Mapping":{ + "engineering":[ + "10b2bd19-f5ca-4905-919f-bf02e95e3b6a" + ] + }, + "AssignDefault":%t + }`, assignDefault) + } + t.Run("AssignDefault,True", func(t *testing.T) { t.Parallel() - legacy := `{ - "Field":"groups", - "Mapping":{ - "engineering":[ - "10b2bd19-f5ca-4905-919f-bf02e95e3b6a" - ] - }, - "AssignDefault":true -}` var settings idpsync.OrganizationSyncSettings settings.AssignDefault = true - err := settings.Set(legacy) + err := settings.Set(legacy(true)) require.NoError(t, err) require.Equal(t, settings.Field, "groups", "field") @@ -50,19 +54,10 @@ func TestFromLegacySettings(t *testing.T) { t.Run("AssignDefault,False", func(t *testing.T) { t.Parallel() - legacy := `{ - "Field":"groups", - "Mapping":{ - "engineering":[ - "10b2bd19-f5ca-4905-919f-bf02e95e3b6a" - ] - }, - "AssignDefault":false -}` var settings idpsync.OrganizationSyncSettings settings.AssignDefault = true - err := settings.Set(legacy) + err := settings.Set(legacy(false)) require.NoError(t, err) require.Equal(t, settings.Field, "groups", "field") @@ -76,19 +71,10 @@ func TestFromLegacySettings(t *testing.T) { t.Run("CorrectAssign", func(t *testing.T) { t.Parallel() - legacy := `{ - "Field":"groups", - "Mapping":{ - "engineering":[ - "10b2bd19-f5ca-4905-919f-bf02e95e3b6a" - ] - }, - "AssignDefault":false -}` var settings idpsync.OrganizationSyncSettings settings.AssignDefault = true - err := settings.Set(legacy) + err := settings.Set(legacy(false)) require.NoError(t, err) require.Equal(t, settings.Field, "groups", "field")