change http

[iceljc]

PR Type

Bug fix, Enhancement

---

Description

• Changed HTTP method from POST to GET for role retrieval endpoint

• Removed JSON property name attributes from RoleFilter class

• Added null-safety checks in SignalRHub connection handler

• Improved logging with fallback for unknown users

---

Diagram Walkthrough

flowchart LR
  A["RoleController"] -->|"Changed POST to GET"| B["GetRoles endpoint"]
  C["RoleFilter"] -->|"Removed JsonPropertyName"| D["Names & ExcludeRoles"]
  E["SignalRHub"] -->|"Added null-safety"| F["OnConnectedAsync"]
  F -->|"Improved logging"| G["Connection info"]

Loading

File Walkthrough

	Relevant files
Enhancement	RoleFilter.cs Remove JSON property name attributes                                          src/Infrastructure/BotSharp.Abstraction/Repositories/Filters/RoleFilter.cs Removed [JsonPropertyName("names")] attribute from Names property Removed [JsonPropertyName("exclude_roles")] attribute from ExcludeRoles property Simplified class by relying on default property naming conventions +0/-3
Bug fix	RoleController.cs Change role retrieval from POST to GET                                      src/Infrastructure/BotSharp.OpenAPI/Controllers/User/RoleController.cs Changed HTTP method from [HttpPost] to [HttpGet] for GetRoles endpoint Changed parameter binding from [FromBody] to [FromQuery] for RoleFilter Aligns with REST conventions for retrieving data +2/-2      SignalRHub.cs Add null-safety checks to SignalRHub                                          src/Plugins/BotSharp.Plugin.ChatHub/SignalRHub.cs Added null-conditional operators (?.) for Context and _user properties Reordered logging statement to occur after null-safe variable initialization Added fallback value "Unknown user" for null identity names Improved constructor formatting with line breaks for readability +7/-5

---

[qodo-merge-pro]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Sensitive information exposure Description: Logging user full name and identity details on connection may expose sensitive personally identifiable information (PII) in logs, which can be accessed or exfiltrated in lower-trust environments; consider minimizing or masking user-identifying data and avoiding logging raw connection IDs. SignalRHub.cs [32-39] Referred Code _logger.LogInformation($"SignalR Hub: {_user?.FirstName} {_user?.LastName} ({Context?.User?.Identity?.Name ?? "Unkown user"}) connected in {Context?.ConnectionId}"); string? conversationId = httpContext?.Request?.Query["conversation-id"].FirstOrDefault(); if (!string.IsNullOrEmpty(conversationId)) { _logger.LogInformation($"Connection {Context?.ConnectionId} is with conversation {conversationId}"); await AddGroup(conversationId);
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Partial logging: The connection event is logged with user and connection context, but it is unclear whether all critical role retrieval actions or permission-related reads are logged elsewhere in this change. Referred Code _logger.LogInformation($"SignalR Hub: {_user?.FirstName} {_user?.LastName} ({Context?.User?.Identity?.Name ?? "Unkown user"}) connected in {Context?.ConnectionId}"); string? conversationId = httpContext?.Request?.Query["conversation-id"].FirstOrDefault(); if (!string.IsNullOrEmpty(conversationId)) { _logger.LogInformation($"Connection {Context?.ConnectionId} is with conversation {conversationId}"); await AddGroup(conversationId); Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Missing validation: Switching to GET with query-bound filter lacks visible validation/error handling for malformed or oversized query inputs, which may require additional guards. Referred Code [HttpGet("/roles")] public async Task<IEnumerable<RoleViewModel>> GetRoles([FromQuery] RoleFilter? filter = null) { if (filter == null) { filter = RoleFilter.Empty(); Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Potential PII in logs: Logging first/last name and identity name may expose PII; consider structured logging and avoiding personal identifiers unless justified and approved. Referred Code _logger.LogInformation($"SignalR Hub: {_user?.FirstName} {_user?.LastName} ({Context?.User?.Identity?.Name ?? "Unkown user"}) connected in {Context?.ConnectionId}"); string? conversationId = httpContext?.Request?.Query["conversation-id"].FirstOrDefault(); if (!string.IsNullOrEmpty(conversationId)) { _logger.LogInformation($"Connection {Context?.ConnectionId} is with conversation {conversationId}"); await AddGroup(conversationId); Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Query input validation: Changing to GET with [FromQuery] introduces external inputs without visible validation or size limits on 'Names' and 'ExcludeRoles', which could risk injection or resource issues. Referred Code [HttpGet("/roles")] public async Task<IEnumerable<RoleViewModel>> GetRoles([FromQuery] RoleFilter? filter = null) { if (filter == null) { filter = RoleFilter.Empty(); Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Avoid race conditions with context To prevent potential race conditions in the async OnConnectedAsync method, capture Context in a local variable at the start and use that variable for subsequent accesses. src/Plugins/BotSharp.Plugin.ChatHub/SignalRHub.cs [29-34] +var context = Context; var convService = _services.GetRequiredService<IConversationService>(); -var httpContext = Context?.GetHttpContext(); +var httpContext = context?.GetHttpContext(); -_logger.LogInformation($"SignalR Hub: {_user?.FirstName} {_user?.LastName} ({Context?.User?.Identity?.Name ?? "Unkown user"}) connected in {Context?.ConnectionId}"); +_logger.LogInformation($"SignalR Hub: {_user?.FirstName} {_user?.LastName} ({context?.User?.Identity?.Name ?? "Unkown user"}) connected in {context?.ConnectionId}"); string? conversationId = httpContext?.Request?.Query["conversation-id"].FirstOrDefault(); Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies a potential race condition in an async method and proposes a valid defensive programming practice to improve code robustness and prevent potential null reference exceptions.	Low
Learned best practice	Validate identity before logging Validate required dependencies and identity upfront and fix the typo in the fallback; if identity is missing, log a clear warning and short-circuit to avoid ambiguous "unknown" connections. src/Plugins/BotSharp.Plugin.ChatHub/SignalRHub.cs [32] -_logger.LogInformation($"SignalR Hub: {_user?.FirstName} {_user?.LastName} ({Context?.User?.Identity?.Name ?? "Unkown user"}) connected in {Context?.ConnectionId}"); +if (_user is null || Context?.User?.Identity is null || !Context.User.Identity.IsAuthenticated) +{ + _logger.LogWarning("SignalR Hub: unauthenticated or missing user attempted to connect. ConnectionId: {ConnectionId}", Context?.ConnectionId); + await base.OnConnectedAsync(); + return; +} +_logger.LogInformation("SignalR Hub: {First} {Last} ({Name}) connected in {ConnectionId}", + _user.FirstName, _user.LastName, Context.User.Identity.Name, Context.ConnectionId); Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Guard against nulls and invalid states before access; avoid masking real issues with overly broad null-coalescing defaults that hide errors and typos.	Low
	Validate query filter and return 400 Explicitly validate query filter inputs (e.g., empty collections) and enforce reasonable limits; return BadRequest for invalid filters to avoid ambiguous processing. src/Infrastructure/BotSharp.OpenAPI/Controllers/User/RoleController.cs [38-49] [BotSharpAuth] [HttpGet("/roles")] -public async Task<IEnumerable<RoleViewModel>> GetRoles([FromQuery] RoleFilter? filter = null) +public async Task<IActionResult> GetRoles([FromQuery] RoleFilter? filter = null) { - if (filter == null) + filter ??= RoleFilter.Empty(); + if (filter.Names != null && !filter.Names.Any()) { - filter = RoleFilter.Empty(); + return BadRequest("Parameter 'names' must contain at least one value when provided."); } var roles = await _roleService.GetRoles(filter); - return roles.Select(x => RoleViewModel.FromRole(x)).ToList(); + var result = roles.Select(x => RoleViewModel.FromRole(x)).ToList(); + return Ok(result); } Apply / Chat Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Use correct HTTP semantics and input validation; when switching from body to query binding, guard against null/empty inputs to prevent invalid states and large payload issues.	Low
More

[qodo-merge-pro]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Sensitive information exposure Description: Logging potentially sensitive user identifiers (full name and identity name) on connection may expose PII in logs; consider redacting or minimizing to a non-sensitive identifier. SignalRHub.cs [32-32] Referred Code _logger.LogInformation($"SignalR Hub: {_user?.FirstName} {_user?.LastName} ({Context?.User?.Identity?.Name ?? "Unkown user"}) connected in {Context?.ConnectionId}");
	Sensitive data in URL Description: Switching roles retrieval to GET with [FromQuery] may leak role filter details (e.g., role names) via URLs, proxies, and logs; ensure no sensitive role data is passed in query strings or enforce server-side defaults. RoleController.cs [39-41] Referred Code [HttpGet("/roles")] public async Task<IEnumerable<RoleViewModel>> GetRoles([FromQuery] RoleFilter? filter = null) {
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Limited audit context: Connection events are logged but may lack a stable user identifier and outcome fields needed for audit trails (e.g., explicit success/failure and user ID), which might limit reconstructing events. Referred Code _logger.LogInformation($"SignalR Hub: {_user?.FirstName} {_user?.LastName} ({Context?.User?.Identity?.Name ?? "Unkown user"}) connected in {Context?.ConnectionId}"); string? conversationId = httpContext?.Request?.Query["conversation-id"].FirstOrDefault(); if (!string.IsNullOrEmpty(conversationId)) { _logger.LogInformation($"Connection {Context?.ConnectionId} is with conversation {conversationId}"); await AddGroup(conversationId); Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Missing exception handling: The connection handler lacks try/catch and does not handle failures of dependency resolution or group addition, potentially causing unhandled exceptions. Referred Code var convService = _services.GetRequiredService<IConversationService>(); var httpContext = Context?.GetHttpContext(); _logger.LogInformation($"SignalR Hub: {_user?.FirstName} {_user?.LastName} ({Context?.User?.Identity?.Name ?? "Unkown user"}) connected in {Context?.ConnectionId}"); string? conversationId = httpContext?.Request?.Query["conversation-id"].FirstOrDefault(); if (!string.IsNullOrEmpty(conversationId)) { _logger.LogInformation($"Connection {Context?.ConnectionId} is with conversation {conversationId}"); await AddGroup(conversationId); var conv = await convService.GetConversation(conversationId); if (conv != null) { var hooks = _services.GetHooks<IConversationHook>(conv.AgentId); Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Potential PII logging: Logs include first and last name and identity name, which may constitute PII and could violate secure logging policies if not permitted. Referred Code _logger.LogInformation($"SignalR Hub: {_user?.FirstName} {_user?.LastName} ({Context?.User?.Identity?.Name ?? "Unkown user"}) connected in {Context?.ConnectionId}"); string? conversationId = httpContext?.Request?.Query["conversation-id"].FirstOrDefault(); if (!string.IsNullOrEmpty(conversationId)) { _logger.LogInformation($"Connection {Context?.ConnectionId} is with conversation {conversationId}"); await AddGroup(conversationId); Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Query input validation: The endpoint now accepts RoleFilter from query without visible validation or normalization, which may risk improper input handling. Referred Code [HttpGet("/roles")] public async Task<IEnumerable<RoleViewModel>> GetRoles([FromQuery] RoleFilter? filter = null) { if (filter == null) { filter = RoleFilter.Empty(); Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Remove redundant null check Remove the redundant if (filter == null) check in the GetRoles method, as the model binder ensures filter is never null for a [FromQuery] complex type. src/Infrastructure/BotSharp.OpenAPI/Controllers/User/RoleController.cs [40-49] -public async Task<IEnumerable<RoleViewModel>> GetRoles([FromQuery] RoleFilter? filter = null) +public async Task<IEnumerable<RoleViewModel>> GetRoles([FromQuery] RoleFilter filter) { - if (filter == null) - { - filter = RoleFilter.Empty(); - } - var roles = await _roleService.GetRoles(filter); return roles.Select(x => RoleViewModel.FromRole(x)).ToList(); } Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out that the if (filter == null) check is redundant because ASP.NET Core's model binder will always instantiate a complex type for a [FromQuery] parameter. Removing this check simplifies the code and improves its correctness.	Low
	Fix typo in log message Correct the typo "Unkown user" to "Unknown user" in the log message on line 32. src/Plugins/BotSharp.Plugin.ChatHub/SignalRHub.cs [32] -_logger.LogInformation($"SignalR Hub: {_user?.FirstName} {_user?.LastName} ({Context?.User?.Identity?.Name ?? "Unkown user"}) connected in {Context?.ConnectionId}"); +_logger.LogInformation($"SignalR Hub: {_user?.FirstName} {_user?.LastName} ({Context?.User?.Identity?.Name ?? "Unknown user"}) connected in {Context?.ConnectionId}"); Apply / Chat Suggestion importance[1-10]: 3 __ Why: The suggestion correctly identifies a typo ("Unkown") in a log message introduced in the PR. While this is a minor issue, fixing it improves the professionalism and clarity of the logs.	Low
More