Skip to content

Commit 8d3cce3

Browse files
scottcrossenscottcrossen
committed
responded to reviewer comments
1 parent 98010cb commit 8d3cce3

File tree

4 files changed

+50
-15641
lines changed

4 files changed

+50
-15641
lines changed

‎packages-exp/auth-exp/src/core/user/additional_user_info.test.ts

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,15 @@
1616
*/
1717

1818
import { expect } from 'chai';
19-
import { fromIdTokenResponse } from './additional_user_info';
19+
import { _fromIdTokenResponse } from './additional_user_info';
2020
import { IdTokenResponse, IdTokenResponseKind } from '../../model/id_token';
21-
import { ProviderId } from '../providers';
22-
import { UserProfile } from '../../model/user';
21+
import {
22+
UserProfile,
23+
ProviderId
24+
} from '@firebase/auth-types-exp';
2325

2426
describe('core/user/additional_user_info', () => {
25-
describe('fromIdTokenResponse', () => {
27+
describe('_fromIdTokenResponse', () => {
2628
const userProfileWithLogin: UserProfile = {
2729
login: 'scott',
2830
friends: [],
@@ -43,7 +45,7 @@ describe('core/user/additional_user_info', () => {
4345
providerId,
4446
username,
4547
profile
46-
} = fromIdTokenResponse(idResponse)!;
48+
} = _fromIdTokenResponse(idResponse)!;
4749
expect(isNewUser).to.be.false;
4850
expect(providerId).to.eq(ProviderId.FACEBOOK);
4951
expect(username).to.be.null;
@@ -60,7 +62,7 @@ describe('core/user/additional_user_info', () => {
6062
providerId,
6163
username,
6264
profile
63-
} = fromIdTokenResponse(idResponse)!;
65+
} = _fromIdTokenResponse(idResponse)!;
6466
expect(isNewUser).to.be.false;
6567
expect(providerId).to.eq(ProviderId.GITHUB);
6668
expect(username).to.eq('scott');
@@ -77,7 +79,7 @@ describe('core/user/additional_user_info', () => {
7779
providerId,
7880
username,
7981
profile
80-
} = fromIdTokenResponse(idResponse)!;
82+
} = _fromIdTokenResponse(idResponse)!;
8183
expect(isNewUser).to.be.false;
8284
expect(providerId).to.eq(ProviderId.GOOGLE);
8385
expect(username).to.be.null;
@@ -95,7 +97,7 @@ describe('core/user/additional_user_info', () => {
9597
providerId,
9698
username,
9799
profile
98-
} = fromIdTokenResponse(idResponse)!;
100+
} = _fromIdTokenResponse(idResponse)!;
99101
expect(isNewUser).to.be.false;
100102
expect(providerId).to.eq(ProviderId.TWITTER);
101103
expect(username).to.eq('scott');
@@ -109,14 +111,14 @@ describe('core/user/additional_user_info', () => {
109111
providerId: ProviderId.FACEBOOK,
110112
rawUserInfo: rawUserInfoWithLogin
111113
});
112-
expect(fromIdTokenResponse(idResponse)!.profile).to.eql(
114+
expect(_fromIdTokenResponse(idResponse)!.profile).to.eql(
113115
userProfileWithLogin
114116
);
115117
});
116118

117119
it('for missing JSON', () => {
118120
const idResponse = idTokenResponse({ providerId: ProviderId.FACEBOOK });
119-
expect(fromIdTokenResponse(idResponse)!.profile).to.be.empty;
121+
expect(_fromIdTokenResponse(idResponse)!.profile).to.be.empty;
120122
});
121123
});
122124

@@ -126,20 +128,20 @@ describe('core/user/additional_user_info', () => {
126128
providerId: ProviderId.FACEBOOK,
127129
isNewUser: true
128130
});
129-
expect(fromIdTokenResponse(idResponse)!.isNewUser).to.be.true;
131+
expect(_fromIdTokenResponse(idResponse)!.isNewUser).to.be.true;
130132
});
131133

132134
it('for new users by toolkit response kind', () => {
133135
const idResponse = idTokenResponse({
134136
providerId: ProviderId.FACEBOOK,
135137
kind: IdTokenResponseKind.SignupNewUser
136138
});
137-
expect(fromIdTokenResponse(idResponse)!.isNewUser).to.be.true;
139+
expect(_fromIdTokenResponse(idResponse)!.isNewUser).to.be.true;
138140
});
139141

140142
it('for old users', () => {
141143
const idResponse = idTokenResponse({ providerId: ProviderId.FACEBOOK });
142-
expect(fromIdTokenResponse(idResponse)!.isNewUser).to.be.false;
144+
expect(_fromIdTokenResponse(idResponse)!.isNewUser).to.be.false;
143145
});
144146
});
145147

@@ -154,7 +156,7 @@ describe('core/user/additional_user_info', () => {
154156
providerId,
155157
username,
156158
profile
157-
} = fromIdTokenResponse(idResponse)!;
159+
} = _fromIdTokenResponse(idResponse)!;
158160
expect(isNewUser).to.be.false;
159161
expect(providerId).to.be.null;
160162
expect(username).to.be.null;
@@ -171,7 +173,7 @@ describe('core/user/additional_user_info', () => {
171173
providerId,
172174
username,
173175
profile
174-
} = fromIdTokenResponse(idResponse)!;
176+
} = _fromIdTokenResponse(idResponse)!;
175177
expect(isNewUser).to.be.false;
176178
expect(providerId).to.be.null;
177179
expect(username).to.be.null;
@@ -184,7 +186,7 @@ describe('core/user/additional_user_info', () => {
184186
providerId,
185187
username,
186188
profile
187-
} = fromIdTokenResponse(
189+
} = _fromIdTokenResponse(
188190
idTokenResponse({ rawUserInfo: rawUserInfoWithLogin })
189191
)!;
190192
expect(isNewUser).to.be.false;
@@ -197,18 +199,18 @@ describe('core/user/additional_user_info', () => {
197199
describe('returns null', () => {
198200
it('for missing provider IDs', () => {
199201
const idResponse = idTokenResponse({});
200-
expect(fromIdTokenResponse(idResponse)!).to.be.null;
202+
expect(_fromIdTokenResponse(idResponse)).to.be.null;
201203
});
202204
});
203205
});
204206
});
205207

206208
function idTokenResponse(partial: Partial<IdTokenResponse>): IdTokenResponse {
207209
return {
208-
idToken: 'Parsing logic not implemented',
209-
refreshToken: "Doesn't matter",
210-
expiresIn: "Doesn't matter",
211-
localId: "Doesn't matter",
210+
idToken: 'id-token',
211+
refreshToken: "refresh-token",
212+
expiresIn: "expires-in",
213+
localId: "local-id",
212214
kind: IdTokenResponseKind.CreateAuthUri,
213215
...partial
214216
};

‎packages-exp/auth-exp/src/core/user/additional_user_info.ts

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -21,38 +21,31 @@ import {
2121
} from '@firebase/auth-types-exp';
2222
import { IdTokenResponse, IdTokenResponseKind } from '../../model/id_token';
2323
import { _parseToken } from './id_token_result';
24-
import { assert } from '../util/assert';
2524

2625
/**
2726
* Parse the `AdditionalUserInfo` from the ID token response.
2827
*/
29-
export function fromIdTokenResponse(
28+
export function _fromIdTokenResponse(
3029
idTokenResponse: IdTokenResponse,
31-
appName: string
3230
): AdditionalUserInfo | null {
3331
const { providerId } = idTokenResponse;
3432
const profile = idTokenResponse.rawUserInfo
3533
? JSON.parse(idTokenResponse.rawUserInfo)
3634
: {};
3735
const isNewUser =
38-
!!idTokenResponse.isNewUser ||
36+
idTokenResponse.isNewUser ||
3937
idTokenResponse.kind === IdTokenResponseKind.SignupNewUser;
4038
if (!providerId && !!idTokenResponse) {
4139
const providerId = _parseToken(idTokenResponse.idToken)?.firebase?.[
4240
'sign_in_provider'
4341
];
44-
assert(
45-
// @ts-ignore - Check to see if string is castable to enum.
46-
!providerId || Object.values(ProviderId).includes(providerId),
47-
appName
48-
);
4942
if (providerId) {
5043
const filteredProviderId =
5144
providerId !== ProviderId.ANONYMOUS && providerId !== ProviderId.CUSTOM
5245
? (providerId as ProviderId)
5346
: null;
5447
// Uses generic class in accordance with the legacy SDK.
55-
return new GenericAdditionalUserInfo(isNewUser, filteredProviderId, null);
48+
return new GenericAdditionalUserInfo(isNewUser, filteredProviderId);
5649
}
5750
}
5851
if (!providerId) {
@@ -73,12 +66,11 @@ export function fromIdTokenResponse(
7366
);
7467
case ProviderId.CUSTOM:
7568
case ProviderId.ANONYMOUS:
76-
return new GenericAdditionalUserInfo(isNewUser, null, null);
69+
return new GenericAdditionalUserInfo(isNewUser, null);
7770
default:
7871
return new FederatedAdditionalUserInfo(
7972
isNewUser,
8073
providerId,
81-
null,
8274
profile
8375
);
8476
}
@@ -88,50 +80,59 @@ class GenericAdditionalUserInfo implements AdditionalUserInfo {
8880
constructor(
8981
readonly isNewUser: boolean,
9082
readonly providerId: ProviderId | null,
91-
readonly username: string | null
9283
) {}
9384
}
9485

9586
class FederatedAdditionalUserInfo extends GenericAdditionalUserInfo {
9687
constructor(
9788
isNewUser: boolean,
98-
providerId: ProviderId | null,
99-
username: string | null,
100-
readonly profile: UserProfile
89+
providerId: ProviderId,
90+
readonly profile: UserProfile,
91+
) {
92+
super(isNewUser, providerId);
93+
}
94+
}
95+
96+
class FederatedAdditionalUserInfoWithUsername extends FederatedAdditionalUserInfo {
97+
constructor(
98+
isNewUser: boolean,
99+
providerId: ProviderId,
100+
profile: UserProfile,
101+
readonly username: string | null,
101102
) {
102-
super(isNewUser, providerId, username);
103+
super(isNewUser, providerId, profile);
103104
}
104105
}
105106

106107
class FacebookAdditionalUserInfo extends FederatedAdditionalUserInfo {
107108
constructor(isNewUser: boolean, profile: UserProfile) {
108-
super(isNewUser, ProviderId.FACEBOOK, null, profile);
109+
super(isNewUser, ProviderId.FACEBOOK, profile);
109110
}
110111
}
111112

112-
class GithubAdditionalUserInfo extends FederatedAdditionalUserInfo {
113+
class GithubAdditionalUserInfo extends FederatedAdditionalUserInfoWithUsername {
113114
constructor(isNewUser: boolean, profile: UserProfile) {
114115
super(
115116
isNewUser,
116117
ProviderId.GITHUB,
117-
typeof profile?.login === 'string' ? profile?.login : null,
118-
profile
118+
profile,
119+
typeof profile?.login === 'string' ? profile?.login : null
119120
);
120121
}
121122
}
122123

123124
class GoogleAdditionalUserInfo extends FederatedAdditionalUserInfo {
124125
constructor(isNewUser: boolean, profile: UserProfile) {
125-
super(isNewUser, ProviderId.GOOGLE, null, profile);
126+
super(isNewUser, ProviderId.GOOGLE, profile);
126127
}
127128
}
128129

129-
class TwitterAdditionalUserInfo extends FederatedAdditionalUserInfo {
130+
class TwitterAdditionalUserInfo extends FederatedAdditionalUserInfoWithUsername {
130131
constructor(
131132
isNewUser: boolean,
132133
profile: UserProfile,
133134
screenName: string | null
134135
) {
135-
super(isNewUser, ProviderId.TWITTER, screenName, profile);
136+
super(isNewUser, ProviderId.TWITTER, profile, screenName);
136137
}
137138
}

‎packages-exp/auth-types-exp/index.d.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,10 +419,10 @@ export interface AdditionalUserInfo {
419419
readonly isNewUser: boolean;
420420
readonly profile?: UserProfile;
421421
readonly providerId: ProviderId | null;
422-
readonly username: string | null;
422+
readonly username?: string | null;
423423
}
424424

425425
/**
426426
* User profile used in `AdditionalUserInfo`
427427
*/
428-
export type UserProfile = { [key: string]: unknown } | null;
428+
export type UserProfile = Record<string, unknown>;

0 commit comments

Comments
 (0)