Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(auth): Add support for Firebase Authentication Emulator #289

Merged
merged 36 commits into from
Mar 25, 2021
Merged

feat(auth): Add support for Firebase Authentication Emulator #289

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
03d9f5a
Add IdToolkitHostResolver with tests
floppydisken Mar 16, 2021
92ea12d
Add resolution for versions of the API with tests
floppydisken Mar 18, 2021
5e22928
Simplify and reduce IdToolkitHostResolver to a Util method
floppydisken Mar 20, 2021
f957a2b
Add TestConfig for testing against FirebaseEmulatorHost
floppydisken Mar 20, 2021
5fa8bc3
Add license for new Utils class file
floppydisken Mar 20, 2021
98a979e
Add documentation for ResolveIdToolkitHost with examples
floppydisken Mar 20, 2021
829f62d
Use the EnvironmentVariable class to access env variables
floppydisken Mar 20, 2021
e2b70f6
Fix formatting
floppydisken Mar 20, 2021
3a06064
Use inheritance instead of TestConfig to set FirebaseUserManager tests
floppydisken Mar 20, 2021
19c386a
Cleanup and rename variable to more meaningful name
floppydisken Mar 24, 2021
85ee525
Move Utils and EnvironmentVariable class to Auth namespace
floppydisken Mar 24, 2021
23e064f
Move emulatorHostEnvVar to where it's used for readability
floppydisken Mar 24, 2021
98948fb
Rename UtilTest to more appropriate name
floppydisken Mar 24, 2021
099a1d8
Rename expected...Host to expected...Url
floppydisken Mar 24, 2021
a78f018
Use the FirebaseAuthEmulatorHostName const instead of string
floppydisken Mar 24, 2021
1f701d8
Add missing license head
floppydisken Mar 24, 2021
c7cbd41
Add missing util import and fix stylecop complaints
floppydisken Mar 24, 2021
002d368
Rename AuthUtil to Util and move to Auth test folder
floppydisken Mar 24, 2021
bddaafc
Simplify emulator host environment variable to be a constant in Utils
floppydisken Mar 24, 2021
afee451
Use string literal for emulator host in FirebaseUserManagerTest
floppydisken Mar 24, 2021
1e3fe6b
Remove unnecessary tests from FirebaseAuthTest
floppydisken Mar 24, 2021
52c7b9e
Use string literal to ensure test fails if constant is changed
floppydisken Mar 24, 2021
248f5e7
Remove the environment simplification since there is only one callsite
floppydisken Mar 24, 2021
778d3c1
Cleanup GetIdToolkitHost documentation
floppydisken Mar 24, 2021
6f3cf7e
Correct typo in file name. Util -> Utils.
floppydisken Mar 25, 2021
76d4aa2
Rename test cass to UtilsTest
floppydisken Mar 25, 2021
c190edc
Add tenant resolution to GetIdToolkitHost
floppydisken Mar 25, 2021
31ec063
Add function for resolving GoogleCredentials when in emulator mode
floppydisken Mar 25, 2021
b18be18
Add test for tenant url resolution
floppydisken Mar 25, 2021
f9c6cae
Use if statement instead of ternary for tenantIdPath
floppydisken Mar 25, 2021
4c62390
Better formatting for GetIdToolkitHost
floppydisken Mar 25, 2021
359dc7c
Add periods to the end of documentation strings
floppydisken Mar 25, 2021
a80cca4
Remove nullable and provide empty string instead
floppydisken Mar 25, 2021
590ffa6
Change to only have one single line at end
floppydisken Mar 25, 2021
855aac4
Add convenience funcs for getting and checking emulator host env vari…
floppydisken Mar 25, 2021
1595d36
Simplify GetEmulatorHost name
floppydisken Mar 25, 2021
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix formatting
  • Loading branch information
@floppydisken
floppydisken committed Mar 20, 2021
commit e2b70f6ce34ca2d3351b9c83ce509e1f4e8a117a
2 changes: 1 addition & 1 deletion FirebaseAdmin/FirebaseAdmin/Util/EnvironmentVariable.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace FirebaseAdmin.Util
internal static class EnvironmentVariable
{
/// <summary>
/// Environment variable for connecting to the Firebase Authentication Emulator.
/// Gets or sets environment variable for connecting to the Firebase Authentication Emulator.
/// </summary>
/// <value>string in the form &lt;host&gt;:&lt;port&gt;, e.g. localhost:9099. Beware: No validation is done.</value>
internal static string FirebaseAuthEmulatorHost
Copy link
Contributor

@hiranya911 hiranya911 Mar 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This is a pretty neat trick, and I do appreciate the power it brings. It basically puts an env variable lookup behind a typesafe constraint that can be checked by the compiler.

However, this is a bit too exposed right now. So:

  1. let's move this constant into the new Utils class (and remove this class entirely), so it's scoped to the Auth namespace, and
  2. remove the setter (it's potentially dangerous to have an API that updates env variables lying around)
Copy link
Contributor Author

@floppydisken floppydisken Mar 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

let's move this constant into the new Utils class (and remove this class entirely), so it's scoped to the Auth namespace, and

I was actually thinking the class to also provide access to the "GOOGLE_CLOUD_HOST" and "GCLOUD_HOST" environment variables as well as other variables. I am quite unaware of the amount of env variables used for the Firebase client libraries, so I don't know if it's a good idea. However, it would make sense that this particular environment variable is scoped to the Auth namespace.

(and remove this class entirely)

Removing the class itself, would result in naming like FirebaseAuthHostEnvironmentVariable instead of EnvironmentVariable.FirebaseAuthHost, which I see no advantage to. Am I misunderstanding you perhaps?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Proposed change in 85ee525
Copy link
Contributor

Choose a reason for hiding this comment

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

Removing the class itself, would result in naming like FirebaseAuthHostEnvironmentVariable instead of EnvironmentVariable.FirebaseAuthHost, which I see no advantage to. Am I misunderstanding you perhaps?

My reasoning is that this class will end up being a wrapper for a single constant. Therefore it's just simpler to use the Utils class itself as the wrapper, and rename the constant to something like EmulatorHostEnvironmentVar.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

#289 (comment)
Expand Down
8 changes: 4 additions & 4 deletions FirebaseAdmin/FirebaseAdmin/Util/Utils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ internal class Utils
{
/// <summary>
/// Resolves to the correct identity toolkit api host.
/// It does this by checking if <see cref="EnvironmentVariable.FirebaseAuthEmulatorHost" /> exists
/// It does this by checking if <see cref="EnvironmentVariable.FirebaseAuthEmulatorHost" /> exists
/// and then prepends the url with that host if it does. Otherwise it returns the regular identity host.
/// Example:
/// If <see cref="EnvironmentVariable.FirebaseAuthEmulatorHost" /> is set to localhost:9099 the host is resolved to http://localhost:9099/identitytoolkit.googleapis.com...
/// If <see cref="EnvironmentVariable.FirebaseAuthEmulatorHost" /> is not set the host resolves to https://identitytoolkit.googleapis.com...
/// </summary>
/// <param name="projectId">The project ID to connect to</param>
/// <param name="version">The version of the API to connect to</param>
/// <returns></returns>
/// <param name="projectId">The project ID to connect to.</param>
/// <param name="version">The version of the API to connect to.</param>
/// <returns>Resolved identity toolkit host.</returns>
internal static string ResolveIdToolkitHost(string projectId, IdToolkitVersion version = IdToolkitVersion.V2)
hiranya911 marked this conversation as resolved.
Show resolved Hide resolved
{
const string IdToolkitUrl = "https://identitytoolkit.googleapis.com/{0}/projects/{1}";
hiranya911 marked this conversation as resolved.
Show resolved Hide resolved
Expand Down