Skip to content

Commit

Permalink
Remove usage of the NODE_ADMIN global in RTDB (#3736)
Browse files Browse the repository at this point in the history
  • Loading branch information
samtstern committed Sep 9, 2020
1 parent f47f990 commit 3d9b5a5
Show file tree
Hide file tree
Showing 9 changed files with 86 additions and 31 deletions.
6 changes: 6 additions & 0 deletions .changeset/polite-readers-wait.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@firebase/database': patch
'@firebase/rules-unit-testing': patch
---

Fix detection of admin context in Realtime Database SDK
13 changes: 10 additions & 3 deletions packages/database/index.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,19 @@ const ServerValue = Database.ServerValue;
* @param app A valid FirebaseApp-like object
* @param url A valid Firebase databaseURL
* @param version custom version e.g. firebase-admin version
* @param nodeAdmin true if the SDK is being initialized from Firebase Admin.
*/
export function initStandalone(app: FirebaseApp, url: string, version: string) {
export function initStandalone(
app: FirebaseApp,
url: string,
version: string,
nodeAdmin: boolean = true
) {
/**
* This should allow the firebase-admin package to provide a custom version
* to the backend
*/
CONSTANTS.NODE_ADMIN = true;
CONSTANTS.NODE_ADMIN = nodeAdmin;
setSDKVersion(version);

/**
Expand All @@ -84,7 +90,8 @@ export function initStandalone(app: FirebaseApp, url: string, version: string) {
instance: RepoManager.getInstance().databaseFromApp(
app,
authProvider,
url
url,
nodeAdmin
) as types.Database,
namespace: {
Reference,
Expand Down
4 changes: 2 additions & 2 deletions packages/database/src/api/Database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,11 @@ export class Database implements FirebaseService {
const apiName = 'database.refFromURL';
this.checkDeleted_(apiName);
validateArgCount(apiName, 1, 1, arguments.length);
const parsedURL = parseRepoInfo(url);
const parsedURL = parseRepoInfo(url, this.repo_.repoInfo_.nodeAdmin);
validateUrl(apiName, 1, parsedURL);

const repoInfo = parsedURL.repoInfo;
if (repoInfo.host !== (this.repo_.repoInfo_ as RepoInfo).host) {
if (repoInfo.host !== this.repo_.repoInfo_.host) {
fatal(
apiName +
': Host name does not match the current database: ' +
Expand Down
12 changes: 7 additions & 5 deletions packages/database/src/core/PersistentConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -796,7 +796,7 @@ export class PersistentConnection extends ServerActions {
.then(null, error => {
self.log_('Failed to get token: ' + error);
if (!canceled) {
if (CONSTANTS.NODE_ADMIN) {
if (this.repoInfo_.nodeAdmin) {
// This may be a critical error for the Admin Node.js SDK, so log a warning.
// But getToken() may also just have temporarily failed, so we still want to
// continue retrying.
Expand Down Expand Up @@ -959,10 +959,12 @@ export class PersistentConnection extends ServerActions {
const stats: { [k: string]: number } = {};

let clientName = 'js';
if (CONSTANTS.NODE_ADMIN) {
clientName = 'admin_node';
} else if (CONSTANTS.NODE_CLIENT) {
clientName = 'node';
if (isNodeSdk()) {
if (this.repoInfo_.nodeAdmin) {
clientName = 'admin_node';
} else {
clientName = 'node';
}
}

stats['sdk.' + clientName + '.' + SDK_VERSION.replace(/\./g, '-')] = 1;
Expand Down
22 changes: 12 additions & 10 deletions packages/database/src/core/RepoInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,21 @@ export class RepoInfo {
internalHost: string;

/**
* @param {string} host Hostname portion of the url for the repo
* @param {boolean} secure Whether or not this repo is accessed over ssl
* @param {string} namespace The namespace represented by the repo
* @param {boolean} webSocketOnly Whether to prefer websockets over all other transports (used by Nest).
* @param {string=} persistenceKey Override the default session persistence storage key
* @param host Hostname portion of the url for the repo
* @param secure Whether or not this repo is accessed over ssl
* @param namespace The namespace represented by the repo
* @param webSocketOnly Whether to prefer websockets over all other transports (used by Nest).
* @param nodeAdmin Whether this instance uses Admin SDK credentials
* @param persistenceKey Override the default session persistence storage key
*/
constructor(
host: string,
public secure: boolean,
public namespace: string,
public webSocketOnly: boolean,
public persistenceKey: string = '',
public includeNamespaceInQueryParams: boolean = false
public readonly secure: boolean,
public readonly namespace: string,
public readonly webSocketOnly: boolean,
public readonly nodeAdmin: boolean = false,
public readonly persistenceKey: string = '',
public readonly includeNamespaceInQueryParams: boolean = false
) {
this.host = host.toLowerCase();
this.domain = this.host.substr(this.host.indexOf('.') + 1);
Expand Down
9 changes: 5 additions & 4 deletions packages/database/src/core/RepoManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ export class RepoManager {
databaseFromApp(
app: FirebaseApp,
authProvider: Provider<FirebaseAuthInternalName>,
url?: string
url?: string,
nodeAdmin?: boolean
): Database {
let dbUrl: string | undefined = url || app.options.databaseURL;
if (dbUrl === undefined) {
Expand All @@ -111,7 +112,7 @@ export class RepoManager {
dbUrl = `${app.options.projectId}-default-rtdb.firebaseio.com`;
}

let parsedUrl = parseRepoInfo(dbUrl);
let parsedUrl = parseRepoInfo(dbUrl, nodeAdmin);
let repoInfo = parsedUrl.repoInfo;

let isEmulator: boolean;
Expand All @@ -124,14 +125,14 @@ export class RepoManager {
if (dbEmulatorHost) {
isEmulator = true;
dbUrl = `http://${dbEmulatorHost}?ns=${repoInfo.namespace}`;
parsedUrl = parseRepoInfo(dbUrl);
parsedUrl = parseRepoInfo(dbUrl, nodeAdmin);
repoInfo = parsedUrl.repoInfo;
} else {
isEmulator = !parsedUrl.repoInfo.secure;
}

const authTokenProvider =
CONSTANTS.NODE_ADMIN && isEmulator
nodeAdmin && isEmulator
? new EmulatorAdminTokenProvider()
: new FirebaseAuthTokenProvider(app, authProvider);

Expand Down
9 changes: 3 additions & 6 deletions packages/database/src/core/util/libs/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,9 @@ function decodeQuery(queryString: string): { [key: string]: string } {
return results;
}

/**
*
* @param {!string} dataURL
* @return {{repoInfo: !RepoInfo, path: !Path}}
*/
export const parseRepoInfo = function (
dataURL: string
dataURL: string,
nodeAdmin: boolean
): { repoInfo: RepoInfo; path: Path } {
const parsedUrl = parseDatabaseURL(dataURL),
namespace = parsedUrl.namespace;
Expand Down Expand Up @@ -101,6 +97,7 @@ export const parseRepoInfo = function (
parsedUrl.host,
parsedUrl.secure,
namespace,
nodeAdmin,
webSocketOnly,
/*persistenceKey=*/ '',
/*includeNamespaceInQueryParams=*/ namespace !== parsedUrl.subdomain
Expand Down
4 changes: 3 additions & 1 deletion packages/database/src/realtime/WebSocketConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export class WebSocketConnection implements Transport {
private stats_: StatsCollection;
private everConnected_: boolean;
private isClosed_: boolean;
private nodeAdmin: boolean;

/**
* @param connId identifier for this transport
Expand All @@ -99,6 +100,7 @@ export class WebSocketConnection implements Transport {
transportSessionId,
lastSessionId
);
this.nodeAdmin = repoInfo.nodeAdmin;
}

/**
Expand Down Expand Up @@ -151,7 +153,7 @@ export class WebSocketConnection implements Transport {

try {
if (isNodeSdk()) {
const device = ENV_CONSTANTS.NODE_ADMIN ? 'AdminNode' : 'Node';
const device = this.nodeAdmin ? 'AdminNode' : 'Node';
// UA Format: Firebase/<wire_protocol>/<sdk_version>/<platform>/<device>
const options: { [k: string]: object } = {
headers: {
Expand Down
38 changes: 38 additions & 0 deletions packages/rules-unit-testing/test/database.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,44 @@ describe('Testing Module Tests', function () {
);
});

it('initializeAdminApp() and initializeTestApp() work together', async function () {
await firebase.loadDatabaseRules({
databaseName: 'foo',
rules: JSON.stringify({
'rules': {
'public': { '.read': true, '.write': true },
'private': { '.read': false, '.write': false }
}
})
});

const adminApp = firebase.initializeAdminApp({
projectId: 'foo',
databaseName: 'foo'
});

const testApp = firebase.initializeTestApp({
projectId: 'foo',
databaseName: 'foo'
});

// Admin app can write anywhere
await firebase.assertSucceeds(
adminApp.database().ref().child('/public/doc').set({ hello: 'admin' })
);
await firebase.assertSucceeds(
adminApp.database().ref().child('/private/doc').set({ hello: 'admin' })
);

// Test app can only write to public, not to private
await firebase.assertSucceeds(
testApp.database().ref().child('/public/doc').set({ hello: 'test' })
);
await firebase.assertFails(
testApp.database().ref().child('/private/doc').set({ hello: 'test' })
);
});

it('loadDatabaseRules() throws if no databaseName or rules', async function () {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
await expect((firebase as any).loadDatabaseRules.bind(null, {})).to.throw(
Expand Down

0 comments on commit 3d9b5a5

Please sign in to comment.