admin felület különválasztva
This commit is contained in:
Roo
266
docs/p0_backoffice_sso_router_audit_report.md
Normal file
266
docs/p0_backoffice_sso_router_audit_report.md
Normal file
@@ -0,0 +1,266 @@
|
||||
# P0 Architect Report: Backoffice SSO & Router Architecture
|
||||
|
||||
**Date:** 2026-06-24
|
||||
**Scope:** Cross-subdomain cookie SSO, backend RBAC dependencies, frontend admin routing
|
||||
**Auditor:** Fast Coder (Core Developer)
|
||||
|
||||
---
|
||||
|
||||
## 1. AUDIT: Cross-Subdomain Cookies (Backend Auth)
|
||||
|
||||
### File: [`backend/app/api/v1/endpoints/auth.py`](backend/app/api/v1/endpoints/auth.py)
|
||||
|
||||
#### Current State (Lines 127–134, 221–228)
|
||||
|
||||
Both the `/login` and `/verify-email` endpoints set the `refresh_token` cookie like this:
|
||||
|
||||
```python
|
||||
response.set_cookie(
|
||||
key="refresh_token",
|
||||
value=refresh,
|
||||
httponly=True,
|
||||
secure=True,
|
||||
samesite="lax",
|
||||
max_age=max_age_sec
|
||||
)
|
||||
```
|
||||
|
||||
#### ❌ Finding: `domain` parameter is MISSING
|
||||
|
||||
The `domain` parameter is **not set** on either `set_cookie()` call. This means the cookie is scoped to the **exact origin** that issued it.
|
||||
|
||||
**Impact for SSO:**
|
||||
- If the user logs in at `app.servicefinder.hu`, the `refresh_token` cookie is only sent to `app.servicefinder.hu`.
|
||||
- When the user navigates to `admin.servicefinder.hu`, the browser **will not send** the cookie because the domain doesn't match.
|
||||
- This breaks cross-subdomain SSO entirely — the admin subdomain cannot silently refresh the access token.
|
||||
|
||||
#### Required Fix
|
||||
|
||||
Add `domain=".servicefinder.hu"` (with leading dot for subdomain wildcard) to both `set_cookie()` calls:
|
||||
|
||||
```python
|
||||
response.set_cookie(
|
||||
key="refresh_token",
|
||||
value=refresh,
|
||||
httponly=True,
|
||||
secure=True,
|
||||
samesite="lax",
|
||||
domain=".servicefinder.hu", # ← ADD THIS
|
||||
max_age=max_age_sec
|
||||
)
|
||||
```
|
||||
|
||||
**Considerations:**
|
||||
- The domain should be **dynamic** based on environment (e.g., `.servicefinder.hu` for production, `localhost` for dev).
|
||||
- Use `settings.COOKIE_DOMAIN` or derive it from `settings.FRONTEND_BASE_URL`.
|
||||
- `samesite="lax"` is correct — it allows the cookie to be sent on top-level navigations between subdomains.
|
||||
|
||||
---
|
||||
|
||||
## 2. AUDIT: Backend Dependencies (Deps for Staff)
|
||||
|
||||
### File: [`backend/app/api/deps.py`](backend/app/api/deps.py)
|
||||
|
||||
#### 2a. Existing `get_current_admin` (Lines 160–177)
|
||||
|
||||
```python
|
||||
async def get_current_admin(current_user: User = Depends(get_current_user)) -> User:
|
||||
allowed_roles = {
|
||||
UserRole.SUPERADMIN,
|
||||
UserRole.ADMIN,
|
||||
UserRole.MODERATOR,
|
||||
}
|
||||
if current_user.role not in allowed_roles:
|
||||
raise HTTPException(status_code=403, detail="...")
|
||||
return current_user
|
||||
```
|
||||
|
||||
**✅ Good:** This already exists and allows `SUPERADMIN`, `ADMIN`, and `MODERATOR`.
|
||||
|
||||
**❌ Gap:** It does **not** include `SALES_REP` or `SERVICE_MGR`. The Architect's vision is that the Backoffice (`admin.servicefinder.hu`) should admit all "staff" roles — not just superadmins.
|
||||
|
||||
#### 2b. Existing `RequireSystemCapability` (Lines 184–218)
|
||||
|
||||
```python
|
||||
def RequireSystemCapability(capability_name: str):
|
||||
async def system_capability_checker(current_user: User = Depends(get_current_user)) -> bool:
|
||||
if current_user.role == UserRole.SUPERADMIN:
|
||||
return True
|
||||
role_key = current_user.role.value
|
||||
if not role_has_capability(role_key, capability_name):
|
||||
raise HTTPException(status_code=403, detail=f"...")
|
||||
return True
|
||||
return system_capability_checker
|
||||
```
|
||||
|
||||
**✅ Good:** This is the scalable, fine-grained approach. Each endpoint can declare exactly which capability it needs.
|
||||
|
||||
**❌ Gap:** There is no **middle-ground** dependency — something between `get_current_admin` (hardcoded role set) and `RequireSystemCapability` (fine-grained). We need a `RequireRole` dependency that accepts a list of allowed roles.
|
||||
|
||||
#### 2c. Proposed: `RequireRole` Dependency
|
||||
|
||||
Add to [`backend/app/api/deps.py`](backend/app/api/deps.py):
|
||||
|
||||
```python
|
||||
def RequireRole(allowed_roles: List[UserRole]):
|
||||
"""
|
||||
🎯 Flexible role-checking dependency.
|
||||
|
||||
Accepts a list of UserRole values. If the current user's role is
|
||||
in the list, access is granted. Otherwise, 403 Forbidden.
|
||||
|
||||
Usage:
|
||||
@router.get("/admin/sales")
|
||||
async def sales_dashboard(
|
||||
current_user: User = Depends(get_current_user),
|
||||
_ = Depends(RequireRole([UserRole.SUPERADMIN, UserRole.SALES_REP]))
|
||||
):
|
||||
"""
|
||||
async def role_checker(
|
||||
current_user: User = Depends(get_current_user),
|
||||
) -> bool:
|
||||
if current_user.role not in allowed_roles:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail="AUTH.INSUFFICIENT_PERMISSIONS"
|
||||
)
|
||||
return True
|
||||
return role_checker
|
||||
```
|
||||
|
||||
And a **convenience** `get_current_staff` that admits all internal staff:
|
||||
|
||||
```python
|
||||
STAFF_ROLES = {
|
||||
UserRole.SUPERADMIN,
|
||||
UserRole.ADMIN,
|
||||
UserRole.MODERATOR,
|
||||
UserRole.SALES_REP,
|
||||
UserRole.SERVICE_MGR,
|
||||
}
|
||||
|
||||
async def get_current_staff(
|
||||
current_user: User = Depends(get_current_user)
|
||||
) -> User:
|
||||
if current_user.role not in STAFF_ROLES:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail="AUTH.STAFF_ONLY"
|
||||
)
|
||||
return current_user
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 3. AUDIT: Frontend Architecture (Admin Domain Routing)
|
||||
|
||||
### File: [`frontend/src/router/index.ts`](frontend/src/router/index.ts)
|
||||
|
||||
#### 3a. Current Admin Route (Lines 112–137)
|
||||
|
||||
```typescript
|
||||
{
|
||||
path: '/admin',
|
||||
component: () => import('../layouts/AdminLayout.vue'),
|
||||
meta: { requiresAuth: true, requiresAdmin: true },
|
||||
children: [
|
||||
{ path: '', name: 'admin-dashboard', component: () => import('../views/admin/AdminDashboardView.vue') },
|
||||
{ path: 'users', name: 'admin-users', component: () => import('../views/admin/AdminUsersView.vue') },
|
||||
{ path: 'packages', name: 'admin-packages', component: () => import('../views/admin/AdminPackagesView.vue') },
|
||||
{ path: 'services', name: 'admin-services', component: () => import('../views/admin/AdminServicesView.vue') },
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
**✅ Good:** The admin routes exist under a single `/admin` path with `requiresAdmin` meta.
|
||||
|
||||
#### 3b. Current Route Guard (Lines 166–177)
|
||||
|
||||
```typescript
|
||||
if (to.meta.requiresAdmin) {
|
||||
const role = authStore.user?.role
|
||||
const adminRoles = ['SUPERADMIN', 'ADMIN', 'MODERATOR']
|
||||
if (!role || !adminRoles.includes(role.toUpperCase())) {
|
||||
return next('/dashboard')
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**❌ Finding: The guard is too restrictive.** It only admits `SUPERADMIN`, `ADMIN`, and `MODERATOR`. `SALES_REP` and `SERVICE_MGR` are blocked from the admin panel entirely.
|
||||
|
||||
**❌ Finding: No separate subdomain routing.** The admin UI is served under the same SPA at `/admin` path, not as a separate deployment for `admin.servicefinder.hu`. This means:
|
||||
- The same Vite build serves both `app.servicefinder.hu` and `admin.servicefinder.hu`.
|
||||
- The Nginx reverse proxy must route `admin.servicefinder.hu` → same SPA, but the router handles the `/admin` prefix.
|
||||
- This is **fine for Phase 1** but limits independent scaling/deployment.
|
||||
|
||||
#### 3c. Auth Store `isAdmin` Getter (Lines 103–110)
|
||||
|
||||
```typescript
|
||||
const isAdmin = computed(() => {
|
||||
const role = user.value?.role
|
||||
if (!role) return false
|
||||
const adminRoles = ['SUPERADMIN', 'ADMIN', 'MODERATOR']
|
||||
return adminRoles.includes(role.toUpperCase())
|
||||
})
|
||||
```
|
||||
|
||||
**❌ Same gap:** `SALES_REP` and `SERVICE_MGR` are not recognized as admin/staff.
|
||||
|
||||
#### 3d. AdminLayout Component
|
||||
|
||||
**File:** [`frontend/src/layouts/AdminLayout.vue`](frontend/src/layouts/AdminLayout.vue)
|
||||
|
||||
**✅ Good:** The layout has a proper sidebar with navigation links, user profile section, and slot for header controls. It references `LanguageSwitcher`, `ModeSwitcher`, and `HeaderProfile`.
|
||||
|
||||
**❌ Finding:** The sidebar links include routes that don't exist in the router (`/admin/vehicles`, `/admin/translations`, `/admin/system`, `/admin/audit`). These will produce 404s when clicked.
|
||||
|
||||
---
|
||||
|
||||
## 4. ACTION PLAN
|
||||
|
||||
### Phase 1: Fix Cookie Domain for SSO (Backend)
|
||||
|
||||
| # | Task | File | Details |
|
||||
|---|------|------|---------|
|
||||
| 1.1 | Add `COOKIE_DOMAIN` to settings | [`backend/app/core/config.py`](backend/app/core/config.py) | Add `COOKIE_DOMAIN: str = ".servicefinder.hu"` field |
|
||||
| 1.2 | Add `domain` param to `/login` cookie | [`backend/app/api/v1/endpoints/auth.py:127`](backend/app/api/v1/endpoints/auth.py:127) | `response.set_cookie(..., domain=settings.COOKIE_DOMAIN)` |
|
||||
| 1.3 | Add `domain` param to `/verify-email` cookie | [`backend/app/api/v1/endpoints/auth.py:221`](backend/app/api/v1/endpoints/auth.py:221) | Same fix |
|
||||
|
||||
### Phase 2: Create Scalable Backend Dependencies
|
||||
|
||||
| # | Task | File | Details |
|
||||
|---|------|------|---------|
|
||||
| 2.1 | Add `RequireRole` dependency | [`backend/app/api/deps.py`](backend/app/api/deps.py) | Generic role-list checker |
|
||||
| 2.2 | Add `get_current_staff` dependency | [`backend/app/api/deps.py`](backend/app/api/deps.py) | Admits SUPERADMIN, ADMIN, MODERATOR, SALES_REP, SERVICE_MGR |
|
||||
| 2.3 | Update `get_current_admin` to use STAFF_ROLES | [`backend/app/api/deps.py`](backend/app/api/deps.py) | Or keep as-is for superadmin-only endpoints |
|
||||
|
||||
### Phase 3: Update Frontend Router Guard & Auth Store
|
||||
|
||||
| # | Task | File | Details |
|
||||
|---|------|------|---------|
|
||||
| 3.1 | Update `requiresAdmin` guard to admit all staff roles | [`frontend/src/router/index.ts:168`](frontend/src/router/index.ts:168) | Change `adminRoles` to include `SALES_REP`, `SERVICE_MGR` |
|
||||
| 3.2 | Update `isAdmin` getter in auth store | [`frontend/src/stores/auth.ts:103`](frontend/src/stores/auth.ts:103) | Same role expansion |
|
||||
| 3.3 | Add missing admin routes or remove dead sidebar links | [`frontend/src/layouts/AdminLayout.vue`](frontend/src/layouts/AdminLayout.vue) | Add routes for vehicles, translations, system, audit — or remove sidebar links |
|
||||
| 3.4 | Consider role-based sidebar visibility | [`frontend/src/layouts/AdminLayout.vue`](frontend/src/layouts/AdminLayout.vue) | SALES_REP should only see sales-related nav items |
|
||||
|
||||
### Phase 4: Nginx / Reverse Proxy (Infrastructure)
|
||||
|
||||
| # | Task | Details |
|
||||
|---|------|---------|
|
||||
| 4.1 | Ensure `admin.servicefinder.hu` → same SPA | Nginx config must serve the same frontend build for both subdomains |
|
||||
| 4.2 | Ensure CORS allows `admin.servicefinder.hu` | Already in [`backend/app/core/config.py:93`](backend/app/core/config.py:93) ✅ |
|
||||
|
||||
---
|
||||
|
||||
## Summary of Findings
|
||||
|
||||
| Area | Status | Issue |
|
||||
|------|--------|-------|
|
||||
| Cookie `domain` | ❌ Missing | SSO broken between subdomains |
|
||||
| `get_current_admin` | ⚠️ Too narrow | Missing SALES_REP, SERVICE_MGR |
|
||||
| `RequireSystemCapability` | ✅ Good | Fine-grained, scalable |
|
||||
| `RequireRole` | ❌ Missing | No flexible role-list dependency |
|
||||
| Frontend `requiresAdmin` guard | ⚠️ Too narrow | Blocks SALES_REP, SERVICE_MGR |
|
||||
| Frontend `isAdmin` getter | ⚠️ Too narrow | Same gap |
|
||||
| AdminLayout sidebar links | ❌ Dead links | Routes for vehicles/translations/system/audit don't exist |
|
||||
| CORS origins | ✅ Good | `admin.servicefinder.hu` already whitelisted |
|
||||
Reference in New Issue
Block a user