Skip to content

Improve auth failure diagnostics #295

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 9, 2025
Merged

Improve auth failure diagnostics #295

merged 4 commits into from
Jul 9, 2025

Conversation

rkistner
Copy link
Contributor

@rkistner rkistner commented Jul 9, 2025

The specific scenario here is that if a JWKS takes longer than 3 seconds to complete, there would be very little info in the logs on what is happening. The logs would include a message such as Known kid values: powersync-api, *, powersync-dev, without any further details.

Note that we have two timeouts applying to JWKS URIs:

  1. The CachedKeyCollector returns a response after 3s, while letting the refresh continue in the background.
  2. The background refresh in RemoteJWKSCollector has a hard timeout of 30s.

This improves the error and timeout timeout handling in a couple of ways:

  1. Log additional details for the keys, to show why certain keys are not matching, such as Known keys: <kid: powersync-api, kty: oct, alg: HS256>, <kid: *, kty: oct, alg: HS256, aud: authenticated>, <kid: powersync-dev, kty: RSA, alg: RS256>.
  2. Treat the 3s request timeouts as errors, instead of just "ignoring" it.
  3. Always log JWKS errors when they happen, instead of relying on the request handler to log it.

For the timeout, the issue was that the CachedKeyCollector would return the current keys after a 3-second timeout, with no error message. But if that was the first request, there would be no keys and no errors, giving no indication on what is happening. This changes it to return an error message. Note that if there are already relevant keys cached, those would still be used.

With these changes, you'd now see this in the logs in the case of a timeout:

Request failed [PSYNC_S2204] JWKS request failed {"cause":{"message":"Key request timed out in 3s","name":"AbortError"}
...
Failed to refresh keys [PSYNC_S2204] JWKS request failed {"cause":{"message":"Request timed out in 30s","name":"AbortError"},"configurationDetails":"JWKS URL: https://my-jwks-uri"

@rkistner rkistner requested a review from stevensJourney July 9, 2025 09:08
Copy link

changeset-bot bot commented Jul 9, 2025

🦋 Changeset detected

Latest commit: 42dfdcd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages
Name Type
@powersync/service-core Patch
@powersync/service-core-tests Patch
@powersync/service-module-core Patch
@powersync/service-module-mongodb-storage Patch
@powersync/service-module-mongodb Patch
@powersync/service-module-mysql Patch
@powersync/service-module-postgres-storage Patch
@powersync/service-module-postgres Patch
@powersync/service-image Patch
test-client Patch
@powersync/service-schema Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Collaborator

@stevensJourney stevensJourney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 🙏

@rkistner rkistner merged commit f1431b6 into main Jul 9, 2025
35 of 36 checks passed
@rkistner rkistner deleted the improve-key-diagnostics branch July 9, 2025 09:31
rkistner added a commit that referenced this pull request Jul 9, 2025
* Expand on key details.

* Improve JWKS timeout handling.

* Add changeset.

* Always log JWKS errors.
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