Fix #262: Moved from Newtonsoft.Json to System.Text.Json #324
Conversation
antoineatstariongroup
requested review from
lxatstariongroup and
samatstariongroup
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
I think that the issue found by SQ should be discarded and non-relevant with the context of the code |
| public override void Write(Utf8JsonWriter writer, PostOperation value, JsonSerializerOptions options) | ||
| { | ||
| writer.WriteStartObject(); | ||
| writer.WriteStartArray("_delete"); |
| { | ||
| domainOfExpertise.Name = jObject["name"].ToObject<string>(); | ||
| if(nameProperty.ValueKind == JsonValueKind.Null) |
There was a problem hiding this comment.
lets log at Trace level
| <PackageTags>CDP COMET ECSS-E-TM-10-25 JSON</PackageTags> | ||
| <PackageLicenseExpression>LGPL-3.0-only</PackageLicenseExpression> | ||
| <PackageReleaseNotes> | ||
| [BUMP] To CDP4Common 26.6.0 | ||
| </PackageReleaseNotes> | ||
| <PackageReadmeFile>README.md</PackageReadmeFile> |
There was a problem hiding this comment.
please restore the README.md
| @@ -0,0 +1,20 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
make sure to add some docs somewhere what the purpose is and how to use it
| emailAddress.ModifiedOn = jObject["modifiedOn"].ToObject<DateTime>(); | ||
| if(modifiedOnProperty.ValueKind == JsonValueKind.Null) | ||
| { | ||
| Logger.Trace("The non-nullabale modifiedOn property of the emailAddress {id} is null", emailAddress.Iid); |
Check warning
Code scanning / CodeQL
Exposure of private information Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 2 months ago
To fix the problem, we should avoid logging sensitive information directly. Instead, we can log a generic message without including the potentially sensitive emailAddress.Iid. This way, we maintain the logging functionality for debugging purposes without exposing private data.
- Modify the logging statements to exclude the
emailAddress.Iid. - Ensure that the log messages still provide useful information for debugging without revealing sensitive data.
| @@ -85,3 +85,3 @@ | ||
| { | ||
| Logger.Trace("The non-nullabale modifiedOn property of the emailAddress {id} is null", emailAddress.Iid); | ||
| Logger.Trace("The non-nullabale modifiedOn property of an emailAddress is null"); | ||
| } | ||
| @@ -97,3 +97,3 @@ | ||
| { | ||
| Logger.Trace("The non-nullabale thingPreference property of the emailAddress {id} is null", emailAddress.Iid); | ||
| Logger.Trace("The non-nullabale thingPreference property of an emailAddress is null"); | ||
| } | ||
| @@ -109,3 +109,3 @@ | ||
| { | ||
| Logger.Trace("The non-nullabale value property of the emailAddress {id} is null", emailAddress.Iid); | ||
| Logger.Trace("The non-nullabale value property of an emailAddress is null"); | ||
| } | ||
| @@ -121,3 +121,3 @@ | ||
| { | ||
| Logger.Trace("The non-nullabale vcardType property of the emailAddress {id} is null", emailAddress.Iid); | ||
| Logger.Trace("The non-nullabale vcardType property of an emailAddress is null"); | ||
| } |
| emailAddress.ThingPreference = jObject["thingPreference"].ToObject<string>(); | ||
| if(thingPreferenceProperty.ValueKind == JsonValueKind.Null) | ||
| { | ||
| Logger.Trace("The non-nullabale thingPreference property of the emailAddress {id} is null", emailAddress.Iid); |
Check warning
Code scanning / CodeQL
Exposure of private information Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 2 months ago
To fix the problem, we need to ensure that sensitive information is not logged directly. Instead of logging the actual emailAddress.Iid, we can log a masked or sanitized version of it. This way, we can still retain useful logging information for debugging purposes without exposing sensitive data.
The best way to fix this problem is to modify the logging statements to mask the emailAddress.Iid before logging it. We can create a helper method to mask the Iid and use this method in the logging statements.
| @@ -45,2 +45,12 @@ | ||
| /// <summary> | ||
| /// Masks the Iid to prevent exposure of sensitive information | ||
| /// </summary> | ||
| /// <param name="iid">The Iid to mask</param> | ||
| /// <returns>The masked Iid</returns> | ||
| private static string MaskIid(Guid iid) | ||
| { | ||
| return iid.ToString().Substring(0, 8) + "****"; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Instantiate and deserialize the properties of a <see cref="CDP4Common.DTO.EmailAddress"/> | ||
| @@ -85,3 +95,3 @@ | ||
| { | ||
| Logger.Trace("The non-nullabale modifiedOn property of the emailAddress {id} is null", emailAddress.Iid); | ||
| Logger.Trace("The non-nullabale modifiedOn property of the emailAddress {id} is null", MaskIid(emailAddress.Iid)); | ||
| } | ||
| @@ -97,3 +107,3 @@ | ||
| { | ||
| Logger.Trace("The non-nullabale thingPreference property of the emailAddress {id} is null", emailAddress.Iid); | ||
| Logger.Trace("The non-nullabale thingPreference property of the emailAddress {id} is null", MaskIid(emailAddress.Iid)); | ||
| } | ||
| @@ -109,3 +119,3 @@ | ||
| { | ||
| Logger.Trace("The non-nullabale value property of the emailAddress {id} is null", emailAddress.Iid); | ||
| Logger.Trace("The non-nullabale value property of the emailAddress {id} is null", MaskIid(emailAddress.Iid)); | ||
| } | ||
| @@ -121,3 +131,3 @@ | ||
| { | ||
| Logger.Trace("The non-nullabale vcardType property of the emailAddress {id} is null", emailAddress.Iid); | ||
| Logger.Trace("The non-nullabale vcardType property of the emailAddress {id} is null", MaskIid(emailAddress.Iid)); | ||
| } |
| emailAddress.Value = jObject["value"].ToObject<string>(); | ||
| if(valueProperty.ValueKind == JsonValueKind.Null) | ||
| { | ||
| Logger.Trace("The non-nullabale value property of the emailAddress {id} is null", emailAddress.Iid); |
Check warning
Code scanning / CodeQL
Exposure of private information Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 2 months ago
To fix the problem, we need to ensure that private information is not logged directly. Instead, we can log a sanitized or masked version of the Iid to prevent exposure of sensitive data. This can be done by hashing the Iid before logging it, which will allow us to log a consistent identifier without revealing the actual value.
The best way to fix the problem without changing existing functionality is to modify the logging statements to log a hashed version of the Iid. We will use a cryptographic hash function to generate a hash of the Iid and log the hash instead of the raw Iid.
| @@ -31,2 +31,4 @@ | ||
| using System.Text.Json; | ||
| using System.Security.Cryptography; | ||
| using System.Text; | ||
|
|
||
| @@ -44,2 +46,11 @@ | ||
|
|
||
| private static string HashIid(Guid iid) | ||
| { | ||
| using (var sha256 = SHA256.Create()) | ||
| { | ||
| var hash = sha256.ComputeHash(Encoding.UTF8.GetBytes(iid.ToString())); | ||
| return Convert.ToBase64String(hash); | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| @@ -85,3 +96,3 @@ | ||
| { | ||
| Logger.Trace("The non-nullabale modifiedOn property of the emailAddress {id} is null", emailAddress.Iid); | ||
| Logger.Trace("The non-nullabale modifiedOn property of the emailAddress {id} is null", HashIid(emailAddress.Iid)); | ||
| } | ||
| @@ -97,3 +108,3 @@ | ||
| { | ||
| Logger.Trace("The non-nullabale thingPreference property of the emailAddress {id} is null", emailAddress.Iid); | ||
| Logger.Trace("The non-nullabale thingPreference property of the emailAddress {id} is null", HashIid(emailAddress.Iid)); | ||
| } | ||
| @@ -109,3 +120,3 @@ | ||
| { | ||
| Logger.Trace("The non-nullabale value property of the emailAddress {id} is null", emailAddress.Iid); | ||
| Logger.Trace("The non-nullabale value property of the emailAddress {id} is null", HashIid(emailAddress.Iid)); | ||
| } | ||
| @@ -121,3 +132,3 @@ | ||
| { | ||
| Logger.Trace("The non-nullabale vcardType property of the emailAddress {id} is null", emailAddress.Iid); | ||
| Logger.Trace("The non-nullabale vcardType property of the emailAddress {id} is null", HashIid(emailAddress.Iid)); | ||
| } |
| emailAddress.VcardType = jObject["vcardType"].ToObject<VcardEmailAddressKind>(); | ||
| if(vcardTypeProperty.ValueKind == JsonValueKind.Null) | ||
| { | ||
| Logger.Trace("The non-nullabale vcardType property of the emailAddress {id} is null", emailAddress.Iid); |
Check warning
Code scanning / CodeQL
Exposure of private information Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 2 months ago
To fix the problem, we need to ensure that sensitive information is not logged. In this case, we should avoid logging the emailAddress.Iid directly. Instead, we can log a generic message without including the potentially sensitive Iid. This change should be made in the FromJsonObject method of the EmailAddressResolver class.
- Replace the logging statements that include
emailAddress.Iidwith generic messages. - Ensure that the functionality of the code remains unchanged, except for the logging behavior.
| @@ -85,3 +85,3 @@ | ||
| { | ||
| Logger.Trace("The non-nullabale modifiedOn property of the emailAddress {id} is null", emailAddress.Iid); | ||
| Logger.Trace("The non-nullabale modifiedOn property of the emailAddress is null"); | ||
| } | ||
| @@ -97,3 +97,3 @@ | ||
| { | ||
| Logger.Trace("The non-nullabale thingPreference property of the emailAddress {id} is null", emailAddress.Iid); | ||
| Logger.Trace("The non-nullabale thingPreference property of the emailAddress is null"); | ||
| } | ||
| @@ -109,3 +109,3 @@ | ||
| { | ||
| Logger.Trace("The non-nullabale value property of the emailAddress {id} is null", emailAddress.Iid); | ||
| Logger.Trace("The non-nullabale value property of the emailAddress is null"); | ||
| } | ||
| @@ -121,3 +121,3 @@ | ||
| { | ||
| Logger.Trace("The non-nullabale vcardType property of the emailAddress {id} is null", emailAddress.Iid); | ||
| Logger.Trace("The non-nullabale vcardType property of the emailAddress is null"); | ||
| } |
| /// Initializes a new <see cref="PostOperationJsonConverter" /> instance. | ||
| /// </summary> | ||
| /// <param name="ignoreCopyProperty">Asserts that the <see cref="PostOperation.Copy"/> have to be ignored</param> | ||
| public PostOperationJsonConverter(bool ignoreCopyProperty = false) |
There was a problem hiding this comment.
the default should be true, since this is special for COMET and not supported by version 1,0.0 of 10-25
|
| telephoneNumber.ModifiedOn = jObject["modifiedOn"].ToObject<DateTime>(); | ||
| if(modifiedOnProperty.ValueKind == JsonValueKind.Null) | ||
| { | ||
| Logger.Trace("The non-nullabale modifiedOn property of the telephoneNumber {id} is null", telephoneNumber.Iid); |
Check warning
Code scanning / CodeQL
Exposure of private information Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 2 months ago
To fix the problem, we should avoid logging sensitive information directly. Instead, we can log a generic message or mask the sensitive parts of the data. In this case, we will replace the logging of telephoneNumber.Iid with a generic message that does not include the potentially sensitive identifier.
- Replace the logging statement on line 86 with a generic message that does not include
telephoneNumber.Iid. - Ensure that no sensitive information is logged in other parts of the code.
| @@ -85,3 +85,3 @@ | ||
| { | ||
| Logger.Trace("The non-nullabale modifiedOn property of the telephoneNumber {id} is null", telephoneNumber.Iid); | ||
| Logger.Trace("The non-nullabale modifiedOn property of a telephoneNumber is null"); | ||
| } | ||
| @@ -97,3 +97,3 @@ | ||
| { | ||
| Logger.Trace("The non-nullabale thingPreference property of the telephoneNumber {id} is null", telephoneNumber.Iid); | ||
| Logger.Trace("The non-nullabale thingPreference property of a telephoneNumber is null"); | ||
| } | ||
| @@ -109,3 +109,3 @@ | ||
| { | ||
| Logger.Trace("The non-nullabale value property of the telephoneNumber {id} is null", telephoneNumber.Iid); | ||
| Logger.Trace("The non-nullabale value property of a telephoneNumber is null"); | ||
| } |
| telephoneNumber.ThingPreference = jObject["thingPreference"].ToObject<string>(); | ||
| if(thingPreferenceProperty.ValueKind == JsonValueKind.Null) | ||
| { | ||
| Logger.Trace("The non-nullabale thingPreference property of the telephoneNumber {id} is null", telephoneNumber.Iid); |
Check warning
Code scanning / CodeQL
Exposure of private information Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 2 months ago
To fix the problem, we need to ensure that sensitive information is not logged directly. Instead of logging the actual telephoneNumber.Iid, we can log a masked or hashed version of it. This way, the identifier is obfuscated, and the risk of exposing private information is mitigated.
The best way to fix the problem without changing existing functionality is to hash the telephoneNumber.Iid before logging it. We can use a cryptographic hash function to generate a fixed-length hash of the identifier, which can then be safely logged.
We need to make the following changes:
- Import the necessary cryptographic library.
- Create a method to hash the
telephoneNumber.Iid. - Use the hashed value in the log message instead of the plain identifier.
| @@ -31,2 +31,4 @@ | ||
| using System.Text.Json; | ||
| using System.Security.Cryptography; | ||
| using System.Text; | ||
|
|
||
| @@ -44,2 +46,16 @@ | ||
|
|
||
| private static string HashIdentifier(Guid id) | ||
| { | ||
| using (SHA256 sha256 = SHA256.Create()) | ||
| { | ||
| byte[] bytes = sha256.ComputeHash(Encoding.UTF8.GetBytes(id.ToString())); | ||
| StringBuilder builder = new StringBuilder(); | ||
| for (int i = 0; i < bytes.Length; i++) | ||
| { | ||
| builder.Append(bytes[i].ToString("x2")); | ||
| } | ||
| return builder.ToString(); | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| @@ -97,3 +113,3 @@ | ||
| { | ||
| Logger.Trace("The non-nullabale thingPreference property of the telephoneNumber {id} is null", telephoneNumber.Iid); | ||
| Logger.Trace("The non-nullabale thingPreference property of the telephoneNumber {id} is null", HashIdentifier(telephoneNumber.Iid)); | ||
| } |
| telephoneNumber.Value = jObject["value"].ToObject<string>(); | ||
| if(valueProperty.ValueKind == JsonValueKind.Null) | ||
| { | ||
| Logger.Trace("The non-nullabale value property of the telephoneNumber {id} is null", telephoneNumber.Iid); |
Check warning
Code scanning / CodeQL
Exposure of private information Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 2 months ago
To fix the problem, we need to ensure that sensitive information is not logged. We can achieve this by either removing the logging statement or by masking the sensitive information before logging it. In this case, we will remove the logging statement to prevent any potential exposure of private information.
| @@ -109,3 +109,3 @@ | ||
| { | ||
| Logger.Trace("The non-nullabale value property of the telephoneNumber {id} is null", telephoneNumber.Iid); | ||
| Logger.Trace("The non-nullabale value property of the telephoneNumber is null"); | ||
| } |
Prerequisites
Description
Fix #262
BREAKING CHANGES :