mirror of
https://github.com/jhd3197/ServerKit.git
synced 2026-10-28 07:37:56 +09:00
feat(workspaces): reassign apps/servers between workspaces (#33)
Completes the organizing loop — existing resources were stuck wherever the backfill put them (Default); now they can be moved into client workspaces. - PUT /apps/<id>/workspace (owner-or-admin on the app) and PUT /servers/<id>/workspace (developer+), both requiring the TARGET be a workspace the caller can access (member or admin). A null target moves the resource back to Default. - UI: an 'Apps' management modal on each Workspaces card (mirrors the Members modal) with move-in / move-out. It fetches with allWorkspaces (an empty X-Workspace-Id header) so it can see apps in other workspaces to move them in, sidestepping the ambient scope. - Fixed a real bug caught by tests: the app ownership check compared app.user_id to the stringified get_jwt_identity() (int != str -> spurious 403); now uses user.id. Verified: 2 new endpoint tests (ownership + target-membership + null->Default + role gates), full suite 122 passed, frontend lint-clean + production build. Servers reassignment is API-only for now; the modal is apps-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
567d6059f7
commit
e89415f3e0
6 files changed
+208
-2
No files matched your search
@@ -334,6 +334,38 @@ def get_apps():
|
||||
}), 200
|
||||
|
||||
|
||||
@apps_bp.route('/<int:app_id>/workspace', methods=['PUT'])
|
||||
@jwt_required()
|
||||
def set_app_workspace(app_id):
|
||||
"""Reassign an application to a workspace (#33). Owner-or-admin on the app;
|
||||
the target must be a workspace the caller can access (member or admin). A
|
||||
null/'default' target moves it back to the default workspace."""
|
||||
from app.models.workspace import Workspace
|
||||
from app.services.workspace_service import WorkspaceService
|
||||
user = User.query.get(get_jwt_identity())
|
||||
app = Application.query.get(app_id)
|
||||
if not app:
|
||||
return jsonify({'error': 'Application not found'}), 404
|
||||
# Use user.id (int) for comparisons — get_jwt_identity() is the stringified token id.
|
||||
if user.role != 'admin' and app.user_id != user.id:
|
||||
return jsonify({'error': 'Access denied'}), 403
|
||||
|
||||
target = (request.get_json() or {}).get('workspace_id')
|
||||
if target in (None, '', 'default'):
|
||||
ws_id = WorkspaceService.ensure_default_workspace().id
|
||||
else:
|
||||
ws = Workspace.query.get(target)
|
||||
if not ws:
|
||||
return jsonify({'error': 'Workspace not found'}), 404
|
||||
if user.role != 'admin' and WorkspaceService.get_user_role(ws.id, user.id) is None:
|
||||
return jsonify({'error': 'Not a member of the target workspace'}), 403
|
||||
ws_id = ws.id
|
||||
|
||||
app.workspace_id = ws_id
|
||||
db.session.commit()
|
||||
return jsonify({'message': 'Workspace updated', 'app': app.to_dict()}), 200
|
||||
|
||||
|
||||
@apps_bp.route('/<int:app_id>', methods=['GET'])
|
||||
@jwt_required()
|
||||
def get_app(app_id):
|
||||
|
||||
@@ -368,6 +368,37 @@ def create_server():
|
||||
return jsonify(result), 201
|
||||
|
||||
|
||||
@servers_bp.route('/<server_id>/workspace', methods=['PUT'])
|
||||
@jwt_required()
|
||||
@developer_required
|
||||
def set_server_workspace(server_id):
|
||||
"""Reassign a server to a workspace (#33). Developer+; the target must be a
|
||||
workspace the caller can access (member or admin). A null/'default' target
|
||||
moves it back to the default workspace."""
|
||||
from app.models import User
|
||||
from app.models.workspace import Workspace
|
||||
from app.services.workspace_service import WorkspaceService
|
||||
user = User.query.get(get_jwt_identity())
|
||||
server = Server.query.get(server_id)
|
||||
if not server:
|
||||
return jsonify({'error': 'Server not found'}), 404
|
||||
|
||||
target = (request.get_json() or {}).get('workspace_id')
|
||||
if target in (None, '', 'default'):
|
||||
ws_id = WorkspaceService.ensure_default_workspace().id
|
||||
else:
|
||||
ws = Workspace.query.get(target)
|
||||
if not ws:
|
||||
return jsonify({'error': 'Workspace not found'}), 404
|
||||
if not user.is_admin and WorkspaceService.get_user_role(ws.id, user.id) is None:
|
||||
return jsonify({'error': 'Not a member of the target workspace'}), 403
|
||||
ws_id = ws.id
|
||||
|
||||
server.workspace_id = ws_id
|
||||
db.session.commit()
|
||||
return jsonify({'message': 'Workspace updated', 'server': server.to_dict()}), 200
|
||||
|
||||
|
||||
@servers_bp.route('/<server_id>', methods=['GET'])
|
||||
@jwt_required()
|
||||
def get_server(server_id):
|
||||
|
||||
@@ -241,6 +241,71 @@ def test_migration_backfill_attaches_resources_and_members(app):
|
||||
|
||||
# ---- API: servers ---------------------------------------------------------
|
||||
|
||||
def test_set_app_workspace_api(app, client):
|
||||
from app import db
|
||||
from app.services.workspace_service import WorkspaceService
|
||||
from app.models import Application
|
||||
|
||||
owner = _mk_user(db, 'mv_owner', 'developer')
|
||||
other = _mk_user(db, 'mv_other', 'developer')
|
||||
admin = _mk_user(db, 'mv_admin', 'admin')
|
||||
ws = WorkspaceService.create_workspace({'name': 'MoveWS'}, admin.id)
|
||||
default_id = WorkspaceService.ensure_default_workspace().id
|
||||
a = Application(name='movable', app_type='php', user_id=owner.id, workspace_id=default_id)
|
||||
db.session.add(a)
|
||||
db.session.commit()
|
||||
app_id = a.id
|
||||
|
||||
# Owner isn't a member of the target -> 403.
|
||||
r = client.put(f'/api/v1/apps/{app_id}/workspace', json={'workspace_id': ws.id}, headers=_token(owner.id))
|
||||
assert r.status_code == 403
|
||||
|
||||
# Once a member, the move succeeds.
|
||||
WorkspaceService.add_member(ws.id, owner.id, 'member')
|
||||
r = client.put(f'/api/v1/apps/{app_id}/workspace', json={'workspace_id': ws.id}, headers=_token(owner.id))
|
||||
assert r.status_code == 200 and r.get_json()['app']['workspace_id'] == ws.id
|
||||
|
||||
# A null target moves it back to Default.
|
||||
r = client.put(f'/api/v1/apps/{app_id}/workspace', json={'workspace_id': None}, headers=_token(owner.id))
|
||||
assert r.status_code == 200 and r.get_json()['app']['workspace_id'] == default_id
|
||||
|
||||
# A non-owner non-admin can't reassign it.
|
||||
r = client.put(f'/api/v1/apps/{app_id}/workspace', json={'workspace_id': None}, headers=_token(other.id))
|
||||
assert r.status_code == 403
|
||||
|
||||
# Unknown target workspace -> 404 (admin passes the ownership gate).
|
||||
r = client.put(f'/api/v1/apps/{app_id}/workspace', json={'workspace_id': 999999}, headers=_token(admin.id))
|
||||
assert r.status_code == 404
|
||||
|
||||
|
||||
def test_set_server_workspace_api(app, client):
|
||||
from app import db
|
||||
from app.services.workspace_service import WorkspaceService
|
||||
from app.models import Server
|
||||
|
||||
admin = _mk_user(db, 'sw_admin', 'admin')
|
||||
dev = _mk_user(db, 'sw_dev', 'developer')
|
||||
viewer = _mk_user(db, 'sw_viewer', 'viewer')
|
||||
ws = WorkspaceService.create_workspace({'name': 'SrvWS'}, admin.id)
|
||||
s = Server(name='srv-move', registered_by=admin.id,
|
||||
workspace_id=WorkspaceService.ensure_default_workspace().id)
|
||||
db.session.add(s)
|
||||
db.session.commit()
|
||||
sid = s.id
|
||||
|
||||
# A developer who isn't a member of the target -> 403.
|
||||
r = client.put(f'/api/v1/servers/{sid}/workspace', json={'workspace_id': ws.id}, headers=_token(dev.id))
|
||||
assert r.status_code == 403
|
||||
|
||||
# Admin can move it (admin bypass).
|
||||
r = client.put(f'/api/v1/servers/{sid}/workspace', json={'workspace_id': ws.id}, headers=_token(admin.id))
|
||||
assert r.status_code == 200 and r.get_json()['server']['workspace_id'] == ws.id
|
||||
|
||||
# A viewer is blocked by @developer_required.
|
||||
r = client.put(f'/api/v1/servers/{sid}/workspace', json={'workspace_id': None}, headers=_token(viewer.id))
|
||||
assert r.status_code == 403
|
||||
|
||||
|
||||
def test_servers_list_global_without_context(app, client):
|
||||
from app import db
|
||||
from app.models import Server
|
||||
|
||||
@@ -301,6 +301,7 @@ Highest-effort, broadest surface — but by now the `WordPressSite` is a first-c
|
||||
- **Landed (workspace foundation slice):** the cross-cutting backbone the rest of #33 needs — a **nullable `workspace_id` FK** on `applications` + `servers` (Alembic **015**, chains off `014`, with an idempotent raw-SQL backfill that creates a **Default** workspace, attaches every workspace-less row, and makes every existing user a member so nobody loses visibility) + a **centralized scope helper** in `WorkspaceService` (`ensure_default_workspace`, `resolve_workspace_id`, `scope_query`). The design rule is **opt-in + narrow-only**: a request only filters by workspace when it carries an `X-Workspace-Id` header / `?workspace_id=`, and `scope_query` *intersects* the workspace filter with the existing per-user ownership filter so it can only ever **narrow** visibility, never broaden it — with no workspace context the behavior is byte-identical to before (admin→all, owner→own apps, servers global). Wired into `apps.py` (list + create) and `servers.py` (list + create), replacing the duplicated `if admin … else filter_by(user_id)` branch and **stamping** new resources with the active-or-default workspace. **Deliberately *not* a 36-route rewrite:** this lands the mechanism + the two core resource surfaces as the proven pattern; broad route adoption, per-site ACL, the workspace↔User role reconciliation, server-side branding, and a frontend active-workspace selector are the follow-on slices. Verified: backend `py_compile`, **10 new pytest tests** (helper branches, scope-narrowing, an explicit *can't-broaden-access* regression, deactivated-user gate, default-stamping, the raw-SQL backfill run against real SQLite + idempotency), full suite **120 passed**, a real end-to-end `alembic upgrade` to `015` on a fresh DB (columns + both indexes created), and a 2-lens adversarial review (3 findings, all fixed — `is_active` gate on the workspace path, FK-add guarded on the `workspaces` table existing, and the missing `workspace_id` indexes for upgraded installs). The earlier draft's biggest risk — that the backfill (everything→Default, everyone→member) plus a workspace filter would let a non-admin enumerate *all* resources via the default workspace — was caught and fixed pre-review by making `scope_query` intersect with ownership.
|
||||
- **Landed (frontend workspace selector):** the keystone that makes the foundation user-visible — the research flagged "no current-workspace context in the frontend" as *the* gap #33 must close. A self-contained `WorkspaceSwitcher` in the sidebar reads/writes an `active_workspace_id` in `localStorage`, which the central API client (`services/api/client.js`) now sends ambiently as the **`X-Workspace-Id` header** on every request (endpoints that don't honor it ignore it; cleared on logout). Switching reloads so all lists re-fetch under the new scope; the switcher self-hides for single-workspace installs and drops a stale selection on load. To make an ambient header safe, backend `resolve_workspace_id` was first made **lenient** (a deleted/forbidden workspace degrades to no-scope instead of 403/404, so a stale header can never break the apps/servers lists — still safe since no-scope only ever shows the user's *own* resources). Verified: frontend lint-clean + production build; the header→scoping path is covered end-to-end by the backend API tests. *(Follow-on: surfacing `workspace_id` in more list endpoints, a workspace management UX beyond the existing admin list, and per-workspace logo override.)*
|
||||
- **Landed (workspace-scoped brand color):** the visible payoff — selecting a client workspace now recolors the whole panel to that client's brand. A workspace's `primary_color` (already on the model; now editable via a color picker added to the Workspaces create modal) is mirrored by the `WorkspaceSwitcher` into a `workspace_accent` localStorage key, and `ThemeContext` applies it via the existing `applyAccentToDOM` with **precedence over the user's personal accent** (`workspace_accent || accentColor`) — recoloring the logo gradient, buttons, and highlights. Deterministic (no effect-ordering races) because switching reloads and the value is read once on mount; cleared on logout and on a stale selection; reverts to the user accent on "All workspaces". Default color is the app accent, so a workspace is unbranded unless a custom color is chosen. Verified: frontend lint-clean (no new warnings) + production build. *(Deferred: per-workspace logo swap in the sidebar brand — the accent recolor is the high-impact piece and the default logo already recolors with the accent.)*
|
||||
- **Landed (resource reassignment):** completes the organizing loop — existing resources were stuck wherever the backfill put them (Default); now they can be moved into client workspaces. New endpoints `PUT /apps/<id>/workspace` (owner-or-admin on the app) and `PUT /servers/<id>/workspace` (developer+), both requiring the **target** be a workspace the caller can access (member or admin); a null target moves a resource back to Default. UI: an **Apps** management modal on each Workspaces card (mirrors the Members modal) listing the workspace's apps with move-in/move-out — it fetches with `allWorkspaces` (an empty `X-Workspace-Id` header) so it can see apps in other workspaces to move them in, sidestepping the ambient scope. **Fixed a real bug** caught by tests: the ownership check compared `app.user_id` to the *stringified* `get_jwt_identity()` (int≠str → spurious 403); now uses `user.id`. Verified: backend `py_compile`, **2 new endpoint tests** (ownership + target-membership + null→Default + role gates), full suite **122 passed**, frontend lint-clean + production build. *(Servers reassignment is API-only for now; the management modal is apps-only.)*
|
||||
|
||||
### #34 — Horizontal scaling `[XL]` ❌ — ⏸️ Deferred (XL + blocked on the same per-site reverse-proxy/upstream layer as #21)
|
||||
- **Today:** `nginx_advanced_service` has load-balancing primitives (round-robin/least-conn/IP-hash) but WP containers are stateful (local uploads, no shared object cache).
|
||||
|
||||
@@ -21,6 +21,8 @@ const Workspaces = () => {
|
||||
const [members, setMembers] = useState([]);
|
||||
const [deleteConfirm, setDeleteConfirm] = useState(null);
|
||||
const [allUsers, setAllUsers] = useState([]);
|
||||
const [showResourcesModal, setShowResourcesModal] = useState(null);
|
||||
const [apps, setApps] = useState([]);
|
||||
const [form, setForm] = useState({ name: '', description: '', max_servers: 0, max_users: 0, primary_color: '#6366f1' });
|
||||
|
||||
const loadWorkspaces = useCallback(async () => {
|
||||
@@ -103,6 +105,28 @@ const Workspaces = () => {
|
||||
}
|
||||
};
|
||||
|
||||
const loadResources = async (wsId) => {
|
||||
try {
|
||||
// allWorkspaces so we can see apps in OTHER workspaces to move them in.
|
||||
const data = await api.getApps({ allWorkspaces: true });
|
||||
setApps(data.apps || []);
|
||||
setShowResourcesModal(wsId);
|
||||
} catch (err) {
|
||||
toast.error('Failed to load applications');
|
||||
}
|
||||
};
|
||||
|
||||
const handleMoveApp = async (appId, workspaceId) => {
|
||||
try {
|
||||
await api.setAppWorkspace(appId, workspaceId);
|
||||
toast.success('Application moved');
|
||||
const data = await api.getApps({ allWorkspaces: true });
|
||||
setApps(data.apps || []);
|
||||
} catch (err) {
|
||||
toast.error(err.message);
|
||||
}
|
||||
};
|
||||
|
||||
if (loading) return <Spinner />;
|
||||
|
||||
return (
|
||||
@@ -146,6 +170,7 @@ const Workspaces = () => {
|
||||
)}
|
||||
<div className="workspace-card__actions">
|
||||
<Button size="sm" variant="outline" onClick={() => loadMembers(ws.id)}>Members</Button>
|
||||
<Button size="sm" variant="outline" onClick={() => loadResources(ws.id)}>Apps</Button>
|
||||
{ws.status === 'active' && (
|
||||
<Button size="sm" variant="secondary" onClick={() => handleArchive(ws.id)}>Archive</Button>
|
||||
)}
|
||||
@@ -245,6 +270,46 @@ const Workspaces = () => {
|
||||
</div>
|
||||
)}
|
||||
|
||||
{showResourcesModal && (() => {
|
||||
const inWs = apps.filter(a => a.workspace_id === showResourcesModal);
|
||||
const elsewhere = apps.filter(a => a.workspace_id !== showResourcesModal);
|
||||
return (
|
||||
<div className="modal-overlay" onClick={() => setShowResourcesModal(null)}>
|
||||
<div className="modal" onClick={e => e.stopPropagation()}>
|
||||
<div className="modal-header">
|
||||
<h2>Workspace Applications</h2>
|
||||
<button className="modal-close" onClick={() => setShowResourcesModal(null)}>×</button>
|
||||
</div>
|
||||
<div className="modal-body">
|
||||
<div className="members-list">
|
||||
{inWs.length === 0 && <p className="form-hint">No applications in this workspace yet.</p>}
|
||||
{inWs.map(a => (
|
||||
<div key={a.id} className="member-row">
|
||||
<div>
|
||||
<strong>{a.name}</strong>
|
||||
<Badge variant="outline" className="ml-2">{a.app_type}</Badge>
|
||||
</div>
|
||||
<Button size="sm" variant="destructive" onClick={() => handleMoveApp(a.id, null)}>Remove</Button>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
<hr />
|
||||
<h4>Add Application</h4>
|
||||
<div className="server-select-list">
|
||||
{elsewhere.length === 0 && <p className="form-hint">No other applications to add.</p>}
|
||||
{elsewhere.map(a => (
|
||||
<div key={a.id} className="server-select-item" onClick={() => handleMoveApp(a.id, showResourcesModal)}>
|
||||
<span>{a.name}</span>
|
||||
<Badge variant="outline" className="ml-2">{a.app_type}</Badge>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
})()}
|
||||
|
||||
{deleteConfirm && (
|
||||
<ConfirmDialog
|
||||
title="Delete Workspace"
|
||||
|
||||
@@ -2,8 +2,20 @@
|
||||
// templates, builds, deployments
|
||||
|
||||
// Apps endpoints
|
||||
export async function getApps() {
|
||||
return this.request('/apps');
|
||||
export async function getApps(options = {}) {
|
||||
// allWorkspaces neutralizes the ambient X-Workspace-Id header (sends it empty)
|
||||
// so a cross-workspace view (e.g. resource management) sees every app, not just
|
||||
// the active workspace's.
|
||||
const config = options.allWorkspaces ? { headers: { 'X-Workspace-Id': '' } } : {};
|
||||
return this.request('/apps', config);
|
||||
}
|
||||
|
||||
// Reassign an application to a workspace (#33). Pass null to move it to Default.
|
||||
export async function setAppWorkspace(appId, workspaceId) {
|
||||
return this.request(`/apps/${appId}/workspace`, {
|
||||
method: 'PUT',
|
||||
body: { workspace_id: workspaceId },
|
||||
});
|
||||
}
|
||||
|
||||
export async function getApp(id) {
|
||||
|
||||
Reference in new issue
Block a user