feat: remove users from deleted organizations in idp org sync

Emyrk · Emyrk · commit c1cc257bdde6 · 2025-04-15T15:54:04.000-05:00
Idp sync should exclude deleted organizations and auto remove
members. They should not be members in the first place.
diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go
@@ -4172,7 +4172,11 @@ func (q *FakeQuerier) GetOrganizationsByUserID(_ context.Context, arg database.G
 			continue
 		}
 		for _, organization := range q.organizations {
-			if organization.ID != organizationMember.OrganizationID || organization.Deleted != arg.Deleted {
+			if organization.ID != organizationMember.OrganizationID {
+				continue
+			}
+
+			if arg.Deleted.Valid && organization.Deleted != arg.Deleted.Bool {
 				continue
 			}
 			organizations = append(organizations, organization)
diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/organizations.sql b/coderd/database/queries/organizations.sql
@@ -55,8 +55,13 @@ SELECT
 FROM
     organizations
 WHERE
-    -- Optionally include deleted organizations
-    deleted = @deleted AND
+    -- Optionally provide a filter for deleted organizations.
+  	CASE WHEN
+  	    sqlc.narg('deleted') :: boolean IS NULL THEN
+			true
+		ELSE
+			deleted = sqlc.narg('deleted')
+	END AND
     id = ANY(
         SELECT
             organization_id
diff --git a/coderd/idpsync/organization.go b/coderd/idpsync/organization.go
@@ -92,14 +92,16 @@ func (s AGPLIDPSync) SyncOrganizations(ctx context.Context, tx database.Store, u
 		return nil // No sync configured, nothing to do
 	}
 
-	expectedOrgs, err := orgSettings.ParseClaims(ctx, tx, params.MergedClaims)
+	expectedOrgIDs, err := orgSettings.ParseClaims(ctx, tx, params.MergedClaims)
 	if err != nil {
 		return xerrors.Errorf("organization claims: %w", err)
 	}
 
+	// Fetch all organizations, even deleted ones. This is to remove a user
+	// from any deleted organizations they may be in.
 	existingOrgs, err := tx.GetOrganizationsByUserID(ctx, database.GetOrganizationsByUserIDParams{
 		UserID:  user.ID,
-		Deleted: false,
+		Deleted: sql.NullBool{},
 	})
 	if err != nil {
 		return xerrors.Errorf("failed to get user organizations: %w", err)
@@ -109,9 +111,22 @@ func (s AGPLIDPSync) SyncOrganizations(ctx context.Context, tx database.Store, u
 		return org.ID
 	})
 
+	expectedOrganizations, err := tx.GetOrganizations(ctx, database.GetOrganizationsParams{
+		IDs: expectedOrgIDs,
+		// Do not include deleted organizations
+		Deleted: false,
+	})
+	if err != nil {
+		return xerrors.Errorf("failed to get expected organizations: %w", err)
+	}
+
+	finalExpected := db2sdk.List(expectedOrganizations, func(org database.Organization) uuid.UUID {
+		return org.ID
+	})
+
 	// Find the difference in the expected and the existing orgs, and
 	// correct the set of orgs the user is a member of.
-	add, remove := slice.SymmetricDifference(existingOrgIDs, expectedOrgs)
+	add, remove := slice.SymmetricDifference(existingOrgIDs, finalExpected)
 	notExists := make([]uuid.UUID, 0)
 	for _, orgID := range add {
 		_, err := tx.InsertOrganizationMember(ctx, database.InsertOrganizationMemberParams{
diff --git a/coderd/users.go b/coderd/users.go
@@ -1340,7 +1340,7 @@ func (api *API) organizationsByUser(rw http.ResponseWriter, r *http.Request) {
 
 	organizations, err := api.Database.GetOrganizationsByUserID(ctx, database.GetOrganizationsByUserIDParams{
 		UserID:  user.ID,
-		Deleted: false,
+		Deleted: sql.NullBool{Bool: false, Valid: true},
 	})
 	if errors.Is(err, sql.ErrNoRows) {
 		err = nil

Original file line number	Diff line number	Diff line change
`@@ -4172,7 +4172,11 @@ func (q *FakeQuerier) GetOrganizationsByUserID(_ context.Context, arg database.G`
`4172`	`4172`	`continue`
`4173`	`4173`	`}`
`4174`	`4174`	`for _, organization := range q.organizations {`
`4175`		`- if organization.ID != organizationMember.OrganizationID \|\| organization.Deleted != arg.Deleted {`
	`4175`	`+ if organization.ID != organizationMember.OrganizationID {`
	`4176`	`+ continue`
	`4177`	`+ }`
	`4178`	`+`
	`4179`	`+ if arg.Deleted.Valid && organization.Deleted != arg.Deleted.Bool {`
`4176`	`4180`	`continue`
`4177`	`4181`	`}`
`4178`	`4182`	`organizations = append(organizations, organization)`