Elgato_dark/fleet
1
0
Fork 0
mirror of https://github.com/fleetdm/fleet synced 2026-05-23 00:49:03 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 106

PR review changes from main IdP department PR (#30418)

Browse source 
Addressing comments on main
[PR](https://github.com/fleetdm/fleet/pull/30375) for #29609.
This commit is contained in:
Lucas Manuel Rodriguez 2025-06-30 12:18:06 -03:00 committed by GitHub
parent 33b1596763
commit 2d5ef59bf3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 140 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

118
ee/server/integrationtest/scim/scim_test.go
View file

@ -975,11 +975,48 @@ func testCreateUser(t *testing.T, s *Suite) {
// Verify externalId is present and correct
assert.Equal(t, "external-system-123456", createResp6["externalId"])
// Test creating a user with department
userWithDepartment := map[string]interface{}{
"schemas": []string{
"urn:ietf:params:scim:schemas:core:2.0:User",
"urn:ietf:params:scim:schemas:extension:enterprise:2.0:User",
},
"userName": "user-with-department@example.com",
"name": map[string]interface{}{
"givenName": "Foo",
"familyName": "Bar",
},
"emails": []map[string]interface{}{
{
"value": "foobar@example.com",
"type": "work",
"primary": true,
},
},
"active": true,
"urn:ietf:params:scim:schemas:extension:enterprise:2.0:User": map[string]interface{}{
"department": "Engineering",
},
}
var createResp7 map[string]interface{}
s.DoJSON(t, "POST", scimPath("/Users"), userWithDepartment, http.StatusCreated, &createResp7)
assert.Equal(t, "user-with-department@example.com", createResp7["userName"])
userID7 := createResp7["id"].(string)
// Verify department is present and correct
m_, ok := createResp7["urn:ietf:params:scim:schemas:extension:enterprise:2.0:User"]
require.True(t, ok)
m, ok := m_.(map[string]interface{})
require.True(t, ok)
assert.Equal(t, "Engineering", m["department"])
// Make sure these users can be deleted.
s.Do(t, "DELETE", scimPath("/Users/"+userID3), nil, http.StatusNoContent)
s.Do(t, "DELETE", scimPath("/Users/"+userID4), nil, http.StatusNoContent)
s.Do(t, "DELETE", scimPath("/Users/"+userID5), nil, http.StatusNoContent)
s.Do(t, "DELETE", scimPath("/Users/"+userID6), nil, http.StatusNoContent)
s.Do(t, "DELETE", scimPath("/Users/"+userID7), nil, http.StatusNoContent)
}
func testUpdateUser(t *testing.T, s *Suite) {
@ -1082,6 +1119,26 @@ func testUpdateUser(t *testing.T, s *Suite) {
// Verify the update was successful.
assert.Equal(t, "FiRsT-uSeR@eXaMpLe.CoM", updateResp2["userName"])
// Test 5: Try to update first user's department (should succeed)
schemas_, ok := updatePayload["schemas"]
require.True(t, ok)
schemas, ok := schemas_.([]string)
require.True(t, ok)
updatePayload["schemas"] = append(schemas, "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User")
updatePayload["urn:ietf:params:scim:schemas:extension:enterprise:2.0:User"] = map[string]interface{}{
"department": "Engineering",
}
var updateResp3 map[string]interface{}
s.DoJSON(t, "PUT", scimPath("/Users/"+firstUserID), updatePayload, http.StatusOK, &updateResp3)
// Verify the update was successful.
m_, ok := updateResp3["urn:ietf:params:scim:schemas:extension:enterprise:2.0:User"]
require.True(t, ok)
m, ok := m_.(map[string]interface{})
require.True(t, ok)
assert.Equal(t, "Engineering", m["department"])
// Delete the users we created.
s.Do(t, "DELETE", scimPath("/Users/"+firstUserID), nil, http.StatusNoContent)
s.Do(t, "DELETE", scimPath("/Users/"+secondUserID), nil, http.StatusNoContent)
@ -2995,6 +3052,67 @@ func testPatchUserAttributes(t *testing.T, s *Suite) {
assert.Equal(t, "Updates", name["familyName"], "familyName should be updated")
})
t.Run("Patch department", func(t *testing.T) {
patchDepartmentPayload := map[string]interface{}{
"schemas": []string{"urn:ietf:params:scim:api:messages:2.0:PatchOp"},
"Operations": []map[string]interface{}{
{
"op": "replace",
"value": map[string]interface{}{
"urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:department": "QA",
},
},
},
}
var patchDepartmentResp map[string]interface{}
s.DoJSON(t, "PATCH", scimPath("/Users/"+userID), patchDepartmentPayload, http.StatusOK, &patchDepartmentResp)
// Verify department is present and correct
m_, ok := patchDepartmentResp["urn:ietf:params:scim:schemas:extension:enterprise:2.0:User"]
require.True(t, ok)
m, ok := m_.(map[string]interface{})
require.True(t, ok)
assert.Equal(t, "QA", m["department"])
// Now remove department using path.
patchDepartmentPayload2 := map[string]interface{}{
"schemas": []string{"urn:ietf:params:scim:api:messages:2.0:PatchOp"},
"Operations": []map[string]interface{}{
{
"op": "remove",
"path": "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:department",
},
},
}
var patchDepartmentResp2 map[string]interface{}
s.DoJSON(t, "PATCH", scimPath("/Users/"+userID), patchDepartmentPayload2, http.StatusOK, &patchDepartmentResp2)
// Verify department is not present (if there are no extension attributes, then the whole map is not there).
_, ok = patchDepartmentResp2["urn:ietf:params:scim:schemas:extension:enterprise:2.0:User"]
require.False(t, ok)
// Now re-add department using path.
patchDepartmentPayload3 := map[string]interface{}{
"schemas": []string{"urn:ietf:params:scim:api:messages:2.0:PatchOp"},
"Operations": []map[string]interface{}{
{
"op": "replace",
"path": "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:department",
"value": "Engineering",
},
},
}
var patchDepartmentResp3 map[string]interface{}
s.DoJSON(t, "PATCH", scimPath("/Users/"+userID), patchDepartmentPayload3, http.StatusOK, &patchDepartmentResp3)
// Verify department is present and correct
m_, ok = patchDepartmentResp3["urn:ietf:params:scim:schemas:extension:enterprise:2.0:User"]
require.True(t, ok)
m, ok = m_.(map[string]interface{})
require.True(t, ok)
assert.Equal(t, "Engineering", m["department"])
})
// ///////////////////////////////////////////////
// Tests for patching with explicit operation path

3
ee/server/integrationtest/scim/suite.go
View file

@ -19,14 +19,13 @@ func SetUpSuite(t *testing.T, uniqueTestName string) *Suite {
license := &fleet.LicenseInfo{
Tier: fleet.TierPremium,
}
ds, redisPool, fleetCfg, fleetSvc, ctx := integrationtest.SetUpMySQLAndRedisAndService(t, uniqueTestName, &service.TestServerOpts{
ds, fleetCfg, fleetSvc, ctx := integrationtest.SetUpMySQLAndService(t, uniqueTestName, &service.TestServerOpts{
License: license,
})
logger := log.NewLogfmtLogger(os.Stdout)
users, server := service.RunServerForTestsWithServiceWithDS(t, ctx, ds, fleetSvc, &service.TestServerOpts{
License: license,
FleetConfig: &fleetCfg,
Pool: redisPool,
Logger: logger,
EnableSCIM: true,
})

5
ee/server/scim/users.go
View file

@ -574,6 +574,11 @@ func (u *UserHandler) Patch(r *http.Request, id string, operations []scim.PatchO
if err != nil {
return scim.Resource{}, err
}
case op.Path.AttributePath.String() == extensionEnterpriseUserAttributes+":"+departmentAttr:
err = u.patchDepartment(op.Op, op.Value, user)
if err != nil {
return scim.Resource{}, err
}
default:
level.Info(u.logger).Log("msg", "unsupported patch path", "path", op.Path)
return scim.Resource{}, errors.ScimErrorBadParams([]string{fmt.Sprintf("%v", op)})

10
server/datastore/mysql/scim.go
View file

@ -12,6 +12,7 @@ import (
"github.com/fleetdm/fleet/v4/server/fleet"
"github.com/go-kit/log"
"github.com/go-kit/log/level"
"github.com/google/go-cmp/cmp"
"github.com/jmoiron/sqlx"
)
@ -314,14 +315,9 @@ func (ds *Datastore) ReplaceScimUser(ctx context.Context, user *fleet.ScimUser)
if rowsAffected == 0 {
return notFound("scim user").WithID(user.ID)
}
usernameChanged := old.UserName != user.UserName
derefStrPtr := func(s *string) string {
if s == nil {
return ""
}
return *s
}
departmentChanged := derefStrPtr(old.Department) != derefStrPtr(user.Department)
departmentChanged := !cmp.Equal(old.Department, user.Department)
// Only update emails if they've changed
if emailsNeedUpdate {

17
server/service/integrationtest/suite.go
View file

@ -48,9 +48,11 @@ func SetUpServerURL(t *testing.T, ds *mysql.Datastore, server *httptest.Server)
require.NoError(t, err)
}
func SetUpMySQLAndRedisAndService(t *testing.T, uniqueTestName string, opts ...*service.TestServerOpts) (*mysql.Datastore, fleet.RedisPool,
func SetUpMySQLAndService(t *testing.T, uniqueTestName string, opts ...*service.TestServerOpts) (
*mysql.Datastore,
config.FleetConfig,
fleet.Service, context.Context) {
fleet.Service, context.Context,
) {
ds := mysql.CreateMySQLDS(t)
test.AddAllHostsLabel(t, ds)
@ -62,10 +64,17 @@ func SetUpMySQLAndRedisAndService(t *testing.T, uniqueTestName string, opts ...*
err = ds.SaveAppConfig(testContext(), appConf)
require.NoError(t, err)
redisPool := redistest.SetupRedis(t, uniqueTestName, false, false, false)
fleetCfg := config.TestConfig()
fleetSvc, ctx := service.NewTestService(t, ds, fleetCfg, opts...)
return ds, fleetCfg, fleetSvc, ctx
}
func SetUpMySQLAndRedisAndService(t *testing.T, uniqueTestName string, opts ...*service.TestServerOpts) (*mysql.Datastore, fleet.RedisPool,
config.FleetConfig,
fleet.Service, context.Context,
) {
redisPool := redistest.SetupRedis(t, uniqueTestName, false, false, false)
ds, fleetCfg, fleetSvc, ctx := SetUpMySQLAndService(t, uniqueTestName, opts...)
return ds, redisPool, fleetCfg, fleetSvc, ctx
}