refactor: add auth headers in the main COMPASS-10818#8218
Draft
paula-stacho wants to merge 4 commits into
Draft
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to centralize Atlas Admin API authentication by injecting auth headers from the main process, while refactoring existing Atlas API callers to use a unified fetch path.
Changes:
- Adds a main-process HTTPS protocol handler intended to attach Atlas Admin API auth headers.
- Removes the
authenticatedFetch/getAuthHeadersabstraction and migrates multiple call sites toAtlasService.fetch. - Introduces a whitelist of Atlas Admin API endpoints that should receive auth headers.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/compass/src/main/application.ts | Registers an HTTPS protocol handler to add auth headers via main-process networking. |
| packages/compass-web/src/connection-storage.tsx | Switches Atlas connection info loading from authenticatedFetch to fetch. |
| packages/compass-web/src/atlas-auth-service.tsx | Removes getAuthHeaders override from cloud auth service. |
| packages/compass-user-data/src/user-data.ts | Replaces authenticatedFetch usage with fetch in Atlas user data operations. |
| packages/compass-settings/test/configure-store.ts | Updates test mock auth service to reflect removed getAuthHeaders. |
| packages/compass-indexes/src/modules/rolling-indexes-service.ts | Migrates rolling index request to fetch and updates inline comment. |
| packages/compass-global-writes/tests/create-store.tsx | Updates test atlasService stub from authenticatedFetch to fetch. |
| packages/compass-global-writes/src/services/atlas-global-writes-service.ts | Migrates multiple Atlas Global Writes calls from authenticatedFetch to fetch. |
| packages/compass-generative-ai/src/atlas-ai-service.ts | Migrates AI service requests from authenticatedFetch to fetch. |
| packages/atlas-service/src/main.ts | Adds endpoint classification + conditional auth header construction; fixes log field typo. |
| packages/atlas-service/src/compass-atlas-auth-service.ts | Removes IPC method wiring for token retrieval and getAuthHeaders. |
| packages/atlas-service/src/atlas-service.ts | Removes authenticatedFetch; updates internal callers to use fetch. |
| packages/atlas-service/src/atlas-auth-service.ts | Removes abstract getAuthHeaders from the auth service contract. |
| packages/atlas-service/src/atlas-admin-api-auth-endpoints.ts | Adds Atlas Admin API endpoint whitelist for auth header injection. |
Comments suppressed due to low confidence (1)
packages/atlas-service/src/atlas-service.ts:164
- This PR removes
AtlasService.authenticatedFetch, but there are still many remaining references in the repo (e.g.packages/atlas-service/src/atlas-service.spec.ts,packages/compass-web/src/connection-storage.spec.ts,packages/compass-user-data/src/user-data.spec.ts,packages/compass-indexes/src/modules/rolling-indexes-service.spec.ts,packages/compass-global-writes/src/store/index.spec.ts). Those will fail at runtime/compile unless updated (orauthenticatedFetchis kept as a compatibility wrapper during the migration).
}
async automationAgentRequest(
atlasMetadata: AtlasClusterMetadata,
opType: string,
opBody: Record<string, unknown>
Comment on lines
+202
to
+211
| protocol.handle('https', (req) => { | ||
| const authHeaders = CompassAuthService.maybeGetAuthHeaders(req); | ||
| return net.fetch(req, { | ||
| headers: { | ||
| ...req.headers, | ||
| ...authHeaders, | ||
| }, | ||
| bypassCustomProtocolHandlers: true, | ||
| }); | ||
| }); |
Comment on lines
+5
to
+15
| const groupId = /^([a-f0-9]{24})$/; | ||
| const clusterName = /^[a-zA-Z0-9][a-zA-Z0-9-]*$/; | ||
|
|
||
| export const ATLAS_ADMIN_API_AUTH_ENDPOINTS = [ | ||
| '/api/atlas/v2/clusters', | ||
| new RegExp(`/api/atlas/v2/groups/${groupId.source}/clusters`), | ||
| new RegExp( | ||
| `/api/atlas/v2/groups/${groupId.source}/clusters/${clusterName.source}` | ||
| ), | ||
| new RegExp(`/api/atlas/v2/groups/${groupId.source}/accessList`), | ||
| ]; |
Comment on lines
+485
to
+495
| static async maybeGetAuthHeaders( | ||
| req: Request | ||
| ): Promise<Record<string, string> | undefined> { | ||
| if (this.isAuthenticatedAtlasAdminAPIRequest(req)) { | ||
| return { | ||
| Authorization: `Bearer ${await this.maybeGetToken({ | ||
| tokenType: 'accessToken', | ||
| })}`, | ||
| }; | ||
| } | ||
| } |
Comment on lines
34
to
38
| type SerializeContent<I> = (content: I) => string; | ||
| type DeserializeContent = (content: string) => unknown; | ||
| type AuthenticatedFetch = ( | ||
| type fetch = ( | ||
| url: RequestInfo | URL, | ||
| options?: RequestInit |
Comment on lines
358
to
367
| // This type exists to avoid a circular dependency between user-data and atlas-service. | ||
| type AtlasServiceLike = { | ||
| userDataEndpoint: ( | ||
| orgId: string, | ||
| projectId: string, | ||
| dataType: UserDataType, | ||
| id?: string | ||
| ) => string; | ||
| authenticatedFetch: AuthenticatedFetch; | ||
| fetch: fetch; | ||
| }; |
Comment on lines
+472
to
+480
| static isAuthenticatedAtlasAdminAPIRequest(req: Request): boolean { | ||
| const url = new URL(req.url); | ||
| return ( | ||
| url.origin === this.config.atlasLogin.issuer && | ||
| ATLAS_ADMIN_API_AUTH_ENDPOINTS.some((endpoint) => { | ||
| if (typeof endpoint === 'string') { | ||
| return url.pathname === endpoint; | ||
| } | ||
| return endpoint.test(url.pathname); |
gribnoysup
reviewed
Jul 8, 2026
| throw err; | ||
| } | ||
| } | ||
| async authenticatedFetch( |
Collaborator
There was a problem hiding this comment.
I think we still want to keep that for two reasons: clearer in the code what is auth-ed vs un-authed endpoint / request, we still want to explicitly tell browser to send the credentials with the request
Collaborator
Author
There was a problem hiding this comment.
hmm.. I thought having the whitelist on the other side is enough, but we can send some custom header that marks the request for authentication (and still whitelist on the main) 👍
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.
Description
Checklist
Motivation and Context
Open Questions
Dependents
Types of changes