Skip to content

Conversation

@ysmoradi
Copy link
Member

@ysmoradi ysmoradi commented Jun 9, 2025

closes #10951

Summary by CodeRabbit

  • Refactor
    • Improved handling of expired access tokens for API and real-time connections, resulting in more consistent and centralized authentication checks.
  • Bug Fixes
    • Enhanced detection of expired or invalid tokens, allowing the system to better prompt users to refresh authentication when necessary.

@ysmoradi ysmoradi requested review from Copilot and msynk June 9, 2025 12:36
@coderabbitai
Copy link

coderabbitai bot commented Jun 9, 2025

Walkthrough

The changes introduce two new extension methods to the HTTP context for detecting expired access tokens and update relevant authentication checks in API and SignalR controllers to use these methods. This centralizes and simplifies the logic for handling expired or invalid tokens in requests that allow anonymous access.

Changes

File(s) Change Summary
.../Controllers/PushNotification/PushNotificationController.cs, .../SignalR/AppHub.cs Refactored authentication checks to use new ContainsExpiredAccessToken/ThrowIfContainsExpiredAccessToken methods.
.../Extensions/HttpContextExtensions.cs Added ThrowIfContainsExpiredAccessToken and ContainsExpiredAccessToken extension methods to centralize logic.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Server (API/SignalR)
    participant HttpContextExtensions

    Client->>Server (API/SignalR): Sends request with/without Authorization header
    Server (API/SignalR)->>HttpContextExtensions: Call ContainsExpiredAccessToken/ThrowIfContainsExpiredAccessToken
    alt Expired Access Token Detected
        HttpContextExtensions-->>Server (API/SignalR): Throws UnauthorizedException/HubException
        Server (API/SignalR)-->>Client: Responds with Unauthorized error
    else Valid or No Token
        HttpContextExtensions-->>Server (API/SignalR): No exception
        Server (API/SignalR)-->>Client: Proceeds with request
    end
Loading

Assessment against linked issues

Objective Addressed Explanation
Implement ThrowIfContainsExpiredAccessToken to detect expired tokens and throw UnauthorizedException (#10951)
Allow API endpoints to differentiate between anonymous and expired-token requests (#10951)
Centralize and simplify expired token detection logic in the Boilerplate template (#10951)

Assessment against linked issues: Out-of-scope changes

No out-of-scope changes found.

Poem

In the meadow of code where tokens may tire,
A rabbit hops in, with logic to inspire.
If your token’s expired, you’ll now be denied,
With one simple check, no need to divide.
Centralized wisdom, authentication’s delight—
Hop on, dear dev, your tokens are right!
🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a centralized mechanism to detect and handle expired access tokens by adding helper methods in the HTTP context extensions and updating related components to use this logic.

  • Removed inline authentication checks in AppHub and PushNotificationController in favor of using new helper methods.
  • Added two extension methods, ContainsExpiredAccessToken and ThrowIfContainsExpiredAccessToken, to encapsulate expired token detection and handling.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppHub.cs Updated token validation logic using ContainsExpiredAccessToken with a HubException for expired tokens.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Extensions/HttpContextExtensions.cs Added extension methods for token checks and centralized exception throwing.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/PushNotification/PushNotificationController.cs Replaced inline token validation with a call to ThrowIfContainsExpiredAccessToken.
Comments suppressed due to low confidence (1)

src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppHub.cs:30

  • There is an inconsistency in the exception types thrown: AppHub.cs throws a HubException with additional connection data, whereas ThrowIfContainsExpiredAccessToken in HttpContextExtensions throws an UnauthorizedException. Consider unifying the exception type for consistent error handling across your API and SignalR endpoints.
if (Context.GetHttpContext()?.ContainsExpiredAccessToken() is true)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Extensions/HttpContextExtensions.cs (1)

41-41: Fix XML documentation syntax error.

There's an extra closing tag in the inheritdoc reference.

-    /// <inheritdoc cref="ThrowIfContainsExpiredAccessToken(HttpContext)"/>/>
+    /// <inheritdoc cref="ThrowIfContainsExpiredAccessToken(HttpContext)"/>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 40c3742 and ced66ae.

📒 Files selected for processing (3)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/PushNotification/PushNotificationController.cs (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Extensions/HttpContextExtensions.cs (2 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppHub.cs (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppHub.cs (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Extensions/HttpContextExtensions.cs (1)
  • ContainsExpiredAccessToken (43-46)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/PushNotification/PushNotificationController.cs (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Extensions/HttpContextExtensions.cs (1)
  • ThrowIfContainsExpiredAccessToken (34-38)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Extensions/HttpContextExtensions.cs (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Extensions/ClaimsPrincipalExtensions.cs (1)
  • IsAuthenticated (5-8)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: build and test
🔇 Additional comments (4)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/PushNotification/PushNotificationController.cs (1)

17-17: LGTM! Clean centralization of expired token detection.

The use of the new extension method effectively centralizes the expired access token detection logic, improving code maintainability and consistency across the API.

src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppHub.cs (1)

30-31: LGTM! Appropriate SignalR-specific implementation.

Good use of the boolean extension method variant with SignalR-specific exception handling. The null-conditional operator provides proper safety, and including the ConnectionId in the exception data aids debugging.

src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Extensions/HttpContextExtensions.cs (2)

26-38: Excellent design with comprehensive documentation.

The method design follows good practices:

  • Clear separation between boolean check and exception throwing
  • Comprehensive XML documentation explaining the use case
  • Appropriate exception type for the scenario

The logic for detecting expired tokens (unauthenticated user + present Authorization header) is a reasonable heuristic that covers the common case where clients have expired tokens.


43-46: Solid implementation of expired token detection logic.

The boolean logic correctly identifies the scenario where a client likely has an expired access token. The combination of an unauthenticated user with a present Authorization header is a good heuristic for this detection.

@msynk msynk merged commit dd193bd into bitfoundation:develop Jun 9, 2025
3 checks passed
@ysmoradi ysmoradi deleted the 10951 branch June 10, 2025 18:42
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.

ThrowIfContainsExpiredAccessToken is missing in Boilerplate project template

2 participants