Skip to content

fix(oauth): record and log why an OAuth start failed - #574

Merged
Bccorb merged 4 commits into
mainfrom
fix/oauth-start-failure-reason
Nov 6, 2026
Merged

Bccorb merged 4 commits into
mainfrom
fix/oauth-start-failure-reason

Conversation

@Bccorb

@Bccorb Bccorb commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Problem

startOAuthLogin caught every error and dropped it. A refused redirect and an unusable provider both left oauth_login_failed with reason start_failed and no log line. RoxTarget production recorded these for three weeks with nothing to say why, and the cause had to be worked out from load balancer logs.

Change

  • The event keeps reason: start_failed and adds detail: redirect_not_allowed or provider_unusable.
  • The error is logged at warn, and the event records the provider. A refused redirect is logged by scheme and host only, since the rest is whatever the caller sent.
  • The answer to the client is unchanged.

Testing

npm test, npm run lint, typecheck and Prettier pass. New test: an app scheme with no allowlist entry answers 400 with detail: redirect_not_allowed.

The catch dropped the error, so a refused redirect and a dead provider left the
same bare start_failed event and nothing in the log.
Comment thread src/controllers/oauth.ts Fixed
CodeQL treats a value read from the provider config as sensitive. The route
parameter is the same id.
Comment thread src/controllers/oauth.ts Fixed
CodeQL flagged the route parameter in the log entry. Line breaks are the part
of it that could forge another entry, so they are removed from the whole
message, including the error text.
Comment thread src/controllers/oauth.ts Fixed
The event already records the provider, so the log entry does not need a value
from the request.
@Bccorb
Bccorb merged commit 1400b20 into main Nov 6, 2026
7 checks passed
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.

2 participants