Migrate from Implicit Grant Flow to Authorization Code with PKCE

.env

@@ -0,0 +1 @@
+HOST=127.0.0.1

README.md

@@ -122,7 +122,7 @@ In the project directory, first run `yarn install` to set up dependencies, then
 **`yarn start`**
 
 Runs the app in the development mode.\
-Open [http://localhost:3000](http://localhost:3000) to view it in the browser.
+Open [http://127.0.0.1:3000](http://127.0.0.1:3000) to view it in the browser.
 
 The page will reload if you make edits.\
 You will also see any lint errors in the console.
@@ -184,7 +184,7 @@ To build and run Exportify with docker, run:
 
 **`docker run -p 3000:3000 exportify`**
 
-And then open [http://localhost:3000](http://localhost:3000) to view it in the browser.
+And then open [http://127.0.0.1:3000](http://127.0.0.1:3000) to view it in the browser.
 
 ## Contributing
 

src/App.test.tsx

@@ -1,17 +1,26 @@
 import React from "react"
 import i18n from "i18n/config"
-import { render, screen } from "@testing-library/react"
+import { render, screen, waitFor } from "@testing-library/react"
 import userEvent from "@testing-library/user-event"
+import { setupServer } from "msw/node"
+import { rest } from "msw"
+import { handlers } from "./mocks/handlers"
 import App from "./App"
+import * as auth from "./auth"
 
 const { location } = window
 
+// Set up MSW server for mocking Spotify API and token exchange
+const server = setupServer(...handlers)
+
 beforeAll(() => {
+  server.listen({ onUnhandledRequest: 'warn' })
   // @ts-ignore
   delete window.location
 })
 
 afterAll(() => {
+  server.close()
   window.location = location
 })
 
@@ -22,6 +31,11 @@ beforeAll(() => {
 
 beforeEach(() => {
   i18n.changeLanguage("en")
+  server.resetHandlers()
+})
+
+afterEach(() => {
+  localStorage.clear()
 })
 
 describe("i18n", () => {
@@ -54,18 +68,110 @@ describe("logging in", () => {
 
     await userEvent.click(linkElement)
 
-    expect(window.location.href).toBe(
-      "https://accounts.spotify.com/authorize?client_id=9950ac751e34487dbbe027c4fd7f8e99&redirect_uri=%2F%2F&scope=playlist-read-private%20playlist-read-collaborative%20user-library-read&response_type=token&show_dialog=false"
-    )
+    // Now uses PKCE flow with authorization code
+    expect(window.location.href).toMatch(/^https:\/\/accounts\.spotify\.com\/authorize\?/)
+    expect(window.location.href).toContain('response_type=code')
+    expect(window.location.href).toContain('client_id=9950ac751e34487dbbe027c4fd7f8e99')
+    expect(window.location.href).toContain('scope=playlist-read-private')
+    expect(window.location.href).toContain('code_challenge_method=S256')
+    expect(window.location.href).toContain('code_challenge=')
+    expect(window.location.href).toContain('show_dialog=false')
   })
 
-  describe("post-login state", () => {
-    beforeAll(() => {
+  describe("OAuth callback with authorization code", () => {
+    let setItemSpy: jest.SpyInstance
+    let replaceStateSpy: jest.SpyInstance
+    let tokenRequestBody: URLSearchParams | null = null
+
+    beforeEach(() => {
+      tokenRequestBody = null
+
+      // Mock localStorage
+      jest.spyOn(Storage.prototype, 'getItem').mockImplementation((key) => {
+        if (key === 'code_verifier') return 'TEST_CODE_VERIFIER'
+        return null
+      })
+      setItemSpy = jest.spyOn(Storage.prototype, 'setItem').mockImplementation(() => {})
+      replaceStateSpy = jest.spyOn(window.history, 'replaceState').mockImplementation(() => {})
+
+      // Mock window.location with code parameter
       // @ts-ignore
-      window.location = { hash: "#access_token=TEST_ACCESS_TOKEN" }
+      window.location = {
+        search: "?code=TEST_AUTH_CODE",
+        pathname: "/exportify",
+        protocol: "https:",
+        host: "exportify.app"
+      }
+
+      // Mock Spotify token endpoint and capture request
+      server.use(
+        rest.post('https://accounts.spotify.com/api/token', async (req, res, ctx) => {
+          // Capture the request body for verification
+          tokenRequestBody = new URLSearchParams(await req.text())
+
+          return res(ctx.json({
+            access_token: 'EXCHANGED_ACCESS_TOKEN',
+            token_type: 'Bearer',
+            expires_in: 3600,
+            refresh_token: 'REFRESH_TOKEN',
+            scope: 'playlist-read-private playlist-read-collaborative user-library-read'
+          }))
+        })
+      )
+    })
+
+    afterEach(() => {
+      jest.restoreAllMocks()
     })
 
-    test("renders playlist component on return from Spotify with auth token", () => {
+    test("exchanges authorization code for access token with correct PKCE parameters", async () => {
+      render(<App />)
+
+      // Wait for token exchange to complete
+      await waitFor(() => {
+        expect(setItemSpy).toHaveBeenCalledWith('access_token', 'EXCHANGED_ACCESS_TOKEN')
+      })
+
+      // Verify the token endpoint was called with correct parameters
+      expect(tokenRequestBody).not.toBeNull()
+      expect(tokenRequestBody?.get('client_id')).toBe('9950ac751e34487dbbe027c4fd7f8e99')
+      expect(tokenRequestBody?.get('grant_type')).toBe('authorization_code')
+      expect(tokenRequestBody?.get('code')).toBe('TEST_AUTH_CODE')
+      expect(tokenRequestBody?.get('code_verifier')).toBe('TEST_CODE_VERIFIER')
+      expect(tokenRequestBody?.get('redirect_uri')).toBe('https://exportify.app/exportify')
+
+      // Verify code is removed from URL
+      expect(replaceStateSpy).toHaveBeenCalledWith({}, expect.any(String), '/exportify')
+
+      // Verify playlist component is rendered
+      expect(screen.getByTestId('playlistTableSpinner')).toBeInTheDocument()
+    })
+  })
+
+  describe("post-login state", () => {
+    let getItemSpy: jest.SpyInstance
+
+    beforeEach(() => {
+      // Mock localStorage for access token
+      getItemSpy = jest.spyOn(Storage.prototype, 'getItem').mockImplementation((key) => {
+        if (key === 'access_token') return 'TEST_ACCESS_TOKEN'
+        return null
+      })
+
+      // @ts-ignore - No code parameter in URL
+      window.location = {
+        pathname: "/exportify",
+        href: "https://www.example.com/exportify",
+        search: "",
+        hash: ""
+      }
+    })
+
+    afterEach(() => {
+      getItemSpy.mockRestore()
+    })
+
+    test("renders playlist component when access token exists in localStorage", () => {
       render(<App />)
 
       expect(screen.getByTestId('playlistTableSpinner')).toBeInTheDocument()
@@ -74,22 +180,42 @@ describe("logging in", () => {
 })
 
 describe("logging out", () => {
-  beforeAll(() => {
-    // @ts-ignore
-    window.location = { hash: "#access_token=TEST_ACCESS_TOKEN", href: "https://www.example.com/#access_token=TEST_ACCESS_TOKEN" }
+  let getItemSpy: jest.SpyInstance
+  let logoutSpy: jest.SpyInstance
+
+  beforeEach(() => {
+    // Mock localStorage with access token
+    getItemSpy = jest.spyOn(Storage.prototype, 'getItem').mockImplementation((key) => {
+      if (key === 'access_token') return 'TEST_ACCESS_TOKEN'
+      return null
+    })
+
+    // Spy on logout function
+    logoutSpy = jest.spyOn(auth, 'logout').mockImplementation(() => {})
+
+    // @ts-ignore - Simple window.location mock
+    window.location = {
+      pathname: "/exportify",
+      href: "https://www.example.com/exportify",
+      search: "",
+      hash: ""
+    }
+  })
+
+  afterEach(() => {
+    getItemSpy.mockRestore()
+    logoutSpy.mockRestore()
   })
 
   test("redirects user to login screen which will force a permission request", async () => {
-    const { rerender } = render(<App />)
+    render(<App />)
 
     const changeUserElement = screen.getByTitle("Change user")
 
     expect(changeUserElement).toBeInTheDocument()
 
     await userEvent.click(changeUserElement)
 
-    expect(window.location.href).toBe("https://www.example.com/?change_user=true")
-
-
+    expect(logoutSpy).toHaveBeenCalledWith(true)
   })
 })

src/App.tsx

@@ -1,35 +1,56 @@
 import './App.scss'
 import "./icons"
 
-import React, { useState } from 'react'
+import React, { useEffect, useState, useRef } from 'react'
 import { useTranslation, Translation } from "react-i18next"
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'
 import "url-search-params-polyfill"
 
 import Login from 'components/Login'
 import PlaylistTable from "components/PlaylistTable"
-import { getQueryParam } from "helpers"
 import TopMenu from "components/TopMenu"
+import { loadAccessToken, exchangeCodeForToken } from "auth"
 
 function App() {
   useTranslation()
   const [subtitle, setSubtitle] = useState(<Translation>{(t) => t("tagline")}</Translation>)
+  const searchParams = new URLSearchParams(window.location.search)
+
+  const [accessToken, setAccessToken] = useState<string | null>(loadAccessToken())
+  const hasProcessedCode = useRef(false)
 
   let view
-  let key = new URLSearchParams(window.location.hash.substring(1))
 
   const onSetSubtitle = (subtitle: any) => {
     setSubtitle(subtitle)
   }
 
-  if (getQueryParam('spotify_error') !== '') {
+  useEffect(() => {
+    const code = searchParams.get("code")
+    if (!code) { return }
+
+    // Prevent multiple executions in StrictMode or re-renders
+    if (hasProcessedCode.current) {
+      return
+    }
+    hasProcessedCode.current = true
+
+    exchangeCodeForToken(code).then((accessToken) => {
+      setAccessToken(accessToken)
+
+      // Remove code from query string
+      window.history.replaceState({}, document.title, window.location.pathname)
+    })
+  })
+
+  if (searchParams.get('spotify_error')) {
     view = <div id="spotifyErrorMessage" className="lead">
       <p><FontAwesomeIcon icon={['fas', 'bolt']} style={{ fontSize: "50px", marginBottom: "20px" }} /></p>
       <p>Oops, Exportify has encountered an unexpected error (5XX) while using the Spotify API. This kind of error is due to a problem on Spotify's side, and although it's rare, unfortunately all we can do is retry later.</p>
       <p style={{ marginTop: "50px" }}>Keep an eye on the <a target="_blank" rel="noreferrer" href="https://status.spotify.dev/">Spotify Web API Status page</a> to see if there are any known problems right now, and then <a rel="noreferrer" href="?">retry</a>.</p>
     </div>
-  } else if (key.has('access_token')) {
-    view = <PlaylistTable accessToken={key.get('access_token')!} onSetSubtitle={onSetSubtitle} />
+  } else if (accessToken) {
+    view = <PlaylistTable accessToken={accessToken!} onSetSubtitle={onSetSubtitle} />
   } else {
     view = <Login />
   }
@@ -38,7 +59,7 @@ function App() {
     <div className="App container">
       <header className="App-header">
         <div className="d-sm-none d-block mb-5" />
-        <TopMenu loggedIn={key.has('access_token')} />
+        <TopMenu loggedIn={!!accessToken} />
         <h1>
           <FontAwesomeIcon icon={['fab', 'spotify']} color="#84BD00" size="sm" /> <a href={process.env.PUBLIC_URL}>Exportify</a>
         </h1>