Develop #479
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| namespace Resgrid.Config | ||
| { | ||
| public static class SessionSecurityConfig | ||
| { | ||
| // Session tracking is required by the Web BFF and is safe for pre-feature | ||
| // credentials because they are adopted lazily by the validation middleware. | ||
| public static bool TrackingEnabled = true; | ||
|
There was a problem hiding this comment.
Immutability issue in Core/Resgrid.Config/SessionSecurityConfig.cs: TrackingEnabled and the declarations at lines 8, 9, 12, 13, 14, 15, 16, 17, 18, 19, 20, and 22 are initialized with compile-time constants but remain mutable. Mark these values const, or readonly if runtime-only assignment is required, to prevent accidental reassignment. Kody rule violation: Use `readonly` or `const` for Immutable Data public const bool TrackingEnabled = true;Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. There was a problem hiding this comment.
Immutable configuration in Core/Resgrid.Config/SessionSecurityConfig.cs uses a compile-time constant as a mutable field, which obscures intent and permits accidental reassignment at line 8 and related declarations at lines 13-20, Core/Resgrid.Config/OidcConfig.cs:16 and :28, and Web/Resgrid.Web/Models/AccountViewModels/ResetPasswordViewModel.cs:20. Mark these values as const, or static readonly if type initialization-time assignment is required. Kody rule violation: Use `readonly` or `const` for Immutable Data public const bool TrackingEnabled = true;Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. There was a problem hiding this comment.
Immutable configuration misuse identified in Kody rule violation: Use `readonly` or `const` for Immutable Data public const bool TrackingEnabled = true;Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| public static bool LegacyAdoptionEnabled = true; | ||
| public static string RequireSessionClaimForCredentialsIssuedAfterUtc = ""; | ||
| // Blank is intentionally disabled at launch. Set to an ISO-8601 UTC timestamp | ||
| // only after previewing stored DepartmentSecurityPolicy session values. | ||
| public static string DepartmentSessionPolicyEnforcementAfterUtc = ""; | ||
| public static int LastActivityWriteIntervalMinutes = 5; | ||
| public static int RevokedSessionRetentionDays = 90; | ||
| public static int PublicResetLinkLifetimeMinutes = 30; | ||
| public static int PublicResetAccountLimitPerHour = 3; | ||
| public static int PublicResetIpLimitPerHour = 10; | ||
| public static int WebBffAccessTokenLifetimeMinutes = 5; | ||
| public static int ClientMetadataMaximumLength = 256; | ||
| public static int UserAgentMaximumLength = 1024; | ||
| // Optional local JSON CIDR database. Leave blank to display location as unavailable. | ||
| public static string IpLocationDatabasePath = ""; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,13 @@ public class AuditEvent | |
| [ProtoMember(11)] | ||
| public bool Successful { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// The user the action was performed on, when that differs from the actor. Recorded on the audit | ||
| /// row as ObjectId so a privileged action can be queried by its subject and not just its actor. | ||
| /// </summary> | ||
| [ProtoMember(12)] | ||
| public string TargetUserId { get; set; } | ||
|
There was a problem hiding this comment.
Nullability mismatch in Core/Resgrid.Model/Events/AuditEvent.cs leaves TargetUserId undefined for events without a distinct target user, which can trigger downstream NullReferenceException in callers across the listed usage sites. Model TargetUserId as string? or initialize it to string.Empty to make the contract explicit. Kody rule violation: Add null checks to prevent NullReferenceException public string? TargetUserId { get; set; }
// or
public string TargetUserId { get; set; } = string.Empty;Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
|
|
||
| public AuditEvent() | ||
| { | ||
| EventId = Guid.NewGuid().ToString(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| namespace Resgrid.Model | ||
| { | ||
| public enum ExternalIdentityLinkMethod | ||
| { | ||
| Subject = 0, | ||
| VerifiedEmail = 1, | ||
| TrustedSamlEmail = 2, | ||
| Scim = 3, | ||
| Administrator = 4 | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,8 +7,10 @@ public interface IEmailProvider | |
| { | ||
| void Configure(object sender, string fromAddress); | ||
|
|
||
| Task<bool> SendWelcomeMail(string name, string departmentName, string userName, string email, int departmentId); | ||
| Task<bool> SendPasswordRecoveryMail(string name, string email, string departmentName, | ||
| string resetUrl, string ipAddress, string userAgent, string requestedOn, bool isSsoManaged); | ||
|
There was a problem hiding this comment.
Sensitive data exposure in Core/Resgrid.Model/Providers/IEmailProvider.cs: passing raw ipAddress and userAgent propagates client-identifying values into downstream logging or templates, with related usage in Web/Resgrid.Web/Controllers/AccountController.cs:709, Web/Resgrid.Web/Areas/User/Controllers/ProfileController.cs:1245, Core/Resgrid.Model/UserSession.cs:39-40, Workers/Resgrid.Workers.Framework/Logic/AuditQueueLogic.cs:43, Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.addArchivedCall.js:144, Web/Resgrid.Web/Areas/User/Controllers/AccountSecurityController.cs:209, Web/Resgrid.Web/wwwroot/js/app/internal/routes/resgrid.routes.edit.js:93, and Web/Resgrid.Web/wwwroot/js/app/internal/routes/resgrid.routes.new.js:201, 243, and 408. Pass redacted or tokenized values such as ipAddressToken and userAgentToken, or use a structured context type that enforces masking. Kody rule violation: Mask PII and secrets in logs string resetUrl, string ipAddressToken, string userAgentToken, string requestedOn, bool isSsoManaged);Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. There was a problem hiding this comment.
Privacy-by-default violation in Core/Resgrid.Model/Providers/IEmailProvider.cs: the method accepts raw personal data in ipAddress and userAgent without indicating minimization, with related usage in Web/Resgrid.Web/Controllers/AccountController.cs:709, Web/Resgrid.Web/Areas/User/Controllers/ProfileController.cs:1245, Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.addArchivedCall.js:144, Core/Resgrid.Model/UserSession.cs:39-40, Workers/Resgrid.Workers.Framework/Logic/AuditQueueLogic.cs:43, Web/Resgrid.Web/wwwroot/js/app/internal/routes/resgrid.routes.edit.js:93, Web/Resgrid.Web/wwwroot/js/app/internal/routes/resgrid.routes.new.js:201, 243, and 408, and Web/Resgrid.Web/Areas/User/Controllers/AccountSecurityController.cs:209. Pass minimized forms such as ipAddressHash and userAgentToken, or encapsulate them in a type that enforces redaction and purpose metadata. Kody rule violation: Redact PII in logs and metrics by default string resetUrl, string ipAddressHash, string userAgentToken, string requestedOn, bool isSsoManaged);Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Comment on lines
+11
to
+12
There was a problem hiding this comment.
Personal-data handling in Core/Resgrid.Model/Providers/IEmailProvider.cs exposes name, email, ipAddress, and userAgent in the SendPasswordRecoveryMail signature, increasing the chance that diagnostics capture raw PII across the listed call sites. Encapsulate these values in a typed request model that separates delivery fields from redacted telemetry fields and enforce redaction or hashing by default. Kody rule violation: Redact PII in logs and metrics by default Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. There was a problem hiding this comment.
Sensitive data overexposure identified in Kody rule violation: Do not log PHI; mask and drop sensitive fields Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| Task<bool> SendPasswordChangedByAdministratorMail(string name, string userName, string email, string departmentName); | ||
| Task<bool> SendSignupMail(string name, string departmentName, string email); | ||
| Task<bool> SendMessageMail(string email, string subject, string messageSubject, string messageBody, string senderEmail, string senderName, string sentOn, int messageId); | ||
| Task<bool> SendCallMail(string email, string subject, string title, string priority, string natureOfCall, string mapPage, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| using System.Collections.Generic; | ||
| using System.Threading.Tasks; | ||
|
|
||
| namespace Resgrid.Model.Repositories | ||
| { | ||
| public interface IUserExternalIdentityLinksRepository : IRepository<UserExternalIdentityLink> | ||
| { | ||
| Task<UserExternalIdentityLink> GetActiveBySubjectAsync(string departmentSsoConfigId, string externalSubject); | ||
| Task<UserExternalIdentityLink> GetActiveByUserAndConfigAsync(string userId, string departmentSsoConfigId); | ||
| Task<IReadOnlyList<UserExternalIdentityLink>> GetActiveByUserAsync(string userId); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mutable public configuration state in Core/Resgrid.Config/OidcConfig.cs allows accidental reassignment of ConnectionString and obscures immutability intent, with the same issue also present at Core/Resgrid.Config/OidcConfig.cs:28-28, Core/Resgrid.Config/SessionSecurityConfig.cs:7-20,22, Tests/Resgrid.Tests/Services/ClientSessionMetadataParserTests.cs:9-9, and Core/Resgrid.Services/DepartmentSettingsService.cs:27-27,40-40. Mark the field readonly, or const if compile-time constant.
Kody rule violation: Use `readonly` or `const` for Immutable Data
Prompt for LLM
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.