fix tranquil pds#2237
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a20a1cdc1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) { | ||
| url.protocol = tokens.oAuthToken.pds.protocol | ||
| url.host = tokens.oAuthToken.pds.host | ||
| val baseUrl = Url(tokens.baseUrl) |
There was a problem hiding this comment.
When an OAuth account uses an entryway/authorization server that is separate from its PDS, tokens.baseUrl is the saved OAuth iss value from login, not necessarily the resource-server URL. This line overrides the request URL after baseUrlFlow/newBaseUrlService() has already resolved the actual DID-doc PDS endpoint, so all OAuth-authenticated XRPC calls for those accounts are sent to the entryway instead of the PDS and will fail or hit the wrong service.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57621e5a74
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| method?.startsWith("app.bsky.") == true -> { | ||
| context.headers["Atproto-Proxy"] = "did:web:api.bsky.app#bsky_appview" |
There was a problem hiding this comment.
This catch-all also matches non-AppView app.bsky endpoints, notably app.bsky.actor.getPreferences/putPreferences used by BlueskyFeedLoader to load and update saved feeds, and app.bsky.notification.* which uses a separate notification service. When those requests now carry Atproto-Proxy: did:web:api.bsky.app#bsky_appview, the PDS forwards private preferences/notification calls to the wrong service instead of handling them (or using #bsky_notif), so saved-feed preference reads/writes and notifications can fail for OAuth/service-proxied accounts.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2056bbecca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| val dpopHeader = | ||
| oAuthApi.createDpopHeaderValue( | ||
| keyPair = tokens.oAuthToken.keyPair, |
There was a problem hiding this comment.
After this change, OAuth requests use whatever host baseUrlFlow has already selected when creating the DPoP proof, but production initializes that flow from OAuthCredential.baseUrl, which is stored as the callback iss in BlueskyOAuthLoginPresenter/BlueskyLoginProvider. For accounts whose authorization server is an entryway separate from the PDS, the first pdsService() call still needs to send getSession to a resource-server/PDS URL before newBaseUrlService() can resolve the DID-doc endpoint; otherwise it calls the entryway with a PDS access token and cannot discover or use the user's PDS. This is separate from the already-commented post-resolution override path: with this commit there is no fallback PDS target before the resolution step runs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8a831270c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| val credential: BlueskyCredential = | ||
| BlueskyCredential.OAuthCredential( | ||
| baseUrl = iss, | ||
| baseUrl = resourceServerBaseUrl, |
There was a problem hiding this comment.
When iss differs from resourceServer, this now stores only the resource server in OAuthCredential.baseUrl. BlueskyService later constructs the OAuthApi used by refreshExpiredToken() from credentialFlow.map { it.baseUrl }, so after the access token expires the refresh path resolves/posts against the PDS base instead of the issuer that returned the token. For OAuth accounts whose authorization server is an entryway separate from the PDS, refresh will fail and the user is logged out; keep the issuer/token endpoint separately from the XRPC resource-server URL instead of replacing it here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41abf416ae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| onCredentialRefreshed: suspend (BlueskyCredential) -> Unit, | ||
| ) : this( | ||
| baseUrlFlow = credentialFlow.map { it.baseUrl }, | ||
| baseUrlFlow = flowOf(accountKey.host.toHttpsBaseUrl()), |
There was a problem hiding this comment.
For accounts whose saved PDS URL includes a non-default scheme or port (for example a self-hosted http://localhost:2583 or https://pds.example:8443), this rebuilds the XRPC base URL from accountKey.host instead of the persisted BlueskyCredential.baseUrl. Password login stores the full working URL in the credential but only Url(baseUrl).host in the account key, so later authenticated calls now go to https://localhost/https://pds.example and lose the port or HTTP scheme before getSession() can resolve the DID doc.
Useful? React with 👍 / 👎.
Tlaster
force-pushed
the
bugfix/tranquil-pds
branch
from
41abf41 to
7ebd8ed
Compare
1 participant
fix #2221