chore: revert "feat(auth): Regional access boundaries main merge (#8665)"#8747
Conversation
…)" This reverts commit 76e6d3b.
There was a problem hiding this comment.
Code Review
This pull request removes the Regional Access Boundary (RAB) feature and its associated manager, helper functions, and tests across the entire codebase. Feedback on these changes highlights a potential issue where the opts parameter is mutated directly by reassigning opts.headers in several client classes, which could lead to unexpected side effects; shallow-copying opts is recommended instead. Additionally, the removal of the isExpired method from AuthClient has introduced code duplication across multiple subclasses, and keeping it as a protected method in the base class is suggested to maintain DRY principles.
I am having trouble creating individual review comments. Click here to see my feedback.
core/packages/google-auth-library-nodejs/src/auth/baseexternalclient.ts (504)
Mutating the opts parameter directly by reassigning opts.headers can lead to unexpected side effects for callers who reuse the options object across multiple requests.
To prevent this, we should shallow-copy opts into a local requestOpts variable and perform all modifications on the copy. Additionally, when cloning or wrapping the request, ensure properties like adapter are stripped to prevent potential infinite recursion or memory leaks:
const {adapter, ...requestOpts} = opts;
try {
const requestHeaders = await this.getRequestHeaders();
requestOpts.headers = Gaxios.mergeHeaders(requestOpts.headers);
this.addUserProjectAndAuthHeaders(requestOpts.headers, requestHeaders);
response = await this.transporter.request<T>(requestOpts);
}References
- When handling custom interceptors or headers in concurrent asynchronous requests, isolate them using a fresh, request-scoped HTTP client instance instead of a shared instance. If cloning or wrapping the request, ensure properties like
adapterare stripped if they can cause infinite recursion or memory leaks.
core/packages/google-auth-library-nodejs/src/auth/externalAccountAuthorizedUserClient.ts (261)
Mutating the opts parameter directly by reassigning opts.headers can lead to unexpected side effects for callers who reuse the options object across multiple requests.
To prevent this, we should shallow-copy opts into a local requestOpts variable and perform all modifications on the copy. Additionally, when cloning or wrapping the request, ensure properties like adapter are stripped to prevent potential infinite recursion or memory leaks:
const {adapter, ...requestOpts} = opts;
try {
const requestHeaders = await this.getRequestHeaders();
requestOpts.headers = Gaxios.mergeHeaders(requestOpts.headers);
this.addUserProjectAndAuthHeaders(requestOpts.headers, requestHeaders);
response = await this.transporter.request<T>(requestOpts);
}References
- When handling custom interceptors or headers in concurrent asynchronous requests, isolate them using a fresh, request-scoped HTTP client instance instead of a shared instance. If cloning or wrapping the request, ensure properties like
adapterare stripped if they can cause infinite recursion or memory leaks.
core/packages/google-auth-library-nodejs/src/auth/oauth2client.ts (1123)
Mutating the opts parameter directly by reassigning opts.headers can lead to unexpected side effects for callers who reuse the options object across multiple requests.
To prevent this, we should shallow-copy opts into a local requestOpts variable and perform all modifications on the copy. Additionally, when cloning or wrapping the request, ensure properties like adapter are stripped to prevent potential infinite recursion or memory leaks:
const {adapter, ...requestOpts} = opts;
try {
const r = await this.getRequestMetadataAsync();
requestOpts.headers = Gaxios.mergeHeaders(requestOpts.headers);
this.addUserProjectAndAuthHeaders(requestOpts.headers, r.headers);
if (this.apiKey) {
requestOpts.headers.set('X-Goog-Api-Key', this.apiKey);
}
return await this.transporter.request<T>(requestOpts);
}References
- When handling custom interceptors or headers in concurrent asynchronous requests, isolate them using a fresh, request-scoped HTTP client instance instead of a shared instance. If cloning or wrapping the request, ensure properties like
adapterare stripped if they can cause infinite recursion or memory leaks.
core/packages/google-auth-library-nodejs/src/auth/authclient.ts (630-635)
Removing isExpired from AuthClient forces multiple subclasses (BaseExternalAccountClient, DownscopedClient, and ExternalAccountAuthorizedUserClient) to duplicate this exact logic as private methods.
To maintain DRY (Don't Repeat Yourself) principles and improve maintainability, consider keeping isExpired as a protected method in AuthClient.
This one needs to be "un-released" for now at the request of the team.