diff --git a/src/features/auth/AuthMain.test.tsx b/src/features/auth/AuthMain.test.tsx index 7af4331..f1a66b5 100644 --- a/src/features/auth/AuthMain.test.tsx +++ b/src/features/auth/AuthMain.test.tsx @@ -9,7 +9,11 @@ describe("CurrentUserStateIndicator", () => { (firestoreAuth.login as jest.Mock).mockClear(); }); - it("can login", async () => { + it("login will set loading ui to prevent double trigger", async () => { + (firestoreAuth.login as jest.Mock).mockResolvedValue( + new Promise((resolve) => setTimeout(resolve, 500)) + ); + const { store } = fullRender(); const loginButtonElement = screen.getByRole("button", { @@ -17,10 +21,11 @@ describe("CurrentUserStateIndicator", () => { }); await act(() => fireEvent.click(loginButtonElement)); - await waitFor(() => expect(store.getState().user.uid).not.toBe("")); + await waitFor(() => expect(store.getState().user.uid).toBe("")); + await waitFor(() => expect(screen.getByText("Checking...")).toBeTruthy()); }); - it("login is integrated with firestore", async () => { + it("triggering login will rely on firestore module", async () => { fullRender(); const loginButtonElement = screen.getByRole("button", { diff --git a/src/services/firestore-auth.test.ts b/src/services/firestore-auth.test.ts index e3777da..ca3999b 100644 --- a/src/services/firestore-auth.test.ts +++ b/src/services/firestore-auth.test.ts @@ -5,8 +5,9 @@ import * as firebaseAuth from "firebase/auth"; describe("firestoreAuth", () => { it("should successfully log in and return user information", async () => { jest.spyOn(firebaseAuth, "getAuth"); - jest.spyOn(firebaseAuth, "signInWithPopup"); - const user = await firestoreAuth.login(); + jest.spyOn(firebaseAuth, "getRedirectResult"); + await firestoreAuth.login(); + const user = await firestoreAuth.loadUser(); expect(user).toEqual({ uid: "abc123def456", displayName: "Jane Doe", @@ -16,7 +17,7 @@ describe("firestoreAuth", () => { it("should block log in when user email is null", async () => { jest.spyOn(firebaseAuth, "getAuth"); - jest.spyOn(firebaseAuth, "signInWithPopup").mockResolvedValue({ + jest.spyOn(firebaseAuth, "getRedirectResult").mockResolvedValue({ operationType: "signIn", providerId: "google.com", user: { @@ -39,7 +40,7 @@ describe("firestoreAuth", () => { photoURL: "", }, }); - await expect(firestoreAuth.login()).rejects.toThrow( + await expect(firestoreAuth.loadUser()).rejects.toThrow( "Login blocked: unidentified user." ); }); diff --git a/src/services/firestore-auth.ts b/src/services/firestore-auth.ts index c89b6e5..81added 100644 --- a/src/services/firestore-auth.ts +++ b/src/services/firestore-auth.ts @@ -3,8 +3,9 @@ import type { User as FirebaseUser } from "firebase/auth"; import { browserLocalPersistence, getAuth, + getRedirectResult, setPersistence, - signInWithPopup, + signInWithRedirect, signOut, } from "firebase/auth"; import { googleAuthProvider } from "./firestore-connection"; @@ -18,54 +19,51 @@ const firestoreAuth = { export default firestoreAuth; +/** Loads user from redirection result, persistence, whatever. */ async function loadUser(): Promise { - const empty: User = { uid: "", displayName: "", email: "" }; - - return new Promise((resolve) => { - const done = (user: FirebaseUser | null) => { - try { - if (user) { - const { uid, displayName, email } = user; - resolve({ - uid, - displayName: displayName ?? email ?? "", - email: email ?? "?@?", - }); - } else { - resolve(empty); - } - } catch (error) { + return new Promise((resolve, reject) => { + const handleResult = (user: FirebaseUser | null) => { + if (!user) { + const empty: User = { uid: "", displayName: "", email: "" }; resolve(empty); + return; + } + + if (!user.uid || !user.email) { + reject(new Error("Login blocked: unidentified user.")); + return; } + + resolve({ + uid: user.uid, + displayName: user.displayName ?? user.email, + email: user.email, + }); }; - setTimeout(() => done(null), 3000); - getAuth().onAuthStateChanged(once(done)); + // prepare auth for one of the following events: + const auth = getAuth(); + const onceHandleResult = once(handleResult); + // case of timeout (this was specially important for popup method but now not so much) + setTimeout(() => onceHandleResult(null), 5 * 1000); + // case of loaded by something else (like persistent successfully loaded) + auth.onAuthStateChanged(onceHandleResult); + // case of got result from redirect flow + getRedirectResult(auth) + .then((credentials) => credentials?.user || null) + .then(onceHandleResult); }); } -async function login(): Promise { +/** Trigger the login function, it redirects to an authentication page. */ +async function login(): Promise { const auth = getAuth(); await setPersistence(auth, browserLocalPersistence); - - const result = await signInWithPopup(auth, googleAuthProvider); - const { - user: { uid, displayName, email }, - } = result; - - if (!email) { - signOut(auth); - throw new Error("Login blocked: unidentified user."); - } - - return { - uid, - displayName: displayName ?? email, - email, - }; + await signInWithRedirect(auth, googleAuthProvider); } +/** Clears the authentication data. */ async function logout() { signOut(getAuth()); } diff --git a/src/state/user.ts b/src/state/user.ts index 6693219..c734310 100644 --- a/src/state/user.ts +++ b/src/state/user.ts @@ -64,7 +64,7 @@ export const loadUser = createAsyncThunk( export const login = createAsyncThunk( "user/login", - () => firestoreAuth.login(), + () => firestoreAuth.login().then(firestoreAuth.loadUser), { condition: (_arg, thunkAPI) => (thunkAPI.getState() as RootState).user.status === "idle", diff --git a/src/testing-helpers/mock-firebase-auth.ts b/src/testing-helpers/mock-firebase-auth.ts index d20e792..0c13f19 100644 --- a/src/testing-helpers/mock-firebase-auth.ts +++ b/src/testing-helpers/mock-firebase-auth.ts @@ -1,7 +1,7 @@ import type { Auth, AuthProvider } from "firebase/auth"; jest.mock("firebase/auth", () => ({ - getAuth: jest.fn(), + getAuth: jest.fn().mockReturnValue({ onAuthStateChanged: jest.fn() }), GoogleAuthProvider: jest.fn(() => ({ Qc: ["client_id", "response_type", "scope", "redirect_uri", "state"], providerId: "google.com", @@ -13,8 +13,9 @@ jest.mock("firebase/auth", () => ({ })), setPersistence: jest.fn(), browserLocalPersistence: jest.fn(), + signInWithRedirect: jest.fn(), // eslint-disable-next-line @typescript-eslint/no-unused-vars - signInWithPopup: jest.fn((_: Auth, __: AuthProvider) => + getRedirectResult: jest.fn((_: Auth, __: AuthProvider) => Promise.resolve({ operationType: "signIn", providerId: "google.com",