Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions docs/officers/EVENTS_SHOP_MEMBERS.md
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,42 @@ Officer review steps after the source merge:

**Escalation:** membership lead plus treasurer and privacy/security owner. Add the platform owner for source or publication evidence. Use the private incident path if real account or membership data appears.

## Strava callback failure privacy — SOURCE ONLY, NOT LIVE

**Status: NOT AVAILABLE YET**

**Purpose:** give a member one plain next step when a Strava connection fails without showing a provider message, callback detail, or technical error on the page.

**Approver:** membership lead plus platform/security owner.

**Prerequisites:** issue [#242](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/242) must be merged for source review. Calling the wording live also requires a protected website publication and an exact revision check on `runmprc.com`. This source change does not deploy Firebase, contact Strava, change provider settings, use production data, or prove live behavior.

Officer review steps after the source merge:

1. Keep the callback wording marked **NOT AVAILABLE YET**.
2. Ask the platform owner for the exact #242 issue, pull request, merged commit, and synthetic frontend test result.
3. Confirm the tests use only made-up callback values and a mocked exchange result.
4. Confirm a signed-out visitor sees only the fixed sign-in instruction.
5. Confirm a made-up provider query failure shows `We could not connect Strava. Please return to My Account and try again.`
6. Confirm a made-up exchange failure shows the same sentence.
7. Confirm no made-up provider detail appears on the page or in browser console output.
8. Confirm missing-code and failed-security-check results still stop before an exchange.
9. Confirm only a successful exchange returns to My Account, and the visible `Back to account` link still works without an exchange.
10. Confirm the failure sentence is announced as an urgent screen-reader alert.
11. Record website publication, `runmprc.com`, Firebase, Strava, production-data, and live-behavior evidence as separate results.

**Expected result:** the reviewed source uses one fixed, actionable sentence for both a callback query failure and an exchange failure. It does not inspect, display, or log the rejected exchange value. Existing sign-in, missing-code, failed-security-check, success, and Back-to-account behavior stays in place. This does not remove the callback details from the browser address.

**Stop conditions:** any real member or Strava account; a request for a callback URL, authorization code, state value, provider error, private browser history, or screenshot containing private values; a real provider call; a production Firebase or Strava change; a raw detail in the page or console; or a claim that source, tests, merge, or a green workflow proves the wording is live.

**Success proof:** for source completion, record the exact #242 issue, reviewed pull request, merged commit, intended old-source failures, green synthetic callback tests, relevant full checks, and independent privacy review. For live availability, separately record the approved website publication, the published revision, and a dated `runmprc.com` check that uses no real account or callback value. Record Firebase deployment, Strava/provider configuration, and production-data actions as **not performed** for this frontend-only change.

**Undo:** before publication, use one reviewed frontend revert or safe roll-forward. After publication, use the same protected website release path and verify the replacement revision on `runmprc.com`. Do not undo by changing a member account, callback value, Firebase record, or Strava setting.

**Escalation:** membership lead plus platform/security owner. Add the privacy owner and use the private incident path if any callback or provider detail appeared. Do not copy the detail into an issue, message, screenshot, or AI tool.

No system diagram changes for this source slice because page structure, data movement, permissions, account ownership, and deployment topology are unchanged.

## Refund amount and returned-result guards — SOURCE ONLY, NOT LIVE

**Purpose:** make an invalid partial amount stop, and record a refund complete only when Stripe returns a matching final success.
Expand Down
210 changes: 209 additions & 1 deletion src/pages/account/Account.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,37 @@ import { join } from 'path';
import {
act, fireEvent, render, screen, waitFor,
} from '@testing-library/react';
import { MemoryRouter } from 'react-router-dom';
import {
MemoryRouter, Route, Routes, useNavigationType,
} from 'react-router-dom';
import { useServiceLocator } from '../../services/ServiceLocatorContext';
import { useAuth } from '../../services/hooks/useAuth';
import {
ensureMyProfile,
getMyProfile,
listMyRegistrations,
updateMyProfile,
} from '../../services/account/accountService';
import {
stravaExchangeCode,
verifyStravaState,
} from '../../services/strava/stravaService';
import { AccountContent } from './Account';
import StravaCallback from './StravaCallback';

jest.mock('../../services/ServiceLocatorContext', () => ({
useServiceLocator: jest.fn(),
}));

jest.mock('../../services/hooks/useAuth', () => ({
useAuth: jest.fn(),
}));

jest.mock('../../services/strava/stravaService', () => ({
stravaExchangeCode: jest.fn(),
verifyStravaState: jest.fn(),
}));

jest.mock('../../services/account/accountService', () => {
const actual = jest.requireActual('../../services/account/accountService');
return {
Expand Down Expand Up @@ -462,3 +479,194 @@ describe('Account profile recovery', () => {
expect(css).toMatch(/\.verification-resend__button:focus-visible\s*\{[\s\S]*outline:/);
});
});

const STRAVA_CALLBACK_FAILURE = 'We could not connect Strava. Please return to My Account and try again.';

function CallbackAccountDestination() {
const navigationType = useNavigationType();

return (
<div>
Account destination
<span data-testid="callback-navigation-type">{navigationType}</span>
</div>
);
}

function renderStravaCallback(
entry = '/account/strava/callback?code=synthetic-code&state=synthetic-state',
) {
return render(
<MemoryRouter
initialEntries={[entry]}
future={{
v7_relativeSplatPath: true,
v7_startTransition: true,
}}
>
<Routes>
<Route path="/account/strava/callback" element={<StravaCallback />} />
<Route path="/account" element={<CallbackAccountDestination />} />
</Routes>
</MemoryRouter>,
);
}

describe('Strava callback error boundary', () => {
beforeEach(() => {
jest.clearAllMocks();
(useServiceLocator as jest.Mock).mockReturnValue({
services: { firebaseResources: { app } },
isReady: true,
});
(useAuth as jest.Mock).mockReturnValue({
isAuthenticated: true,
isLoading: false,
});
(verifyStravaState as jest.Mock).mockReturnValue(true);
(stravaExchangeCode as jest.Mock).mockResolvedValue({
ok: true,
athleteId: null,
});
});

afterEach(() => {
jest.restoreAllMocks();
});

test.each([
['Firebase services are not ready', false, false],
['authentication is loading', true, true],
])('waits without callback work when %s', async (_case, isReady, isLoading) => {
(useServiceLocator as jest.Mock).mockReturnValue({
services: isReady ? { firebaseResources: { app } } : null,
isReady,
});
(useAuth as jest.Mock).mockReturnValue({
isAuthenticated: true,
isLoading,
});

renderStravaCallback();

expect(await screen.findByText('Connecting your Strava...')).toBeInTheDocument();
expect(verifyStravaState).not.toHaveBeenCalled();
expect(stravaExchangeCode).not.toHaveBeenCalled();
});

test('requires sign-in before exposing or acting on query failure details', async () => {
(useAuth as jest.Mock).mockReturnValue({
isAuthenticated: false,
isLoading: false,
});

renderStravaCallback(
'/account/strava/callback?error=signed-out-provider-canary&code=hidden-code&state=hidden-state',
);

expect(await screen.findByText('You need to be signed in to connect Strava.'))
.toBeInTheDocument();
expect(document.body).not.toHaveTextContent(/signed-out-provider-canary|hidden-code|hidden-state/);
expect(verifyStravaState).not.toHaveBeenCalled();
expect(stravaExchangeCode).not.toHaveBeenCalled();
});

test('replaces a provider query error with one fixed actionable alert', async () => {
renderStravaCallback(
'/account/strava/callback?error=provider%3Aprivate-query-canary&code=hidden-code&state=hidden-state',
);

const alert = await screen.findByRole('alert');
expect(alert).toHaveTextContent(STRAVA_CALLBACK_FAILURE);
expect(alert).toHaveAttribute('aria-live', 'assertive');
expect(alert).toHaveAttribute('aria-atomic', 'true');
expect(document.body).not.toHaveTextContent(/private-query-canary|hidden-code|hidden-state/);
expect(verifyStravaState).not.toHaveBeenCalled();
expect(stravaExchangeCode).not.toHaveBeenCalled();
expect(screen.getByRole('link', { name: 'Back to account' }))
.toHaveAttribute('href', '/account');
});

test('stops a missing code before state verification or exchange', async () => {
renderStravaCallback('/account/strava/callback?state=synthetic-state');

expect(await screen.findByRole('alert')).toHaveTextContent(
'Missing authorization code from Strava.',
);
expect(verifyStravaState).not.toHaveBeenCalled();
expect(stravaExchangeCode).not.toHaveBeenCalled();
});

test('stops an invalid state before exchange', async () => {
(verifyStravaState as jest.Mock).mockReturnValue(false);

renderStravaCallback();

expect(await screen.findByRole('alert')).toHaveTextContent(
'Security check failed (state mismatch). Please try connecting again.',
);
expect(verifyStravaState).toHaveBeenCalledTimes(1);
expect(verifyStravaState).toHaveBeenCalledWith('synthetic-state');
expect(stravaExchangeCode).not.toHaveBeenCalled();
});

test('verifies state before one exact exchange and replaces the account route on success', async () => {
const calls: string[] = [];
(verifyStravaState as jest.Mock).mockImplementation(() => {
calls.push('verify');
return true;
});
(stravaExchangeCode as jest.Mock).mockImplementation(async () => {
calls.push('exchange');
return { ok: true, athleteId: null };
});

renderStravaCallback();

expect(await screen.findByText('Account destination')).toBeInTheDocument();
expect(calls).toEqual(['verify', 'exchange']);
expect(verifyStravaState).toHaveBeenCalledWith('synthetic-state');
expect(stravaExchangeCode).toHaveBeenCalledWith(app, 'synthetic-code');
expect(stravaExchangeCode).toHaveBeenCalledTimes(1);
expect(screen.getByTestId('callback-navigation-type')).toHaveTextContent('REPLACE');
});

test('does not navigate while the exchange result is pending', async () => {
(stravaExchangeCode as jest.Mock).mockReturnValue(new Promise(() => {
// Keep this synthetic exchange pending until the test unmounts the page.
}));

renderStravaCallback();

await waitFor(() => expect(stravaExchangeCode).toHaveBeenCalledTimes(1));
expect(screen.getByText('Connecting your Strava...')).toBeInTheDocument();
expect(screen.queryByText('Account destination')).not.toBeInTheDocument();
});

test.each([
['ordinary error', () => new Error('ordinary-provider-canary')],
['hostile message getter', () => Object.defineProperty({}, 'message', {
configurable: true,
get() {
throw new Error('message-getter-canary');
},
})],
])('does not inspect or render an %s rejection', async (_case, makeRejection) => {
const consoleSpies = [
jest.spyOn(console, 'log').mockImplementation(() => undefined),
jest.spyOn(console, 'warn').mockImplementation(() => undefined),
jest.spyOn(console, 'error').mockImplementation(() => undefined),
];
(stravaExchangeCode as jest.Mock).mockRejectedValueOnce(makeRejection());

renderStravaCallback();

const alert = await screen.findByRole('alert');
expect(alert).toHaveTextContent(STRAVA_CALLBACK_FAILURE);
expect(document.body).not.toHaveTextContent(/ordinary-provider-canary|message-getter-canary/);
expect(screen.queryByText('Account destination')).not.toBeInTheDocument();
expect(verifyStravaState).toHaveBeenCalledTimes(1);
expect(stravaExchangeCode).toHaveBeenCalledTimes(1);
consoleSpies.forEach((spy) => expect(spy).not.toHaveBeenCalled());
});
});
17 changes: 13 additions & 4 deletions src/pages/account/StravaCallback.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import { useAuth } from '../../services/hooks/useAuth';
import SEO from '../../components/SEO';
import { stravaExchangeCode, verifyStravaState } from '../../services/strava/stravaService';

const STRAVA_CALLBACK_FAILURE = 'We could not connect Strava. Please return to My Account and try again.';

function StravaCallback() {
const [params] = useSearchParams();
const { services, isReady } = useServiceLocator();
Expand All @@ -25,7 +27,7 @@ function StravaCallback() {
}
if (err) {
setStatus('error');
setMessage(`Strava returned an error: ${err}`);
setMessage(STRAVA_CALLBACK_FAILURE);
return;
}
if (!code) {
Expand All @@ -42,9 +44,9 @@ function StravaCallback() {

stravaExchangeCode(services.firebaseResources.app, code)
.then(() => setStatus('done'))
.catch((e) => {
.catch(() => {
setStatus('error');
setMessage(e?.message || 'Failed to exchange code with Strava.');
setMessage(STRAVA_CALLBACK_FAILURE);
});
}, [services, isReady, isAuthenticated, isLoading, code, state, err]);

Expand All @@ -65,7 +67,14 @@ function StravaCallback() {
{status === 'error' && (
<>
<h1 className="text-2xl font-bold mb-2 text-red-600">Connection failed</h1>
<p className="text-gray-700">{message}</p>
<p
className="text-gray-700"
role="alert"
aria-live="assertive"
aria-atomic="true"
>
{message}
</p>
<a href="/account" className="inline-block mt-4 text-blue-600 hover:underline">
Back to account
</a>
Expand Down