test(webui): add standalone SSO session and multi-user isolation coverage - #6849
Merged
Merged
Conversation
italic-jinxin
added
size: XL
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6849
July 29, 2026 10:39
Destroyed
Contributor
Author
|
@claude review |
github-actions
Bot
added
scope: ci
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds a guarded local Google OAuth endpoint seam, queued mock OIDC identities, a dedicated SSO binary, and standalone Reborn WebUI v2 E2E coverage for authentication, multi-user isolation, SSE denial, and logout revocation. ChangesReborn standalone SSO coverage
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant MockOAuthIdp
participant RebornServe
participant WebChatAPI
Browser->>RebornServe: Request Google login
RebornServe-->>Browser: Redirect to MockOAuthIdp
Browser->>MockOAuthIdp: Authorize Alice or Bob
MockOAuthIdp-->>Browser: Callback with authorization code
Browser->>MockOAuthIdp: Exchange authorization code
MockOAuthIdp-->>Browser: Google-shaped id_token
Browser->>RebornServe: Exchange login ticket
RebornServe-->>Browser: Session bearer token
Browser->>WebChatAPI: Access threads, timelines, and SSE
WebChatAPI-->>Browser: Scoped data or not_found
Browser->>RebornServe: Logout
RebornServe-->>Browser: Alice session revoked
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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 |
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Contributor
🔎 Review · PR #6849
Submitted review →Reviewed the complete trusted base-to-head comparison. No concrete, actionable correctness, security, CI, or test-coverage defects were found. The test OAuth seam remains off by default, requires the explicit feature in a debug build, validates paired literal-loopback endpoints, and the workflows preserve the default binary separately from the SSO test binary. Automatic · PR opened · attempt 1 of 3 · completed in 1m 31s Run details
|
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🔍 Review complete · PR #6849
✅ No actionable findings
Reviewed the complete trusted base-to-head comparison. No concrete, actionable correctness, security, CI, or test-coverage defects were found. The test OAuth seam remains off by default, requires the explicit feature in a debug build, validates paired literal-loopback endpoints, and the workflows preserve the default binary separately from the SSO test binary.
Validation and technical details
- Inspected all 11 changed files and surrounding CLI OAuth-provider, Google token-decoding, E2E fixture, harness, manifest, and workflow code.
- Verified the comparison with git diff refs/ironloop/base..refs/ironloop/head and git diff --check; no whitespace errors were reported.
- Python byte-compilation succeeded for all changed Python files.
- scripts/ci/check-reborn-responses-e2e-manifest.py passed with 21 tests.
- Rust tests could not be executed because cargo is unavailable in the review environment; pytest collection and workflow YAML parsing were also unavailable because pytest/PyYAML or Ruby are not installed.
- Base:
main - Head:
issue-4636-sso-e2eatb63d04e - Run:
b39b43f8-7ca9-4e44-ad22-f148f28ccbb8
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_reborn_cli/Cargo.toml`:
- Around line 35-40: Update the Cargo feature comment to state the approved
feature bar that justifies enabling this dev-only seam, and rename
e2e-test-support to the existing test-support seam consistently across
conftest.py, both workflows, and all cfg(feature = ...) references; preserve the
current dependency activation and debug/loopback gating.
In `@crates/ironclaw_reborn_cli/src/commands/serve_sso.rs`:
- Around line 240-255: Add a caller-level test under #[cfg(feature =
"e2e-test-support")] that invokes sso_startup_config_from_env with Google test
endpoints configured but IRONCLAW_REBORN_WEBUI_GOOGLE_CLIENT_ID absent, and
assert startup aborts with an error mentioning that environment variable. Keep
the test focused on the fail-closed branch in the provider setup flow.
- Around line 346-363: Remove build_google_provider and fold the test-endpoint
selection into its call site, using the normal GoogleProvider::new construction
for default builds and GoogleProvider::with_endpoints only when e2e-test-support
is enabled. Do not add a #[cfg(not(feature = ...))] fallback or panic path;
validate both default and e2e-test-support builds.
In `@tests/e2e/CLAUDE.md`:
- Line 145: Update the reborn_v2_sso_server row in the session-scoped fixture
table to mark it as module-scoped, matching the reborn_v2_server row above, and
identify tests/e2e/reborn_webui_harness.py as the defining module.
In `@tests/e2e/conftest.py`:
- Around line 487-515: Both feature variants overwrite target/debug/ironclaw, so
isolate the SSO build and simplify the workflow build steps. In
tests/e2e/conftest.py lines 487-515, update ironclaw_reborn_sso_binary to use
target/e2e-sso, build with that target directory, return
target/e2e-sso/debug/ironclaw, and remove unlink/copy logic. In
.github/workflows/reborn-e2e.yml lines 311-323, remove the restore/canary
packaging workaround and trust comment, leaving plain builds with separate
target directories. In .github/workflows/coverage.yml lines 242-251, likewise
remove rm/copy/restore steps and build each variant into its own target
directory.
In `@tests/e2e/scenarios/test_reborn_webui_v2_sso.py`:
- Around line 176-185: The cross-user SSE denial checks in the scenario
currently use httpx response buffering, making them depend on the stream closing
promptly. Replace the /events requests in the alice_streams_bob and
bob_streams_alice flow with the existing aiohttp-based sse_stream() helper, then
assert the denied stream_error frame through that incremental SSE path while
preserving both cross-user denial checks.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 763d4037-8b09-418b-84b2-59e576257fa7
📒 Files selected for processing (11)
.github/workflows/coverage.yml.github/workflows/reborn-e2e.ymlcrates/ironclaw_reborn_cli/Cargo.tomlcrates/ironclaw_reborn_cli/src/commands/serve_sso.rstests/e2e/CLAUDE.mdtests/e2e/README.mdtests/e2e/conftest.pytests/e2e/fixtures/mock_oauth_idp.pytests/e2e/reborn_coverage_tests.txttests/e2e/reborn_webui_harness.pytests/e2e/scenarios/test_reborn_webui_v2_sso.py
Contributor
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.55% — 315532 / 368828 lines Per-crate breakdown (60 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (3 entry/entries excluded from the accounting above)
|
|
🚅 Deployed to the ironclaw-pr-6849 environment in ironclaw-ci-preview
|
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6849
July 29, 2026 11:22
Destroyed
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6849
July 29, 2026 11:46
Destroyed
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6849
July 29, 2026 12:05
Destroyed
# Conflicts: # tests/e2e/CLAUDE.md # tests/e2e/reborn_coverage_tests.txt # tests/e2e/reborn_webui_harness.py
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6849
July 30, 2026 05:03
Destroyed
hanakannzashi
previously approved these changes
Jul 30, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Jul 30, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
Jul 30, 2026
# Conflicts: # tests/e2e/reborn_webui_harness.py
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6849
July 30, 2026 10:45
Destroyed
hanakannzashi
approved these changes
Jul 30, 2026
l3ocifer
pushed a commit
to l3ocifer/frick-ironclaw
that referenced
this pull request
Sep 3, 2026
…rage (nearai#6849) * test(webui): add guarded SSO provider E2E seam * test(e2e): cover WebUI SSO multi-user isolation * fix(test): address SSO E2E review feedback
This branch was successfully deployed
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
2 participants
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.
Summary
ironclaw serveprocess.Change Type
Linked Issue
Closes #4636
Validation
cargo fmt --all -- --checkcargo clippy -p ironclaw --features e2e-test-support --all-targets -- -D warningscargo test -p ironclaw --features e2e-test-support --bin ironclaw— 464 passedgit diff --checkTest Strategy
User behavior:
A user can complete SSO login through the standalone server, exchange the login ticket for a session bearer, access protected WebUI APIs, and log out. Two admitted users in the same tenant remain isolated.
Risk areas:
Tests added or updated:
What the tests prove:
The shipping server path can complete a hermetic SSO session flow, maps distinct provider identities to distinct users, denies cross-user thread/timeline/event-stream access, and revokes only the logged-out session.
Commands run:
cargo fmt --all -- --checkcargo clippy -p ironclaw --features e2e-test-support --all-targets -- -D warningscargo test -p ironclaw --features e2e-test-support --bin ironclawcargo test -p ironclaw --features e2e-test-support serve_sso -- --nocapturepytest tests/e2e/scenarios/test_reborn_webui_v2_sso.py -v --timeout=120python3 scripts/ci/check-reborn-responses-e2e-manifest.pygit diff --checkSecurity Impact
Yes, but test-only. The endpoint override requires an explicit Cargo feature, a debug build, paired environment variables, and HTTP loopback IP literals without URL credentials. Default and release builds reject activation. Mock credentials and identities are local test data and are not written to artifacts.
Trust-Boundary Checklist
Database Impact
None. No schema, migration, persistence format, or backend behavior changes.
Blast Radius
Limited to the WebUI SSO test seam, Python E2E harness, and the E2E/coverage workflows. Default runtime behavior is unchanged unless the test feature and guarded environment variables are explicitly enabled.
Rollback Plan
Revert the E2E commit followed by the test-seam commit. No data rollback or migration is required.
Review Follow-Through
Self-review identified missing explicit SSE isolation coverage. The E2E now verifies that cross-user stream attempts receive only a redacted
not_foundstream error and no business events. No remaining actionable findings are known.Review track: C