Skip to content

RG-T133 User Session, Password Reset Changes, More Audits - #476

Closed
ucswift wants to merge 1 commit into
masterfrom
develop
Closed

RG-T133 User Session, Password Reset Changes, More Audits#476
ucswift wants to merge 1 commit into
masterfrom
develop

Conversation

@ucswift

@ucswift ucswift commented Aug 21, 2026

Copy link
Copy Markdown
Member

Summary

This PR adds account session tracking, stronger password reset flows, and expanded authentication auditing across the platform. It also tightens SSO/local-login behavior and moves the web app away from browser-stored API tokens toward a same-origin BFF pattern.

Key changes

User session management and revocation

  • Introduces persistent user session tracking for web and API authentication.
  • Adds support for:
    • listing active sessions,
    • revoking a single session,
    • revoking all other sessions,
    • revoking all sessions account-wide,
    • revoking sessions for a specific department membership.
  • Enforces session validity on:
    • website requests,
    • API requests,
    • SignalR/eventing hub calls.
  • Tracks session metadata such as client app, device, browser/OS, IP, and approximate location.
  • Revokes sessions automatically when:
    • passwords change,
    • usernames change,
    • emails change,
    • memberships are disabled/deleted,
    • accounts are deactivated.

Password reset and credential security changes

  • Adds a department setting to require administrator-initiated password resets to be done by email link instead of allowing admins to choose a user’s new password.
  • Adds UI and localization for this new department policy.
  • Reworks password recovery to use:
    • short-lived,
    • single-use,
    • rate-limited reset links.
  • Public password recovery now sends a reset email instead of assigning a generated password.
  • Admin resets can now either:
    • directly set a new password, or
    • send a reset email, depending on department policy.
  • Completing a password change or reset revokes existing sessions and tokens.
  • Adds email notifications for:
    • password recovery requests,
    • administrator-changed passwords.
  • Prevents password reset for SSO-managed accounts through Resgrid flows.

SSO and external identity handling

  • Adds durable external identity link records for SSO/SCIM-managed users.
  • Tightens SSO account linking to rely on stable external subject identifiers.
  • Allows email-based linking only when the external email is explicitly verified.
  • Uses external identity state to determine whether local password login is allowed.
  • Blocks local login when an account is SSO-managed or when department SSO policy requires it.
  • Records SCIM-created users as externally managed identities.

Web authentication and BFF changes

  • Removes reliance on browser-stored API bearer tokens for the web app.
  • Adds a same-origin Web BFF endpoint for proxied web API access.
  • Adds a dedicated eventing token flow for browser SignalR connections.
  • Updates web frontend calls and SignalR clients to use the BFF/eventing token flow instead of localStorage tokens.
  • Adds anti-forgery protection to more web actions and web-side API usage.

Audit and security logging improvements

  • Expands system audit data with target user, session, and correlation context.
  • Adds new audit types for:
    • password changes,
    • admin password resets,
    • public password reset completion,
    • username changes,
    • email changes,
    • session revocations,
    • external identity link events,
    • admin-sent password reset links.
  • Adds additional audit log types for password reset and session revocation events.
  • Adds console-origin audit support for administrative password reset command usage.

Other security and behavior updates

  • Adds session-aware authentication generation fields to users so credentials can be invalidated account-wide.
  • Adds cleanup of revoked/expired session history in the worker cleanup task.
  • Hardens secret comparison for SCIM bearer token validation.
  • Secures SignalR eventing hubs so only authenticated callers can subscribe and only the internal publisher can broadcast.
  • Prevents Weather Alert messages from sending through SMS/chatbot channels; they now remain email/push only.
  • Removes embedded default OIDC connection string value from config.

Functional impact

  • Users and administrators can now manage and revoke active sessions.
  • Password reset behavior is more secure and policy-driven.
  • SSO-managed users are more clearly separated from locally managed credentials.
  • Session/token invalidation is consistently enforced after credential-sensitive changes.
  • Web/API/eventing authentication behavior is more tightly controlled and auditable.

@request-info

request-info Bot commented Aug 21, 2026

Copy link
Copy Markdown

Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details?

@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your current included review allowance is based on your included PR review attempts over the past 7 days.

Next review available in: 11 minutes

Limit details: You’ve used the included review currently available. Your 60 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: da1940dc-924f-47a5-a770-8424c0911029

📥 Commits

Reviewing files that changed from the base of the PR and between 61c717b and a8e51b5.

⛔ Files ignored due to path filters (21)
  • Core/Resgrid.Localization/Areas/User/Department/Department.en.resx is excluded by !**/*.resx
  • Core/Resgrid.Localization/Areas/User/Department/Department.resx is excluded by !**/*.resx
  • Core/Resgrid.Localization/Areas/User/Profile/Profile.ar.resx is excluded by !**/*.resx
  • Core/Resgrid.Localization/Areas/User/Profile/Profile.de.resx is excluded by !**/*.resx
  • Core/Resgrid.Localization/Areas/User/Profile/Profile.el.resx is excluded by !**/*.resx
  • Core/Resgrid.Localization/Areas/User/Profile/Profile.en.resx is excluded by !**/*.resx
  • Core/Resgrid.Localization/Areas/User/Profile/Profile.es.resx is excluded by !**/*.resx
  • Core/Resgrid.Localization/Areas/User/Profile/Profile.fr.resx is excluded by !**/*.resx
  • Core/Resgrid.Localization/Areas/User/Profile/Profile.it.resx is excluded by !**/*.resx
  • Core/Resgrid.Localization/Areas/User/Profile/Profile.pl.resx is excluded by !**/*.resx
  • Core/Resgrid.Localization/Areas/User/Profile/Profile.sv.resx is excluded by !**/*.resx
  • Core/Resgrid.Localization/Areas/User/Profile/Profile.uk.resx is excluded by !**/*.resx
  • Tests/Resgrid.Tests/Services/ClientSessionMetadataParserTests.cs is excluded by !**/Tests/**
  • Tests/Resgrid.Tests/Services/CommunicationServiceTests.cs is excluded by !**/Tests/**
  • Tests/Resgrid.Tests/Services/DepartmentSettingsServicePasswordResetTests.cs is excluded by !**/Tests/**
  • Tests/Resgrid.Tests/Services/DepartmentSsoServiceTests.cs is excluded by !**/Tests/**
  • Tests/Resgrid.Tests/Services/ExternalIdentityLinkServiceTests.cs is excluded by !**/Tests/**
  • Tests/Resgrid.Tests/Services/LocalIpLocationProviderTests.cs is excluded by !**/Tests/**
  • Tests/Resgrid.Tests/Services/PasswordRecoveryServiceTests.cs is excluded by !**/Tests/**
  • Tests/Resgrid.Tests/Services/UserSessionServiceTests.cs is excluded by !**/Tests/**
  • Tests/Resgrid.Tests/Web/Services/ConnectControllerSsoTests.cs is excluded by !**/Tests/**
📒 Files selected for processing (136)
  • Core/Resgrid.Config/OidcConfig.cs
  • Core/Resgrid.Config/SessionSecurityConfig.cs
  • Core/Resgrid.Model/AuditLogTypes.cs
  • Core/Resgrid.Model/DepartmentSettingTypes.cs
  • Core/Resgrid.Model/ExternalIdentityLinkMethod.cs
  • Core/Resgrid.Model/Identity/IdentityUser.cs
  • Core/Resgrid.Model/Providers/IEmailProvider.cs
  • Core/Resgrid.Model/Repositories/IIdentityRepository.cs
  • Core/Resgrid.Model/Repositories/IUserExternalIdentityLinksRepository.cs
  • Core/Resgrid.Model/Repositories/IUserSessionsRepository.cs
  • Core/Resgrid.Model/Security/PasswordRecoveryContracts.cs
  • Core/Resgrid.Model/Security/SessionClaimTypes.cs
  • Core/Resgrid.Model/Security/SessionCreationDeniedException.cs
  • Core/Resgrid.Model/Security/UserSessionContracts.cs
  • Core/Resgrid.Model/Services/IClientSessionMetadataParser.cs
  • Core/Resgrid.Model/Services/IDepartmentSettingsService.cs
  • Core/Resgrid.Model/Services/IEmailService.cs
  • Core/Resgrid.Model/Services/IExternalIdentityLinkService.cs
  • Core/Resgrid.Model/Services/IIpLocationProvider.cs
  • Core/Resgrid.Model/Services/IPasswordRecoveryService.cs
  • Core/Resgrid.Model/Services/IUserSessionService.cs
  • Core/Resgrid.Model/Services/IUsersService.cs
  • Core/Resgrid.Model/SystemAudit.cs
  • Core/Resgrid.Model/SystemAuditSystems.cs
  • Core/Resgrid.Model/SystemAuditTypes.cs
  • Core/Resgrid.Model/UserExternalIdentityLink.cs
  • Core/Resgrid.Model/UserSession.cs
  • Core/Resgrid.Model/UserSessionAuthenticationMethod.cs
  • Core/Resgrid.Model/UserSessionClientApplication.cs
  • Core/Resgrid.Model/UserSessionRevocationReason.cs
  • Core/Resgrid.Model/UserSessionState.cs
  • Core/Resgrid.Services/ClientSessionMetadataParser.cs
  • Core/Resgrid.Services/CommunicationService.cs
  • Core/Resgrid.Services/DeleteService.cs
  • Core/Resgrid.Services/DepartmentSettingsService.cs
  • Core/Resgrid.Services/DepartmentSsoService.cs
  • Core/Resgrid.Services/EmailService.cs
  • Core/Resgrid.Services/ExternalIdentityLinkService.cs
  • Core/Resgrid.Services/LocalIpLocationProvider.cs
  • Core/Resgrid.Services/PasswordRecoveryService.cs
  • Core/Resgrid.Services/ServicesModule.cs
  • Core/Resgrid.Services/UserSessionService.cs
  • Core/Resgrid.Services/UsersService.cs
  • Providers/Resgrid.Providers.Bus/SignalrProvider.cs
  • Providers/Resgrid.Providers.Claims/ClaimsPrincipalFactory.cs
  • Providers/Resgrid.Providers.Email/PostmarkTemplateProvider.cs
  • Providers/Resgrid.Providers.Email/Resgrid.Providers.Email.csproj
  • Providers/Resgrid.Providers.Email/Template/PasswordChangedByAdministrator.html
  • Providers/Resgrid.Providers.Email/Template/PasswordRecovery.html
  • Providers/Resgrid.Providers.Email/Template/PasswordReset.html
  • Providers/Resgrid.Providers.Migrations/Migrations/M0120_AddUserAuthenticationState.cs
  • Providers/Resgrid.Providers.Migrations/Migrations/M0121_AddUserSessions.cs
  • Providers/Resgrid.Providers.Migrations/Migrations/M0122_AddUserExternalIdentityLinks.cs
  • Providers/Resgrid.Providers.Migrations/Migrations/M0123_AddAuthenticationAuditContext.cs
  • Providers/Resgrid.Providers.MigrationsPg/Migrations/M0120_AddUserAuthenticationStatePg.cs
  • Providers/Resgrid.Providers.MigrationsPg/Migrations/M0121_AddUserSessionsPg.cs
  • Providers/Resgrid.Providers.MigrationsPg/Migrations/M0122_AddUserExternalIdentityLinksPg.cs
  • Providers/Resgrid.Providers.MigrationsPg/Migrations/M0123_AddAuthenticationAuditContextPg.cs
  • Repositories/Resgrid.Repositories.DataRepository/IdentityRepository.cs
  • Repositories/Resgrid.Repositories.DataRepository/Modules/ApiDataModule.cs
  • Repositories/Resgrid.Repositories.DataRepository/Modules/DataModule.cs
  • Repositories/Resgrid.Repositories.DataRepository/Modules/NonWebDataModule.cs
  • Repositories/Resgrid.Repositories.DataRepository/Modules/TestingDataModule.cs
  • Repositories/Resgrid.Repositories.DataRepository/UserExternalIdentityLinksRepository.cs
  • Repositories/Resgrid.Repositories.DataRepository/UserSessionsRepository.cs
  • Tools/Resgrid.Console/Commands/ResetPasswordCommand.cs
  • Web/Resgrid.Web.Eventing/Hubs/EventingHub.cs
  • Web/Resgrid.Web.Eventing/Middleware/SessionValidationHubFilter.cs
  • Web/Resgrid.Web.Eventing/Middleware/SessionValidationMiddleware.cs
  • Web/Resgrid.Web.Eventing/Startup.cs
  • Web/Resgrid.Web.Mcp/Startup.cs
  • Web/Resgrid.Web.Services/Controllers/v4/ConnectController.cs
  • Web/Resgrid.Web.Services/Controllers/v4/ScimController.cs
  • Web/Resgrid.Web.Services/Controllers/v4/SessionsController.cs
  • Web/Resgrid.Web.Services/Helpers/IpAddressHelper.cs
  • Web/Resgrid.Web.Services/Hubs/EventingHub.cs
  • Web/Resgrid.Web.Services/Middleware/SessionValidationHubFilter.cs
  • Web/Resgrid.Web.Services/Middleware/SessionValidationMiddleware.cs
  • Web/Resgrid.Web.Services/Resgrid.Web.Services.xml
  • Web/Resgrid.Web.Services/Startup.cs
  • Web/Resgrid.Web/Areas/User/Apps/src/components/chat/chatHub.ts
  • Web/Resgrid.Web/Areas/User/Apps/src/runtime/api.ts
  • Web/Resgrid.Web/Areas/User/Apps/src/runtime/auth.ts
  • Web/Resgrid.Web/Areas/User/Apps/src/runtime/browserConfig.ts
  • Web/Resgrid.Web/Areas/User/Apps/src/runtime/eventingToken.ts
  • Web/Resgrid.Web/Areas/User/Apps/src/runtime/signalr.ts
  • Web/Resgrid.Web/Areas/User/Controllers/AccountController.cs
  • Web/Resgrid.Web/Areas/User/Controllers/AccountSecurityController.cs
  • Web/Resgrid.Web/Areas/User/Controllers/DepartmentController.cs
  • Web/Resgrid.Web/Areas/User/Controllers/HomeController.cs
  • Web/Resgrid.Web/Areas/User/Controllers/PersonnelController.cs
  • Web/Resgrid.Web/Areas/User/Controllers/ProfileController.cs
  • Web/Resgrid.Web/Areas/User/Models/DepartmentSettingsModel.cs
  • Web/Resgrid.Web/Areas/User/Models/EditProfileModel.cs
  • Web/Resgrid.Web/Areas/User/Models/Personnel/PersonnelForJson.cs
  • Web/Resgrid.Web/Areas/User/Models/PersonnelModel.cs
  • Web/Resgrid.Web/Areas/User/Models/Profile/ResetPasswordForUserView.cs
  • Web/Resgrid.Web/Areas/User/Models/Security/AccountCredentialViews.cs
  • Web/Resgrid.Web/Areas/User/Models/Security/ActiveSessionsView.cs
  • Web/Resgrid.Web/Areas/User/Views/AccountSecurity/ChangePassword.cshtml
  • Web/Resgrid.Web/Areas/User/Views/AccountSecurity/ChangeUsername.cshtml
  • Web/Resgrid.Web/Areas/User/Views/AccountSecurity/Sessions.cshtml
  • Web/Resgrid.Web/Areas/User/Views/Department/Settings.cshtml
  • Web/Resgrid.Web/Areas/User/Views/Home/EditUserProfile.cshtml
  • Web/Resgrid.Web/Areas/User/Views/Personnel/Index.cshtml
  • Web/Resgrid.Web/Areas/User/Views/Profile/ResetPasswordForUser.cshtml
  • Web/Resgrid.Web/Areas/User/Views/Profile/YourDepartments.cshtml
  • Web/Resgrid.Web/Areas/User/Views/Shared/_Navigation.cshtml
  • Web/Resgrid.Web/Areas/User/Views/Shared/_TopNavbar.cshtml
  • Web/Resgrid.Web/Areas/User/Views/Shared/_UserLayout.cshtml
  • Web/Resgrid.Web/Areas/User/Views/WeatherAlerts/History.cshtml
  • Web/Resgrid.Web/Areas/User/Views/WeatherAlerts/Index.cshtml
  • Web/Resgrid.Web/Areas/User/Views/WeatherAlerts/Settings.cshtml
  • Web/Resgrid.Web/Areas/User/Views/WeatherAlerts/Zones.cshtml
  • Web/Resgrid.Web/Controllers/AccountController.cs
  • Web/Resgrid.Web/Controllers/WebApiBffController.cs
  • Web/Resgrid.Web/Helpers/ApiAuthHelper.cs
  • Web/Resgrid.Web/Helpers/IpAddressHelper.cs
  • Web/Resgrid.Web/Helpers/JavasriptHelpers.cs
  • Web/Resgrid.Web/Middleware/SessionValidationMiddleware.cs
  • Web/Resgrid.Web/Models/AccountViewModels/ForcePasswordChangeViewModel.cs
  • Web/Resgrid.Web/Models/AccountViewModels/ResetPasswordViewModel.cs
  • Web/Resgrid.Web/Startup.cs
  • Web/Resgrid.Web/Views/Account/ForcePasswordChange.cshtml
  • Web/Resgrid.Web/Views/Account/ResetPassword.cshtml
  • Web/Resgrid.Web/Views/Shared/Error.cshtml
  • Web/Resgrid.Web/Views/Shared/Unauthorized.cshtml
  • Web/Resgrid.Web/Views/Shared/_RecoveryLayout.cshtml
  • Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.addArchivedCall.js
  • Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.editcall.js
  • Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.newcall.js
  • Web/Resgrid.Web/wwwroot/js/app/internal/profile/resgrid.profile.yourdepartments.js
  • Web/Resgrid.Web/wwwroot/js/app/internal/routes/resgrid.routes.edit.js
  • Web/Resgrid.Web/wwwroot/js/app/internal/routes/resgrid.routes.new.js
  • Web/Resgrid.Web/wwwroot/js/app/public/resgrid.password-recovery.js
  • Workers/Resgrid.Workers.Console/Tasks/CleanOIDCScheduleTask.cs

Comment @coderabbitai help to get the list of available commands.

@Resgrid-Bot

Resgrid-Bot commented Aug 21, 2026

Copy link
Copy Markdown

Code Review Could Not Complete ⚠️

The review failed before suggestions could be generated.

Reason: Rate limit reached on the provider (openai). Try again in a few minutes.

After fixing the issue, comment @kody review on this PR to re-run the review.

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the @kody start-review command at the root of your PR.

  • Validate Business Logic: Ask Kody to validate your code against business rules by adding a comment with the @kody -v business-logic command.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug
Performance
Security
Business Logic

Access your configuration settings here.

/// Comma-separated, registered client IDs allowed to receive the longer mobile
/// refresh-token lifetime. Anonymous requests and caller-supplied scopes never qualify.
/// </summary>
public static string TrustedLongLivedClientIds = "";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules high

Mutable static configuration in Core/Resgrid.Config/OidcConfig.cs and the related declarations at Web/Resgrid.Web/Models/AccountViewModels/ResetPasswordViewModel.cs:20-20, Core/Resgrid.Config/SessionSecurityConfig.cs:7-7, Core/Resgrid.Config/SessionSecurityConfig.cs:8-8, Core/Resgrid.Config/SessionSecurityConfig.cs:9-9, Core/Resgrid.Config/SessionSecurityConfig.cs:12-12, Core/Resgrid.Config/SessionSecurityConfig.cs:13-13, Core/Resgrid.Config/SessionSecurityConfig.cs:14-14, Core/Resgrid.Config/SessionSecurityConfig.cs:15-15, Core/Resgrid.Config/SessionSecurityConfig.cs:16-16, Core/Resgrid.Config/SessionSecurityConfig.cs:17-17, Core/Resgrid.Config/SessionSecurityConfig.cs:18-18, Core/Resgrid.Config/SessionSecurityConfig.cs:19-19, Core/Resgrid.Config/SessionSecurityConfig.cs:20-20, Core/Resgrid.Config/SessionSecurityConfig.cs:22-22, and Core/Resgrid.Services/DepartmentSettingsService.cs:27-27 obscures immutability and permits accidental reassignment. Mark TrustedLongLivedClientIds as readonly where the value is only supplied through configuration or bootstrap.

Kody rule violation: Use `readonly` or `const` for Immutable Data

public static readonly string TrustedLongLivedClientIds = string.Empty;
Prompt for LLM

File Core/Resgrid.Config/OidcConfig.cs:

Line 28:

Mutable static configuration in `Core/Resgrid.Config/OidcConfig.cs` and the related declarations at `Web/Resgrid.Web/Models/AccountViewModels/ResetPasswordViewModel.cs:20-20`, `Core/Resgrid.Config/SessionSecurityConfig.cs:7-7`, `Core/Resgrid.Config/SessionSecurityConfig.cs:8-8`, `Core/Resgrid.Config/SessionSecurityConfig.cs:9-9`, `Core/Resgrid.Config/SessionSecurityConfig.cs:12-12`, `Core/Resgrid.Config/SessionSecurityConfig.cs:13-13`, `Core/Resgrid.Config/SessionSecurityConfig.cs:14-14`, `Core/Resgrid.Config/SessionSecurityConfig.cs:15-15`, `Core/Resgrid.Config/SessionSecurityConfig.cs:16-16`, `Core/Resgrid.Config/SessionSecurityConfig.cs:17-17`, `Core/Resgrid.Config/SessionSecurityConfig.cs:18-18`, `Core/Resgrid.Config/SessionSecurityConfig.cs:19-19`, `Core/Resgrid.Config/SessionSecurityConfig.cs:20-20`, `Core/Resgrid.Config/SessionSecurityConfig.cs:22-22`, and `Core/Resgrid.Services/DepartmentSettingsService.cs:27-27` obscures immutability and permits accidental reassignment. Mark `TrustedLongLivedClientIds` as `readonly` where the value is only supplied through configuration or bootstrap.

Suggested Code:

		public static readonly string TrustedLongLivedClientIds = string.Empty;

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

ModerationRequestCompleted,
ModerationEvidenceDownloaded
ModerationEvidenceDownloaded,
PasswordResetByAdministrator,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules high

Security audit coverage gap in Core/Resgrid.Model/AuditLogTypes.cs for PasswordResetByAdministrator and at Core/Resgrid.Model/AuditLogTypes.cs:196-196 can leave a privileged action without the required immutable and tamper-evident record structure. Ensure this audit action writes timestamp, actor.user_id, actor.role, action, resource.id, result, trace_id, ip, and user_agent, and store the records in append-only or WORM-backed logging forwarded to the SIEM.

Kody rule violation: Emit tamper-evident audit logs with required fields

Prompt for LLM

File Core/Resgrid.Model/AuditLogTypes.cs:

Line 195:

Security audit coverage gap in `Core/Resgrid.Model/AuditLogTypes.cs` for `PasswordResetByAdministrator` and at `Core/Resgrid.Model/AuditLogTypes.cs:196-196` can leave a privileged action without the required immutable and tamper-evident record structure. Ensure this audit action writes `timestamp`, `actor.user_id`, `actor.role`, `action`, `resource.id`, `result`, `trace_id`, `ip`, and `user_agent`, and store the records in append-only or WORM-backed logging forwarded to the SIEM.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

ModerationRequestCompleted,
ModerationEvidenceDownloaded
ModerationEvidenceDownloaded,
PasswordResetByAdministrator,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules high

Privileged password reset flow in Core/Resgrid.Model/AuditLogTypes.cs for PasswordResetByAdministrator lacks a step-up authentication requirement. Require MFA re-authentication within the last 5 minutes for this action and record mfa_verified_at in the audit log.

Kody rule violation: Require step-up MFA for privileged operations

Prompt for LLM

File Core/Resgrid.Model/AuditLogTypes.cs:

Line 195:

Privileged password reset flow in `Core/Resgrid.Model/AuditLogTypes.cs` for `PasswordResetByAdministrator` lacks a step-up authentication requirement. Require MFA re-authentication within the last 5 minutes for this action and record `mfa_verified_at` in the audit log.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

Task<bool> SendPasswordResetMail(string name, string password, string userName, string email, string departmentName);
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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules high

Sensitive metadata propagation in Core/Resgrid.Model/Providers/IEmailProvider.cs and the related usages at Providers/Resgrid.Providers.Email/Template/PasswordRecovery.html:26-26, Providers/Resgrid.Providers.Email/Template/PasswordChangedByAdministrator.html:25-25, Web/Resgrid.Web/Middleware/SessionValidationMiddleware.cs:101-101, Web/Resgrid.Web/Middleware/SessionValidationMiddleware.cs:131-131, Providers/Resgrid.Providers.Email/Template/PasswordRecovery.html:27-27, Providers/Resgrid.Providers.Email/Template/PasswordRecovery.html:38-38, Web/Resgrid.Web/Controllers/AccountController.cs:145-145, Core/Resgrid.Model/UserSession.cs:27-27, Web/Resgrid.Web.Services/Controllers/v4/ConnectController.cs:302-302, Providers/Resgrid.Providers.Email/Template/PasswordRecovery.html:40-40, Web/Resgrid.Web.Services/Controllers/v4/ConnectController.cs:309-309, Providers/Resgrid.Providers.Email/Template/PasswordRecovery.html:39-39, and Web/Resgrid.Web.Services/Controllers/v4/ConnectController.cs:327-327 increases the chance that ipAddress and userAgent are logged or persisted outside a dedicated audit path. Remove raw ipAddress and userAgent from the mail provider contract and pass only minimal non-identifying context such as a trace or request ID when needed.

Kody rule violation: Do not log PHI; mask and drop sensitive fields

string resetUrl, string requestedOn, bool isSsoManaged);
Prompt for LLM

File Core/Resgrid.Model/Providers/IEmailProvider.cs:

Line 12:

Sensitive metadata propagation in `Core/Resgrid.Model/Providers/IEmailProvider.cs` and the related usages at `Providers/Resgrid.Providers.Email/Template/PasswordRecovery.html:26-26`, `Providers/Resgrid.Providers.Email/Template/PasswordChangedByAdministrator.html:25-25`, `Web/Resgrid.Web/Middleware/SessionValidationMiddleware.cs:101-101`, `Web/Resgrid.Web/Middleware/SessionValidationMiddleware.cs:131-131`, `Providers/Resgrid.Providers.Email/Template/PasswordRecovery.html:27-27`, `Providers/Resgrid.Providers.Email/Template/PasswordRecovery.html:38-38`, `Web/Resgrid.Web/Controllers/AccountController.cs:145-145`, `Core/Resgrid.Model/UserSession.cs:27-27`, `Web/Resgrid.Web.Services/Controllers/v4/ConnectController.cs:302-302`, `Providers/Resgrid.Providers.Email/Template/PasswordRecovery.html:40-40`, `Web/Resgrid.Web.Services/Controllers/v4/ConnectController.cs:309-309`, `Providers/Resgrid.Providers.Email/Template/PasswordRecovery.html:39-39`, and `Web/Resgrid.Web.Services/Controllers/v4/ConnectController.cs:327-327` increases the chance that `ipAddress` and `userAgent` are logged or persisted outside a dedicated audit path. Remove raw `ipAddress` and `userAgent` from the mail provider contract and pass only minimal non-identifying context such as a trace or request ID when needed.

Suggested Code:

			string resetUrl, string requestedOn, bool isSsoManaged);

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

Task<bool> SendPasswordResetMail(string name, string password, string userName, string email, string departmentName);
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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules high

PII propagation risk in Core/Resgrid.Model/Providers/IEmailProvider.cs and the related usages at Web/Resgrid.Web/Controllers/AccountController.cs:145-145, Web/Resgrid.Web.Services/Middleware/SessionValidationHubFilter.cs:86-86, Web/Resgrid.Web.Services/Middleware/SessionValidationHubFilter.cs:94-94, Providers/Resgrid.Providers.Email/Template/PasswordChangedByAdministrator.html:26-26, Providers/Resgrid.Providers.Email/Template/PasswordChangedByAdministrator.html:25-25, Web/Resgrid.Web/Middleware/SessionValidationMiddleware.cs:101-101, Web/Resgrid.Web/Middleware/SessionValidationMiddleware.cs:131-131, Core/Resgrid.Model/UserSession.cs:39-39, Core/Resgrid.Model/UserSession.cs:40-40, and Web/Resgrid.Web.Services/Controllers/v4/SessionsController.cs:89-89 increases the likelihood that raw ipAddress and userAgent reach logs or telemetry without redaction. Keep those values out of the email provider interface, or replace them with redacted or hashed forms or a request or trace ID handled through structured logging.

Kody rule violation: Redact PII in logs and metrics by default

string resetUrl, string requestedOn, bool isSsoManaged);
Prompt for LLM

File Core/Resgrid.Model/Providers/IEmailProvider.cs:

Line 12:

PII propagation risk in `Core/Resgrid.Model/Providers/IEmailProvider.cs` and the related usages at `Web/Resgrid.Web/Controllers/AccountController.cs:145-145`, `Web/Resgrid.Web.Services/Middleware/SessionValidationHubFilter.cs:86-86`, `Web/Resgrid.Web.Services/Middleware/SessionValidationHubFilter.cs:94-94`, `Providers/Resgrid.Providers.Email/Template/PasswordChangedByAdministrator.html:26-26`, `Providers/Resgrid.Providers.Email/Template/PasswordChangedByAdministrator.html:25-25`, `Web/Resgrid.Web/Middleware/SessionValidationMiddleware.cs:101-101`, `Web/Resgrid.Web/Middleware/SessionValidationMiddleware.cs:131-131`, `Core/Resgrid.Model/UserSession.cs:39-39`, `Core/Resgrid.Model/UserSession.cs:40-40`, and `Web/Resgrid.Web.Services/Controllers/v4/SessionsController.cs:89-89` increases the likelihood that raw `ipAddress` and `userAgent` reach logs or telemetry without redaction. Keep those values out of the email provider interface, or replace them with redacted or hashed forms or a request or trace ID handled through structured logging.

Suggested Code:

			string resetUrl, string requestedOn, bool isSsoManaged);

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

}
catch (Exception ex)
{
Logging.LogException(ex);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules high

Diagnostic context loss in Core/Resgrid.Services/CommunicationService.cs and the related sites including Providers/Resgrid.Providers.Bus/SignalrProvider.cs:149-149, Core/Resgrid.Services/EmailService.cs:92-92, and Tools/Resgrid.Console/Commands/ResetPasswordCommand.cs:48-48 leaves Logging.LogException(ex); without the operation or identifiers needed for correlation. Log the failure with structured fields such as operation = "SendToUserAsync", receivingUserId, departmentId, and messageId alongside ex.

Kody rule violation: Include error context in structured logs

logger.Error("Chatbot outbound send failed", new { operation = "SendToUserAsync", receivingUserId = message.ReceivingUserId, departmentId, messageId = message.MessageId, error = ex });
Prompt for LLM

File Core/Resgrid.Services/CommunicationService.cs:

Line 149:

Diagnostic context loss in `Core/Resgrid.Services/CommunicationService.cs` and the related sites including `Providers/Resgrid.Providers.Bus/SignalrProvider.cs:149-149`, `Core/Resgrid.Services/EmailService.cs:92-92`, and `Tools/Resgrid.Console/Commands/ResetPasswordCommand.cs:48-48` leaves `Logging.LogException(ex);` without the operation or identifiers needed for correlation. Log the failure with structured fields such as `operation = "SendToUserAsync"`, `receivingUserId`, `departmentId`, and `messageId` alongside `ex`.

Suggested Code:

					logger.Error("Chatbot outbound send failed", new { operation = "SendToUserAsync", receivingUserId = message.ReceivingUserId, departmentId, messageId = message.MessageId, error = ex });

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

// Soft-delete the membership last (this also writes the audit event and clears caches).
var member = await _departmentsService.DeleteUserAsync(departmentId, userId, revokingUserId, cancellationToken);
if (member != null && member.IsDeleted)
await _userSessionService.RevokeDepartmentSessionsAsync(userId, departmentId,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules high

Unobserved external call failure in Core/Resgrid.Services/DeleteService.cs around _userSessionService.RevokeDepartmentSessionsAsync(userId, departmentId, ...) can hide session revocation errors and remove operational context. Wrap the call in try/catch, log Operation = "RevokeDepartmentSessionsAsync", UserId = userId, and DepartmentId = departmentId, then rethrow or map the exception deliberately.

Kody rule violation: Add try-catch blocks for external calls

try
{
	await _userSessionService.RevokeDepartmentSessionsAsync(userId, departmentId,
		UserSessionRevocationReason.MembershipDisabled, cancellationToken);
}
catch (Exception ex)
{
	_logService.LogException(ex, new { Operation = "RevokeDepartmentSessionsAsync", UserId = userId, DepartmentId = departmentId });
	throw;
}
Prompt for LLM

File Core/Resgrid.Services/DeleteService.cs:

Line 122:

Unobserved external call failure in `Core/Resgrid.Services/DeleteService.cs` around `_userSessionService.RevokeDepartmentSessionsAsync(userId, departmentId, ...)` can hide session revocation errors and remove operational context. Wrap the call in `try/catch`, log `Operation = "RevokeDepartmentSessionsAsync"`, `UserId = userId`, and `DepartmentId = departmentId`, then rethrow or map the exception deliberately.

Suggested Code:

				try
				{
					await _userSessionService.RevokeDepartmentSessionsAsync(userId, departmentId,
						UserSessionRevocationReason.MembershipDisabled, cancellationToken);
				}
				catch (Exception ex)
				{
					_logService.LogException(ex, new { Operation = "RevokeDepartmentSessionsAsync", UserId = userId, DepartmentId = departmentId });
					throw;
				}

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.


foreach (var departmentLinks in links.GroupBy(link => link.DepartmentId))
{
var configs = (await _ssoConfigRepository.GetAllByDepartmentIdAsync(departmentLinks.Key))?.ToList()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules high

N+1 repository access in Core/Resgrid.Services/ExternalIdentityLinkService.cs awaits _ssoConfigRepository.GetAllByDepartmentIdAsync(departmentLinks.Key) inside the group iteration, multiplying database calls by department count. Prefetch configs for all distinct DepartmentId values and iterate over the in-memory results after Task.WhenAll completes.

Kody rule violation: Detect N+1 style queries and suggest batching

var departmentIds = links.Select(link => link.DepartmentId).Distinct().ToList();
var configTasks = departmentIds.ToDictionary(id => id, id => _ssoConfigRepository.GetAllByDepartmentIdAsync(id));
await Task.WhenAll(configTasks.Values);
foreach (var departmentLinks in links.GroupBy(link => link.DepartmentId))
{
	var configs = (await configTasks[departmentLinks.Key])?.ToList() ?? new System.Collections.Generic.List<DepartmentSsoConfig>();
	// ...
}
Prompt for LLM

File Core/Resgrid.Services/ExternalIdentityLinkService.cs:

Line 96:

N+1 repository access in `Core/Resgrid.Services/ExternalIdentityLinkService.cs` awaits `_ssoConfigRepository.GetAllByDepartmentIdAsync(departmentLinks.Key)` inside the group iteration, multiplying database calls by department count. Prefetch configs for all distinct `DepartmentId` values and iterate over the in-memory results after `Task.WhenAll` completes.

Suggested Code:

			var departmentIds = links.Select(link => link.DepartmentId).Distinct().ToList();
			var configTasks = departmentIds.ToDictionary(id => id, id => _ssoConfigRepository.GetAllByDepartmentIdAsync(id));
			await Task.WhenAll(configTasks.Values);
			foreach (var departmentLinks in links.GroupBy(link => link.DepartmentId))
			{
				var configs = (await configTasks[departmentLinks.Key])?.ToList() ?? new System.Collections.Generic.List<DepartmentSsoConfig>();
				// ...
			}

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

public async Task<PasswordRecoveryRequest> GetAsync(string token, CancellationToken cancellationToken = default)
{
if (string.IsNullOrWhiteSpace(token))
return null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules critical

Null result contract in Core/Resgrid.Services/PasswordRecoveryService.cs and at Core/Resgrid.Services/PasswordRecoveryService.cs:71-71, Core/Resgrid.Services/PasswordRecoveryService.cs:80-80, and Core/Resgrid.Services/PasswordRecoveryService.cs:95-96 returns null from an async Task<T> method, which weakens call-site guarantees and increases null-handling risk. Return an explicit result object, option pattern, or clearly nullable result value instead of a bare null.

Kody rule violation: Avoid Returning Null in Non-Async Task Methods

return null; // If keeping Task<PasswordRecoveryRequest>, consider Task.FromResult<PasswordRecoveryRequest>(null) only for non-async methods; otherwise keep async and return a value object or nullable result explicitly.
Prompt for LLM

File Core/Resgrid.Services/PasswordRecoveryService.cs:

Line 67:

Null result contract in `Core/Resgrid.Services/PasswordRecoveryService.cs` and at `Core/Resgrid.Services/PasswordRecoveryService.cs:71-71`, `Core/Resgrid.Services/PasswordRecoveryService.cs:80-80`, and `Core/Resgrid.Services/PasswordRecoveryService.cs:95-96` returns `null` from an `async Task<T>` method, which weakens call-site guarantees and increases null-handling risk. Return an explicit result object, option pattern, or clearly nullable result value instead of a bare `null`.

Suggested Code:

				return null; // If keeping Task<PasswordRecoveryRequest>, consider Task.FromResult<PasswordRecoveryRequest>(null) only for non-async methods; otherwise keep async and return a value object or nullable result explicitly.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

// bypass SSL certificate validation
clientHandler.ServerCertificateCustomValidationCallback +=
(sender, certificate, chain, sslPolicyErrors) => { return true; };
clientHandler.ServerCertificateCustomValidationCallback =

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules critical

Certificate validation bypass in Providers/Resgrid.Providers.Bus/SignalrProvider.cs and at Providers/Resgrid.Providers.Bus/SignalrProvider.cs:136-136 and Web/Resgrid.Web/Startup.cs:144-144 allows TLS server impersonation and interception of supposedly secure traffic. Remove clientHandler.ServerCertificateCustomValidationCallback = certificate skipping so normal SSL/TLS server certificate verification remains enforced.

Kody rule violation: Verify SSL/TLS Server Certificates

Prompt for LLM

File Providers/Resgrid.Providers.Bus/SignalrProvider.cs:

Line 104:

Certificate validation bypass in `Providers/Resgrid.Providers.Bus/SignalrProvider.cs` and at `Providers/Resgrid.Providers.Bus/SignalrProvider.cs:136-136` and `Web/Resgrid.Web/Startup.cs:144-144` allows TLS server impersonation and interception of supposedly secure traffic. Remove `clientHandler.ServerCertificateCustomValidationCallback =` certificate skipping so normal SSL/TLS server certificate verification remains enforced.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

newEmail.To.Add(email);
return await _emailSender.Send(newEmail);
}
catch (Exception)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules high

Exception suppression in Providers/Resgrid.Providers.Email/PostmarkTemplateProvider.cs and Core/Resgrid.Services/PasswordRecoveryService.cs:78-80 uses catch (Exception) without preserving failure details, which hides operational errors. Capture the exception as ex, log it with contextual fields such as email, departmentName, and userName, and then return a deliberate failure result or rethrow.

Kody rule violation: Avoid empty catch blocks

catch (Exception ex)
{
    _logger.LogError(ex, "Failed to send password changed by administrator email", new { email, departmentName, userName });
    return false;
}
Prompt for LLM

File Providers/Resgrid.Providers.Email/PostmarkTemplateProvider.cs:

Line 374:

Exception suppression in `Providers/Resgrid.Providers.Email/PostmarkTemplateProvider.cs` and `Core/Resgrid.Services/PasswordRecoveryService.cs:78-80` uses `catch (Exception)` without preserving failure details, which hides operational errors. Capture the exception as `ex`, log it with contextual fields such as `email`, `departmentName`, and `userName`, and then return a deliberate failure result or rethrow.

Suggested Code:

catch (Exception ex)
{
    _logger.LogError(ex, "Failed to send password changed by administrator email", new { email, departmentName, userName });
    return false;
}

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<title>Your Resgrid password was changed</title>
<style>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules high

Global style leakage in Providers/Resgrid.Providers.Email/Template/PasswordChangedByAdministrator.html and Web/Resgrid.Web/Views/Shared/_RecoveryLayout.cshtml:13-13 can make template styles affect unrelated content. Keep the <style> block scoped to the specific email or component styling mechanism instead of introducing unscoped global CSS.

Kody rule violation: Use component-scoped styling

Prompt for LLM

File Providers/Resgrid.Providers.Email/Template/PasswordChangedByAdministrator.html:

Line 7:

Global style leakage in `Providers/Resgrid.Providers.Email/Template/PasswordChangedByAdministrator.html` and `Web/Resgrid.Web/Views/Shared/_RecoveryLayout.cshtml:13-13` can make template styles affect unrelated content. Keep the `<style>` block scoped to the specific email or component styling mechanism instead of introducing unscoped global CSS.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

.WithColumn("unlinkedon").AsDateTime2().Nullable()
.WithColumn("unlinkedbyuserid").AsCustom("citext").Nullable();

Create.Index("ux_userexternalidentitylinks_config_subject")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules critical

Write-lock risk in Providers/Resgrid.Providers.MigrationsPg/Migrations/M0122_AddUserExternalIdentityLinksPg.cs and the related migrations at Providers/Resgrid.Providers.Migrations/Migrations/M0121_AddUserSessions.cs:60-60, Providers/Resgrid.Providers.MigrationsPg/Migrations/M0120_AddUserAuthenticationStatePg.cs:11-11, Providers/Resgrid.Providers.MigrationsPg/Migrations/M0121_AddUserSessionsPg.cs:60-60, Providers/Resgrid.Providers.MigrationsPg/Migrations/M0122_AddUserExternalIdentityLinksPg.cs:36-36, Providers/Resgrid.Providers.Migrations/Migrations/M0122_AddUserExternalIdentityLinks.cs:31-31, Providers/Resgrid.Providers.MigrationsPg/Migrations/M0122_AddUserExternalIdentityLinksPg.cs:41-41, Providers/Resgrid.Providers.Migrations/Migrations/M0122_AddUserExternalIdentityLinks.cs:36-36, and Providers/Resgrid.Providers.Migrations/Migrations/M0122_AddUserExternalIdentityLinks.cs:41-41 creates a PostgreSQL index without an online strategy, which can block writes on large tables. Use a concurrent index build such as CREATE INDEX CONCURRENTLY or the FluentMigrator equivalent .WithOptions().Concurrent() and document rollback constraints if concurrency is unavailable.

Kody rule violation: Block risky database migrations (locking ops, downtime risk)

Create.Index("ux_userexternalidentitylinks_config_subject")
    .OnTable("userexternalidentitylinks")
    .OnColumn("departmentssoconfigid").Ascending()
    .OnColumn("externalsubject").Ascending()
    .WithOptions().Unique()
    .WithOptions().Concurrent();
Prompt for LLM

File Providers/Resgrid.Providers.MigrationsPg/Migrations/M0122_AddUserExternalIdentityLinksPg.cs:

Line 31:

Write-lock risk in `Providers/Resgrid.Providers.MigrationsPg/Migrations/M0122_AddUserExternalIdentityLinksPg.cs` and the related migrations at `Providers/Resgrid.Providers.Migrations/Migrations/M0121_AddUserSessions.cs:60-60`, `Providers/Resgrid.Providers.MigrationsPg/Migrations/M0120_AddUserAuthenticationStatePg.cs:11-11`, `Providers/Resgrid.Providers.MigrationsPg/Migrations/M0121_AddUserSessionsPg.cs:60-60`, `Providers/Resgrid.Providers.MigrationsPg/Migrations/M0122_AddUserExternalIdentityLinksPg.cs:36-36`, `Providers/Resgrid.Providers.Migrations/Migrations/M0122_AddUserExternalIdentityLinks.cs:31-31`, `Providers/Resgrid.Providers.MigrationsPg/Migrations/M0122_AddUserExternalIdentityLinksPg.cs:41-41`, `Providers/Resgrid.Providers.Migrations/Migrations/M0122_AddUserExternalIdentityLinks.cs:36-36`, and `Providers/Resgrid.Providers.Migrations/Migrations/M0122_AddUserExternalIdentityLinks.cs:41-41` creates a PostgreSQL index without an online strategy, which can block writes on large tables. Use a concurrent index build such as `CREATE INDEX CONCURRENTLY` or the FluentMigrator equivalent `.WithOptions().Concurrent()` and document rollback constraints if concurrency is unavailable.

Suggested Code:

Create.Index("ux_userexternalidentitylinks_config_subject")
    .OnTable("userexternalidentitylinks")
    .OnColumn("departmentssoconfigid").Ascending()
    .OnColumn("externalsubject").Ascending()
    .WithOptions().Unique()
    .WithOptions().Concurrent();

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

public override void Up()
{
if (!Schema.Table("systemaudits").Column("targetuserid").Exists())
Alter.Table("systemaudits").AddColumn("targetuserid").AsCustom("citext").Nullable();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules high

Audit query performance gap in Providers/Resgrid.Providers.MigrationsPg/Migrations/M0123_AddAuthenticationAuditContextPg.cs and at Providers/Resgrid.Providers.MigrationsPg/Migrations/M0123_AddAuthenticationAuditContextPg.cs:13-13 and Providers/Resgrid.Providers.MigrationsPg/Migrations/M0123_AddAuthenticationAuditContextPg.cs:15-15 adds targetuserid to systemaudits without a supporting index, which can degrade investigation queries that filter or join on that identifier. Add an index for targetuserid after verifying expected query patterns and execution plans.

Kody rule violation: Add database indexes for query optimization

Prompt for LLM

File Providers/Resgrid.Providers.MigrationsPg/Migrations/M0123_AddAuthenticationAuditContextPg.cs:

Line 11:

Audit query performance gap in `Providers/Resgrid.Providers.MigrationsPg/Migrations/M0123_AddAuthenticationAuditContextPg.cs` and at `Providers/Resgrid.Providers.MigrationsPg/Migrations/M0123_AddAuthenticationAuditContextPg.cs:13-13` and `Providers/Resgrid.Providers.MigrationsPg/Migrations/M0123_AddAuthenticationAuditContextPg.cs:15-15` adds `targetuserid` to `systemaudits` without a supporting index, which can degrade investigation queries that filter or join on that identifier. Add an index for `targetuserid` after verifying expected query patterns and execution plans.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

Assert.That(storedLifetime, Is.EqualTo(TimeSpan.FromMinutes(
Math.Max(5, SessionSecurityConfig.PublicResetLinkLifetimeMinutes))));
var request = JsonConvert.DeserializeObject<PasswordRecoveryRequest>(storedValue);
Assert.That(request.UserId, Is.EqualTo("user-1"));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules high

Null dereference risk in Tests/Resgrid.Tests/Services/PasswordRecoveryServiceTests.cs and the related sites including Web/Resgrid.Web/Controllers/AccountController.cs:750-750, Web/Resgrid.Web/Controllers/AccountController.cs:810-810, and Tools/Resgrid.Console/Commands/ResetPasswordCommand.cs:46-46 reads request.UserId even though deserialization may return null. Assert that request is non-null before dereferencing, or use null-safe access such as request?.UserId.

Kody rule violation: Add null checks to prevent NullReferenceException

Assert.That(request, Is.Not.Null);
Assert.That(request?.UserId, Is.EqualTo("user-1"));
Prompt for LLM

File Tests/Resgrid.Tests/Services/PasswordRecoveryServiceTests.cs:

Line 43:

Null dereference risk in `Tests/Resgrid.Tests/Services/PasswordRecoveryServiceTests.cs` and the related sites including `Web/Resgrid.Web/Controllers/AccountController.cs:750-750`, `Web/Resgrid.Web/Controllers/AccountController.cs:810-810`, and `Tools/Resgrid.Console/Commands/ResetPasswordCommand.cs:46-46` reads `request.UserId` even though deserialization may return `null`. Assert that `request` is non-null before dereferencing, or use null-safe access such as `request?.UserId`.

Suggested Code:

			Assert.That(request, Is.Not.Null);
			Assert.That(request?.UserId, Is.EqualTo("user-1"));

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

}
else
{
logger.LogError("Failed to reset the Password: " + result.Errors.FirstOrDefault()?.Description);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules high

Invariant mismatch in Tools/Resgrid.Console/Commands/ResetPasswordCommand.cs uses result.Errors.FirstOrDefault()?.Description, which implies the error collection may be empty even though a failure path is expected to contain at least one entry. Use First() if that invariant is guaranteed so the code expresses the contract explicitly.

Kody rule violation: Use `First`/`Single` Instead of `FirstOrDefault`/`SingleOrDefault` for Non-Empty Collections

logger.LogError("Failed to reset the Password: " + result.Errors.First()?.Description);
Prompt for LLM

File Tools/Resgrid.Console/Commands/ResetPasswordCommand.cs:

Line 78:

Invariant mismatch in `Tools/Resgrid.Console/Commands/ResetPasswordCommand.cs` uses `result.Errors.FirstOrDefault()?.Description`, which implies the error collection may be empty even though a failure path is expected to contain at least one entry. Use `First()` if that invariant is guaranteed so the code expresses the contract explicitly.

Suggested Code:

logger.LogError("Failed to reset the Password: " + result.Errors.First()?.Description);

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

if (call == null || call.DepartmentId != GetDepartmentId())
throw new HubException("Not authorized for this call.");

await Groups.AddToGroupAsync(Context.ConnectionId, $"CallUpdated:${callId}");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules high

Group subscription lifecycle leak in Web/Resgrid.Web.Eventing/Hubs/EventingHub.cs adds the connection to Groups.AddToGroupAsync(Context.ConnectionId, $"CallUpdated:${callId}") without a deterministic removal path. Register cleanup on Context.ConnectionAborted or unsubscribe so Groups.RemoveFromGroupAsync always runs when the connection closes.

Kody rule violation: Provide error handlers to subscription/listener APIs

await Groups.AddToGroupAsync(Context.ConnectionId, $"CallUpdated:${callId}");
Context.ConnectionAborted.Register(() =>
{
	_ = Groups.RemoveFromGroupAsync(Context.ConnectionId, $"CallUpdated:${callId}");
});
Prompt for LLM

File Web/Resgrid.Web.Eventing/Hubs/EventingHub.cs:

Line 75:

Group subscription lifecycle leak in `Web/Resgrid.Web.Eventing/Hubs/EventingHub.cs` adds the connection to `Groups.AddToGroupAsync(Context.ConnectionId, $"CallUpdated:${callId}")` without a deterministic removal path. Register cleanup on `Context.ConnectionAborted` or unsubscribe so `Groups.RemoveFromGroupAsync` always runs when the connection closes.

Suggested Code:

			await Groups.AddToGroupAsync(Context.ConnectionId, $"CallUpdated:${callId}");
			Context.ConnectionAborted.Register(() =>
			{
				_ = Groups.RemoveFromGroupAsync(Context.ConnectionId, $"CallUpdated:${callId}");
			});

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<meta name="request-verification-token" content="@Antiforgery.GetAndStoreTokens(Context).RequestToken" />

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules high

Security token exposure in Web/Resgrid.Web/Areas/User/Views/Shared/_UserLayout.cshtml and Web/Resgrid.Web.Eventing/Hubs/EventingHub.cs:71-71 renders @Antiforgery.GetAndStoreTokens(Context).RequestToken into a globally readable meta tag, expanding client-side access to a server-generated token. Avoid emitting this antiforgery value into shared layout markup unless the client-side use is required and tightly scoped.

Kody rule violation: Never expose secrets to the client

Prompt for LLM

File Web/Resgrid.Web/Areas/User/Views/Shared/_UserLayout.cshtml:

Line 11:

Security token exposure in `Web/Resgrid.Web/Areas/User/Views/Shared/_UserLayout.cshtml` and `Web/Resgrid.Web.Eventing/Hubs/EventingHub.cs:71-71` renders `@Antiforgery.GetAndStoreTokens(Context).RequestToken` into a globally readable meta tag, expanding client-side access to a server-generated token. Avoid emitting this antiforgery value into shared layout markup unless the client-side use is required and tightly scoped.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

} catch (e) {}
return '';
function getAntiForgeryToken() {
return document.querySelector('meta[name="request-verification-token"]').content;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules high

Null DOM access in Web/Resgrid.Web/Areas/User/Views/WeatherAlerts/Zones.cshtml and the related usages at Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.editcall.js:351-351, Web/Resgrid.Web/Areas/User/Controllers/ProfileController.cs:1353-1353, Web/Resgrid.Web/Areas/User/Controllers/ProfileController.cs:1358-1358, Web/Resgrid.Web/Areas/User/Views/WeatherAlerts/Settings.cshtml:304-304, Web/Resgrid.Web/Areas/User/Controllers/ProfileController.cs:1366-1366, and Web/Resgrid.Web/Areas/User/Controllers/ProfileController.cs:1361-1361 assumes document.querySelector('meta[name="request-verification-token"]') always returns an element, which can throw at .content. Use optional chaining with a fallback, such as ?.content ?? '', to avoid runtime errors when the meta tag is missing.

Kody rule violation: Add null checks before accessing properties

return document.querySelector('meta[name="request-verification-token"]')?.content ?? '';
Prompt for LLM

File Web/Resgrid.Web/Areas/User/Views/WeatherAlerts/Zones.cshtml:

Line 166:

Null DOM access in `Web/Resgrid.Web/Areas/User/Views/WeatherAlerts/Zones.cshtml` and the related usages at `Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.editcall.js:351-351`, `Web/Resgrid.Web/Areas/User/Controllers/ProfileController.cs:1353-1353`, `Web/Resgrid.Web/Areas/User/Controllers/ProfileController.cs:1358-1358`, `Web/Resgrid.Web/Areas/User/Views/WeatherAlerts/Settings.cshtml:304-304`, `Web/Resgrid.Web/Areas/User/Controllers/ProfileController.cs:1366-1366`, and `Web/Resgrid.Web/Areas/User/Controllers/ProfileController.cs:1361-1361` assumes `document.querySelector('meta[name="request-verification-token"]')` always returns an element, which can throw at `.content`. Use optional chaining with a fallback, such as `?.content ?? ''`, to avoid runtime errors when the meta tag is missing.

Suggested Code:

return document.querySelector('meta[name="request-verification-token"]')?.content ?? '';

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

Successful = false,
IpAddress = IpAddressHelper.GetRequestIP(Request, true),
ServerName = Environment.MachineName,
Data = $"Web LogOn blocked by SSO policy {Request.Headers["User-Agent"]} {Request.Headers["Accept-Language"]}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules high

Log payload exposure in Web/Resgrid.Web/Controllers/AccountController.cs and the related usages at Web/Resgrid.Web.Services/Middleware/SessionValidationHubFilter.cs:86-86, Web/Resgrid.Web.Services/Controllers/v4/ConnectController.cs:302-302, Web/Resgrid.Web.Services/Middleware/SessionValidationHubFilter.cs:94-94, Providers/Resgrid.Providers.Email/Template/PasswordChangedByAdministrator.html:25-25, Providers/Resgrid.Providers.Email/Template/PasswordChangedByAdministrator.html:26-26, Web/Resgrid.Web.Services/Controllers/v4/ConnectController.cs:309-309, Web/Resgrid.Web/Middleware/SessionValidationMiddleware.cs:101-101, Web/Resgrid.Web/Middleware/SessionValidationMiddleware.cs:131-131, Core/Resgrid.Model/UserSession.cs:39-39, Core/Resgrid.Model/UserSession.cs:40-40, Web/Resgrid.Web.Services/Controllers/v4/ConnectController.cs:327-327, and Web/Resgrid.Web.Services/Controllers/v4/SessionsController.cs:89-89 records raw Request.Headers["User-Agent"] and Request.Headers["Accept-Language"], which may contain personal data. Log only non-sensitive context and use HttpContext.TraceIdentifier or other correlation metadata instead of full header values.

Kody rule violation: Mask PII and secrets in logs

Data = "Web LogOn blocked by SSO policy",
CorrelationId = HttpContext.TraceIdentifier
Prompt for LLM

File Web/Resgrid.Web/Controllers/AccountController.cs:

Line 145:

Log payload exposure in `Web/Resgrid.Web/Controllers/AccountController.cs` and the related usages at `Web/Resgrid.Web.Services/Middleware/SessionValidationHubFilter.cs:86-86`, `Web/Resgrid.Web.Services/Controllers/v4/ConnectController.cs:302-302`, `Web/Resgrid.Web.Services/Middleware/SessionValidationHubFilter.cs:94-94`, `Providers/Resgrid.Providers.Email/Template/PasswordChangedByAdministrator.html:25-25`, `Providers/Resgrid.Providers.Email/Template/PasswordChangedByAdministrator.html:26-26`, `Web/Resgrid.Web.Services/Controllers/v4/ConnectController.cs:309-309`, `Web/Resgrid.Web/Middleware/SessionValidationMiddleware.cs:101-101`, `Web/Resgrid.Web/Middleware/SessionValidationMiddleware.cs:131-131`, `Core/Resgrid.Model/UserSession.cs:39-39`, `Core/Resgrid.Model/UserSession.cs:40-40`, `Web/Resgrid.Web.Services/Controllers/v4/ConnectController.cs:327-327`, and `Web/Resgrid.Web.Services/Controllers/v4/SessionsController.cs:89-89` records raw `Request.Headers["User-Agent"]` and `Request.Headers["Accept-Language"]`, which may contain personal data. Log only non-sensitive context and use `HttpContext.TraceIdentifier` or other correlation metadata instead of full header values.

Suggested Code:

							Data = "Web LogOn blocked by SSO policy",
							CorrelationId = HttpContext.TraceIdentifier

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

_antiforgery = antiforgery;
}

[AcceptVerbs("GET", "POST", "PUT", "PATCH", "DELETE")]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules high

HTTP method ambiguity in Web/Resgrid.Web/Controllers/WebApiBffController.cs uses [AcceptVerbs("GET", "POST", "PUT", "PATCH", "DELETE")], which obscures the supported routes and weakens verb-specific metadata. Replace it with explicit [HttpGet("{**path}")], [HttpPost("{**path}")], [HttpPut("{**path}")], [HttpPatch("{**path}")], and [HttpDelete("{**path}")] attributes.

Kody rule violation: Annotate REST API Actions with HTTP Verb Attributes

[HttpGet("{**path}")]
[HttpPost("{**path}")]
[HttpPut("{**path}")]
[HttpPatch("{**path}")]
[HttpDelete("{**path}")]
Prompt for LLM

File Web/Resgrid.Web/Controllers/WebApiBffController.cs:

Line 48:

HTTP method ambiguity in `Web/Resgrid.Web/Controllers/WebApiBffController.cs` uses `[AcceptVerbs("GET", "POST", "PUT", "PATCH", "DELETE")]`, which obscures the supported routes and weakens verb-specific metadata. Replace it with explicit `[HttpGet("{**path}")]`, `[HttpPost("{**path}")]`, `[HttpPut("{**path}")]`, `[HttpPatch("{**path}")]`, and `[HttpDelete("{**path}")]` attributes.

Suggested Code:

		[HttpGet("{**path}")]
		[HttpPost("{**path}")]
		[HttpPut("{**path}")]
		[HttpPatch("{**path}")]
		[HttpDelete("{**path}")]

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

return;

fetch(resgrid.absoluteApiBaseUrl + '/api/v4/Geocoding/ForwardGeocode?address=' + encodeURIComponent(where), { headers: { 'Authorization': 'Bearer ' + getAuthToken() } })
fetch('/api/web-bff/api/v4/Geocoding/ForwardGeocode?address=' + encodeURIComponent(where))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kody code-review Kody Rules high

Unhandled promise rejection risk in Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.addArchivedCall.js and the related fetch chains, including Web/Resgrid.Web/wwwroot/js/app/internal/routes/resgrid.routes.new.js:76-76, Web/Resgrid.Web/wwwroot/js/app/internal/routes/resgrid.routes.edit.js:82-82, and Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.addArchivedCall.js:374-374, leaves network and r.json() parsing failures unobserved. Terminate the chain with .catch(...) or use async/await with try/catch so failures are handled and logged.

Kody rule violation: Handle async operations with proper error handling

fetch('/api/web-bff/api/v4/Geocoding/ForwardGeocode?address=' + encodeURIComponent(where))
	.then(function(r) { return r.json(); })
	.then(function(result) {
		// handle result
	})
	.catch(function(err) {
		logger.error('ForwardGeocode failed', { op: 'ForwardGeocode', address: where, err: err });
	});
Prompt for LLM

File Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.addArchivedCall.js:

Line 126:

Unhandled promise rejection risk in `Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.addArchivedCall.js` and the related fetch chains, including `Web/Resgrid.Web/wwwroot/js/app/internal/routes/resgrid.routes.new.js:76-76`, `Web/Resgrid.Web/wwwroot/js/app/internal/routes/resgrid.routes.edit.js:82-82`, and `Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.addArchivedCall.js:374-374`, leaves network and `r.json()` parsing failures unobserved. Terminate the chain with `.catch(...)` or use `async/await` with `try/catch` so failures are handled and logged.

Suggested Code:

					fetch('/api/web-bff/api/v4/Geocoding/ForwardGeocode?address=' + encodeURIComponent(where))
						.then(function(r) { return r.json(); })
						.then(function(result) {
							// handle result
						})
						.catch(function(err) {
							logger.error('ForwardGeocode failed', { op: 'ForwardGeocode', address: where, err: err });
						});

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

[HttpPost("revoke-others")]
[ProducesResponseType(StatusCodes.Status200OK)]
[ProducesResponseType(StatusCodes.Status409Conflict)]
public async Task<IActionResult> RevokeOthers(CancellationToken cancellationToken)

[HttpPost("revoke-all")]
[ProducesResponseType(StatusCodes.Status200OK)]
public async Task<IActionResult> RevokeAll(CancellationToken cancellationToken)
@ucswift ucswift closed this Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants