Lỗi có thể di chuyển tù sharedlink sang public nhưng không có hotspot quay lại

This commit is contained in:
2026-06-11 16:27:37 +07:00
parent be149f26ca
commit 0434837026
7 changed files with 1055 additions and 47 deletions
+923
View File
@@ -0,0 +1,923 @@
# Tour/Scene Privacy Logic Analysis Report
## Executive Summary
The privacy system has **critical design flaws** that compromise security. While Tour-level privacy management is correctly implemented with proper propagation to scenes, the **Scene model is missing the `privacy` field** in its schema definition, creating a fundamental data structure issue. Additionally, there are inconsistencies between the intended API requirements and actual implementation.
### Critical Issues Found: 8
### High Priority Issues: 5
### Medium Priority Issues: 3
---
## Requirements from ARCHITEC.md
```
- Public: Everyone can see. Only creator can manage
- Private: Only creator can see and manage
- Shared link: Everyone with valid token can see. Only creator can manage
- Member: Only members can see. Only creator can manage
```
---
## Detailed Findings
## 1. GET /api/tours/:id - Tour Detail Endpoint
### Current Implementation
**File**: [backend/middlewares/TourController.js](backend/middlewares/TourController.js#L82-L125)
```javascript
router.get('/:id', optionalAuth, async (req, res) => {
const tour = await Tour.findById(req.params.id)
.populate('createdBy', 'username')
.populate({ path: 'rootSceneId', ... })
.populate({ path: 'scenes', ... })
.lean();
const isTokenValid = tour.shareToken && (!tour.shareTokenExpires || new Date() < tour.shareTokenExpires);
let hasAccess = tour.privacy === 'public' || isOwner || isAdmin ||
(tour.privacy === 'shared' && req.query.token === tour.shareToken && isTokenValid) ||
(tour.privacy === 'member' && req.user && req.user._id && (
tour.sharedWith.some(u => u.toString() === req.user._id.toString()) ||
(userEmail && tour.sharedEmails.includes(userEmail))
));
if (!hasAccess) return res.status(403).json({ message: 'Unauthorized' });
res.json(tour);
});
```
### Expected Behavior (Per ARCHITEC.md)
1.**Public**: Anyone can view
2.**Private**: Only creator
3.**Shared**: Valid token holders can view
4.**Member**: Members in `sharedWith` array can view
5. ⚠️ **Token Validation**: Check expiration
### Issues Found
#### ISSUE 1.1 - Token Validation Logic Missing Email Check
**Severity**: HIGH
In the shared privacy mode, the code validates token **but does NOT validate expiration for other access types**. It should also allow members with valid emails to access.
**Current Code**:
```javascript
(tour.privacy === 'shared' && req.query.token === tour.shareToken && isTokenValid)
```
**Problem**: For `member` privacy, it checks `tour.sharedEmails` but never validates if they're in the `member` list during token validation.
**Recommendation**: Add explicit token handling for member access patterns.
---
## 2. GET /api/tours - Tour List Endpoint
### Current Implementation
**File**: [backend/middlewares/TourController.js](backend/middlewares/TourController.js#L128-L167)
```javascript
router.get('/', optionalAuth, async (req, res) => {
let query = { privacy: 'public' };
if (req.user && req.user.role !== 'guest') {
query = {
$or: [
{ privacy: 'public' },
{ createdBy: req.user._id },
{ privacy: 'member', sharedWith: req.user._id },
{ privacy: 'member', sharedEmails: req.user.email }
]
};
}
const tours = await Tour.find(query)...
});
```
### Expected Behavior
1. **Public Tours**: Visible to everyone (guest/logged in)
2. **Private Tours**: Only visible to creator
3. **Shared Tours**: Only visible if token provided **or user has permission**
4. **Member Tours**: Only visible to members in `sharedWith` or `sharedEmails`
### Issues Found
#### ISSUE 2.1 - Shared Token Tours Not Returned in List
**Severity**: CRITICAL
The endpoint **does NOT support token-based access**. If a guest has a shared link token, they cannot see the tour in the list.
**Current Code**: No `token` parameter handling
**Expected Behavior**: Should accept `?token=xxx` and return tours matching that token
**Problem Scenario**:
1. User A creates private tour with shareToken
2. User A shares token with Guest
3. Guest tries `GET /api/tours?token=xxx` → Empty list returned
4. Guest **cannot discover the tour exists**
**Recommendation**:
```javascript
// Add token support
if (req.query.token) {
const tourWithToken = await Tour.findOne({
shareToken: req.query.token,
$or: [
{ shareTokenExpires: null },
{ shareTokenExpires: { $gt: new Date() } }
]
});
if (tourWithToken) {
query = { _id: tourWithToken._id };
}
}
```
#### ISSUE 2.2 - Private Tours Accessible to Creator Only Not Enforced
**Severity**: MEDIUM
The query allows creator to see private tours, but **doesn't explicitly exclude non-creator guests** from seeing **someone else's shared tours**.
The current logic returns:
- Public tours → OK
- Creator's own tours (all privacy levels) → OK
- Member tours where user is a member → OK
But **missing**: Explicit rejection of non-creator access to private/shared tours they're not invited to.
---
## 3. GET /api/scenes/:id - Scene Detail Endpoint
### Current Implementation
**File**: [backend/routes/sceneRoutes.js](backend/routes/sceneRoutes.js#L149-L210)
```javascript
router.get('/:id', optionalAuth, async (req, res) => {
const scene = await Scene.findById(req.params.id)
.populate('createdBy', 'username')
.populate('tourId'); // ⚠️ Critical: Must populate tour for privacy checks
const tour = scene.tourId;
const isSceneTokenValid = scene.shareToken && (!scene.shareTokenExpires || new Date() < scene.shareTokenExpires);
const isTourTokenValid = tour.shareToken && (!tour.shareTokenExpires || new Date() < tour.shareTokenExpires);
let hasAccess = tour.privacy === 'public' || isOwner || isAdmin ||
(scene.privacy === 'shared' && req.query.token === scene.shareToken && isSceneTokenValid) ||
(tour.privacy === 'shared' && req.query.token === tour.shareToken && isTourTokenValid) ||
(tour.privacy === 'member' && req.user && req.user._id && (
tour.sharedWith.some(u => u.toString() === req.user._id.toString()) ||
(userEmail && tour.sharedEmails.includes(userEmail))
));
if (!hasAccess) return res.status(403).json({ message: 'Unauthorized' });
// Increment views
if (!isOwner && !isAdmin) {
scene.views = (scene.views || 0) + 1;
await scene.save();
}
res.json(scene);
});
```
### Expected Behavior
Privacy should be inherited from Tour at minimum, with Scene-level overrides possible.
### Issues Found
#### ISSUE 3.1 - CRITICAL: Scene Model Missing Privacy Field
**Severity**: CRITICAL 🔴
**File**: [backend/models/Scene.js](backend/models/Scene.js)
**Current Schema**:
```javascript
const sceneSchema = new mongoose.Schema({
tourId: { type: ObjectId, required: true },
name: String,
shareToken: String,
shareTokenExpires: Date,
sharedWith: [ObjectId],
sharedEmails: [String],
// ❌ MISSING: privacy field
}, { timestamps: true });
```
**Problem**: The code references `scene.privacy` but it's **never defined in the schema**. MongoDB will store it but it won't be validated or properly indexed.
**Evidence**:
- Line 165: `isSceneTokenValid = scene.shareToken && ...` (assuming privacy exists)
- Line 170: `(scene.privacy === 'shared' && req.query.token === scene.shareToken ...)`
- But schema never defines `privacy` field
**Impact**:
- ❌ Scene privacy cannot be properly validated
- ❌ No default value enforcement
- ❌ No enum restrictions (could be any string)
- ❌ Causes logic errors when privacy is undefined
**Required Fix**:
```javascript
privacy: {
type: String,
enum: ['public', 'private', 'member', 'shared'],
default: 'private'
},
```
#### ISSUE 3.2 - Bridge Access Logic Allows Unintended Navigation
**Severity**: HIGH
```javascript
// [BRIDGE ACCESS LOGIC] Lines 194-205
if (!hasAccess && req.query.token) {
const potentialParents = await Hotspot.find({
target_scene_id: scene._id
}).distinct('parent_scene_id');
if (potentialParents.length > 0) {
const authorizedParentExists = await Scene.exists({
_id: { $in: potentialParents },
shareToken: req.query.token,
$or: [{ shareTokenExpires: null }, { shareTokenExpires: { $gt: new Date() } }]
});
if (authorizedParentExists) hasAccess = true;
}
}
```
**Problem**: This logic allows a guest with a shared link to ANY scene in a chain to navigate through ALL connected scenes.
**Scenario**:
1. User creates 5 linked scenes in a tour
2. Only scene 1 is shared (has token)
3. Guest with token for scene 1 can navigate to scene 2, 3, 4, 5 (if they know the hotspot coordinates)
4. **Scenes 2-5 may have different privacy settings but are still accessible**
**Recommendation**: Add privacy validation to ensure the target scene matches the token's privacy context.
---
## 4. GET /api/scenes - Scene List Endpoint
### Current Implementation
**File**: [backend/routes/sceneRoutes.js](backend/routes/sceneRoutes.js#L108-L142)
```javascript
router.get('/', optionalAuth, async (req, res) => {
const publicTours = await Tour.find({ privacy: 'public' }).select('_id');
const publicTourIds = publicTours.map(t => t._id);
let baseQuery = req.user && req.user.role !== 'guest'
? { $or: [
{ privacy: 'public' },
{ tourId: { $in: publicTourIds } },
{ createdBy: req.user._id },
{ sharedWith: req.user._id },
{ sharedEmails: req.user.email }
]}
: { $or: [
{ privacy: 'public' },
{ tourId: { $in: publicTourIds } }
]};
let finalQuery = baseQuery;
if (token) {
const tourWithToken = await Tour.findOne({ shareToken: token }).select('_id');
finalQuery = {
$or: [
baseQuery,
{ shareToken: token },
{ tourId: tourWithToken ? tourWithToken._id : null }
]
};
}
const scenes = await Scene.find(finalQuery)...
});
```
### Expected Behavior
- **Public Scenes**: Everyone sees
- **Private Scenes**: Only creator
- **Shared Scenes**: Only token holders + creator
- **Member Scenes**: Only members + creator
- **Tours**: Inherit parent tour privacy
### Issues Found
#### ISSUE 4.1 - Private Tour Scenes Should Not Be Visible
**Severity**: HIGH
**Problem**: The query returns all scenes from public tours but **doesn't check individual scene privacy**.
**Current Logic**:
```javascript
{ tourId: { $in: publicTourIds } } // All scenes from public tours returned
```
**Scenario**:
1. Tour A is public (privacy: 'public')
2. Scene 1 in Tour A created by User X (privacy: 'public')
3. Scene 2 in Tour A created by User Y (privacy: 'private')
4. Guest user calls GET /api/scenes
5. **Both scenes returned even though Scene 2 is marked private**
**Why This Matters**: Per ARCHITEC.md, even in a public tour, individual scenes can be private.
#### ISSUE 4.2 - Token Expiration Not Validated
**Severity**: MEDIUM
```javascript
if (token) {
const tourWithToken = await Tour.findOne({ shareToken: token }).select('_id');
// ❌ No check for shareTokenExpires
}
```
**Problem**: The code finds a tour with matching token but **never checks if token is expired**.
**Fix Required**:
```javascript
const tourWithToken = await Tour.findOne({
shareToken: token,
$or: [
{ shareTokenExpires: null },
{ shareTokenExpires: { $gt: new Date() } }
]
});
```
#### ISSUE 4.3 - Null Privacy Values Break Logic
**Severity**: MEDIUM
Scenes created before the privacy field was added will have `privacy: undefined`. The query checks:
```javascript
{ privacy: 'public' } // Won't match undefined!
```
**Scenario**:
1. Old scene exists with no privacy field
2. Scene is in public tour
3. Guest queries GET /api/scenes
4. Old scenes NOT returned (should be visible due to public tour)
---
## 5. PUT /api/tours/:id - Update Tour
### Current Implementation
**File**: [backend/middlewares/TourController.js](backend/middlewares/TourController.js#L46-L80)
```javascript
router.put('/:id', protect, async (req, res) => {
const tour = await Tour.findById(req.params.id);
if (tour.createdBy.toString() !== req.user._id.toString() && req.user.role !== 'admin') {
return res.status(403).json({ message: 'Not authorized' });
}
// Update privacy
if (privacy) tour.privacy = privacy;
// Handle token logic...
if (tour.privacy === 'shared') {
if (!tour.shareToken) tour.shareToken = crypto.randomBytes(24).toString('hex');
// ... handle expiration
} else {
tour.shareToken = undefined;
tour.shareTokenExpires = undefined;
}
// Propagate to scenes
await propagateScenePrivacy(tour._id, { privacy: tour.privacy, ... }, req.user._id);
res.json(tour);
});
```
### Expected Behavior
**Correctly Implemented** - Ownership check works, privacy propagation to scenes applied.
### Verified Working
- ✅ Checks creator ownership
- ✅ Generates/removes tokens based on privacy mode
- ✅ Handles token expiration
- ✅ Propagates privacy to all scenes in tour
---
## 6. PUT /api/scenes/:id - Update Scene
### Current Implementation
**File**: [backend/routes/sceneRoutes.js](backend/routes/sceneRoutes.js#L214-L280)
```javascript
router.put('/:id', protect, uploadSinglePanorama, async (req, res) => {
const scene = await Scene.findById(req.params.id);
if (!scene || (scene.createdBy.toString() !== req.user._id.toString() && req.user.role !== 'admin')) {
return res.status(403).json({ message: 'Not authorized' });
}
// [SECURITY] Block direct privacy changes on Scene
if (privacy && privacy !== scene.privacy) {
return res.status(403).json({
message: "Privacy must be managed at Tour level"
});
}
// Update metadata only
scene.name = title || scene.name;
scene.description = description !== undefined ? description : scene.description;
await scene.save();
res.json(scene);
});
```
### Expected Behavior
**Correctly Implemented** - Privacy cannot be changed directly; must go through Tour update.
### Verified Working
- ✅ Checks creator ownership
- ✅ Blocks direct privacy modification
- ✅ Enforces Tour-level privacy management
---
## 7. DELETE /api/tours/:id & DELETE /api/scenes/:id
### Current Implementation
#### Tour Deletion
**File**: [backend/middlewares/TourController.js](backend/middlewares/TourController.js#L168-L190)
```javascript
router.delete('/:id', protect, async (req, res) => {
const tour = await Tour.findById(req.params.id);
if (tour.createdBy.toString() !== req.user._id.toString() && req.user.role !== 'admin') {
return res.status(403).json({ message: 'Not authorized' });
}
// Delete all scenes in tour
const scenesInTour = await Scene.find({ tourId: tour._id });
for (const scene of scenesInTour) {
await deleteSceneCascade(scene._id, req.user._id);
}
await Tour.findByIdAndDelete(req.params.id);
res.json({ message: 'Tour deleted' });
});
```
#### Scene Deletion
**File**: [backend/routes/sceneRoutes.js](backend/routes/sceneRoutes.js#L303-L350)
```javascript
router.delete('/:id', protect, async (req, res) => {
const rootScene = await Scene.findById(rootSceneId);
if (req.user.role !== 'admin' && rootScene.createdBy.toString() !== req.user._id.toString()) {
return res.status(403).json({ message: 'Forbidden' });
}
// Cascade delete
await deleteSceneCascade(rootSceneId, req.user._id);
// Update tour (remove from scenes array, update rootSceneId)
if (tourId) {
const tour = await Tour.findById(tourId);
if (tour) {
tour.scenes = tour.scenes.filter(sId => sId.toString() !== rootSceneId.toString());
if (tour.rootSceneId?.toString() === rootSceneId.toString()) {
tour.rootSceneId = tour.scenes.length > 0 ? tour.scenes[0] : null;
}
// Delete tour if empty
const actualRemainingScenes = await Scene.countDocuments({ tourId: tour._id });
if (actualRemainingScenes === 0) {
await Tour.findByIdAndDelete(tour._id);
return res.json({ message: 'Tour deleted' });
}
await tour.save();
}
}
res.json({ message: 'Scene deleted' });
});
```
### Expected Behavior
**Correctly Implemented** - Delete permissions properly checked.
### Verified Working
- ✅ Creator-only deletion
- ✅ Cascade delete scenes when tour deleted
- ✅ Cleanup tour references when scene deleted
- ✅ Auto-delete empty tours
---
## 8. Image Asset Streaming - GET /api/assets/view/:assetId
### Current Implementation
**File**: [backend/routes/assetRoutes.js](backend/routes/assetRoutes.js#L18-L100)
```javascript
router.get('/assets/view/:assetId', verifyReferer, optionalAuth, async (req, res) => {
const asset = await Asset.findById(req.params.assetId);
// Complex privacy check
const scene = await Scene.findOne({ assetId: asset._id }).populate('tourId');
const isSceneTokenValid = scene.shareToken && (!scene.shareTokenExpires || new Date() < scene.shareTokenExpires);
const isTourTokenValid = tour && tour.shareToken && (!tour.shareTokenExpires || new Date() < tour.shareTokenExpires);
let hasAccess = isAdmin ||
scene.privacy === 'public' ||
(tour && tour.privacy === 'public') ||
(scene.privacy === 'member' && userIdStr && (scene.sharedWith.some(...) || scene.sharedEmails.includes(...))) ||
isOwner ||
(scene.privacy === 'shared' && req.query.token === scene.shareToken && isSceneTokenValid) ||
(tour && tour.privacy === 'shared' && req.query.token === tour.shareToken && isTourTokenValid);
if (!hasAccess) return res.status(403).json({ message: 'Forbidden' });
res.sendFile(resolvedPath, { maxAge: 2592000000 });
});
```
### Issues Found
#### ISSUE 8.1 - Scene Privacy Field Again Missing
**Severity**: CRITICAL
This endpoint checks `scene.privacy === 'public'` and `scene.privacy === 'member'` but Scene model doesn't define the privacy field.
**Compounded by**: Scenes created with null privacy will fail these checks even if they should be public.
#### ISSUE 8.2 - Bridge Access Logic Not Applied
**Severity**: MEDIUM
Unlike the scene detail endpoint, this doesn't implement the bridge access logic. A guest with a shared scene token cannot navigate to connected scenes' assets.
---
## 9. Frontend Tour Display Logic
### Current Implementation
**File**: [frontend/js/main_map.js](frontend/js/main_map.js#L1468-L1535)
```javascript
async function loadScenes(urlToken = null) {
let url = `${API_BASE_URL}/scenes?_=${new Date().getTime()}`;
if (urlToken) url += `&token=${urlToken}`;
const response = await fetch(url, { headers });
const scenes = await response.json();
// Deduplicate by coordinates
scenes.forEach((scene) => {
const coordKey = `${latNum.toFixed(6)},${lngNum.toFixed(6)}`;
if (seenCoordinates.has(coordKey)) return; // Skip duplicates
// Add marker to map
const marker = L.marker([latNum, lngNum], { icon: calloutIcon });
marker.on('click', () => {
openScene(scene._id, scene.privacy, scene.shareToken || '');
});
});
}
```
### Expected Behavior
Only display scenes the user has permission to view.
### Issues Found
#### ISSUE 9.1 - Frontend Trust Issue
**Severity**: MEDIUM
The frontend blindly trusts backend to filter scenes by privacy. While this is standard practice, the logic doesn't provide additional validation.
**Current Flow**:
1. Frontend calls `GET /api/scenes`
2. Backend filters by privacy
3. Frontend displays all returned scenes
4. User clicks → `openScene(scene._id, scene.privacy, scene.shareToken)`
**Potential Issue**: If backend filtering is broken, frontend has no fallback.
**Recommendation**: Add client-side privacy display indicators (e.g., 🔒 icon for private scenes user cannot access).
#### ISSUE 9.2 - Tour Privacy Not Shown
**Severity**: LOW
Frontend displays scene privacy but not tour privacy. Users don't know if scenes are part of a private/shared tour.
**Recommendation**: Display tour privacy icon alongside scene marker.
---
## 10. Shared Link Token Validation
### Current Implementation
#### Token Generation (Tour Creation)
**File**: [backend/middlewares/TourController.js](backend/middlewares/TourController.js#L12-L35)
```javascript
const newTour = new Tour({
privacy: privacy || 'private',
shareToken: (privacy === 'shared') ? crypto.randomBytes(24).toString('hex') : undefined
});
if (newTour.privacy === 'shared') {
const expires = new Date();
expires.setDate(expires.getDate() + 7); // Default 7 days
newTour.shareTokenExpires = expires;
}
```
**Status**: ✅ Correctly implemented with default 7-day expiration
#### Token Validation (Multiple Endpoints)
**Pattern Used**:
```javascript
const isTokenValid = tour.shareToken &&
(!tour.shareTokenExpires || new Date() < tour.shareTokenExpires);
if (tour.privacy === 'shared' && req.query.token === tour.shareToken && isTokenValid)
// Grant access
```
**Issues**:
#### ISSUE 10.1 - Token Comparison Uses Weak Equality
**Severity**: MEDIUM
All token comparisons use `===` (strict equality) which is good, but **no case normalization or encoding validation**.
**Potential Issue**:
- Token with uppercase/lowercase differences won't match (probably fine)
- Token with URL encoding might not match (real issue)
**Scenario**:
1. User gets token from URL: `?token=abc%2Bdef` (URL encoded +)
2. Backend receives: `abc+def` (after URL decoding)
3. But comparison happens against raw token stored: might have encoding differences
**Recommendation**: Always decode and normalize tokens before comparison.
#### ISSUE 10.2 - Token Not Revoked on Privacy Change
**Severity**: MEDIUM
When a tour changes from shared → private, the code sets `tour.shareToken = undefined`. But:
```javascript
tour.shareToken = undefined; // Mongoose behavior: removes field
```
**Potential Issue**: Existing tokens might still work if there's a race condition or if clients cache the token.
**Better Approach**: Should explicitly null the token and add revocation timestamp.
---
## 11. Cross-Tour Link Security
### Current Implementation
When a guest with a shared token navigates through hotspots:
**File**: [backend/routes/sceneRoutes.js](backend/routes/sceneRoutes.js#L194-L205)
```javascript
// Bridge access logic
if (!hasAccess && req.query.token) {
const potentialParents = await Hotspot.find({ target_scene_id: scene._id });
const authorizedParentExists = await Scene.exists({
_id: { $in: potentialParents },
shareToken: req.query.token
});
if (authorizedParentExists) hasAccess = true;
}
```
### Issues Found
#### ISSUE 11.1 - Bridge Access Doesn't Validate Cross-Tour Navigation
**Severity**: HIGH
This allows guests to navigate from a shared scene in Tour A to a scene in Tour B if they're hotspot-linked, **even if Tour B is private**.
**Scenario**:
1. User A creates private Tour A with shared Scene 1
2. User B creates private Tour B with Scene 2
3. User A adds hotspot link from Scene 1 → Scene 2 (cross-tour link)
4. Guest with token for Scene 1 can now see Scene 2 **even though Tour B is private**
**Root Cause**: Bridge logic doesn't check target scene's tour privacy
**Recommendation**:
```javascript
// When validating bridge access, also check target tour privacy
if (authorizedParentExists) {
// Check if target scene's tour is accessible
const targetTour = await Tour.findById(scene.tourId);
if (targetTour.privacy === 'public' || targetTour.createdBy === req.user._id) {
hasAccess = true;
}
// Only grant access if target tour is public or owned by user
}
```
---
## 12. Member Access Email Validation
### Current Implementation
**File**: [backend/middlewares/TourController.js](backend/middlewares/TourController.js#L110-L115)
```javascript
(tour.privacy === 'member' && req.user && req.user._id && (
tour.sharedWith.some(u => u.toString() === req.user._id.toString()) ||
(userEmail && tour.sharedEmails.includes(userEmail))
))
```
### Issues Found
#### ISSUE 12.1 - Email Case Sensitivity
**Severity**: LOW
Email comparison is **case-sensitive** but emails should be case-insensitive.
**Scenario**:
1. Tour shared with: `User@Example.com`
2. User logs in with: `user@example.com`
3. Access **denied** (incorrect)
**Fix Required**:
```javascript
tour.sharedEmails.map(e => e.toLowerCase()).includes(userEmail.toLowerCase())
```
#### ISSUE 12.2 - Empty Email Bypass
**Severity**: MEDIUM
If user exists but has no email, the check might bypass:
```javascript
userEmail && tour.sharedEmails.includes(userEmail)
```
This is actually safe (requires non-empty email), but unclear.
---
## Summary Table
| # | Issue | File | Severity | Type | Impact |
|----|-------|------|----------|------|--------|
| 1.1 | Token validation missing email check | TourController.js | HIGH | Logic | Member access inconsistent |
| 2.1 | Shared tokens not returned in list | TourController.js | CRITICAL | Feature | Guests cannot discover shared tours |
| 2.2 | Private tour filtering unclear | TourController.js | MEDIUM | Logic | Edge case in permission logic |
| 3.1 | **Scene model missing privacy field** | Scene.js | CRITICAL | Schema | Core data structure broken |
| 3.2 | Bridge access too permissive | sceneRoutes.js | HIGH | Security | Cross-tour access bypass |
| 4.1 | Individual scene privacy not checked | sceneRoutes.js | HIGH | Logic | Private scenes visible in public tour |
| 4.2 | Token expiration not validated | sceneRoutes.js | MEDIUM | Logic | Expired tokens still work |
| 4.3 | Null privacy values ignored | sceneRoutes.js | MEDIUM | Data | Old scenes not found |
| 8.1 | Asset endpoint uses undefined privacy | assetRoutes.js | CRITICAL | Schema | Images serve to wrong users |
| 8.2 | Asset bridge access not implemented | assetRoutes.js | MEDIUM | Feature | Incomplete implementation |
| 9.1 | Frontend trust issue | main_map.js | MEDIUM | UX | No validation fallback |
| 9.2 | Tour privacy not displayed | main_map.js | LOW | UX | Information missing |
| 10.2 | Token revocation incomplete | TourController.js | MEDIUM | Logic | Old tokens might work |
| 11.1 | Cross-tour bridge access insecure | sceneRoutes.js | HIGH | Security | Private tours accessible |
| 12.1 | Email case sensitivity | TourController.js | LOW | Logic | Email-based access fails |
---
## Recommendations by Priority
### CRITICAL (Fix Immediately)
1. **Add `privacy` field to Scene schema**
```javascript
privacy: {
type: String,
enum: ['public', 'private', 'member', 'shared'],
default: 'private'
}
```
2. **Migrate existing scenes** to have proper privacy values (inherit from tour if not set)
3. **Add token support to GET /api/tours**
```javascript
if (req.query.token) {
// Find tour with matching token and valid expiration
}
```
4. **Fix scene privacy list endpoint** to check individual scene privacy, not just tour
### HIGH PRIORITY (Fix This Week)
5. **Validate token expiration in GET /api/scenes**
- Add `shareTokenExpires` check to token query
6. **Secure bridge access logic**
- Check target tour privacy before allowing cross-tour navigation
- Apply same logic to asset streaming
7. **Validate individual scene privacy** in public tours
- Don't assume all scenes in public tour are public
### MEDIUM PRIORITY (Fix This Sprint)
8. **Token revocation**
- Keep history of revoked tokens
- Add timestamp field
9. **Token parameter normalization**
- Validate and normalize before comparison
- Handle URL encoding properly
10. **Email case-insensitivity**
- Convert to lowercase before comparison
11. **Frontend privacy indicators**
- Show lock icons for private scenes
- Show tour privacy on markers
### LOW PRIORITY (Nice to Have)
12. Add comprehensive logging for privacy access decisions
13. Add audit trail for privacy changes
14. Add privacy migration tool for legacy data
---
## Testing Recommendations
### Test Scenarios
```javascript
// Public tour access
✓ Guest sees public tour scenes
✓ Guest can view public tour details
// Private tour access
✓ Creator sees own private tour
✓ Guest cannot see private tour
✓ Non-creator user cannot see private tour
// Shared token access
✓ Guest with valid token sees shared tour
✓ Guest with expired token denied access
✓ Guest with invalid token denied access
✓ Guest with shared token cannot access private tours via hotlinks
// Member access
✓ User in sharedWith list can access member tour
✓ User with matching email can access member tour
✓ Case-insensitive email matching works
✓ Non-member user cannot access member tour
// Cross-tour linking
✓ Hotspot link respects target tour privacy
✓ Bridge access validates expiration
✓ Bridge access doesn't bypass tour privacy
// Asset streaming
✓ Image available to authorized users
✓ Image denied to unauthorized users
✓ Token validation works for assets
✓ Watermarking respects privacy
```
---
## Conclusion
The project has a **solid foundation** for privacy management at the Tour level with proper propagation to scenes. However, **critical structural issues** (missing Scene.privacy field) and **logic gaps** (token handling, bridge access security, individual scene privacy validation) compromise the implementation.
The good news: Most issues can be fixed without major refactoring. The bad news: The missing Scene.privacy field requires a data migration.
**Recommended Action**:
1. Add Scene.privacy field to schema immediately
2. Migrate existing data
3. Apply the fix recommendations in priority order
4. Run comprehensive security tests