Skip to content
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

[ServerApp] Update feature branch Auth implementation to use the authIdToken #7944

Merged
merged 47 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
1216622
initial pass of additions
DellaBitta Sep 19, 2023
2efe043
another pass, optional callback failures
DellaBitta Sep 20, 2023
fda3060
compilation fixes
DellaBitta Sep 21, 2023
5fa9c21
FirebaseServerAppImpl now extends FirebaseAppImpl
DellaBitta Sep 21, 2023
0ef3bb2
removed Headers object, added three callbacks instead
DellaBitta Oct 2, 2023
8cd1072
added deleteOnDeref support
DellaBitta Oct 2, 2023
d42f676
lint fixes
DellaBitta Oct 2, 2023
612f990
deleteOnDeref type converted from WeakRef to object.
DellaBitta Oct 3, 2023
76f2fe1
update cookie func signatures. Check for existence of FinalizationReg…
DellaBitta Oct 4, 2023
d68f986
added baseline firebaseServerApp.test.ts
DellaBitta Oct 5, 2023
09e3157
update callback method siganatures with shorter names and alt undef p…
DellaBitta Oct 5, 2023
3750aa4
added some firebaseServerApp tests
DellaBitta Oct 10, 2023
316309c
Merge branch 'master' into ddb-server-app
DellaBitta Dec 4, 2023
6cc428b
format
DellaBitta Dec 4, 2023
53c15e2
Merge branch 'master' into ddb-server-app
DellaBitta Dec 6, 2023
22b9bd9
rework builds, tests need updating
DellaBitta Dec 7, 2023
3c39866
another API update and doc generation
DellaBitta Dec 14, 2023
d5e461e
Merge branch 'master' into ddb-server-app
DellaBitta Dec 15, 2023
a7d111a
reflect new API proposal in source
DellaBitta Dec 15, 2023
0427934
FirbaseServerAppSettings name?: undefined
DellaBitta Dec 18, 2023
db9b941
merge master
DellaBitta Dec 18, 2023
e146815
remove firebaseServerApp name
DellaBitta Dec 20, 2023
e9f28ed
remove engines from package.json
DellaBitta Dec 20, 2023
a13f418
FirebaseApp / Options typeguard. More tests.
DellaBitta Dec 21, 2023
8a745d1
Update FirebaseServerAppConfig name to be undefined.
DellaBitta Dec 21, 2023
7b02583
throw if firebaseServerApp constructed in browser. Remove lib ESNext …
DellaBitta Dec 31, 2023
d86eeb7
throw in initializeServerApp if invoked in a browser env.
DellaBitta Dec 31, 2023
b34b878
Update index.d.ts
DellaBitta Dec 31, 2023
4aeb9a6
Update index.d.ts
DellaBitta Dec 31, 2023
04af931
Remove the new FirebaseServerApp from the app-types.
DellaBitta Jan 3, 2024
968e176
Leverage the authIdToken to login users in FirebaseServerApp instances
DellaBitta Jan 12, 2024
b5a84b0
merged in feature branch
DellaBitta Jan 12, 2024
826a3c4
void promise
DellaBitta Jan 16, 2024
e2a0f9a
const providerData
DellaBitta Jan 16, 2024
a6bdf3e
provider data initialized inline
DellaBitta Jan 16, 2024
c323811
format and getToken test update
DellaBitta Jan 16, 2024
e8151d6
more stsmanager tests
DellaBitta Jan 16, 2024
8e6ab2d
fix test deep-equal
DellaBitta Jan 16, 2024
1980c7b
[ServerApp] Add integration tests for FirebaseServerApp (#7960)
DellaBitta Jan 22, 2024
6543206
add comment about authIdToken string member of FirebaseServerAppImpl
DellaBitta Jan 22, 2024
29fa942
remove duplicate assertions in getToken.
DellaBitta Jan 22, 2024
bb9ba8a
Merge branch 'feature-firebaseserverapp' into ddb-serverapp-use-auth-…
DellaBitta Jan 23, 2024
940ec40
test for invalid token
DellaBitta Jan 23, 2024
8bf5286
Fix tests for emulators
DellaBitta Jan 24, 2024
3b066fa
Fix isFirebaseSeverAppImpl export problem
DellaBitta Jan 25, 2024
020687b
formatter.
DellaBitta Jan 25, 2024
63cf1fb
Skip loading users from persistence with FirebaseServerApps.
DellaBitta Jan 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions common/api-review/app.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,11 @@ export function initializeServerApp(options: FirebaseOptions | FirebaseApp, conf
// @internal (undocumented)
export function _isFirebaseApp(obj: FirebaseApp | FirebaseOptions): obj is FirebaseApp;

// Warning: (ae-forgotten-export) The symbol "FirebaseServerAppImpl" needs to be exported by the entry point index.d.ts
Copy link
Contributor

Choose a reason for hiding this comment

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

If you're exporting this out of @firebase/app you should also export FirebaseServerAppImpl, as its typing depends on that type, otherwise you could run into some TS errors. The other alternative is to be a little looser with the typings and not do the obj is FirebaseServerAppImpl and just do boolean.

Copy link
Contributor Author

@DellaBitta DellaBitta Jan 24, 2024

Choose a reason for hiding this comment

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

Ah yes, I was exporting FirebaseServerAppImpl so that Auth could get the authIdToken from it. I didn't realize that it would then be part of the public API, too, but that makes total sense that it is.

I'll work to make it FirebaseServerAppImpl to say withinapp only, and figure out something else for auth.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, to be clear, FirebaseServerAppImpl isn't being exported at the moment, as far as I can tell, which is what api-extractor is warning you about. _isFirebaseServerAppImpl is being exported. But since the typing of _isFirebaseServerAppImpl depends on FirebaseServerAppImpl, api-extractor is complaining that you have to export that too. The way to get around it is to not use the obj is return type and return a boolean instead.

The internal.ts file isn't "public" in that it's not meant to be used be users of the SDK, it's meant to export "internal" methods used by other packages, like auth, so I think this is fine to export this function in that file (if you check dist/ after building you should see it show up in the "internal" bundle and not in the main bundle). The only problem is that there may possibly be some TS issue at some point if you don't resolve this warning, so I think the simplest way is to change the return type to boolean.

Copy link
Contributor Author

@DellaBitta DellaBitta Jan 25, 2024

Choose a reason for hiding this comment

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

Right, I was saying to fix the error I would have needed to export FirebaseServerAppImpl, and I didn't want to do that.

This should be fixed now. I've updated the function to simple by isFirebaseServerApp and not isFirebaseServerAppImpl. FirebasServerApp is already a public interface so we should be good to go here.

//
// @internal (undocumented)
export function _isFirebaseServerAppImpl(obj: FirebaseApp | FirebaseServerApp): obj is FirebaseServerAppImpl;

// @public
export function onLog(logCallback: LogCallback | null, options?: LogOptions): void;

Expand Down
4 changes: 4 additions & 0 deletions packages/app/src/firebaseServerApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ export class FirebaseServerAppImpl
this.automaticCleanup
);

this.authIdToken = serverConfig.authIdToken;

if (this._serverConfig.releaseOnDeref !== undefined) {
this._finalizationRegistry.register(
this._serverConfig.releaseOnDeref,
Expand All @@ -88,6 +90,8 @@ export class FirebaseServerAppImpl
return this._serverConfig;
}

authIdToken?: string;

authIdTokenVerified(): Promise<void> {
this.checkDestroyed();
// TODO
Expand Down
14 changes: 14 additions & 0 deletions packages/app/src/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,20 @@ export function _isFirebaseApp(
return (obj as FirebaseApp).options !== undefined;
}

/**
*
* @param obj - an object of type FirebaseApp.
*
* @returns true if the provided object is of type FirebaseServerAppImpl.
*
* @internal
*/
export function _isFirebaseServerAppImpl(
obj: FirebaseApp | FirebaseServerApp
): obj is FirebaseServerAppImpl {
return (obj as FirebaseServerAppImpl).authIdToken !== undefined;
}

/**
* Test only
*
Expand Down
3 changes: 2 additions & 1 deletion packages/auth/karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ function getTestFiles(argv) {
'src/**/*.test.ts',
'test/helpers/**/*.test.ts',
'test/integration/flows/anonymous.test.ts',
'test/integration/flows/email.test.ts'
'test/integration/flows/email.test.ts',
'test/integration/flows/firebaseserverapp.test.ts'
];
}
}
Expand Down
4 changes: 3 additions & 1 deletion packages/auth/scripts/run_node_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ let testConfig = [
];

if (argv.integration) {
testConfig = ['test/integration/flows/{email,anonymous}.test.ts'];
testConfig = [
'test/integration/flows/{email,anonymous,firebaseserverapp}.test.ts'
];
if (argv.local) {
testConfig.push('test/integration/flows/*.local.test.ts');
}
Expand Down
33 changes: 31 additions & 2 deletions packages/auth/src/core/auth/initialize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,21 @@
* limitations under the License.
*/

import { _getProvider, FirebaseApp } from '@firebase/app';
import {
_getProvider,
_isFirebaseServerAppImpl,
FirebaseApp
} from '@firebase/app';
import { deepEqual } from '@firebase/util';
import { Auth, Dependencies } from '../../model/public_types';

import { AuthErrorCode } from '../errors';
import { PersistenceInternal } from '../persistence';
import { _fail } from '../util/assert';
import { _getInstance } from '../util/instantiator';
import { AuthImpl } from './auth_impl';
import { AuthImpl, _castAuth } from './auth_impl';
import { UserImpl } from '../user/user_impl';
import { getAccountInfo } from '../../api/account_management/account';

/**
* Initializes an {@link Auth} instance with fine-grained control over
Expand Down Expand Up @@ -65,9 +71,32 @@ export function initializeAuth(app: FirebaseApp, deps?: Dependencies): Auth {

const auth = provider.initialize({ options: deps }) as AuthImpl;

if (_isFirebaseServerAppImpl(app)) {
if (app.authIdToken !== undefined) {
void _loadUserFromIdToken(auth, app.authIdToken);
}
}

return auth;
}

export async function _loadUserFromIdToken(
auth: Auth,
idToken: string
): Promise<void> {
const response = await getAccountInfo(auth, { idToken });
const authInternal = _castAuth(auth);
await authInternal._initializationPromise;

const user = await UserImpl._fromGetAccountInfoResponse(
authInternal,
response,
idToken
);

await authInternal._updateCurrentUser(user);
}

export function _initializeAuthInstance(
auth: AuthImpl,
deps?: Dependencies
Expand Down
2 changes: 1 addition & 1 deletion packages/auth/src/core/user/reload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ function mergeProviderData(
return [...deduped, ...newData];
}

function extractProviderData(providers: ProviderUserInfo[]): UserInfo[] {
export function extractProviderData(providers: ProviderUserInfo[]): UserInfo[] {
return providers.map(({ providerId, ...provider }) => {
return {
providerId,
Expand Down
31 changes: 29 additions & 2 deletions packages/auth/src/core/user/token_manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,35 @@ describe('core/user/token_manager', () => {
});
});

it('returns null if the refresh token is missing', async () => {
expect(await stsTokenManager.getToken(auth)).to.be.null;
it('returns non-null if the refresh token is missing but token still valid', async () => {
Object.assign(stsTokenManager, {
accessToken: 'token',
expirationTime: now + 100_000
});
const tokens = await stsTokenManager.getToken(auth, false);
expect(tokens).to.eql('token');
});

it('throws an error if the refresh token is missing and force refresh is true', async () => {
Object.assign(stsTokenManager, {
accessToken: 'token',
expirationTime: now + 100_000
});
await expect(stsTokenManager.getToken(auth, true)).to.be.rejectedWith(
FirebaseError,
"Firebase: The user's credential is no longer valid. The user must sign in again. (auth/user-token-expired)"
);
});

it('throws an error if the refresh token is missing and token is no longer valid', async () => {
Object.assign(stsTokenManager, {
accessToken: 'old-access-token',
expirationTime: now - 1
});
await expect(stsTokenManager.getToken(auth)).to.be.rejectedWith(
FirebaseError,
"Firebase: The user's credential is no longer valid. The user must sign in again. (auth/user-token-expired)"
);
});

it('throws an error if expired but refresh token is missing', async () => {
Expand Down
18 changes: 12 additions & 6 deletions packages/auth/src/core/user/token_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,20 +73,26 @@ export class StsTokenManager {
);
}

updateFromIdToken(idToken: string): void {
_assert(idToken.length !== 0, AuthErrorCode.INTERNAL_ERROR);
const expiresIn = _tokenExpiresIn(idToken);
this.updateTokensAndExpiration(idToken, null, expiresIn);
}

async getToken(
auth: AuthInternal,
forceRefresh = false
): Promise<string | null> {
_assert(
!this.accessToken || this.refreshToken,
auth,
AuthErrorCode.TOKEN_EXPIRED
);
if (!this.accessToken) {
_assert(this.refreshToken, auth, AuthErrorCode.TOKEN_EXPIRED);
}

if (!forceRefresh && this.accessToken && !this.isExpired) {
return this.accessToken;
}

_assert(this.refreshToken, auth, AuthErrorCode.TOKEN_EXPIRED);

if (this.refreshToken) {
await this.refresh(auth, this.refreshToken!);
return this.accessToken;
Expand All @@ -113,7 +119,7 @@ export class StsTokenManager {

private updateTokensAndExpiration(
accessToken: string,
refreshToken: string,
refreshToken: string | null,
expiresInSec: number
): void {
this.refreshToken = refreshToken || null;
Expand Down
61 changes: 58 additions & 3 deletions packages/auth/src/core/user/user_impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@
* limitations under the License.
*/

import { IdTokenResult } from '../../model/public_types';
import { IdTokenResult, UserInfo } from '../../model/public_types';
import { NextFn } from '@firebase/util';

import {
APIUserInfo,
GetAccountInfoResponse,
deleteAccount
} from '../../api/account_management/account';
import { FinalizeMfaResponse } from '../../api/authentication/mfa';
Expand All @@ -36,7 +36,7 @@ import { _assert } from '../util/assert';
import { getIdTokenResult } from './id_token_result';
import { _logoutIfInvalidated } from './invalidation';
import { ProactiveRefresh } from './proactive_refresh';
import { _reloadWithoutSaving, reload } from './reload';
import { extractProviderData, _reloadWithoutSaving, reload } from './reload';
import { StsTokenManager } from './token_manager';
import { UserMetadata } from './user_metadata';
import { ProviderId } from '../../model/enums';
Expand Down Expand Up @@ -333,4 +333,59 @@ export class UserImpl implements UserInternal {
await _reloadWithoutSaving(user);
return user;
}

/**
* Initialize a User from an idToken server response
* @param auth
* @param idTokenResponse
*/
static async _fromGetAccountInfoResponse(
auth: AuthInternal,
response: GetAccountInfoResponse,
idToken: string
): Promise<UserInternal> {
const coreAccount = response.users[0];
_assert(coreAccount.localId !== undefined, AuthErrorCode.INTERNAL_ERROR);

const providerData: UserInfo[] =
coreAccount.providerUserInfo !== undefined
? extractProviderData(coreAccount.providerUserInfo)
: [];

const isAnonymous =
!(coreAccount.email && coreAccount.passwordHash) && !providerData?.length;

const stsTokenManager = new StsTokenManager();
stsTokenManager.updateFromIdToken(idToken);

// Initialize the Firebase Auth user.
const user = new UserImpl({
uid: coreAccount.localId,
auth,
stsTokenManager,
isAnonymous
});

// update the user with data from the GetAccountInfo response.
const updates: Partial<UserInternal> = {
uid: coreAccount.localId,
displayName: coreAccount.displayName || null,
photoURL: coreAccount.photoUrl || null,
email: coreAccount.email || null,
emailVerified: coreAccount.emailVerified || false,
phoneNumber: coreAccount.phoneNumber || null,
tenantId: coreAccount.tenantId || null,
providerData,
metadata: new UserMetadata(
coreAccount.createdAt,
coreAccount.lastLoginAt
),
isAnonymous:
!(coreAccount.email && coreAccount.passwordHash) &&
!providerData?.length
};

Object.assign(user, updates);
return user;
}
}
Loading