feat(auth): verify-token endpoint and quieter expected-error logging - #3748
Open
jmthomas wants to merge 3 commits into
Open
feat(auth): verify-token endpoint and quieter expected-error logging#3748jmthomas wants to merge 3 commits into
jmthomas wants to merge 3 commits into
Conversation
4 tasks
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3748 +/- ##
==========================================
- Coverage 80.02% 79.99% -0.04%
==========================================
Files 885 885
Lines 65382 65396 +14
Branches 2543 2543
==========================================
- Hits 52322 52311 -11
- Misses 12396 12421 +25
Partials 664 664
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds a dedicated endpoint for validating existing session tokens (without impacting password rate limiting), and reduces noise in JSON-RPC logs by downgrading routine/expected exceptions to single-line messages.
Changes:
- Add
POST /auth/verify-tokenroute + controller action to validate session tokens via prefix-gating before Redis access. - Add controller specs covering session token validation, prefix enforcement, OTP non-consumption, and rate-limit non-interaction.
- Adjust
json_drbexception logging to emit concise logs for common/expected auth and operator-confirmation exceptions.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| openc3/lib/openc3/io/json_drb.rb | Downgrades logging for expected exception types; keeps stack filtering for unexpected errors |
| openc3-cosmos-cmd-tlm-api/spec/controllers/auth_controller_spec.rb | Adds spec coverage for the new verify_token behavior and invariants |
| openc3-cosmos-cmd-tlm-api/config/routes.rb | Registers the new POST /auth/verify-token endpoint |
| openc3-cosmos-cmd-tlm-api/app/controllers/auth_controller.rb | Implements verify_token with session-prefix gating and token verification |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+285
to
+296
| if AuthError === e or ForbiddenError === e | ||
| # Auth failures are expected (e.g. browser requests before login or with an | ||
| # expired session token) so log a single line without the stack trace | ||
| Logger.warn "#{e.class.name.split('::')[-1]} : #{e.message} calling #{request.method.downcase()}" | ||
| elsif HazardousError === e or CriticalCmdError === e | ||
| # Not errors at all: these are the signal that the UI should raise an | ||
| # operator confirmation dialog, and the user retries with the hazardous | ||
| # or critical flag set. A stack trace per prompt is pure noise. Note | ||
| # HazardousError#formatted is the formatted command, not a backtrace, | ||
| # so the else branch below logged the command with no context anyway. | ||
| detail = e.message == e.class.name ? '' : " : #{e.message}" | ||
| Logger.info "#{e.class.name.split('::')[-1]}#{detail} calling #{request.method.downcase()}" |
jmthomas
force-pushed
the
auth-verify-token-logging
branch
from
August 20, 2026 21:27
deb154e to
5eb0acc
Compare
The login page previously checked an existing session token via auth#verify, but a session token is never a valid password, so every check failed as a bad password attempt and a stale localStorage token ate into the rate limit. verify-token requires the session prefix before touching Redis, which also keeps an OTP token out of verify_no_service where it would be consumed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AuthError/ForbiddenError happen routinely (browser requests before login or with an expired token) and HazardousError/CriticalCmdError are the signal to raise an operator confirmation dialog, not failures. All four dumped a filtered stack trace per occurrence. Log them as warn/info one-liners instead; HazardousError#formatted is the command, not a backtrace, so the old path was logging a bare command with no context anyway. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
jmthomas
force-pushed
the
auth-verify-token-logging
branch
from
August 20, 2026 23:24
1477d80 to
03ddfa1
Compare
|
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
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.
Backend half of a stacked pair. Frontend follow-up (auth guard + batched settings) is stacked on top of this branch.
POST /auth/verify-token(cmd-tlm-api)The login page needs to know whether the browser already holds a valid session token so it can skip the login form. It was asking
auth#verify, but a session token is never a valid password — the check always failed and counted as a bad password attempt, so a stale token in localStorage burned through the rate limit.verify_tokenis deliberately separate:SESSION_PREFIXbefore touching Redis, so an unauthenticated caller can't make usHGETALLthe whole session hash per request.verify_no_service, which would otherwise consume it.Quieter JSON-RPC error logging (openc3 gem)
json_drblogged a filtered stack trace for every exception. Four classes are routine, not failures:AuthError/ForbiddenError— browser requests before login or with an expired session token →warn, one lineHazardousError/CriticalCmdError— the signal for the UI to raise an operator confirmation dialog; the user then retries with the flag set →info, one lineHazardousError#formattedis the formatted command, not a backtrace, so the old path was logging a bare command with no context anyway.Test plan
auth_controller_spec.rbcovers verify-token: valid session token, wrong/absent prefix, bad token🤖 Generated with Claude Code