Skip to content

use fake "owner" credential when rtdb emulator env var is set#2029

Merged
jmwski merged 15 commits intomasterfrom
wyszynski/db-emulator-omit-access-token
Aug 9, 2019
Merged

use fake "owner" credential when rtdb emulator env var is set#2029
jmwski merged 15 commits intomasterfrom
wyszynski/db-emulator-omit-access-token

Conversation

@jmwski
Copy link
Copy Markdown
Contributor

@jmwski jmwski commented Jul 27, 2019

Today, the RTDB emulator accepts any production access token. We would like to support fully-offline emulator interaction that doesn't require an external auth provider. Some questions to follow up on:

  • Should we store a bit describing whether the SDK is talking to an emulator in the RepoInfo class?
  • Should we only infer that we're talking to an emulator if the environment variable is set? Can we infer it in the additional situation in which an ns query parameter is needed (RepoInfo#needsQueryParam)?

@mikelehen
Copy link
Copy Markdown
Contributor

mikelehen commented Jul 29, 2019

Hrm... I guess I have several interrelated questions...

  1. What is the overall user experience we're trying to build here? I'm surprised that we want to always communicate with the emulator as "owner". Don't we want to be able to test security rules, etc. which will require authenticating with different credentials?
  2. There are a couple of existing "hooks" to replace auth already. FirebaseAuth hooks into FirebaseApp like (this)[https://github.com/firebase/firebase-js-sdk/blob/8271edfb49a17082c379bae160a0ebcbfbe215cb/packages/auth/src/exports_auth.js#L618] to provide auth functionality. And RTDB wraps FirebaseApp's token fetching capabilities in its own abstraction:

    firebase-js-sdk/packages/database/src/core/AuthTokenProvider.ts

    Line 23 in 8271edf

    * Abstraction around FirebaseApp's token fetching capabilities.
    . Would it make sense to modify / replace this AuthTokenProvider to incorporate whatever emulator functionality we're aiming for?
  3. The Cloud Firestore SDK already has emulator support. Are we doing something similar there? Or is RTDB different for some reason?

@jmwski
Copy link
Copy Markdown
Contributor Author

jmwski commented Jul 29, 2019 •
edited
Loading

Hrm... I guess I have several interrelated questions...

  1. What is the overall user experience we're trying to build here? I'm surprised that we want to always communicate with the emulator as "owner". Don't we want to be able to test security rules, etc. which will require authenticating with different credentials?
  2. There are a couple of existing "hooks" to replace auth already. FirebaseAuth hooks into FirebaseApp like (this)[https://github.com/firebase/firebase-js-sdk/blob/8271edfb49a17082c379bae160a0ebcbfbe215cb/packages/auth/src/exports_auth.js#L618] to provide auth functionality. And RTDB wraps FirebaseApp's token fetching capabilities in its own abstraction:

    firebase-js-sdk/packages/database/src/core/AuthTokenProvider.ts

    Line 23 in 8271edf

    * Abstraction around FirebaseApp's token fetching capabilities.

    . Would it make sense to modify / replace this AuthTokenProvider to incorporate whatever emulator functionality we're aiming for?
  3. The Cloud Firestore SDK already has emulator support. Are we doing something similar there? Or is RTDB different for some reason?

The use case we want to support here is to enable the emulators in environments where we can't get a production access token (e.g., CI). The issue I'm trying to work around in this PR is that we're using a single global application credential for all emulators.

I just talked offline with @ryanpbrewster and @yuchenshi and I think one path forward is to enforce the following policy in the admin sdk: if either FIRESTORE_EMULATOR_HOST or FIREBASE_DATABASE_EMULATOR_HOST is set, we default to using a single "fake" credential globally. We are still discussing this, but if we decide to move forward, I will restore this PR in the admin sdk and get rid of this one.

Apologies for the back-and-forth on this.

@mikelehen
Copy link
Copy Markdown
Contributor

mikelehen commented Jul 30, 2019

Per chat discussion, if we move forward with this approach, I think the special handling for RTDB should probably live in a new EmulatorAuthTokenProvider class akin to

firebase-js-sdk/packages/database/src/core/AuthTokenProvider.ts

Line 25 in 4e84bef

export class AuthTokenProvider {
that just returns "owner" or whatever and swap it in when the env var is set.

@jmwski jmwski changed the title omit access token exchange if we're talking to an emulator use fake "owner" credential when rtdb emulator env var is set Aug 1, 2019
yuchenshi
yuchenshi reviewed Aug 2, 2019
View reviewed changes
Comment thread packages/database/src/core/EmulatorAuthTokenProvider.ts Outdated
@mikelehen
Copy link
Copy Markdown
Contributor

mikelehen commented Aug 5, 2019

When this is ready for another review, can you please assign it to me?

@jmwski jmwski assigned mikelehen and unassigned jmwski and yuchenshi Aug 6, 2019
mikelehen
mikelehen suggested changes Aug 6, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@mikelehen mikelehen left a comment

Choose a reason for hiding this comment

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

I think this mostly LGTM but a couple minor suggestions. Let me know what you think.

Comment thread packages/database/src/core/Constants.ts Outdated
Comment thread packages/database/src/core/Constants.ts Outdated
Comment thread packages/database/src/core/AuthTokenProvider.ts Outdated
@mikelehen mikelehen assigned jmwski and unassigned mikelehen Aug 6, 2019
@jmwski jmwski assigned mikelehen and unassigned jmwski Aug 7, 2019
mikelehen
mikelehen approved these changes Aug 7, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@mikelehen mikelehen left a comment

Choose a reason for hiding this comment

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

LGTM with one final nit. Thanks!

Comment thread packages/database/src/core/ReadonlyRestClient.ts
@mikelehen mikelehen removed their assignment Aug 7, 2019
schmidt-sebastian
schmidt-sebastian approved these changes Aug 9, 2019
View reviewed changes
Comment thread packages/database/src/core/AuthTokenProvider.ts
@jmwski jmwski merged commit 4ebedd0 into master Aug 9, 2019
@firebase firebase locked and limited conversation to collaborators Oct 9, 2019
@paulb777 paulb777 deleted the wyszynski/db-emulator-omit-access-token branch February 20, 2026 01:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@yuchenshi yuchenshi yuchenshi left review comments

@jsdt jsdt Awaiting requested review from jsdt

+2 more reviewers

@mikelehen mikelehen mikelehen approved these changes

@schmidt-sebastian schmidt-sebastian schmidt-sebastian approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jmwski jmwski

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jmwski @mikelehen @schmidt-sebastian @yuchenshi