-
Notifications
You must be signed in to change notification settings - Fork 890
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.
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
Use Admin AuthTokenProvider when targeting Emulator #3228
Changes from 8 commits
9d69fd9
88e8f22
84a9b29
f891b19
7a19477
617dd05
8a978d8
54c19f1
f0ea3f0
6ba08b7
004f58d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
--- | ||
"firebase": patch | ||
"@firebase/database": patch | ||
--- | ||
|
||
[fix] Instead of using production auth, the SDK will use test credentials | ||
to connect to the Emulator when the RTDB SDK is used via the Firebase | ||
Admin SDK. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,7 @@ | |
*/ | ||
|
||
import { FirebaseApp } from '@firebase/app-types'; | ||
import { safeGet } from '@firebase/util'; | ||
import { safeGet, CONSTANTS } from '@firebase/util'; | ||
import { Repo } from './Repo'; | ||
import { fatal } from './util/util'; | ||
import { parseRepoInfo } from './util/libs/parser'; | ||
|
@@ -26,6 +26,11 @@ import { Database } from '../api/Database'; | |
import { RepoInfo } from './RepoInfo'; | ||
import { FirebaseAuthInternalName } from '@firebase/auth-interop-types'; | ||
import { Provider } from '@firebase/component'; | ||
import { | ||
AuthTokenProvider, | ||
EmulatorAdminTokenProvider, | ||
FirebaseAuthTokenProvider | ||
} from './AuthTokenProvider'; | ||
|
||
/** @const {string} */ | ||
const DATABASE_URL_OPTION = 'databaseURL'; | ||
|
@@ -108,16 +113,30 @@ export class RepoManager { | |
let parsedUrl = parseRepoInfo(dbUrl); | ||
let repoInfo = parsedUrl.repoInfo; | ||
|
||
let authTokenProvider: AuthTokenProvider; | ||
|
||
let isEmulator: boolean; | ||
|
||
let dbEmulatorHost: string | undefined = undefined; | ||
if (typeof process !== 'undefined') { | ||
dbEmulatorHost = process.env[FIREBASE_DATABASE_EMULATOR_HOST_VAR]; | ||
} | ||
|
||
if (dbEmulatorHost) { | ||
isEmulator = true; | ||
dbUrl = `http://${dbEmulatorHost}?ns=${repoInfo.namespace}`; | ||
parsedUrl = parseRepoInfo(dbUrl); | ||
repoInfo = parsedUrl.repoInfo; | ||
} else { | ||
isEmulator = | ||
parsedUrl.repoInfo.host === 'localhost' && !parsedUrl.repoInfo.secure; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We do see people running their emulator on non-localhost hosts. Like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. I will verify this manually on Monday, which should allow us to release this next week. |
||
} | ||
|
||
authTokenProvider = | ||
CONSTANTS.NODE_ADMIN && isEmulator | ||
? new EmulatorAdminTokenProvider() | ||
: new FirebaseAuthTokenProvider(app, authProvider); | ||
|
||
validateUrl('Invalid Firebase Database URL', 1, parsedUrl); | ||
if (!parsedUrl.path.isEmpty()) { | ||
fatal( | ||
|
@@ -126,7 +145,7 @@ export class RepoManager { | |
); | ||
} | ||
|
||
const repo = this.createRepo(repoInfo, app, authProvider); | ||
const repo = this.createRepo(repoInfo, app, authTokenProvider); | ||
|
||
return repo.database; | ||
} | ||
|
@@ -159,7 +178,7 @@ export class RepoManager { | |
createRepo( | ||
repoInfo: RepoInfo, | ||
app: FirebaseApp, | ||
authProvider: Provider<FirebaseAuthInternalName> | ||
authTokenProvider: AuthTokenProvider | ||
): Repo { | ||
let appRepos = safeGet(this.repos_, app.name); | ||
|
||
|
@@ -174,7 +193,7 @@ export class RepoManager { | |
'Database initialized multiple times. Please make sure the format of the database URL matches with each database() call.' | ||
); | ||
} | ||
repo = new Repo(repoInfo, this.useRestClient_, app, authProvider); | ||
repo = new Repo(repoInfo, this.useRestClient_, app, authTokenProvider); | ||
appRepos[repoInfo.toURLString()] = repo; | ||
|
||
return repo; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this branch
if (dbEmulatorHost) { }
trigger only if the emulator connection is made via env var? What about if the user does this:You might consider using the
EmulatorAdminTokenProvider
any timeConstants.NODE_ADMIN
is true and the database ishttp://
(nothttps://
). That's what we do in the Firebase CLI, it catches more cases and it also has the nice side benefit of not allowing someone's admin credentials to ever go to an insecure host.