diff --git a/.env b/.env new file mode 100644 index 0000000..8083471 --- /dev/null +++ b/.env @@ -0,0 +1 @@ +HOST=127.0.0.1 diff --git a/README.md b/README.md index d114376..0b6561b 100644 --- a/README.md +++ b/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 diff --git a/src/App.test.tsx b/src/App.test.tsx index dee9b14..5d2734c 100644 --- a/src/App.test.tsx +++ b/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() + + // 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() expect(screen.getByTestId('playlistTableSpinner')).toBeInTheDocument() @@ -74,13 +180,35 @@ 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() + render() const changeUserElement = screen.getByTitle("Change user") @@ -88,8 +216,6 @@ describe("logging out", () => { await userEvent.click(changeUserElement) - expect(window.location.href).toBe("https://www.example.com/?change_user=true") - - + expect(logoutSpy).toHaveBeenCalledWith(true) }) }) diff --git a/src/App.tsx b/src/App.tsx index c03c281..70c5ba8 100644 --- a/src/App.tsx +++ b/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({(t) => t("tagline")}) + const searchParams = new URLSearchParams(window.location.search) + + const [accessToken, setAccessToken] = useState(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 =

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.

Keep an eye on the Spotify Web API Status page to see if there are any known problems right now, and then retry.

- } else if (key.has('access_token')) { - view = + } else if (accessToken) { + view = } else { view = } @@ -38,7 +59,7 @@ function App() {
- +

Exportify

diff --git a/src/auth.ts b/src/auth.ts new file mode 100644 index 0000000..039b35f --- /dev/null +++ b/src/auth.ts @@ -0,0 +1,126 @@ +// See: +// - https://developer.spotify.com/documentation/web-api/tutorials/migration-implicit-auth-code +// - https://developer.spotify.com/documentation/web-api/tutorials/code-pkce-flow + +import axios from "axios" + +const SPOTIFY_CLIENT_ID = "9950ac751e34487dbbe027c4fd7f8e99" +const SPOTIFY_AUTH_URL = "https://accounts.spotify.com/authorize" +const SPOTIFY_TOKEN_URL = "https://accounts.spotify.com/api/token" +const SPOTIFY_SCOPES = "playlist-read-private playlist-read-collaborative user-library-read" + +// Access token management +export function loadAccessToken(): string | null { + return localStorage.getItem('access_token') +} + +export function saveAccessToken(token: string): void { + localStorage.setItem('access_token', token) +} + +export function clearAccessToken(): void { + localStorage.removeItem('access_token') + localStorage.removeItem('code_verifier') +} + +// Generate code verifier for PKCE flow +function generateCodeVerifier(): string { + const possible = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789" + const randomValues = crypto.getRandomValues(new Uint8Array(64)) + return Array.from(randomValues) + .map((value) => possible[value % possible.length]) + .join("") +} + +// Generate code challenge from verifier +async function generateCodeChallenge(codeVerifier: string): Promise { + const encoder = new TextEncoder() + const data = encoder.encode(codeVerifier) + const hashed = await crypto.subtle.digest("SHA-256", data) + return base64urlencode(hashed) +} + +// Encode array buffer to base64 URL +function base64urlencode(arrayBuffer: ArrayBuffer): string { + const bytes = new Uint8Array(arrayBuffer) + let str = "" + bytes.forEach((byte) => { + str += String.fromCharCode(byte) + }) + return btoa(str).replace(/\+/g, "-").replace(/\//g, "_").replace(/=+$/, "") +} + +// Get the redirect URI for the current location +function getRedirectUri(): string { + return [window.location.protocol, '//', window.location.host, window.location.pathname].join('') +} + +// Initiate Spotify OAuth authorization flow +export async function initiateSpotifyAuth(options?: { clientId?: string; changeUser?: boolean }): Promise { + const clientId = options?.clientId || SPOTIFY_CLIENT_ID + const changeUser = options?.changeUser || false + + // Generate and store PKCE code verifier + const codeVerifier = generateCodeVerifier() + const codeChallenge = await generateCodeChallenge(codeVerifier) + localStorage.setItem("code_verifier", codeVerifier) + + // Construct authorization URL + const authUrl = new URL(SPOTIFY_AUTH_URL) + const params = { + response_type: "code", + client_id: clientId, + scope: SPOTIFY_SCOPES, + code_challenge_method: "S256", + code_challenge: codeChallenge, + redirect_uri: getRedirectUri(), + show_dialog: changeUser.toString() + } + + authUrl.search = new URLSearchParams(params).toString() + + // Redirect to Spotify authorization page + window.location.href = authUrl.toString() +} + +// Exchange authorization code for access token +export async function exchangeCodeForToken(code: string): Promise { + const redirectUri = getRedirectUri() + const codeVerifier = localStorage.getItem('code_verifier') + + if (!codeVerifier) { + throw new Error('Code verifier not found in localStorage') + } + + const response = await axios.post( + SPOTIFY_TOKEN_URL, + new URLSearchParams({ + client_id: SPOTIFY_CLIENT_ID, + grant_type: "authorization_code", + code: code, + redirect_uri: redirectUri, + code_verifier: codeVerifier, + }), + { + headers: { + "Content-Type": "application/x-www-form-urlencoded", + }, + } + ) + + const accessToken = response.data.access_token + + // Save token and clean up code verifier + saveAccessToken(accessToken) + + return accessToken +} + +// Logout and optionally redirect to change user +export function logout(changeUser: boolean = false): void { + clearAccessToken() + const url = changeUser + ? `${window.location.pathname}?change_user=true` + : window.location.pathname + window.location.href = url +} diff --git a/src/components/Login.tsx b/src/components/Login.tsx index eac9c1e..d2b15bb 100644 --- a/src/components/Login.tsx +++ b/src/components/Login.tsx @@ -2,24 +2,15 @@ import React from "react" import { withTranslation, WithTranslation } from "react-i18next" import { Button } from "react-bootstrap" import { FontAwesomeIcon } from "@fortawesome/react-fontawesome" -import { getQueryParam } from "helpers" +import { initiateSpotifyAuth } from "auth" class Login extends React.Component { - authorize() { - let clientId = getQueryParam("app_client_id") - let changeUser = getQueryParam("change_user") !== "" + authorize = async () => { + const searchParams = new URLSearchParams(window.location.search) + const clientId = searchParams.get("app_client_id") || undefined + const changeUser = searchParams.has("change_user") - // Use Exportify application clientId if none given - if (clientId === '') { - clientId = "9950ac751e34487dbbe027c4fd7f8e99" - } - - window.location.href = "https://accounts.spotify.com/authorize" + - "?client_id=" + clientId + - "&redirect_uri=" + encodeURIComponent([window.location.protocol, '//', window.location.host, window.location.pathname].join('')) + - "&scope=playlist-read-private%20playlist-read-collaborative%20user-library-read" + - "&response_type=token" + - "&show_dialog=" + changeUser; + await initiateSpotifyAuth({ clientId, changeUser }) } render() { diff --git a/src/components/PlaylistTable.test.tsx b/src/components/PlaylistTable.test.tsx index a5363f0..5d0ff62 100644 --- a/src/components/PlaylistTable.test.tsx +++ b/src/components/PlaylistTable.test.tsx @@ -71,12 +71,12 @@ test("playlist loading", async () => { test("redirecting when access token is invalid", async () => { // @ts-ignore - window.location = { href: "http://www.example.com/exportify#access_token=INVALID_ACCESS_TOKEN" } + window.location = { pathname: "/exportify", href: "http://www.example.com/exportify" } render() await waitFor(() => { - expect(window.location.href).toBe("http://www.example.com/exportify") + expect(window.location.href).toBe("/exportify") }) }) diff --git a/src/components/TopMenu.tsx b/src/components/TopMenu.tsx index 84fd3ac..c8cdd59 100644 --- a/src/components/TopMenu.tsx +++ b/src/components/TopMenu.tsx @@ -2,6 +2,7 @@ import React from "react" import { withTranslation, WithTranslation, Trans } from "react-i18next" import { Button, Modal, Table, Dropdown, Form } from "react-bootstrap" import { FontAwesomeIcon } from "@fortawesome/react-fontawesome" +import { logout } from "auth" interface TopMenuProps extends WithTranslation { loggedIn: boolean @@ -17,7 +18,7 @@ class TopMenu extends React.Component { } handleLogoutClick = () => { - window.location.href = `${window.location.href.split('#')[0]}?change_user=true` + logout(true) } handleDarkModeClick = () => { diff --git a/src/helpers.ts b/src/helpers.ts index 420894b..babe1f6 100644 --- a/src/helpers.ts +++ b/src/helpers.ts @@ -1,14 +1,7 @@ import Bugsnag from "@bugsnag/js" import axios from "axios" import Bottleneck from "bottleneck" - -// http://stackoverflow.com/a/901144/4167042 -export function getQueryParam(name: string) { - name = name.replace(/[[]/, "\\[").replace(/[\]]/, "\\]"); - var regex = new RegExp("[\\?&]" + name + "=([^&#]*)"), - results = regex.exec(window.location.search); - return results === null ? "" : decodeURIComponent(results[1].replace(/\+/g, " ")); -} +import { clearAccessToken } from "./auth" const REQUEST_RETRY_BUFFER = 1000 const MAX_RATE_LIMIT_RETRIES = 2 // 3 attempts in total @@ -50,12 +43,13 @@ export const apiCall = limiter.wrap(function(url: string, accessToken: string) { export function apiCallErrorHandler(error: any) { if (error.isAxiosError) { if (error.request.status === 401) { - // Return to home page after auth token expiry - window.location.href = window.location.href.split('#')[0] + // Clear token and return to home page after auth token expiry + clearAccessToken() + window.location.href = window.location.pathname return } else if (error.request.status >= 500 && error.request.status < 600) { // Show error page when we get a 5XX that fails retries - window.location.href = `${window.location.href.split('#')[0]}?spotify_error=true` + window.location.href = `${window.location.pathname}?spotify_error=true` return } } diff --git a/src/setupTests.ts b/src/setupTests.ts index 8f2609b..e164433 100644 --- a/src/setupTests.ts +++ b/src/setupTests.ts @@ -3,3 +3,28 @@ // expect(element).toHaveTextContent(/react/i) // learn more: https://github.com/testing-library/jest-dom import '@testing-library/jest-dom'; +import { TextEncoder, TextDecoder } from 'util'; + +// Mock TextEncoder/TextDecoder for tests +global.TextEncoder = TextEncoder; +// @ts-ignore +global.TextDecoder = TextDecoder; + +// Mock Web Crypto API for tests +Object.defineProperty(global, 'crypto', { + value: { + getRandomValues: (arr: Uint8Array) => { + // Fill with deterministic values for testing + for (let i = 0; i < arr.length; i++) { + arr[i] = i % 256; + } + return arr; + }, + subtle: { + digest: async (algorithm: string, data: ArrayBuffer) => { + // Return a mock hash + return new ArrayBuffer(32); + } + } + } +});