firebase / firebase-admin-node

Firebase Admin Node.js SDK
https://firebase.google.com/docs/admin/setup
Apache License 2.0
1.62k stars 369 forks source link

[FR] Multi-Tenancy doesn't check for existence of a tenant #700

Open Dinuz opened 4 years ago

Dinuz commented 4 years ago

Is your feature request related to a problem? Please describe. The issue is related with the creation of a tenant using the sdk server side. The sdk is missing a way to check if the tenant (the displayName to be precise) already exist in the project. The sdk will create a tenant with a different id but with the same name if the create operation is performed multiple times with the same parameter. I do understand that the sdk distinguish the tenants using the id, but it is not very friendly.

Describe the solution you'd like It would be much better if the sdk would mock the behavior and strategy used in the creation of a user (it checks for example if the email is already in use, and if it is it throws an exception). The sdk checks if the displayname is already taken and if so throw an exception. This feature could easily been optional, and the developer decide if enforce it.

Describe alternatives you've considered For now I am just checking if the displayname exist in a json object that works as map between tenant's name and tenant's id. It easier to deal with a name (decided upfront by the developer) than relies on an ID automatically generated by the cloud.

google-oss-bot commented 4 years ago

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

hiranya911 commented 4 years ago

I don't think display name is expected to be unique. And it is also pretty easy to update, if multiple tenants having the same display name is causing any confusion.

@bojeil-google what do you think?

wuyanna commented 4 years ago

It's true that display name is not expected to be unique. We do have a plan to allow customers to specify tenant ID when creating tenants in the future, which could solve your problem.

Dinuz commented 4 years ago

@hiranya911 it is not creating so much confusion as it is, what create a bit of a mess is the following situation, using multi-tenants:

let's say I do have an app that implements a multi-tenancy strategy. Now let's say that I do decide to use a subdomain strategy to allow the tenants to access the app in a personalized way. In this case, it is just natural to use as subdomain the name of the tenant and not its id:

It will be absolutely weird using a tenant id ( eg google.mydomain.com and not google-Tr6W.mydomain.com). Now said so, the subdomain string can be used to trigger the specific tenant (in the backend), after clearly perform all the validation. But in this case if I use the tenant name (natural) I do need to map it to the ID of the specific tenant (otherwise I cannot use tenantManager and tenantAuth).

This will require or a call to get the whole tenant list and sort it out or loading a tenant map from storage.

Hope this clarify where I came from. On top of this, I don't get why the tenant name should not be unique, in which situation in the same app there are multiple tenant id with same name? I don't see a practical case.

From this point of view, specifying my own ID will not really solve the issue (actually it can make it worst, because how the unicity is guarantee?) I see the tenant name as the email of a registered user, it should be unique. Maybe, you can add a parameter that identify uniquely the tenant (a corporate name for example), and leave the display name free (exactly as it is done with the user).

hiranya911 commented 4 years ago

I'll let @wuyanna or @bojeil-google from the Auth team provide a more detailed answer. But from what I can understand, displayName is just that -- a name for display purposes. Uniqueness is not guaranteed or even required. Tenant ID is the only unique key. Somebody may wish to implement their own tenant management dashboard, and group certain tenants together on the screen by assigning them the same or similar display names. It's a somewhat convoluted use case, but a case can be made for that.

Once it is possible to set user-defined tenant IDs, it should be pretty straightforward to implement your particular scenario. If you want to treat foo.mydomain.com and bar.mydomain.com as separate tenants, just map "foo" and "bar" to tenant IDs. You can also use the same as display names if you like. Something like:

const subdomain = 'foo';
await admin.auth().tenantManager.createTenant({
  id: `tenant-${subdomain}`,
  displayName: subdomain,
  // ...
})

const client = admin.auth().tenantManager.forTenant(`tenant-${subdomain}`);
wuyanna commented 4 years ago

@hiranya911 is correct. Unlike email, which is used to uniquely identify a user, displayName is just for displaying purpose, which is just like the displayName in UserRecord. While not so often, it is possible that 2 companies happen to have the same name just like users usually do.

By allowing customer-specified tenant ID, the tenant ID becomes essentially the same thing as what you referred to as 'tenant name'. @hiranya911 provided a good code example above.

Dinuz commented 4 years ago

@wuyanna and @hiranya911 thank you very much for the answers. I then correctly understood. I am sorry I couldn’t really visualize a scenario where you would use different tenantIDs with the same display name. I am still not really convinced of the utility of it, and looks a bit of a stretch, but you guys said so, and I am taking it for granted.

Again you just confirmed what I thought: the tenantId will become the “name” with the new feature. However I do think that keeping the ID as random generated, and adding an extra parameter (similar concept of email) would be more beneficial in the long run, considering that the auth would be used server side anyway. My 2 cents.

wuyanna commented 4 years ago

Thanks for the valuable input, @Dinuz ! We will for sure take that into consideration in our planning.

Dinuz commented 4 years ago

@wuyanna after some more thoughts about this, I do have another point to make (that I do think it’s kind of compelling).

Without a unique tenant display name, as we agree before, the only way to connect to the tenant is to pass (and then expose) on the client side the tenantId. This is kind of bad, as I pointed out before, because, in order to use a subdomain strategy I should: 1) or use the tenantId as subdomain (with the consequence of having a weird and unnatural url) 2) or map the name with the id in the code

On top of this, if I want to use a single point of entry (sign in) for every tenant, I will need to expose the full map of all tenants. To be honest I don’t really like this option, because I don’t want to expose which are the tenants that use the product to everybody to see in the client.

The only option that remains to perform all this, is to move the whole signin in the backend (server side e.g. using an express api). This strategy allow me to keep everything private, and I must use the firebase client sdk directly in the server to perform the signin, get the token and send it in the response to the client. The client will use after this token to make subsequently calls to the backend api that requires authentication.

I don’t mind this at all, but I do suspect that firebase has some limitation for signin from same ip. How I handle this limitation? If the signin is made server side, the ip will not really change.

What if I use the endpoints (google) solution in front of my backend api? I still have the limitations? Is the endpoints able to perform authentication with the token returned by firebase sdk server side? Or I need to exchange the tokenid for token access and pass this to the client app to use in the header?

Last but not least, if my backend server, is not a monolithic, but a series of microservices deployed as containers on cloud run (fully managed), I do still have the IP limitation issue? or due to the nature of cloud run (container instances) the behavior is different?

Sorry for all these questions, but things can get pretty messy and confusion can arise when dealing with documentations (google) that are so different depending on which service of google you are using.

Thank you for every answer or clarity you can provide.

wuyanna commented 4 years ago

You are right, if you are using Firebase client SDK to perform sign-in from server side the IP will not change, and all your tenants will be consuming the same quota since the requests are from the same IP. Even with Endpoints and Cloud run, the per-IP quota limit still applies. This is because the per-IP quota limit is to prevent sign-up/sign-in generated from scripts.

Firebase client SDKs are not intended to be used on server side. One option is to still do the client side sign-in, and enforce the display name uniqueness on your server side. However, this might not be sufficient for you, because even if the display name is unique, you still need to map it to the tenant ID on your server as you need tenant ID to call the sign-in APIs.

Alternatively, if you still prefer to do sign-in on server side, I recommend reaching out to Cloud support (https://cloud.google.com/support-hub/) so that we can look at your use case in detail and raise quota if necessary.

Dinuz commented 4 years ago

@wuyanna thank you very much for your answer. Unfortunately you confirmed my suspicions:(

The client side sign-in it’s a no-go (believe me it would be much simpler doing it just client side), because it will require to expose the tenant id of all the tenants. I don’t see other ways to initialize firebase auth without the api_key and the tenant id.

The client side sign-in works pretty well for a single tenant, and I guess it does cause it was conceived from the beginning with this idea in mind. With a single tenant you don’t need to expose any id or names.

If you think about it, the way in which the multi-tenant (without server side support) works, it is the equivalent of a single tenant having on the client side an email list of all the users (or Ids if you prefer) to use to make the call to the right auth.

It’s just awkward having in the open (visible to everyone) who your tenants are and how many. It’s bad from a tech point of view (I don’t find it very elegant, but rather sloppy) and it’s even worst from a business point of you. You are just opening your client portfolio to the world, and increase the perception of low security in your clients mind.

Maybe the functionality of signing in made server side should be part of the admin sdk (like the custom claims and the revoke token), and allow it to don’t be ip limited (server side the scripting it’s just not a real concern, especially if the server is in gcp). This with the ability of refreshing a token server side would make the product really really great.

I am giving you my rationale and what I would improve or at least consider in order to make the product more versatile.

I am still experimenting, but sure when ready I will reach out the support hub as you suggested.

wuyanna commented 4 years ago

Thanks for the improvement suggestions, @Dinuz! Supporting sign-in requests generated from customer's server without any per-IP quota limitation is a reasonable ask. We will definitely consider that in our future feature planning.

However, I'm not sure if server-side sign-in is the only option you have. You can initialize Firebase Auth without tenant ID, and programmatically set the tenant ID based on the entry point (eg. subdomain). You can refer to the documentation for JS SDK here: https://cloud.google.com/identity-platform/docs/multi-tenancy-authentication

You can maintain a subdomain -> tenant ID mapping on your server, and for each login determine the subdomain from the request and pass the corresponding tenant ID to your frontend, then in your javascript, you can set the tenant ID by calling firebase.auth().tenantId = tenantId. In the future when we provide the capability for customers to assign tenant ID, this would become much easier - you can just use the subdomain as the tenant ID, and then you don't need to maintain server side mapping.

Dinuz commented 4 years ago

@wuyanna Thank you again for the answer.

I don't understand how I can perform sign-in on client side without providing the tenant id. I know I can initialize Firebase Auth without the tenant ID, and yes this I can do client side.

However, in order to use the sign-in-with-email-and-password, and get the token for the user (the user is under a tenant), I need to provide the tenant ID, and I don't have it on the client.

What I can do is creating a function that call the server, and in the request body or in the header has the tenant name (that I grab from the url), the server then fetch the map and return the tenantID in the response to client (I can set an axios call async/await for this). The client gets back the tenantID, and then set the firebase.auth().tenantId, and finally perform the sign_in_with_password_and_email.

Well, this is exactly what I am doing server side, with a big difference, I do only one call to the backend from the client (I call my login endpoint), and I perform the mapping operation directly there. The mapping is easy, because I can use the admin sdk to get a list of tenants, and map the name I get from the client to the corresponding ID (this allow me to graciously handle a request made with a not existing tenant). The mapping is just a middleware in my express setting, the service is exactly the sign_in_with_email_and_password called with firebase.auth().tenantId = tenantId_from_mapping, and my controller handle the response and failure of my api.

I guess it's important pointing out that I don't have any user creation in the frontend, all the users operations are made in the back end using the admin sdk, that are accessible only through authorization using openapi in google endpoints. The detail of what is possible to do with each api call depends on the role of the specific user (eg admin, user etc). Every tenant has a complete isolation from other tenants (has its own firebase app, its own firestore, its own storage unit). Nothing is really exposed on the outside, except the client.

The first tenant user is created server side by us during tenant setup. The rest of the tenant users are created one at the time by the tenant admin, or by us in batch using again the admin sdk (the batch creation is made separately from the app).

All this allow me to have only one code for the backend (designed as a bunch of microservices, interacting using service account permission) and only one code for the frontend. The backend is not accessible from the outside world, because I do have a gateway (the google endpoints) in front of it.

The firebase auth for signin is strongly advocated by google, I do like it, and I do love the idea that is google that takes care of securing, encrypting etc sensitive data. I do also love the idea of the tenants in the auth, because it removes completely the annoyance of dealing with it in a database. Moreover it separate completely auth from app.

I do understand that firebase sdk was born specifically for client, but firebase (following the google doc suggestions) is now a "plus" on top of gcp (eg storage is the same, firestore is the same) with clearly the advantage of a friendly and easy set of tools to use.

Clearly all this is possible only because I am using node server side (the firebase client sdk can be included in it easily). To be honest when I started looking into it, I built all this using python admin sdk, but python lacks the multi-tenant capability (@hiranya911 knows), and I decided to switch to node just for the auth.

One thing that I lose in moving the process server side is the ability to auto-refresh the token, but this again can be handled easily checking the expiration client side and refresh it server side.

What I am not sure it's possible (at least with the multi-tenancy) is the ability to create session cookies for the client (I know that without multi-tenant I can do it).

PS. Clearly if I was having the tenantName unicity, I could do all the signin on the client (no need to call the server, no need of a map, no need of anything of that).

PS. I could also perform all the operation server side without using at all the firebase sdk auth, but this will not work for multi-tenant (I was doing it in python, using directly the REST API, and making my own sign_in_with_email_and_password method).

wuyanna commented 4 years ago

Thanks again for your feedback @Dinuz. These are very valuable inputs for us to further improve our multi-tenancy offering in the future.

Answering some of your questions:

  1. Create session cookies is not currently supported in multi-tenancy.
  2. You should be able to use REST API directly for multi-tenancy as well. For email and password sign-in, you will need to pass the tenantId in the post body. Please refer to the REST API doc here: https://cloud.google.com/identity-platform/docs/reference/rest/client/#section-sign-in-email-password
Dinuz commented 4 years ago

@wuyanna I used the rest API, it works and I wanted to let you know:) But you were faster then me and suggested the same thing.

I am kind of satisfied with the solution, the only thing that remains unclear is about the limit of the rest api. The rest api has the same limitation of the firebase auth sdk (in terms of same IP sign-in ) or I can use it server side without incurring in the limit (we discussed above this topic)??

Thank you again very very much, your feedback were precious and really appreciated.

wuyanna commented 4 years ago

They have the same per-IP limitations for sign-ins. The only exception currently is signUp (https://cloud.google.com/identity-platform/docs/reference/rest/client/#section-create-email-password). For signUp, you can pass a service credential (https://cloud.google.com/iam/docs/creating-short-lived-service-account-credentials) in the http header and that will bypass the IP limitation. This is essentially the same as the createUser api on Firebase Admin SDK

bencalnan commented 4 years ago

Just to add, I would love the ability to be able to set the tenancy id programmatically so you can retrieve the tenancy ID from a users email address (on a login page) without having to show a long list of tenants to the user and asking them to pick their one.

Charles-Dahl commented 4 years ago

I have the same use case as Dinuz. A different tenant for each subdomain. I would love to be able to set the tenant id as the subdomain. Any idea when this will be available? From what I can tell it's not in the docs yet.

Madsim commented 3 years ago

I have the same use case as @Dinuz and also am not a fan of exposing the ability to guess valid tenants via public API that performs subdomain -> tenantId checks. Is there news on this feature?