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
37 changes: 37 additions & 0 deletions docs/officers/EVENTS_SHOP_MEMBERS.md
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,43 @@ Officer review steps after the source merge:

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

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

**Status: NOT AVAILABLE YET**

**Purpose:** give a signed-in member one safe next step when My Account cannot confirm a Strava disconnect, without showing a provider or technical error or guessing whether the disconnect completed.

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

**Prerequisites:** issue [#252](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/252) must be merged for source review. Calling the sentence 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, revoke access, use production data, or prove live behavior.

Officer review steps after the source merge:

1. Keep the disconnect-failure sentence marked **NOT AVAILABLE YET**.
2. Ask the platform owner for the exact #252 issue, pull request, merged commit, and synthetic frontend test result.
3. Confirm the tests use only a made-up connection, made-up activity, a mocked confirmation, and a mocked disconnect result.
4. Confirm cancelling the browser question sends no disconnect request and shows no failure.
5. Confirm a made-up rejected request shows `We could not confirm the Strava disconnect. Please refresh this page before trying again.`
6. Confirm the connected athlete and activity remain visible because the actual result is not known.
7. Confirm the Disconnect button becomes available again, but the instructions say to refresh before another attempt.
8. Confirm a later activity-load failure cannot replace the disconnect refresh instruction.
9. Confirm no made-up provider detail appears on the page or in browser console output.
10. Confirm a hostile rejected value is not inspected.
11. Confirm a successful made-up result still changes the page to `Connect Strava` and clears the old activity view.
12. Record website publication, `runmprc.com`, Firebase, Strava, production-data, and live-behavior evidence as separate results.

**Expected result:** the reviewed source uses one fixed refresh-before-retry sentence for a rejected disconnect request. It does not inspect, display, or log the rejected value, and a later activity-load failure cannot replace that higher-priority instruction. It keeps the current connected view because the result is unknown, ends the busy state, and preserves the existing successful disconnect transition. This source slice does not prove that Strava access was revoked or that a retry is safe.

**Stop conditions:** any real member or Strava account; a request for a token, provider error, private account detail, or screenshot containing private values; a real disconnect or provider call; a production Firebase or Strava change; a raw detail on the page or in the console; an immediate retry without first refreshing; or a claim that source, tests, merge, or a green workflow proves the sentence or disconnect behavior is live.

**Success proof:** for source completion, record the exact #252 issue, reviewed pull request, merged commit, intended old-source failures, green synthetic tests, relevant full checks, and independent privacy review. For live availability, separately record the approved website publication, published revision, and a dated `runmprc.com` check using no real account. Record Firebase deployment, Strava/provider configuration, revoke actions, 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 disconnecting an account, changing a member record, editing Firebase, or changing a Strava setting.

**Escalation:** membership lead plus platform/security owner. Add the privacy owner and use the private incident path if any provider or technical detail appeared. If a disconnect result is unclear after refresh, stop and escalate; do not repeat the request. Do not copy private 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
137 changes: 137 additions & 0 deletions src/pages/account/Account.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
} from '../../services/account/accountService';
import {
getStravaConnection,
stravaDisconnect,
stravaExchangeCode,
stravaFetchStats,
verifyStravaState,
Expand All @@ -39,6 +40,7 @@ jest.mock('../../services/strava/stravaService', () => {
return {
...actual,
getStravaConnection: jest.fn(),
stravaDisconnect: jest.fn(),
stravaExchangeCode: jest.fn(),
stravaFetchStats: jest.fn(),
verifyStravaState: jest.fn(),
Expand Down Expand Up @@ -630,6 +632,141 @@ describe('Strava activity browser failure boundary', () => {
});
});

const STRAVA_DISCONNECT_FAILURE = 'We could not confirm the Strava disconnect. Please refresh this page before trying again.';

describe('Strava disconnect browser failure boundary', () => {
beforeEach(() => {
jest.clearAllMocks();
(useServiceLocator as jest.Mock).mockReturnValue({
services: { firebaseResources: { app, firestore } },
isReady: true,
});
(getStravaConnection as jest.Mock).mockResolvedValue(STRAVA_CONNECTION);
(stravaFetchStats as jest.Mock).mockResolvedValue(STRAVA_STATS);
(stravaDisconnect as jest.Mock).mockResolvedValue({ ok: true });
});

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

test('replaces rejected disconnect details with one fixed refresh-before-retry alert', async () => {
const consoleSpies = ['debug', 'error', 'info', 'log', 'warn']
.map((method) => jest.spyOn(console, method as any).mockImplementation(() => undefined));
let rejectDisconnect: ((reason?: unknown) => void) | undefined;
jest.spyOn(window, 'confirm').mockReturnValue(true);
(stravaDisconnect as jest.Mock).mockReturnValueOnce(new Promise((_resolve, reject) => {
rejectDisconnect = reject;
}));
const privateFailure = Object.assign(
new Error('disconnect-private-canary member@example.test'),
{
code: 'functions/disconnect-private-canary',
endpoint: 'https://provider.example.test/?token=secret-canary',
},
);

renderActualStravaSection();
expect(await screen.findByText('Synthetic Morning Run')).toBeInTheDocument();
fireEvent.click(screen.getByRole('button', { name: 'Disconnect' }));

const pendingButton = screen.getByRole('button', { name: 'Disconnecting...' });
expect(pendingButton).toBeDisabled();
fireEvent.click(pendingButton);
expect(stravaDisconnect).toHaveBeenCalledTimes(1);
await act(async () => { rejectDisconnect?.(privateFailure); });

const alert = await screen.findByRole('alert');
expect(alert.textContent).toBe(STRAVA_DISCONNECT_FAILURE);
expect(alert).toHaveAttribute('aria-live', 'assertive');
expect(alert).toHaveAttribute('aria-atomic', 'true');
expect(document.body).toHaveTextContent('Synthetic Athlete');
expect(document.body).toHaveTextContent('Synthetic Morning Run');
expect(document.body).not.toHaveTextContent(
/disconnect-private-canary|member@example\.test|provider\.example|secret-canary/i,
);
expect(screen.getByRole('button', { name: 'Disconnect' })).toBeEnabled();
expect(stravaDisconnect).toHaveBeenCalledWith(app);
expect(stravaDisconnect).toHaveBeenCalledTimes(1);
consoleSpies.forEach((spy) => expect(spy).not.toHaveBeenCalled());
});

test('does not inspect a hostile disconnect rejection', async () => {
const messageGetter = jest.fn(() => {
throw new Error('disconnect-message-getter-canary');
});
jest.spyOn(window, 'confirm').mockReturnValue(true);
(stravaDisconnect as jest.Mock).mockRejectedValueOnce(
Object.defineProperty({}, 'message', {
configurable: true,
get: messageGetter,
}),
);

renderActualStravaSection();
expect(await screen.findByText('Synthetic Morning Run')).toBeInTheDocument();
fireEvent.click(screen.getByRole('button', { name: 'Disconnect' }));

expect((await screen.findByRole('alert')).textContent).toBe(STRAVA_DISCONNECT_FAILURE);
expect(messageGetter).not.toHaveBeenCalled();
expect(document.body).not.toHaveTextContent('disconnect-message-getter-canary');
expect(screen.getByRole('button', { name: 'Disconnect' })).toBeEnabled();
});

test('keeps the disconnect warning when a pending stats request later fails', async () => {
let rejectStats: ((reason?: unknown) => void) | undefined;
jest.spyOn(window, 'confirm').mockReturnValue(true);
(stravaFetchStats as jest.Mock).mockReturnValueOnce(new Promise((_resolve, reject) => {
rejectStats = reject;
}));
(stravaDisconnect as jest.Mock).mockRejectedValueOnce(
new Error('disconnect-priority-canary'),
);

renderActualStravaSection();
expect(await screen.findByText('Loading recent activity...')).toBeInTheDocument();
fireEvent.click(screen.getByRole('button', { name: 'Disconnect' }));

expect((await screen.findByRole('alert')).textContent).toBe(STRAVA_DISCONNECT_FAILURE);
await act(async () => { rejectStats?.(new Error('stats-priority-canary')); });

expect(screen.getByRole('alert').textContent).toBe(STRAVA_DISCONNECT_FAILURE);
expect(document.body).not.toHaveTextContent(
/disconnect-priority-canary|stats-priority-canary/i,
);
expect(document.body).toHaveTextContent('Synthetic Athlete');
expect(screen.getByRole('button', { name: 'Disconnect' })).toBeEnabled();
expect(stravaDisconnect).toHaveBeenCalledTimes(1);
});

test('does not request a disconnect when confirmation is cancelled', async () => {
jest.spyOn(window, 'confirm').mockReturnValue(false);

renderActualStravaSection();
expect(await screen.findByText('Synthetic Morning Run')).toBeInTheDocument();
fireEvent.click(screen.getByRole('button', { name: 'Disconnect' }));

expect(stravaDisconnect).not.toHaveBeenCalled();
expect(screen.queryByRole('alert')).not.toBeInTheDocument();
expect(screen.getByRole('button', { name: 'Disconnect' })).toBeEnabled();
});

test('preserves the successful disconnect transition', async () => {
jest.spyOn(window, 'confirm').mockReturnValue(true);

renderActualStravaSection();
expect(await screen.findByText('Synthetic Morning Run')).toBeInTheDocument();
fireEvent.click(screen.getByRole('button', { name: 'Disconnect' }));

expect(await screen.findByRole('button', { name: 'Connect Strava' })).toBeInTheDocument();
expect(screen.queryByText('Synthetic Athlete')).not.toBeInTheDocument();
expect(screen.queryByText('Synthetic Morning Run')).not.toBeInTheDocument();
expect(screen.queryByRole('alert')).not.toBeInTheDocument();
expect(stravaDisconnect).toHaveBeenCalledWith(app);
expect(stravaDisconnect).toHaveBeenCalledTimes(1);
});
});

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

function CallbackAccountDestination() {
Expand Down
8 changes: 4 additions & 4 deletions src/pages/account/StravaSection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ function StravaSection({ uid }: { uid: string }) {
const [loadingConn, setLoadingConn] = useState(true);
const [loadingStats, setLoadingStats] = useState(false);
const [error, setError] = useState<string | null>(null);
const [disconnectError, setDisconnectError] = useState<string | null>(null);
const [disconnecting, setDisconnecting] = useState(false);

useEffect(() => {
if (!services) return;
getStravaConnection(services.firebaseResources.firestore, uid)
Expand Down Expand Up @@ -166,8 +166,8 @@ function StravaSection({ uid }: { uid: string }) {
await stravaDisconnect(services.firebaseResources.app);
setConn(null);
setStats(null);
} catch (err: any) {
setError(err?.message || 'Failed to disconnect');
} catch {
setDisconnectError('We could not confirm the Strava disconnect. Please refresh this page before trying again.');
} finally {
setDisconnecting(false);
}
Expand All @@ -183,7 +183,7 @@ function StravaSection({ uid }: { uid: string }) {
conn={conn}
stats={stats}
loading={loadingStats}
error={error}
error={disconnectError || error}
onDisconnect={handleDisconnect}
disconnecting={disconnecting}
/>
Expand Down