Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

House cleaning #635

Merged
merged 29 commits into from
Oct 11, 2024
Merged

House cleaning #635

merged 29 commits into from
Oct 11, 2024

Conversation

JoeShook
Copy link
Owner

@JoeShook JoeShook commented Oct 7, 2024

Removed all warnings.
Adding more Cancellation Token plumbing.
Removed code that was UdapEd only, such as IAccessTokenProvider and AuthTokenHttpMessageHandler
Package updates

dependabot bot and others added 20 commits September 10, 2024 12:13
@dependabot
Bump FluentAssertions from 6.12.0 to 6.12.1
df8df64 
Bumps [FluentAssertions](https://github.com/fluentassertions/fluentassertions) from 6.12.0 to 6.12.1.
- [Release notes](https://github.com/fluentassertions/fluentassertions/releases)
- [Changelog](https://github.com/fluentassertions/fluentassertions/blob/develop/AcceptApiChanges.ps1)
- [Commits](fluentassertions/fluentassertions@6.12.0...6.12.1)

---
updated-dependencies:
- dependency-name: FluentAssertions
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
@dependabot
Bump Firely.Fhir.Packages from 4.7.0 to 4.8.0
0db49af 
Bumps [Firely.Fhir.Packages](https://github.com/FirelyTeam/Firely.Fhir.Packages) from 4.7.0 to 4.8.0.
- [Release notes](https://github.com/FirelyTeam/Firely.Fhir.Packages/releases)
- [Commits](FirelyTeam/Firely.Fhir.Packages@v4.7.0...v4.8.0)

---
updated-dependencies:
- dependency-name: Firely.Fhir.Packages
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
@dependabot
Bump Npgsql.EntityFrameworkCore.PostgreSQL and Npgsql
9a9fd92 
Bumps [Npgsql.EntityFrameworkCore.PostgreSQL](https://github.com/npgsql/efcore.pg) and [Npgsql](https://github.com/npgsql/npgsql). These dependencies needed to be updated together.

Updates `Npgsql.EntityFrameworkCore.PostgreSQL` from 8.0.4 to 8.0.8
- [Release notes](https://github.com/npgsql/efcore.pg/releases)
- [Commits](npgsql/efcore.pg@v8.0.4...v8.0.8)

Updates `Npgsql` from 8.0.3 to 8.0.4
- [Release notes](https://github.com/npgsql/npgsql/releases)
- [Commits](npgsql/npgsql@v8.0.3...v8.0.4)

---
updated-dependencies:
- dependency-name: Npgsql.EntityFrameworkCore.PostgreSQL
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: Npgsql
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
@JoeShook
Minor adjustments. Mostly for example servers.
009e823
@JoeShook
Minor updates to builders
65f586d 
It will make it easier on consumers concerning warnings.  And the nullable parameters will express whether a null will be allowed.
@JoeShook
Fixing compiler warnings
021806b
@JoeShook
Fixing compiler warnings
736dced
@JoeShook
Fixing compiler warnings
f0a77ad
@JoeShook
Dropping IAccessTokenProvider and AuthTokenHttpMessageHandler
ed99c3c 
Moving it to UdapEd.
Also fixing up more compiler warnings.
@JoeShook
Merge pull request #620 from JoeShook/dependabot/nuget/develop/Fluent…
836a06c 
…Assertions-6.12.1

Bump FluentAssertions from 6.12.0 to 6.12.1
@JoeShook
Merge pull request #633 from JoeShook/dependabot/nuget/develop/multi-…
3ae48a8 
…f7a557f0b7

Bump Npgsql.EntityFrameworkCore.PostgreSQL and Npgsql
@JoeShook
Merge pull request #630 from JoeShook/dependabot/nuget/develop/Firely…
bd60900 
….Fhir.Packages-4.8.0

Bump Firely.Fhir.Packages from 4.7.0 to 4.8.0
@JoeShook
Fix up so Properties are accessible
67e8d26
@JoeShook
Fixing more warnings
b622a5c
@JoeShook
Fixup Tracing so it sets the class and method names rather than hard …
4882ff8 
…coding them.
@JoeShook
Fixing Warnings
a0ba9a1
@JoeShook
Fixup warnings and adding Cancellation token into missing areas.
6d6ab57
@JoeShook
fixup warnings
79caef1
@JoeShook
Almost all warnings removed.
16c6dd8
@JoeShook
Package updates
96b64a5
github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 7, 2024
View reviewed changes
Udap.Common/Metadata/UdapMetaDataBuilder.cs Fixed Show fixed Hide fixed
Udap.Common/Metadata/UdapMetaDataBuilder.cs Fixed Show fixed Hide fixed
Udap.Server/Registration/UdapDynamicClientRegistrationValidator.cs
UdapDynamicClientRegistrationErrorDescriptions.UntrustedCertificate);
_logger.LogWarning("{Error}::{Description}",
UdapDynamicClientRegistrationErrors.UnapprovedSoftwareStatement,
UdapDynamicClientRegistrationErrorDescriptions.UntrustedCertificate);

Check failure

Code scanning / CodeQL

Clear text storage of sensitive information High

This stores sensitive data returned by
access to constant UntrustedCertificate
Loading
as clear text.

Copilot Autofix AI 3 months ago

To fix the problem, we need to ensure that sensitive information, such as certificate thumbprints, is encrypted before being logged. We can use the System.Security.Cryptography.ProtectedData class to encrypt the thumbprints before logging them. This will ensure that even if the logs are accessed by unauthorized users, the sensitive information remains protected.

  1. Encrypt the thumbprints before appending them to the log message.
  2. Update the logging statements to include the encrypted thumbprints instead of the plain text ones.
  3. Add necessary imports for the encryption functionality.
Suggested changeset 1
Udap.Server/Registration/UdapDynamicClientRegistrationValidator.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Udap.Server/Registration/UdapDynamicClientRegistrationValidator.cs b/Udap.Server/Registration/UdapDynamicClientRegistrationValidator.cs
--- a/Udap.Server/Registration/UdapDynamicClientRegistrationValidator.cs
+++ b/Udap.Server/Registration/UdapDynamicClientRegistrationValidator.cs
@@ -38,2 +38,3 @@
 using Udap.Util.Extensions;
+using System.Security.Cryptography;
 using static Udap.Model.UdapConstants;
@@ -42,2 +43,9 @@
 
+private string Encrypt(string data)
+{
+    byte[] dataBytes = Encoding.UTF8.GetBytes(data);
+    byte[] encryptedBytes = ProtectedData.Protect(dataBytes, null, DataProtectionScope.CurrentUser);
+    return Convert.ToBase64String(encryptedBytes);
+}
+
 /// <summary>
@@ -305,3 +313,3 @@
             var sb = new StringBuilder();
-            sb.AppendLine($"Client Thumbprint: {publicCert.Thumbprint}");
+            sb.AppendLine($"Client Thumbprint: {Encrypt(publicCert.Thumbprint)}");
             
@@ -310,3 +318,3 @@
                 sb.AppendLine(
-                    $"Intermediate Thumbprints: {string.Join(" | ", intermediateCertificates.Select(a => a.Thumbprint))}");
+                    $"Intermediate Thumbprints: {string.Join(" | ", intermediateCertificates.Select(a => Encrypt(a.Thumbprint)))}");
 
@@ -314,3 +322,3 @@
 
-            sb.AppendLine($"Anchor Certificate Thumbprints: {string.Join(" | ", anchorCertificates.Select(a => a.Thumbprint))}");
+            sb.AppendLine($"Anchor Certificate Thumbprints: {string.Join(" | ", anchorCertificates.Select(a => Encrypt(a.Thumbprint)))}");
             _logger.LogWarning("{Message}", sb.ToString());
EOF
@@ -38,2 +38,3 @@
using Udap.Util.Extensions;
using System.Security.Cryptography;
using static Udap.Model.UdapConstants;
@@ -42,2 +43,9 @@

private string Encrypt(string data)
{
byte[] dataBytes = Encoding.UTF8.GetBytes(data);
byte[] encryptedBytes = ProtectedData.Protect(dataBytes, null, DataProtectionScope.CurrentUser);
return Convert.ToBase64String(encryptedBytes);
}

/// <summary>
@@ -305,3 +313,3 @@
var sb = new StringBuilder();
sb.AppendLine($"Client Thumbprint: {publicCert.Thumbprint}");
sb.AppendLine($"Client Thumbprint: {Encrypt(publicCert.Thumbprint)}");

@@ -310,3 +318,3 @@
sb.AppendLine(
$"Intermediate Thumbprints: {string.Join(" | ", intermediateCertificates.Select(a => a.Thumbprint))}");
$"Intermediate Thumbprints: {string.Join(" | ", intermediateCertificates.Select(a => Encrypt(a.Thumbprint)))}");

@@ -314,3 +322,3 @@

sb.AppendLine($"Anchor Certificate Thumbprints: {string.Join(" | ", anchorCertificates.Select(a => a.Thumbprint))}");
sb.AppendLine($"Anchor Certificate Thumbprints: {string.Join(" | ", anchorCertificates.Select(a => Encrypt(a.Thumbprint)))}");
_logger.LogWarning("{Message}", sb.ToString());
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Udap.Server/Security/Authentication/TieredOAuth/TieredOAuthAuthenticationHandler.cs Fixed Show fixed Hide fixed
JoeShook and others added 9 commits October 7, 2024 07:02
@JoeShook @github-advanced-security
Fix code scanning alert no. 40: Log entries created from user input
8e3f07a 
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@JoeShook @github-advanced-security
Fix code scanning alert no. 41: Log entries created from user input
2583a6d 
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@JoeShook @github-advanced-security
Fix code scanning alert no. 39: Log entries created from user input
9e212fa 
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@JoeShook
CdsHooks building blocks
4348a15 
The idea is to build a functioning CdsHooks service that works with version 2.0 and another mode where it can run in a UDAP enabled experience.
@JoeShook
Add CdsHooks packages
ddfa649 
This is experimental for testing what it would be like to integrate UDAP and CDS Hooks.
@JoeShook
Fixup Package prep for CdsHooks
1bdb23d
@JoeShook
Package updates
2e568f8
@JoeShook
Package updates
8064960
@JoeShook
fixup package image reference.
5d3cc2d
@JoeShook JoeShook merged commit fe35398 into main Oct 11, 2024
3 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@JoeShook