SimpleIdServer icon indicating copy to clipboard operation
SimpleIdServer copied to clipboard

Bulk updates only change string columns

Open RobTF opened this issue 1 year ago • 2 comments

Hi,

When testing SCIM functionality with Entra ID (formally Azure AD) I noticed that when a user is disabled their active flag never goes back to "0" in the "ValueBoolean" column.

This appears to be a bug in the BulkUpdate method in EFSCIMRepresentationCommandRepository where at some point only two columns are named to be part of the bulk update;

Notably line 226 of EFSCIMRepresentationCommandRepository.cs where the bulkConfig variable is initialized.

        public async Task BulkUpdate(IEnumerable<SCIMRepresentationAttribute> scimRepresentationAttributes, bool isReference = false)
        {
            scimRepresentationAttributes = scimRepresentationAttributes.Where(r => !string.IsNullOrWhiteSpace(r.RepresentationId));
            foreach (var attr in scimRepresentationAttributes)
                attr.SchemaAttributeId = attr.SchemaAttribute?.Id;

            // **************************************************************************************
            // Why only ValueString and ComputedValueIndex????
            var bulkConfig = new BulkConfig
            {
                PropertiesToInclude = new List<string> { nameof(SCIMRepresentationAttribute.ValueString), nameof(SCIMRepresentationAttribute.ComputedValueIndex) }
            };
            // ***************************************************************************************

            bulkConfig = GetBulkConfig(BulkOperations.UPDATE, bulkConfig);
            await _scimDbContext.BulkUpdateAsync(scimRepresentationAttributes.ToList(), bulkConfig);
        }

If I add nameof(SCIMRepresentationAttribute.ValueBoolean) to the list of properties the active attribute is set successfully.

Is this a simple oversight? Is there any other implication to adding more properties to this list?

thanks

RobTF avatar Apr 22 '24 12:04 RobTF

Hello,

Firstly, thank you for conducting the investigation :)

Indeed, there is an error in the BulkUpdate operation of the EFSCIMRepresentationCommandRepository class. All properties that begin with the name Value and ComputedValueIndex must be added to the PropertiesToInclude list.

I have made some modifications in the master branch to address this issue.

Kind regards,

SID

simpleidserver avatar Apr 23 '24 19:04 simpleidserver

Great, thanks for the confirmation!

RobTF avatar Apr 24 '24 10:04 RobTF

This issue is fixed in the version 5.0.0

simpleidserver avatar Jun 07 '24 14:06 simpleidserver