[Storage,Functions] Fix credentials for emulator - #16395
Conversation
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enforces stricter transport layer security across Firebase Functions and Firebase Storage by preventing Auth, FCM, and AppCheck tokens from being sent over insecure HTTP connections to non-loopback hosts. Review feedback suggests refactoring duplicated URL validation logic in Functions.swift into a private URL extension to improve maintainability, and guarding the warning log in StorageTokenAuthorizer.swift to prevent false positive logs when the request or URL is nil.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enforces stricter transport layer security in both Firebase Functions and Firebase Storage by preventing Auth, FCM, and AppCheck tokens from being attached to outbound HTTP requests directed to non-loopback hosts. While the implementation correctly identifies "localhost", "127.0.0.1", and "::1" as loopback addresses, the review feedback highlights that Foundation's URL.host returns IPv6 loopback addresses enclosed in square brackets (i.e., "[::1]"). To ensure robust IPv6 loopback detection, it is recommended to update the loopback checks in both Functions.swift and StorageTokenAuthorizer.swift to also match "[::1]".
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enforces stricter transport layer security in both FirebaseFunctions and FirebaseStorage by preventing Auth, FCM, and AppCheck tokens from being attached to outbound HTTP requests directed to non-loopback hosts. Feedback on the changes suggests improving consistency and developer experience by adding warning logs in FirebaseFunctions when tokens are withheld (matching the behavior in FirebaseStorage), and adding a unit test to verify that loopback hosts successfully attach tokens over HTTP.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enforces stricter transport layer security in both Firebase Functions and Firebase Storage by preventing Auth, FCM, and AppCheck tokens from being attached to outbound requests over insecure HTTP connections to non-loopback hosts. While the implementation is solid, the new unit tests in FunctionsTests.swift pass nil for the fake auth provider, which prevents them from properly verifying whether the Authorization header is correctly attached or omitted based on the connection security. Updating these tests to use a fake auth provider will make the test suite more robust.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enforces stricter transport layer security when using the emulator for both Firebase Functions and Firebase Storage. Auth, FCM, and AppCheck tokens are no longer attached to outbound requests if the connection is made over HTTP to a non-loopback host. Appropriate warnings are logged when refusing to send tokens, and unit tests have been added to verify these changes. No review comments were provided, so I have no additional feedback.
ncooke3
left a comment
There was a problem hiding this comment.
LGTM
One optional refactor would be to prefer a guard statement that catches the else blocks.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enforces stricter transport layer security in Firebase Functions and Firebase Storage when using the emulator by preventing Auth, FCM, and AppCheck tokens from being attached to outbound HTTP requests to non-loopback hosts. The feedback suggests a minor optimization in StorageTokenAuthorizer.swift to extract the lowercased URL scheme into a local variable to avoid redundant evaluations and improve readability.
See internal b/511890380. Review with https://github.com/firebase/firebase-ios-sdk/pull/16395/changes?w=1
Enforce token transport security for non-loopback HTTP connections
Description
This PR introduces stricter transport security checks for how Auth, FCM, and AppCheck tokens are handled over HTTP connections within the Firebase Storage and Firebase Functions iOS SDKs.
Previously, when developers configured their SDK to use an emulator running on a remote, non-loopback host (e.g.,
10.0.0.x) over standard HTTP, the SDKs would unconditionally attach the app's bearer tokens to outgoing network requests.To align with modern transport security best practices (and mirror the default behavior of gRPC in Firestore), this change ensures that we only attach session tokens if the underlying request is securely encrypted (
https://) or targeting a local loopback interface (localhost,127.0.0.1,::1).Changes Include
StorageTokenAuthorizerto evaluate the outgoing request's scheme and host. If the request is over HTTP and destined for a non-loopback host, the token fetching and attachment logic is safely bypassed, and a warning is logged viaFirebaseLogger.makeFetcherandmakeRequestForStreamableContentto apply identical scheme/host validation logic before attaching tokens from theFunctionsContext.StorageAuthorizerTests.swiftandFunctionsTests.swiftto verify that headers are properly omitted under these conditions while retaining normal behavior for secure/local emulator workflows.