Skip to content

feat(firestore-bigquery-export): authenticate TRANSFORM_FUNCTION calls#2868

Open
Murayu0225 wants to merge 3 commits into
firebase:nextfrom
Murayu0225:feat/transform-function-oidc-auth
Open

feat(firestore-bigquery-export): authenticate TRANSFORM_FUNCTION calls#2868
Murayu0225 wants to merge 3 commits into
firebase:nextfrom
Murayu0225:feat/transform-function-oidc-auth

Conversation

@Murayu0225
Copy link
Copy Markdown

Resolves #2804

transformRows now mints an OIDC ID token for the configured TRANSFORM_FUNCTION URL and attaches it as an Authorization: Bearer ... header. Users can then restrict the transform endpoint via Cloud Run IAM by granting roles/run.invoker to the extension's runtime service account; the extension itself does not modify IAM policies.

Backward compatibility:

  • Public transform functions ignore the extra header and behave exactly as before.
  • If minting the token fails (e.g., local dev, non-Google URL), the request falls back to an unauthenticated state and a warning is logged.

google-auth-library is added as a direct dependency (previously pulled in transitively via firebase-admin). Unit tests cover the authenticated, fallback, and disabled paths.

@Murayu0225 Murayu0225 requested a review from a team as a code owner May 27, 2026 10:24
@google-cla
Copy link
Copy Markdown

google-cla Bot commented May 27, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds support for authenticating calls to the configured transform function in the Firestore BigQuery Change Tracker. It introduces google-auth-library to mint an ID token for the transform function URL and appends the authorization headers to the outgoing fetch request, falling back to an unauthenticated call if token minting fails. The review feedback suggests optimizing performance by caching the resolved IdTokenClient on the class instance to avoid repeated initialization, and improving error handling by checking response.ok before parsing the response as JSON.

Comment on lines +124 to +133
let authHeaders = {};
try {
const auth = new GoogleAuth();
const client = await auth.getIdTokenClient(
this.config.transformFunction
);
authHeaders = await client.getRequestHeaders();
} catch (err) {
logs.transformFunctionAuthFailed(err);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Instantiating new GoogleAuth() and calling getIdTokenClient on every single transformRows invocation introduces unnecessary overhead, especially since the transformFunction URL is constant for the tracker instance.

We can optimize this by caching the resolved IdTokenClient on the class instance (e.g., using (this as any).authClient). This avoids repeated initialization and improves performance during high-throughput document writes while maintaining clean test isolation.

      let authHeaders = {};
      try {
        if (!(this as any).authClient) {
          const auth = new GoogleAuth();
          (this as any).authClient = await auth.getIdTokenClient(
            this.config.transformFunction
          );
        }
        authHeaders = await (this as any).authClient.getRequestHeaders();
      } catch (err) {
        logs.transformFunctionAuthFailed(err);
      }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This would be a nice optimization but not blocking for this PR

Comment on lines 135 to 140
const response = await fetch(this.config.transformFunction, {
method: "post",
body: JSON.stringify({ data: rows }),
headers: { "Content-Type": "application/json" },
headers: { "Content-Type": "application/json", ...authHeaders },
});
const responseJson = await response.json();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

When the transform function returns a non-2xx status code (e.g., 401 Unauthorized or 403 Forbidden due to misconfigured IAM permissions), the response body is often HTML rather than JSON. Calling response.json() directly in these cases will throw a cryptic JSON parsing error.

Checking response.ok first and throwing a descriptive error makes debugging authentication and configuration issues much easier.

      const response = await fetch(this.config.transformFunction, {
        method: "post",
        body: JSON.stringify({ data: rows }),
        headers: { "Content-Type": "application/json", ...authHeaders },
      });
      if (!response.ok) {
        throw new Error(
          'Transform function failed with status ' + response.status + ': ' + response.statusText
        );
      }
      const responseJson = await response.json();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think this needs addressing

Copy link
Copy Markdown

@IzaakGough IzaakGough 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 overall! Can we address the gemini comment regarding the response.ok handling?

Comment on lines +124 to +133
let authHeaders = {};
try {
const auth = new GoogleAuth();
const client = await auth.getIdTokenClient(
this.config.transformFunction
);
authHeaders = await client.getRequestHeaders();
} catch (err) {
logs.transformFunctionAuthFailed(err);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This would be a nice optimization but not blocking for this PR

Comment on lines 135 to 140
const response = await fetch(this.config.transformFunction, {
method: "post",
body: JSON.stringify({ data: rows }),
headers: { "Content-Type": "application/json" },
headers: { "Content-Type": "application/json", ...authHeaders },
});
const responseJson = await response.json();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think this needs addressing

@Murayu0225
Copy link
Copy Markdown
Author

Thanks for the review! I've addressed the comment regarding response.ok handling. Could you take another look?

@Murayu0225 Murayu0225 requested a review from IzaakGough May 27, 2026 14:41
Copy link
Copy Markdown

@IzaakGough IzaakGough left a comment

Choose a reason for hiding this comment

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

@Murayu0225 This looks great! Thanks for addressing the comments.

@Murayu0225
Copy link
Copy Markdown
Author

@IzaakGough Thanks for checking! I've fixed the linting error that was causing the CI to fail.

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.

🐛 [firestore-bigquery-export] Authenticate calls to TRANSFORM_FUNCTION so it can be IAM-protected

3 participants