next-auth icon indicating copy to clipboard operation
next-auth copied to clipboard

[firebase adapter] - Firebase adapter requires open security rules we cannot allow in production

Open Chibionos opened this issue 1 year ago • 11 comments

Adapter type

@next-auth/firebase-adapter

Environment

System: OS: Windows 10 10.0.22000 CPU: (24) x64 AMD Ryzen 9 3900X 12-Core Processor Memory: 9.27 GB / 31.95 GB Binaries: Node: 16.13.2 - C:\Program Files\nodejs\node.EXE Yarn: 1.22.19 - C:\Program Files\nodejs\yarn.CMD npm: 8.1.2 - C:\Program Files\nodejs\npm.CMD Browsers: Edge: Spartan (44.22000.120.0), Chromium (103.0.1264.62) Internet Explorer: 11.0.22000.120 npmPackages: next: ^12.2.3 => 12.2.3 next-auth: ^4.10.2 => 4.10.2 react: 18.2.0 => 18.2.0

npmPackages: @next-auth/firebase-adapter: ^1.0.1 => 1.0.1

Reproduction URL

https://github.com/nextauthjs/next-auth-example

Describe the issue

Firebase adapter is using client side library and it is expecting the security rules to be open for it to edit sessions, accounts and other collections managed by it. We cannot allow those collections to have an open security rule.

Is there a way we can safely open security rules for the firebase adapter to write and access these files as part of user context ensuring that a malicious user cannot access the records.

eg:

match /users/{uid} {
   allow read, write:  if request.auth.uid == uid;
}

match /sessions/{id} {
   allow read, write:  if request.auth.uid == resource.data.userId;
}

match /accounts/{id} {
  allow read, write: if request.auth.uid == resource.data.userId;
}

I tried a few with no luck. If you have the documentation for the rules the adapter is very viable to be used in production and would solve a lot of manual work that need to be done.

The ideal fix here would be move the firebase adapter to require to use firebase-admin package as it would then have the right permissions without us opening the security rules.

Great work on the adapter !


How to reproduce

Setup the adapter and lock down security rules

rules_version = '2';
service cloud.firestore {
  match /databases/{database}/documents {
    match /{document=**} {
      allow read, write: if false;
    }
  }
}

Expected behavior

We should have a clear security rule setup ensuring there is no security vulnerabilities exposed by opening up permissions.

Chibionos avatar Jul 29 '22 23:07 Chibionos

Since the adapter is being used server-side only, you should be safe as users cannot interact with the database directly. 🤔 cc @chanceaclark

balazsorban44 avatar Jul 30 '22 15:07 balazsorban44

Not the ideal approach, but the following Cloud Firestore Security Rule works. Just make sure that you don't expose Firebase config on client side code.

rules_version = '2';
service cloud.firestore {
  match /databases/{database}/documents {
    match /{document=**} {
      allow read, write: if false;
    }
    match /users/{document} {
    	allow read, write: if true;
    }
    match /sessions/{document} {
    	allow read, write: if true;
    }
    match /accounts/{document} {
    	allow read, write: if true;
    }
    match /verificationTokens/{document} {
    	allow read, write: if true;
    }
  }
}

rupampoddar avatar Aug 01 '22 07:08 rupampoddar

I thought the same thing and came to this issue!

My product will use client-side firebase in Browser so it's not realistic to hide firebase config.

The ideal fix here would be move the firebase adapter to require to use firebase-admin package as it would then have the right permissions without us opening the security rules.

I think so too. Using firebase-admin and serviceAccount private key is an ideal solution.

kesoji avatar Aug 15 '22 03:08 kesoji

Not the ideal approach, but the following Cloud Firestore Security Rule works. Just make sure that you don't expose Firebase config on client side code.

rules_version = '2';
service cloud.firestore {
  match /databases/{database}/documents {
    match /{document=**} {
      allow read, write: if false;
    }
    match /users/{document} {
    	allow read, write: if true;
    }
    match /sessions/{document} {
    	allow read, write: if true;
    }
    match /accounts/{document} {
    	allow read, write: if true;
    }
    match /verificationTokens/{document} {
    	allow read, write: if true;
    }
  }
}

Those are not secure rules, if that is enabled your firebase can be compromised very easily. Remember even with public config a malicious user can delete all users and collections with these rules.

Chibionos avatar Aug 17 '22 08:08 Chibionos

Since the adapter is being used server-side only, you should be safe as users cannot interact with the database directly. 🤔 cc @chanceaclark

I don't think we can use it server side with client side firebase config. If the Adapter is supposed to be used on the server side, it should accept the firebase-admin instance than the firebase instance.

As I mentioned opening up security rules for read and write true is extremely dangerous and should never be done on Firebase. It can lead to massive issues down the line. Check their security rules guidelines.

https://firebase.google.com/docs/rules/insecure-rules#open_access

Chibionos avatar Aug 17 '22 08:08 Chibionos

+1 to firestore-adapter accepting instance of firebase-admin

firebase-admin is already required server-side to createCustomToken(), so passing it to the adapter would be natural

mcgrealife avatar Aug 22 '22 22:08 mcgrealife

+1 here. This is not a secure setup.

subimage avatar Aug 23 '22 00:08 subimage

So are we fine if we stick to SSR?

NayamAmarshe avatar Sep 04 '22 21:09 NayamAmarshe

Is there any good examples of Firestore rules to use that are safe & work as intended? Or at least some guidelines / tips on how to create a ruleset that does?

The most I have gathered is that this rule recommended above that, while works, is obviously a huge no no & should never be used, especially in production.

NuroDev avatar Sep 07 '22 02:09 NuroDev

Is there any good examples of Firestore rules to use that are safe & work as intended? Or at least some guidelines / tips on how to create a ruleset that does?

The most I have gathered is that this rule recommended above that, while works, is obviously a huge no no & should never be used, especially in production.

I think if you use a middleware that checks valid JWT with getToken, then you should be fine with open security rules.

Think like this, when creating applications with MongoDB we'd usually just check the JWT and if the JWT is valid, we'd allow the user request to modify the database. Same thing here, security rules don't matter as long as you're checking if the requests are valid and as long as the CRUD operations are performed by the server, not client.

NayamAmarshe avatar Sep 07 '22 02:09 NayamAmarshe

Looks like the thread stagnated, I will write another with firebase admin adapter this week send out a PR.

Chibionos avatar Sep 09 '22 17:09 Chibionos

@Chibionos did you ever get to write another adapter? In my mind it would be best to simply re-write the current adapter to use the admin sdk?

nhuethmayr avatar Sep 23 '22 14:09 nhuethmayr

I've created a version of the adapter that uses the Firebase server-side SDK. There is a PR incoming. Please let me know how you feel about it.

@balazsorban44 @Chibionos

nhuethmayr avatar Sep 24 '22 15:09 nhuethmayr

Since the adapter is being used server-side only, you should be safe as users cannot interact with the database directly. 🤔 cc @chanceaclark

You could hijack an endpoint that connects to your database and have open access because, security rules are practically non-existent. Also, firebase databases are technically public by default. This much is stated rather clearly upon creation. Open rules + public database = Severe security issues. Stating that it's server sided and equating that to safety is like saying there's a robber w/ a gun outside a glass door and we're "safe" because there's a door. The door is glass. One shot and it's gone. Same principle applies here.

I thought the same thing and came to this issue!

My product will use client-side firebase in Browser so it's not realistic to hide firebase config.

The ideal fix here would be move the firebase adapter to require to use firebase-admin package as it would then have the right permissions without us opening the security rules.

I think so too. Using firebase-admin and serviceAccount private key is an ideal solution.

The biggest issue with this is that the current adapter doesn't support this. Changing it to support this would probably warrant a new adapter because firebase-admin works very differently from firebase. There still needs to be a better way to create rules based on what we already have so that we can create less friction on the users already using this. Fwiw, firebase-admin adapter would take some time to complete and get through review.

Personally, I think that the adapter should've been service account compatible to begin with because adapters are meant to be server sided by default. So, it was a bit immature that the current adapter was pushed up. Especially since there's no real security rule practice which makes it a glaring flaw in basically everything related to production. That being said, storing database instances / creds on client is horrible practice and should be avoided. Details should be stored securely on the server with the least amount of friction. Realisticness (?) is not a valid concern when you're throwing basic security practice out the window for simplicity. So, stating that it's not realistic to hide firebase config when attempting client-only adoption isn't really a valid complaint or concern here when the that line of thought is severely flawed to begin with. Moving it to a service account wouldn't solve your issue in this scenario, because you're exposing api keys on the client. It's actually more dangerous given that the service account could have full access / rights to the database instance.

@kesoji , @Chibionos

sbhadr avatar Sep 29 '22 06:09 sbhadr

Since the adapter is being used server-side only, you should be safe as users cannot interact with the database directly. 🤔 cc @chanceaclark

You could hijack an endpoint that connects to your database and have open access because, security rules are practically non-existent. Also, firebase databases are technically public by default. This much is stated rather clearly upon creation. Open rules + public database = Severe security issues. Stating that it's server sided and equating that to safety is like saying there's a robber w/ a gun outside a glass door and we're "safe" because there's a door. The door is glass. One shot and it's gone. Same principle applies here.

I thought the same thing and came to this issue! My product will use client-side firebase in Browser so it's not realistic to hide firebase config.

The ideal fix here would be move the firebase adapter to require to use firebase-admin package as it would then have the right permissions without us opening the security rules.

I think so too. Using firebase-admin and serviceAccount private key is an ideal solution.

The biggest issue with this is that the current adapter doesn't support this. Changing it to support this would probably warrant a new adapter because firebase-admin works very differently from firebase. There still needs to be a better way to create rules based on what we already have so that we can create less friction on the users already using this. Fwiw, firebase-admin adapter would take some time to complete and get through review.

Personally, I think that the adapter should've been service account compatible to begin with because adapters are meant to be server sided by default. So, it was a bit immature that the current adapter was pushed up. Especially since there's no real security rule practice which makes it a glaring flaw in basically everything related to production. That being said, storing database instances / creds on client is horrible practice and should be avoided. Details should be stored securely on the server with the least amount of friction. Realisticness (?) is not a valid concern when you're throwing basic security practice out the window for simplicity. So, stating that it's not realistic to hide firebase config when attempting client-only adoption isn't really a valid complaint or concern here when the that line of thought is severely flawed to begin with. Moving it to a service account wouldn't solve your issue in this scenario, because you're exposing api keys on the client. It's actually more dangerous given that the service account could have full access / rights to the database instance.

@kesoji , @Chibionos

The only solution is to use the firebase admin SDK - I've implemented the adapter already and its working like a charm. My PR has received no attention. You wannt try?

nhuethmayr avatar Sep 29 '22 11:09 nhuethmayr

Since the adapter is being used server-side only, you should be safe as users cannot interact with the database directly. thinking cc @chanceaclark

You could hijack an endpoint that connects to your database and have open access because, security rules are practically non-existent. Also, firebase databases are technically public by default. This much is stated rather clearly upon creation. Open rules + public database = Severe security issues. Stating that it's server sided and equating that to safety is like saying there's a robber w/ a gun outside a glass door and we're "safe" because there's a door. The door is glass. One shot and it's gone. Same principle applies here.

I thought the same thing and came to this issue! My product will use client-side firebase in Browser so it's not realistic to hide firebase config.

The ideal fix here would be move the firebase adapter to require to use firebase-admin package as it would then have the right permissions without us opening the security rules.

I think so too. Using firebase-admin and serviceAccount private key is an ideal solution.

The biggest issue with this is that the current adapter doesn't support this. Changing it to support this would probably warrant a new adapter because firebase-admin works very differently from firebase. There still needs to be a better way to create rules based on what we already have so that we can create less friction on the users already using this. Fwiw, firebase-admin adapter would take some time to complete and get through review.

Personally, I think that the adapter should've been service account compatible to begin with because adapters are meant to be server sided by default. So, it was a bit immature that the current adapter was pushed up. Especially since there's no real security rule practice which makes it a glaring flaw in basically everything related to production. That being said, storing database instances / creds on client is horrible practice and should be avoided. Details should be stored securely on the server with the least amount of friction. Realisticness (?) is not a valid concern when you're throwing basic security practice out the window for simplicity. So, stating that it's not realistic to hide firebase config when attempting client-only adoption isn't really a valid complaint or concern here when the that line of thought is severely flawed to begin with. Moving it to a service account wouldn't solve your issue in this scenario, because you're exposing api keys on the client. It's actually more dangerous given that the service account could have full access / rights to the database instance.

@kesoji , @Chibionos

I'm considering to use the Firebase Adapter, and this is very interesting.

While I agree to most of your stand points on the security enhancement, how about the practical stand point?

As far as I understand so far, Nextjs handle both client and server securely hence it's a 'fullstack' solution, so if the connection is always client -> nextjs frontend -> nextjs backend -> firebase, and the firebase configuration resides only in the nextjs backend, so I'm just not genuinely sure where and how the endpoint can be hijacked.

The concern here is that the attacker can have access to the firebase directly without going through Nextjs entirely (and that's a huge issue since, as you mentioned, we have to pretty much open access to anyone), but that would require the endpoint url, which is hard to guess since it's randomized, and client API key, which isn't visible if it's handled by nextjs backend entirely (but I might be missing something since I'm pretty new to Nextjs. But as NextAuth is installed in the page/api/ folder, it shouldn't be exposed to the client if I understand it correctly).

So the only scenario I can imagine is the firebase endpoint url being correctly guessed and so is the client API key, thus, the attacker can directly access the firebase. Well, to be honest, this make me feel quite uneasy since I have no control in case of the possible security breach, yet I'm trying to see if it's worth the risk to try it out.

I would also love to know what NextAuth team's recommendation is since this adapter is introduced in the official documentation.

wysohn avatar Jan 24 '23 08:01 wysohn