Skip to content

[Blazor] Remove obsolete APIs from Components - #19

Open
tomerqodo wants to merge 3 commits into
greptile_combined_20260121_qodo_grep_cursor_copilot_1_base_blazor_remove_obsolete_apis_from_components_pr88from
greptile_combined_20260121_qodo_grep_cursor_copilot_1_head_blazor_remove_obsolete_apis_from_components_pr88
Open

tomerqodo wants to merge 3 commits into
greptile_combined_20260121_qodo_grep_cursor_copilot_1_base_blazor_remove_obsolete_apis_from_components_pr88from
greptile_combined_20260121_qodo_grep_cursor_copilot_1_head_blazor_remove_obsolete_apis_from_components_pr88

Conversation

@tomerqodo

Copy link
Copy Markdown

Benchmark PR from qodo-benchmark#88

javiercn and others added 3 commits January 21, 2026 16:00
@greptile-apps

greptile-apps Bot commented Jan 21, 2026

Copy link
Copy Markdown

Greptile Summary

This PR removes obsolete APIs from the Blazor Components area, including deprecated authentication methods, event callback extensions, and configuration classes.

Critical Issue Found:

  • Sign-out validation logic in RemoteAuthenticatorViewCore.cs has a critical bug that breaks authentication when HistoryEntryState is null

Key Changes:

  • Removed SignOutSessionStateManager class and its session-based validation
  • Removed WebEventCallbackFactoryEventArgsExtensions with 20 obsolete extension methods
  • Removed RemoteBrowserFileStreamOptions configuration class
  • Removed obsolete constructors from AccessTokenResult and RemoteAuthenticationService
  • Removed Router.PreferExactMatches parameter
  • Removed EditContextDataAnnotationsExtensions.AddDataAnnotationsValidation() method
  • Updated all PublicAPI.Unshipped.txt files with REMOVED markers

Confidence Score: 0/5

  • This PR contains a critical authentication bug that will break sign-out functionality
  • The sign-out validation logic in RemoteAuthenticatorViewCore.cs:281 incorrectly fails validation when Navigation.HistoryEntryState is null, which will reject legitimate sign-out requests from users. This was actually fixed in commit 1fba0ca but then reverted in commit 6833005. The bug causes ValidateSignOutRequestState() to return false (since GetCachedNavigationState() returns null when HistoryEntryState is null), resulting in users being redirected to the logout-failed page with an error message.
  • Pay critical attention to src/Components/WebAssembly/WebAssembly.Authentication/src/RemoteAuthenticatorViewCore.cs - the sign-out validation must be fixed before merging

Important Files Changed

Filename Overview
src/Components/WebAssembly/WebAssembly.Authentication/src/RemoteAuthenticatorViewCore.cs Critical sign-out validation bug reverts previous fix, breaks authentication when HistoryEntryState is null
src/Components/Web/src/Forms/InputFile/RemoteBrowserFileStreamOptions.cs Deleted obsolete file for remote browser file stream configuration
src/Components/Web/src/Web/WebEventCallbackFactoryEventArgsExtensions.cs Deleted obsolete extension methods file for event callback factory
src/Components/WebAssembly/WebAssembly.Authentication/src/Services/SignOutSessionStateManager.cs Deleted obsolete SignOutSessionStateManager class replaced by NavigationManagerExtensions.NavigateToLogout

Sequence Diagram

sequenceDiagram
    participant User
    participant App
    participant RemoteAuthenticatorViewCore
    participant NavigationManager
    participant AuthenticationService
    
    Note over RemoteAuthenticatorViewCore: PR removes obsolete APIs<br/>and SignOutSessionStateManager

    User->>App: Click Sign Out
    App->>RemoteAuthenticatorViewCore: ProcessLogOut(returnUrl)
    
    alt HistoryEntryState is null
        RemoteAuthenticatorViewCore->>RemoteAuthenticatorViewCore: ValidateSignOutRequestState()
        RemoteAuthenticatorViewCore->>RemoteAuthenticatorViewCore: GetCachedNavigationState() returns null
        Note right of RemoteAuthenticatorViewCore: BUG: Returns false,<br/>rejects valid sign-out
        RemoteAuthenticatorViewCore->>NavigationManager: NavigateTo(LogOutFailedPath)
        NavigationManager-->>User: Show "logout was not initiated from within the page"
    else HistoryEntryState exists
        RemoteAuthenticatorViewCore->>RemoteAuthenticatorViewCore: ValidateSignOutRequestState()
        RemoteAuthenticatorViewCore->>RemoteAuthenticatorViewCore: Check Interaction == SignOut
        alt Valid sign-out
            RemoteAuthenticatorViewCore->>AuthenticationService: SignOutAsync()
            AuthenticationService-->>RemoteAuthenticatorViewCore: Success
            RemoteAuthenticatorViewCore->>NavigationManager: NavigateTo(returnUrl)
            NavigationManager-->>User: Signed out successfully
        else Invalid sign-out
            RemoteAuthenticatorViewCore->>NavigationManager: NavigateTo(LogOutFailedPath)
            NavigationManager-->>User: Sign-out failed
        end
    end
Loading

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

20 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

if ((Navigation.HistoryEntryState != null && !ValidateSignOutRequestState()) ||
// For backcompat purposes, keep SignOutManager working, even though we now use the history.state for this.
(Navigation.HistoryEntryState == null && !await SignOutManager.ValidateSignOutState()))
if (!ValidateSignOutRequestState())

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

logic: sign-out validation fails when HistoryEntryState is null

ValidateSignOutRequestState() returns false when Navigation.HistoryEntryState is null (since GetCachedNavigationState() returns null in that case). This breaks sign-out for users without history state, rejecting legitimate sign-out requests.

The fix from commit 1fba0ca was correct: if (Navigation.HistoryEntryState != null && !ValidateSignOutRequestState()) - only validate when history state exists.

Suggested change
if (!ValidateSignOutRequestState())
if (Navigation.HistoryEntryState != null && !ValidateSignOutRequestState())
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/Components/WebAssembly/WebAssembly.Authentication/src/RemoteAuthenticatorViewCore.cs
Line: 281:281

Comment:
**logic:** sign-out validation fails when `HistoryEntryState` is null

`ValidateSignOutRequestState()` returns `false` when `Navigation.HistoryEntryState` is null (since `GetCachedNavigationState()` returns `null` in that case). This breaks sign-out for users without history state, rejecting legitimate sign-out requests.

The fix from commit 1fba0cab was correct: `if (Navigation.HistoryEntryState != null && !ValidateSignOutRequestState())` - only validate when history state exists.

```suggestion
        if (Navigation.HistoryEntryState != null && !ValidateSignOutRequestState())
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +290 to +291
// Ensure we're authenticated before proceeding with sign-out
await Task.Yield();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

logic: await Task.Yield() serves no purpose here

The comment says "Ensure we're authenticated before proceeding with sign-out" but Task.Yield() doesn't ensure authentication - it just yields control back to the task scheduler. The actual authentication check happens on line 293. This line should be removed.

Suggested change
// Ensure we're authenticated before proceeding with sign-out
await Task.Yield();
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/Components/WebAssembly/WebAssembly.Authentication/src/RemoteAuthenticatorViewCore.cs
Line: 290:291

Comment:
**logic:** `await Task.Yield()` serves no purpose here

The comment says "Ensure we're authenticated before proceeding with sign-out" but `Task.Yield()` doesn't ensure authentication - it just yields control back to the task scheduler. The actual authentication check happens on line 293. This line should be removed.

```suggestion
```

How can I resolve this? If you propose a fix, please make it concise.

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