Skip to content

Commit 7d458aa

Browse files
theredmooseclaude
andauthored
Fix: add userId filter to getGearItemsByOwner to prevent cross-user data access (#45)
getGearItemsByOwner() was querying only by ownerId, which could allow a user to fetch another user's gear if they knew the ownerId. Added a compound query filtering by both userId and ownerId. Also added ski base condition analyzer feature to TODO.md Future Enhancements. Co-authored-by: theredmoose <theredmoose@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 39bebb9 commit 7d458aa

4 files changed

Lines changed: 24 additions & 3 deletions

File tree

TODO.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
- [x] Real gear photo analysis via Claude Vision API (with mock fallback)
2020
- [x] Fischer and Evosports Nordic sizing models with FA Value
2121
- [x] Range vs single size display toggle (per-session + default in Settings)
22+
- [x] Square UI (all rounded corners removed globally)
2223

2324
## Next Up
2425
- [x] Persist skill levels per member in database
@@ -37,11 +38,20 @@
3738
- [ ] Age-specific sizing adjustments for children
3839
- [ ] Growth projections for kids (next season sizing)
3940
- [ ] Waist width recommendations for alpine skis
41+
- [ ] Binding safety check — warn when DIN setting is outside safe range for user's weight/skill
42+
- [ ] Multi-brand sizing comparison view when adding gear
43+
- [ ] Equipment lifespan tracking based on condition progression
44+
- [ ] Stance width measurement guide (visual diagram)
45+
- [ ] Coach/instructor notes on family member profiles
46+
- [ ] Ski base condition analyzer — take a photo of the bottom of a ski and use Claude Vision to determine if it needs waxing, base grinding, edge sharpening, or base repair (ptex fill)
4047

4148
## Technical Debt
4249
- [x] Code-split Firebase to reduce bundle size — chunks exceed 500KB after minification (currently 680KB), use dynamic imports
4350
- [ ] Add E2E tests with Playwright
4451
- [x] Increase test coverage to 80%+ for branches/functions — 88% stmts, 82% branches, 86% functions, 90% lines (516 tests)
52+
- [ ] Increase PhotoCapture test coverage (currently 38.2% — lowest in codebase)
53+
- [ ] Extract GEAR_TYPE_LABELS and SPORT_LABELS to a shared constants file (currently duplicated across GearCard, MemberDetail, SettingsScreen, etc.)
54+
- [ ] Add useMemo to MemberDetail sizingCards calculation (recalculates on every render)
4555

4656
## Known Issues
4757

@@ -50,6 +60,11 @@
5060
- [x] photos/extendedDetails fields lost on Firestore read — was already fixed in `docToGearItem()`
5161
- [x] No user-scoped Firebase queries — was already fixed in `getAllFamilyMembers()` and `getAllGearItems()`
5262
- [x] status/location/checkedOutTo/checkedOutDate lost on Firestore read — fixed in `docToGearItem()` (PR #39)
63+
- [ ] `getGearItemsByOwner()` not filtered by userId — queries by ownerId only, not `userId + ownerId`; a user could fetch another user's gear if they know the ownerId (`src/services/firebase.ts`)
64+
65+
### High
66+
- [ ] Boot unit preference (MP/EU/US) resets on every visit — `bootUnit` state in MemberDetail is local and not persisted in AppSettings
67+
- [ ] Hockey skate fallback size formula wrong — `MemberInfoTable.tsx:55` uses `footLength * 1.5 + 2 - 32` fallback instead of `getShoeSizesFromFootLength()` from shoeSize service
5368

5469
### Medium
5570
- [x] Negative/zero size values after conversion — MemberDetail now guards against footLength <= 0
@@ -58,6 +73,9 @@
5873
- [x] Missing Firebase env var validation — `src/config/firebase.ts` now throws with clear error listing missing vars
5974
- [ ] No account linking flow — users get stuck signing in with Google after creating email account with same address
6075
- [ ] No offline error handling — Firebase operations show raw errors when offline
76+
- [ ] Year field in GearForm accepts invalid values (0, negative, far future) — needs validation to 1980–current year + 1
77+
- [ ] GearForm numeric fields (tip/waist/tail profile) use parseFloat/parseInt without isNaN guard — NaN silently stored on submit
78+
- [ ] Foot measurement bounds not validated — MemberForm accepts values outside reasonable range (12–30 cm)
6179

6280
### Low
6381
- [x] `parseFloat(value) || undefined` treats 0 as undefined — optional numeric fields now use `e.target.value === '' ? undefined : parseFloat()`
@@ -68,6 +86,8 @@
6886
- [x] No loading state for gear operations — gear delete/submit in App.tsx show no loading indicators; mutations now catch errors and surface a dismissible toast
6987
- [x] Race condition in useAuth — `setLoading(false)` and `setError()` now guarded by mounted ref
7088
- [ ] No email verification on signup — email accounts created without verifying address
89+
- [ ] Hockey skate width thresholds (0.36, 0.40) hardcoded without source documentation (`sizing.ts:653`)
90+
- [ ] Firestore timestamp conversion drops nanoseconds (`firebase.ts:32`) — minor precision loss
7191

7292
## Notes
7393
- App URL: https://gearguru-b3bc8.web.app

src/hooks/useFirebase.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ export function useGearItems(userId: string | null, ownerId?: string): UseGearIt
245245
setState((prev) => ({ ...prev, loading: true, error: null }));
246246
try {
247247
const items = ownerId
248-
? await firebaseService.getGearItemsByOwner(ownerId)
248+
? await firebaseService.getGearItemsByOwner(userId, ownerId)
249249
: await firebaseService.getAllGearItems(userId);
250250
setState({ data: items, loading: false, error: null });
251251
} catch (err) {

src/services/__tests__/firebase.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,7 @@ describe('firebase service', () => {
386386

387387
describe('getGearItemsByOwner', () => {
388388
it('returns empty array when no gear for owner', async () => {
389-
const result = await getGearItemsByOwner('member-1');
389+
const result = await getGearItemsByOwner('user-1', 'member-1');
390390
expect(result).toEqual([]);
391391
});
392392
});

src/services/firebase.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,11 +212,12 @@ export async function getAllGearItems(userId: string): Promise<GearItem[]> {
212212
.sort((a, b) => new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime());
213213
}
214214

215-
export async function getGearItemsByOwner(ownerId: string): Promise<GearItem[]> {
215+
export async function getGearItemsByOwner(userId: string, ownerId: string): Promise<GearItem[]> {
216216
const { collection, query, where, getDocs } = await import('firebase/firestore');
217217
const db = await getDb();
218218
const q = query(
219219
collection(db, COLLECTIONS.GEAR_ITEMS),
220+
where('userId', '==', userId),
220221
where('ownerId', '==', ownerId)
221222
);
222223
const snapshot = await getDocs(q);

0 commit comments

Comments
 (0)