auth: implement backend-proxy support for GitHub device flow#441
Merged
Conversation
- Introduce `AuthPath` (Backend or Direct) to support authentication through a proxy for users on restricted networks. - Add `BACKEND_BASE_URL` and `BACKEND_ORIGIN` constants to centralize backend endpoint configuration. - Implement backend-first authentication in `AuthenticationRepositoryImpl`, falling back to the `Direct` path only on infrastructure errors (timeouts or 5xx responses). - Update `AuthenticationViewModel` to persist and restore the active `AuthPath` via `SavedStateHandle` to ensure session continuity. - Enhance `GitHubAuthApi` with backend-specific `start` and `poll` methods, including support for `X-Request-ID` headers for better error tracking. - Refactor polling logic to allow dynamic escalation from `Backend` to `Direct` path during an active session if infrastructure failures occur.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughDevice-flow auth now prefers a backend proxy path by default, persists per-session path selection, and implements controlled fallback to direct GitHub calls only for infrastructure/5xx failures; polling and start endpoints can be invoked via backend or direct APIs with request-id propagation and explicit rate-limit guidance. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant VM as AuthenticationViewModel
participant Repo as AuthenticationRepository
participant BackendAPI as GitHubAuthApi (Backend)
participant DirectAPI as GitHubAuthApi (Direct)
participant GitHub
User->>VM: startLogin()
VM->>Repo: startDeviceFlow()
Repo->>BackendAPI: startDeviceFlowViaBackend()
alt Backend infra (timeout/5xx)
BackendAPI-->>Repo: infra error
Repo->>DirectAPI: startDeviceFlowDirect()
DirectAPI->>GitHub: POST /device
GitHub-->>DirectAPI: Device start response
DirectAPI-->>Repo: GithubDeviceStart
Repo-->>VM: DeviceFlowStart(path: Direct)
else Backend success
BackendAPI->>GitHub: POST /device (proxy)
GitHub-->>BackendAPI: Device start response
BackendAPI-->>Repo: GithubDeviceStart
Repo-->>VM: DeviceFlowStart(path: Backend)
end
VM->>VM: begin polling with authPath
loop Polling
VM->>Repo: pollDeviceTokenOnce(deviceCode, authPath)
alt authPath == Backend
Repo->>BackendAPI: pollDeviceTokenViaBackend()
alt Backend infra (timeout/5xx)
BackendAPI-->>Repo: infra error
Repo->>DirectAPI: pollDeviceTokenDirect()
DirectAPI->>GitHub: POST /token
GitHub-->>DirectAPI: Token / pending
DirectAPI-->>Repo: Result
Repo-->>VM: PollOutcome(result, path: Direct)
else Backend success/failure
BackendAPI->>GitHub: POST /token (proxy)
GitHub-->>BackendAPI: Token / pending
BackendAPI-->>Repo: Result
Repo-->>VM: PollOutcome(result, path: Backend)
end
else authPath == Direct
Repo->>DirectAPI: pollDeviceTokenDirect()
DirectAPI->>GitHub: POST /token
GitHub-->>DirectAPI: Token / pending
DirectAPI-->>Repo: Result
Repo-->>VM: PollOutcome(result, path: Direct)
end
alt outcome.path changed
VM->>VM: update & persist authPath
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/auth/data/src/commonMain/kotlin/zed/rainxch/auth/data/network/GitHubAuthApi.kt (1)
19-19:⚠️ Potential issue | 🟠 MajorRethrow coroutine cancellation before wrapping poll failures.
CancellationExceptionis caught by these genericcatch (e: Exception)blocks, converting cancelled polling into a normalResult.failureinstead of properly propagating cancellation, which breaks coroutine cancellation semantics.🐛 Proposed fix
+import kotlinx.coroutines.CancellationException import kotlinx.coroutines.delay- } catch (e: Exception) { + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { Result.failure(e) }- } catch (e: Exception) { + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { Result.failure(e) }Applies to lines 152-154 and 253-255 in the poll helper functions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/auth/data/src/commonMain/kotlin/zed/rainxch/auth/data/network/GitHubAuthApi.kt` at line 19, The generic catch (e: Exception) in the poll helper functions is swallowing coroutine cancellations; modify the catch blocks in the poll helpers (the functions named poll... in GitHubAuthApi, e.g., the two poll helper functions around the reported lines) to rethrow CancellationException before wrapping errors: check if e is CancellationException and throw it, otherwise convert to Result.failure/handle as before; ensure kotlinx.coroutines.CancellationException is imported if not already.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/auth/data/src/commonMain/kotlin/zed/rainxch/auth/data/repository/AuthenticationRepositoryImpl.kt`:
- Around line 329-332: The else branch in AuthenticationRepositoryImpl that
currently logs "Single poll unknown error" and returns DevicePollResult.Pending
must surface non-device-flow failures as DevicePollResult.Failed instead of
hiding them; change the fallback to return DevicePollResult.Failed with the
captured error info (preserve the errorMsg/details) so backend/client errors
stop polling. Also ensure AuthPath selection logic in
AuthenticationRepositoryImpl only escalates from Backend to Direct on
infrastructure/network errors (e.g., timeouts, connection errors) and not on
HTTP 4xx responses or valid GitHub responses—treat 4xx as terminal failures and
propagate them as Failed rather than switching path or continuing with Pending.
---
Outside diff comments:
In
`@feature/auth/data/src/commonMain/kotlin/zed/rainxch/auth/data/network/GitHubAuthApi.kt`:
- Line 19: The generic catch (e: Exception) in the poll helper functions is
swallowing coroutine cancellations; modify the catch blocks in the poll helpers
(the functions named poll... in GitHubAuthApi, e.g., the two poll helper
functions around the reported lines) to rethrow CancellationException before
wrapping errors: check if e is CancellationException and throw it, otherwise
convert to Result.failure/handle as before; ensure
kotlinx.coroutines.CancellationException is imported if not already.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7083f520-31af-410b-a1f6-3d70e46d6824
📒 Files selected for processing (7)
CLAUDE.mdcore/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendApiClient.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendEndpoints.ktfeature/auth/data/src/commonMain/kotlin/zed/rainxch/auth/data/network/GitHubAuthApi.ktfeature/auth/data/src/commonMain/kotlin/zed/rainxch/auth/data/repository/AuthenticationRepositoryImpl.ktfeature/auth/domain/src/commonMain/kotlin/zed/rainxch/auth/domain/repository/AuthenticationRepository.ktfeature/auth/presentation/src/commonMain/kotlin/zed/rainxch/auth/presentation/AuthenticationViewModel.kt
… Failed - Change the fallback case in `AuthenticationRepositoryImpl` to return `DevicePollResult.Failed` instead of `DevicePollResult.Pending` when an unrecognized error occurs. - Update logging to reflect that unknown errors are now surfaced as failures.
This was referenced Apr 28, 2026
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AuthPath(Backend or Direct) to support authentication through a proxy for users on restricted networks.BACKEND_BASE_URLandBACKEND_ORIGINconstants to centralize backend endpoint configuration.AuthenticationRepositoryImpl, falling back to theDirectpath only on infrastructure errors (timeouts or 5xx responses).AuthenticationViewModelto persist and restore the activeAuthPathviaSavedStateHandleto ensure session continuity.GitHubAuthApiwith backend-specificstartandpollmethods, including support forX-Request-IDheaders for better error tracking.BackendtoDirectpath during an active session if infrastructure failures occur.Summary by CodeRabbit