Skip to content

Commit 9ef1e1a

Browse files
authored
Explicitly load credentials from Firebase (#1611)
* Log uncaught errors from sagas to the console * Explicitly load credentials from Firebase When Popcode first integrated Firebase, the Firebase client stored provider credentials and exposed them in the `onAuthStateChange` callback. However, that changed a long time ago, obliging us to store provider credentials in the Firebase database. At the time, I decided to keep the interface of the Firebase client the same, keeping the storage and retrieval of credentials abstracted from the caller (in this case, the saga). Ultimately, this was a path-dependent outcome, and there’s no reason the saga shouldn’t just handle that task itself. With an eye toward both becoming more robust to mismatches between stored credential information and actual linked accounts; and to loading and storing more information about the user’s provider accounts (specifically the provider profiles), move handling of credential persistence into the saga. While in general we’re able to handle this reasonably elegantly in the saga, there is one wrinkle, which is that the channel for auth state changes fires regardless of whether the auth state change is due to an explicit user action or an implicit change (e.g. the user logged in/out in a different tab). Oddly, the Firebase client fires the auth state change handler _before_ resolving the promise returned by the call to log in. So, when the user logs in explicitly, we now wait for the promise to resolve (which gives us a credential we want to store and use), and in parallel we pull the next auth state change off the channel, simply discarding it, since it’s redundant with the login promise but contains less information. * Disable “ghost mode” for BrowserStack Also set a couple of other useful options
1 parent 989a008 commit 9ef1e1a

File tree

15 files changed

+496
-294
lines changed

15 files changed

+496
-294
lines changed

gulpfile.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,10 @@ gulp.task('browserSync', ['static'], () => {
141141
const compiler = webpack(webpackConfiguration(process.env.NODE_ENV));
142142
compiler.plugin('invalid', browserSync.reload);
143143
browserSync.init({
144+
ghostMode: false,
145+
notify: false,
144146
open: !isDocker(),
147+
reloadOnRestart: true,
145148
server: {
146149
baseDir: distDir,
147150
middleware: [

src/channels/index.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1 @@
1-
import loginState from './loginState';
2-
3-
export {loginState};
1+
export {default as makeLoginState} from './makeLoginState';

src/channels/loginState.js

Lines changed: 0 additions & 9 deletions
This file was deleted.

src/channels/makeLoginState.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import {eventChannel} from 'redux-saga';
2+
3+
import {onAuthStateChanged} from '../clients/firebase';
4+
5+
export default function makeLoginState() {
6+
return eventChannel(
7+
emitter => onAuthStateChanged(({user}) => {
8+
emitter({user});
9+
}),
10+
);
11+
}

src/clients/firebase.js

Lines changed: 17 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
import Cookies from 'js-cookie';
22
import get from 'lodash-es/get';
33
import isEmpty from 'lodash-es/isEmpty';
4-
import isEqual from 'lodash-es/isEqual';
54
import isNil from 'lodash-es/isNil';
65
import isNull from 'lodash-es/isNull';
7-
import map from 'lodash-es/map';
86
import omit from 'lodash-es/omit';
97
import values from 'lodash-es/values';
108
import uuid from 'uuid/v4';
@@ -60,14 +58,9 @@ function buildFirebase(appName = undefined) {
6058
}
6159

6260
export function onAuthStateChanged(listener) {
63-
const unsubscribe = auth.onAuthStateChanged(async(user) => {
64-
if (isNull(user)) {
65-
listener({user: null});
66-
} else {
67-
listener(await decorateUserWithCredentials(user));
68-
}
61+
return auth.onAuthStateChanged((user) => {
62+
listener({user});
6963
});
70-
return unsubscribe;
7164
}
7265

7366
async function workspace(uid) {
@@ -107,22 +100,11 @@ export async function saveProject(uid, project) {
107100
setWithPriority(project, -Date.now());
108101
}
109102

110-
async function decorateUserWithCredentials(user) {
103+
export async function loadCredentialsForUser(uid) {
111104
const database = await loadDatabase();
112105
const credentialEvent =
113-
await database.ref(`authTokens/${user.uid}`).once('value');
114-
const credentials = values(credentialEvent.val() || {});
115-
if (
116-
!isEqual(
117-
map(credentials, 'providerId').sort(),
118-
map(user.providerData, 'providerId').sort(),
119-
)
120-
) {
121-
await auth.signOut();
122-
return {user: null};
123-
}
124-
125-
return {user, credentials};
106+
await database.ref(`authTokens/${uid}`).once('value');
107+
return values(credentialEvent.val());
126108
}
127109

128110
export async function signIn(provider) {
@@ -135,7 +117,6 @@ export async function signIn(provider) {
135117
} else if (provider === 'google') {
136118
userCredential = await signInWithGoogle();
137119
}
138-
await saveUserCredential(userCredential);
139120
return userCredential;
140121
} finally {
141122
setTimeout(() => {
@@ -145,10 +126,9 @@ export async function signIn(provider) {
145126
}
146127

147128
export async function linkGithub() {
148-
const userCredential =
129+
const {credential} =
149130
await auth.currentUser.linkWithPopup(githubAuthProvider);
150-
await saveUserCredential(userCredential);
151-
return userCredential.credential;
131+
return credential;
152132
}
153133

154134
export async function migrateAccount(inboundAccountCredential) {
@@ -174,7 +154,7 @@ export async function migrateAccount(inboundAccountCredential) {
174154
async function migrateCredential(credential, {auth: inboundAccountAuth}) {
175155
await inboundAccountAuth.currentUser.unlink(credential.providerId);
176156
await auth.currentUser.linkAndRetrieveDataWithCredential(credential);
177-
await saveUserCredential({user: auth.currentUser, credential});
157+
await saveCredentialForCurrentUser(credential);
178158
}
179159

180160
async function migrateProjects({
@@ -235,10 +215,18 @@ export async function signOut() {
235215
return auth.signOut();
236216
}
237217

238-
async function saveUserCredential({
218+
export async function saveUserCredential({
239219
user: {uid},
240220
credential,
241221
}) {
222+
await saveCredentialForUser(uid, credential);
223+
}
224+
225+
export async function saveCredentialForCurrentUser(credential) {
226+
await saveCredentialForUser(auth.currentUser.uid, credential);
227+
}
228+
229+
async function saveCredentialForUser(uid, credential) {
242230
const database = await loadDatabase();
243231
await database.
244232
ref(`authTokens/${providerPath(uid, credential.providerId)}`).

src/createApplicationStore.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ const compose = get(
1919
export default function createApplicationStore() {
2020
const sagaMiddleware = createSagaMiddleware({
2121
onError(error) {
22+
if (get(console, 'error')) {
23+
// eslint-disable-next-line no-console
24+
console.error(error);
25+
}
2226
bugsnagClient.notify(error);
2327
},
2428
});

src/sagas/index.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import {all} from 'redux-saga/effects';
2+
3+
import manageUserState from './manageUserState';
24
import watchErrors from './errors';
35
import watchProjects from './projects';
46
import watchUi from './ui';
@@ -8,6 +10,7 @@ import watchCompiledProjects from './compiledProjects';
810

911
export default function* rootSaga() {
1012
yield all([
13+
manageUserState(),
1114
watchErrors(),
1215
watchProjects(),
1316
watchUi(),

src/sagas/manageUserState.js

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
import {all, call, fork, put, race, take} from 'redux-saga/effects';
2+
import isEmpty from 'lodash-es/isEmpty';
3+
import isError from 'lodash-es/isError';
4+
import isNil from 'lodash-es/isNil';
5+
import isString from 'lodash-es/isString';
6+
import reject from 'lodash-es/reject';
7+
8+
import {bugsnagClient} from '../util/bugsnag';
9+
import {
10+
getSessionUid,
11+
loadCredentialsForUser,
12+
saveUserCredential,
13+
signIn,
14+
signOut,
15+
startSessionHeartbeat,
16+
} from '../clients/firebase';
17+
import {makeLoginState} from '../channels';
18+
import {notificationTriggered} from '../actions/ui';
19+
import {userAuthenticated, userLoggedOut} from '../actions/user';
20+
21+
export function* handleInitialAuth(user) {
22+
if (isNil(user)) {
23+
yield put(userLoggedOut());
24+
return;
25+
}
26+
27+
const sessionUid = yield call(getSessionUid);
28+
if (user.uid !== sessionUid) {
29+
yield call(signOut);
30+
return;
31+
}
32+
33+
const credentials = yield call(loadCredentialsForUser, user.uid);
34+
35+
if (isEmpty(credentials)) {
36+
yield call(signOut);
37+
return;
38+
}
39+
40+
yield put(userAuthenticated(user, credentials));
41+
}
42+
43+
export function* handleAuthChange(user, {newCredential} = {}) {
44+
if (isNil(user)) {
45+
yield put(userLoggedOut());
46+
} else {
47+
if (!isNil(newCredential)) {
48+
yield fork(saveUserCredential, {user, credential: newCredential});
49+
}
50+
let credentials;
51+
52+
const storedCredentials = yield call(loadCredentialsForUser, user.uid);
53+
if (isNil(newCredential)) {
54+
credentials = storedCredentials;
55+
} else {
56+
credentials = reject(
57+
storedCredentials,
58+
{providerId: newCredential.providerId},
59+
);
60+
credentials.push(newCredential);
61+
}
62+
63+
yield put(userAuthenticated(user, credentials));
64+
}
65+
}
66+
export function* handleAuthError(e) {
67+
if ('message' in e && e.message === 'popup_closed_by_user') {
68+
yield put(notificationTriggered('user-cancelled-auth'));
69+
return;
70+
}
71+
72+
switch (e.code) {
73+
case 'auth/popup-closed-by-user':
74+
yield put(notificationTriggered('user-cancelled-auth'));
75+
break;
76+
77+
case 'auth/network-request-failed':
78+
yield put(notificationTriggered('auth-network-error'));
79+
break;
80+
81+
case 'auth/cancelled-popup-request':
82+
break;
83+
84+
case 'auth/web-storage-unsupported':
85+
case 'auth/operation-not-supported-in-this-environment':
86+
yield put(
87+
notificationTriggered('auth-third-party-cookies-disabled'),
88+
);
89+
break;
90+
91+
case 'access_denied':
92+
case 'auth/internal-error':
93+
yield put(notificationTriggered('auth-error'));
94+
break;
95+
96+
default:
97+
yield put(notificationTriggered('auth-error'));
98+
99+
if (isError(e)) {
100+
yield call([bugsnagClient, 'notify'], e, {metaData: {code: e.code}});
101+
} else if (isString(e)) {
102+
yield call([bugsnagClient, 'notify'], new Error(e));
103+
}
104+
break;
105+
}
106+
}
107+
108+
export default function* manageUserState(loginState = makeLoginState()) {
109+
yield fork(startSessionHeartbeat);
110+
111+
const {user: initialUser} = yield take(loginState);
112+
yield call(handleInitialAuth, initialUser);
113+
114+
while (true) {
115+
const {loginStateChange, logInAction} = yield race({
116+
logInAction: take('LOG_IN'),
117+
loginStateChange: take(loginState),
118+
});
119+
120+
if (isNil(logInAction)) {
121+
yield call(handleAuthChange, loginStateChange.user);
122+
} else {
123+
try {
124+
const [{user, credential}] = yield all([
125+
call(signIn, logInAction.payload.provider),
126+
take(loginState),
127+
]);
128+
yield call(handleAuthChange, user, {newCredential: credential});
129+
} catch (e) {
130+
yield handleAuthError(e);
131+
}
132+
}
133+
}
134+
}

0 commit comments

Comments
 (0)