feat: per-account device isolation with Firebase scoped paths#127
Merged
Conversation
Contributor
Reviewer's GuideImplements per-account Firebase device isolation by scoping all device access to user-specific paths, adding a namespace mode switch, wiring routes and smarthome handlers to pass user scope, and updating the devices dashboard to render live, scoped device state. Sequence diagram for scoped smarthome request handlingsequenceDiagram
actor User
participant GoogleHome as GoogleHome
participant Routes as FlaskRoutes_smarthome
participant Actions as actions
participant Devices as action_devices
participant Firebase as FirebaseRealtimeDB
User->>GoogleHome: Voice command
GoogleHome->>Routes: POST /smarthome
Routes->>Routes: _resolve_smarthome_user_scope(req)
Routes->>Actions: actions(req, user_id, agent_user_id)
loop For each intent
Actions->>Actions: Inspect intent
alt SYNC intent
Actions->>Devices: onSync(user_id, agent_user_id)
Devices->>Devices: _normalize_user_scope(user_id)
Devices->>Devices: rsync(user_scope)
Devices->>Devices: _get_scoped_snapshot(user_scope)
alt namespace_mode legacy or user_scope None
Devices->>Firebase: GET /devices
else namespace_mode hybrid or strict
Devices->>Firebase: GET /users/{user_scope}/devices
opt hybrid fallback when empty
Devices->>Firebase: GET /devices
end
end
Devices->>Actions: devices list
else QUERY intent
Actions->>Devices: onQuery(body, user_id)
loop For each deviceId
Devices->>Devices: rquery(deviceId, user_id)
Devices->>Devices: _get_scoped_device_states(deviceId, user_id)
alt namespace_mode legacy or user_scope None
Devices->>Firebase: GET /devices/{deviceId}/states
else namespace_mode hybrid or strict
Devices->>Firebase: GET /users/{user_scope}/devices/{deviceId}/states
opt hybrid fallback when None
Devices->>Firebase: GET /devices/{deviceId}/states
end
end
end
Devices->>Actions: states per device
else EXECUTE intent
Actions->>Devices: onExecute(body, user_id)
loop For each device command
Devices->>Devices: commands(payload, deviceId, execCommand, params, user_id)
Devices->>Devices: rexecute(deviceId, params, user_id)
alt namespace_mode legacy or user_scope None
Devices->>Firebase: UPDATE /devices/{deviceId}/states
Devices->>Firebase: GET /devices/{deviceId}/states
else namespace_mode hybrid or strict
Devices->>Firebase: UPDATE /users/{user_scope}/devices/{deviceId}/states
Devices->>Firebase: GET /users/{user_scope}/devices/{deviceId}/states
end
end
Devices->>Actions: updated states
end
end
Actions->>Routes: payload (agentUserId, devices, states)
Routes->>GoogleHome: JSON response
GoogleHome->>User: Confirmation
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces user-level scoping for devices stored in Firebase, enabling multi-tenancy through a configurable namespace mode (legacy, hybrid, or strict). The changes include refactoring core device interaction functions to propagate user IDs, implementing scoped snapshot retrieval with fallback logic, and updating the web dashboard to display user-specific device data. Review feedback highlights an inconsistency in default configuration values, a potential logic issue in rexecute that could lead to partial records in hybrid mode, and the need to use configured fallbacks instead of hardcoded strings in exception handlers.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="action_devices.py" line_range="90-93" />
<code_context>
return current[keys[-1]]
+def _device_namespace_mode():
+ """Return configured device namespace mode."""
+ mode = str(current_app.config.get('DEVICE_NAMESPACE_MODE', 'hybrid')).strip().lower()
+ return mode if mode in {'legacy', 'hybrid', 'strict'} else 'hybrid'
+
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Align DEVICE_NAMESPACE_MODE default between config and helper to avoid hidden mode changes.
`Config.DEVICE_NAMESPACE_MODE` defaults to `strict`, but `_device_namespace_mode` falls back to `hybrid` when the config value is missing or invalid. This means code that reads `current_app.config` directly (e.g. tests or ad‑hoc apps) may run with a different namespace mode than code using `Config`. Please either make `Config.DEVICE_NAMESPACE_MODE` the single source of truth (and drop the `'hybrid'` fallback here) or change the config default to `hybrid` so both paths share the same default.
Suggested implementation:
```python
def _device_namespace_mode():
"""Return configured device namespace mode.
Uses the same default as Config.DEVICE_NAMESPACE_MODE to keep
current_app.config and Config in sync.
"""
# Fall back to Config.DEVICE_NAMESPACE_MODE so both code paths share
# the same default and we don't silently switch to a different mode.
mode = str(
current_app.config.get('DEVICE_NAMESPACE_MODE', Config.DEVICE_NAMESPACE_MODE)
).strip().lower()
return mode if mode in {'legacy', 'hybrid', 'strict'} else Config.DEVICE_NAMESPACE_MODE
```
1. Ensure `Config` is imported or otherwise available in this module. For example, near the top of `action_devices.py`, add an appropriate import such as `from .config import Config` or `from yourpackage.config import Config`, matching how the rest of the codebase accesses `Config`.
2. Confirm that `Config.DEVICE_NAMESPACE_MODE` retains the intended default (currently `strict` per your comment) so that both direct `Config` users and `_device_namespace_mode()` share the same default and fallback behavior.
</issue_to_address>
### Comment 2
<location path="action_devices.py" line_range="160-166" />
<code_context>
+ return reference(None).get() or {}
+
+
+def _get_scoped_device_states(device_id, user_id):
+ """Get device states under user scope with hybrid fallback for reads."""
+ mode = _device_namespace_mode()
+ user_scope = _normalize_user_scope(user_id)
+
+ if mode == 'legacy' or not user_scope:
+ return reference(None).child(device_id).child('states').get()
+
+ scoped_states = reference(user_scope).child(device_id).child('states').get()
</code_context>
<issue_to_address>
**issue (bug_risk):** Consider aligning hybrid write behavior with read fallback to avoid stale state reads.
In `_get_scoped_device_states`, hybrid mode reads from the user scope but falls back to `/devices` when no scoped states exist, while `rexecute` in hybrid mode writes only to `reference(user_scope)`:
```python
mode = _device_namespace_mode()
user_scope = _normalize_user_scope(user_id)
target_reference = reference(None) if mode == 'legacy' or not user_scope else reference(user_scope)
```
This can produce stale reads: initial reads hit legacy, writes go only to the scoped path, and subsequent reads may still hit legacy if `scoped_states` is `None`, missing the new state.
To keep hybrid behavior consistent and avoid this mismatch, you could either:
- Write to both scoped and legacy locations in hybrid mode, or
- Adjust `_get_scoped_device_states` so a `None` scoped value triggers promotion from legacy and/or prevents further fallback once scoped data exists.
Aligning these will reduce confusing state discrepancies between reads and writes.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements per-account device isolation backed by Firebase Realtime Database. Each user's devices are stored and served from a user-scoped path (
/users/{user_id}/devices), with no cross-account leakage.Changes
Device Management
action_devices.py: Refactored all device functions (rsync,rstate,rquery,rexecute,onSync,onQuery,onExecute,commands,actions,get_dashboard_devices) to accept and useuser_idfor scoped Firebase reads/writes_device_namespace_mode(),_normalize_user_scope(),_get_scoped_snapshot(), and_get_scoped_device_states()helpersget_dashboard_devices(user_id)to power the devices dashboard with real Firebase dataRoutes
routes.py:/devices,/device/<device_id>, and/smarthomeroutes all passcurrent_user.idas scope_resolve_smarthome_user_scope()to extract user scope from Google Home requests (agentUserIdor authenticated session)Config
config.py: AddedDEVICE_NAMESPACE_MODEenv var (defaultstrict) — controls whether device reads fall back to the legacy global path (legacy|hybrid|strict)Templates
templates/devices.html: Updated to render real device data (icon, display name, type label, online status) from FirebaseFirebase Path Schema
/users/{user_id}/devices/{device_id}//devices/Isolation Behaviour
In
strictmode, each account only reads from its own/users/{user_id}/devicespath. No fallback to shared data.Tests
38 tests passing. New tests added:
test_devices_route_scopes_to_logged_in_usertest_smarthome_uses_agent_user_scopetest_devices_route_passes_logged_in_user_scopetest_get_dashboard_devices_uses_live_statetest_devices_route_uses_dashboard_devicesSummary by Sourcery
Scope device management to per-user Firebase paths and wire this through web routes and smarthome intents, with configurable fallback behavior.
New Features:
Enhancements:
Tests: