Skip to content

feat(auth): login self-heal, logout, status, OAuth best practices#73

Open
sVIKs wants to merge 1 commit into
corezoid:mainfrom
sVIKs:fix/login-account-url-selfheal
Open

feat(auth): login self-heal, logout, status, OAuth best practices#73
sVIKs wants to merge 1 commit into
corezoid:mainfrom
sVIKs:fix/login-account-url-selfheal

Conversation

@sVIKs

@sVIKs sVIKs commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

What

Self-diagnosing, self-healing auth for the simulator MCP server, plus OAuth 2.1 / RFC 8414 / RFC 8252 alignment. Sibling PR on the corezoid plugin: sVIKs/corezoid-ai-plugin PR #69 (shared .env semantics are coordinated).

login self-heals ACCOUNT_URL

The .env keys are shared with the corezoid plugin; a sibling tool can overwrite ACCOUNT_URL with a host that serves no OAuth endpoints (field incident: the browser opened the admin dashboard and login died silently). login now asks the chosen gateway's public config for its declared account service (saUrl — works for cloud and on-prem) before opening a browser, corrects .env on mismatch, and reports what was fixed. Failures explain what to check and show the consent URL that was used.

New tools

  • logout — removes ACCESS_TOKEN/REFRESH_TOKEN with a .env.bak backup first (backup failure aborts), warns about the shared .env.
  • status — no-side-effect diagnosis: environment, token state, .env path; probe=true verifies ACCOUNT_URL against the gateway.

OAuth hardening (verified against the live account service)

  • Endpoint discovery via /.well-known/oauth-authorization-server (the service publishes it — verified) with a conventional-path fallback for older on-prem installs.
  • Silent renewal via the refresh_token grant before any browser round-trip. The service currently answers that grant with a session-scoped token papi rejects (verified live), so a refreshed token is trusted only after one authenticated papi call succeeds; otherwise the flow falls back to the browser, respecting refresh-token rotation and dropping a useless stored token.
  • ACCOUNT_URL scheme validation (https, or http for loopback only) before the browser opens; ctx-aware, timeout-bounded token exchange honouring --insecure; token-endpoint bodies never echoed into errors (field names only).

Test hygiene

TestMain pins SIMULATOR_WORK_DIR to a sandbox for the whole suite — a test that forgets t.Setenv can no longer touch a real .env.

Testing

  • go build ./..., go vet ./..., go test -race ./... green; +20 tests (self-heal, discovery, refresh rotation/validation, redaction, logout roundtrip, ctx cancel).
  • Live: status MISMATCH detection against a poisoned .env on the real mw gateway; full OAuth login through the real consent flow; logout backup/idempotence; refresh grant + papi rejection verified with real tokens.

🤖 Generated with Claude Code

login now self-diagnoses before opening a browser: it verifies
ACCOUNT_URL against the gateway's public config (saUrl — works for
cloud and on-prem) and self-heals .env when a sibling tool sharing the
file overwrote it, reporting what was fixed. OAuth failures explain
what to check and include the authorization URL for a manual retry;
the callback wait honours client cancellation.

New tools: logout (removes ACCESS_TOKEN with a .env.bak backup first,
warns about the shared .env) and status (no-side-effect diagnosis of
environment, token state and .env path; probe=true checks ACCOUNT_URL
against the gateway).

OAuth hardening per OAuth 2.1 / RFC 8414 / RFC 8252: endpoint
discovery via /.well-known/oauth-authorization-server with a
conventional-path fallback, silent refresh_token renewal validated by
one authenticated papi call before being trusted, ACCOUNT_URL scheme
validation, bounded token requests, standard access_token accepted,
and token-endpoint bodies never echoed into error messages.

Tests run in a sandboxed SIMULATOR_WORK_DIR via TestMain so a suite
run can never touch a real .env.
@gh-corezoid

Copy link
Copy Markdown

AI Review

Adds OAuth 2.1/RFC 8414 hardening to the simulator MCP server: login now self-heals ACCOUNT_URL from the gateway's public config before opening a browser, plus two new tools (logout with .env.bak backup and status for no-side-effect diagnosis), silent refresh-token renewal, PKCE flow context-cancellation, and a TestMain sandbox in both test packages.

Checklist

Check Result
U1 — Conventional commit format ✅ pass
U2 — No leaked credentials ✅ pass
U3 — No merge commits ✅ pass
U4 — PR targets correct base branch ❌ fail
U5 — Build & tests (Go) ✅ pass
U6 — Architectural & design consequences ⚠️ warning
S1 — No manual edits to public/ ✅ skip
S2 — API path parameter names match openapi ✅ skip
S3 — New tools have eval scenarios ⚠️ warning
S4 — Discovery artifacts committed if source changed ✅ pass
S5 — All six manifest files version-synced ✅ skip
S6 — README and ARCHITECTURE.md §4 updated for new tools ⚠️ warning

Issues found

[error] U4 — Wrong base branch
The PR targets main but simulator-ai-plugin requires base branch develop. This is a feature PR (feat(auth)) with new tools, not a docs-only or chore change, so the mismatch is hard — CI guards on simulator-ai-plugin reject non-develop bases.
Fix: close this PR and reopen (or retarget) against develop.


[warning] U6 — validateSimToken duplicates TLS transport logic from authHTTPClient
build.go's validateSimToken() (the papi-probe used to validate a refresh-minted token) hand-builds its own http.Transport with InsecureSkipVerify logic:

tr := &http.Transport{}
if insecure {
    tr.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
}
resp, err := (&http.Client{Timeout: 15 * time.Second, Transport: tr}).Do(req)

This duplicates auth.authHTTPClient(timeout) in oauth.go, which was introduced in this same PR for exactly this purpose. If transport behavior needs updating (proxy settings, retry policy, cipher list), there are now two places to change. Concrete failure scenario: --insecure support for the token-exchange path gets fixed in authHTTPClient but the papi probe keeps the old behavior silently.
Fix: export authHTTPClient (or add a thin auth.NewHTTPClient(insecure, timeout) constructor) and use it in validateSimToken.


[warning] S3 — logout and status missing from eval-scenarios.json and from knownToolNames()
build.go now registers logout and status, but neither appears in testdata/eval-scenarios.json. Per S3, new tools should have at least one evaluation scenario so the corpus stays representative of the real tool surface.
Additionally, eval_test.go's knownToolNames() function explicitly lists auth helpers ("login", "set-workspace") but was not updated to include "logout" and "status". Any future scenario that references these tools will fail TestEvalScenariosReferenceRealTools with an "unknown tool" error, which will block whoever adds the scenario rather than the PR author.
Fix: add "logout": true and "status": true to knownToolNames(), and add at least one scenario each to eval-scenarios.json.


[warning] S6 — docs/ARCHITECTURE.md §4 not updated
The Setup row in §4 still reads:

| Setup | set-environment ... login getWorkspaces set-workspace |

logout and status appear in README.md (correctly updated) but not in ARCHITECTURE.md §4.
Fix: add logout and status to the Setup row in docs/ARCHITECTURE.md §4 with their one-line descriptions.


This review was generated automatically. A human maintainer should still make the merge decision.

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