diff --git a/lang/da.json b/lang/da.json index d8b96fb..8c962a6 100644 --- a/lang/da.json +++ b/lang/da.json @@ -532,6 +532,9 @@ "m365_profile_save_tip": "Gem aktuelle indstillinger som profil", "m365_profile_save_prompt": "Profilnavn:", "m365_profile_applied": "Profil indlæst", + "m365_profile_acct_manual": "Manuel", + "m365_profile_acct_unresolved": "Ikke indlæst", + "m365_profile_manual_not_found": "Manuel konto ikke fundet i M365", "m365_profile_saved": "Profil gemt", "m365_profile_manage_btn": "Profiler", "m365_profile_clear_btn": "Ryd", diff --git a/lang/de.json b/lang/de.json index 80a880a..a5fabbf 100644 --- a/lang/de.json +++ b/lang/de.json @@ -532,6 +532,9 @@ "m365_profile_save_tip": "Aktuelle Einstellungen als Profil speichern", "m365_profile_save_prompt": "Profilname:", "m365_profile_applied": "Profil geladen", + "m365_profile_acct_manual": "Manuell", + "m365_profile_acct_unresolved": "Nicht geladen", + "m365_profile_manual_not_found": "Manuelles Konto in M365 nicht gefunden", "m365_profile_saved": "Profil gespeichert", "m365_profile_manage_btn": "Profile", "m365_profile_clear_btn": "Löschen", diff --git a/lang/en.json b/lang/en.json index 3e17284..ecbc541 100644 --- a/lang/en.json +++ b/lang/en.json @@ -532,6 +532,9 @@ "m365_profile_save_tip": "Save current settings as a profile", "m365_profile_save_prompt": "Profile name:", "m365_profile_applied": "Profile loaded", + "m365_profile_acct_manual": "Manual", + "m365_profile_acct_unresolved": "Not loaded", + "m365_profile_manual_not_found": "Manual account not found in M365", "m365_profile_saved": "Profile saved", "m365_profile_manage_btn": "Profiles", "m365_profile_clear_btn": "Clear", diff --git a/m365_connector.py b/m365_connector.py index 210f259..ce3c31a 100644 --- a/m365_connector.py +++ b/m365_connector.py @@ -224,6 +224,10 @@ class M365Connector: return {"displayName": "App (service account)", "id": "", "mail": ""} return self._get("/me") + def lookup_user(self, upn: str) -> dict: + """Look up one directory user by UPN or email (requires User.Read.All).""" + return self._get(f"/users/{upn}", {"$select": "id,displayName,mail,userPrincipalName"}) + def list_users(self, top: int = 999) -> list: """List licensed domain users in the tenant (requires User.Read.All). diff --git a/routes/CLAUDE.md b/routes/CLAUDE.md index 715004e..99eec0c 100644 --- a/routes/CLAUDE.md +++ b/routes/CLAUDE.md @@ -115,5 +115,6 @@ Optional AI-powered NER replacing spaCy. Activated via `config.json` keys `claud ## Gotchas - **`_load_settings()` return** — does NOT include `file_sources`. Returns only: sources, user_ids, options, retention_years, fiscal_year_end, email_to. -- **`_save_settings()` clobbers profile fields** — called on every M365 scan start with only M365 sources/user_ids/options. The fix in `app_config.py` preserves `google_sources` and `file_sources` and rebuilds `sources` as `m365_src + google_src + file_src`. Do not simplify away this merge logic. +- **`POST /api/scan/start` must not write profiles** — it used to call `_save_settings()` with the sidebar state, which overwrote the active profile's `user_ids` (dropping Google-only users and role-filtered-out users) and, with no active profile, clobbered `profiles[0]`. Profiles change only via `/api/profiles/save`. `profile_id` in the start payload is audit-only. Do not re-add the call. +- **`_save_settings()` merge logic** — still used by `/api/settings/save`. It preserves `google_sources` and `file_sources` and rebuilds `sources` as `m365_src + google_src + file_src`. Do not simplify away this merge logic. - **`loadLastScanSummary()` timing** — must only be called after the first `/api/scan/status` poll resolves (inside `_sseWatchdog` in `results.js`, guarded by `_initialStatusChecked`). Calling it on `DOMContentLoaded` shows a stale "no results" card during a live scan after a hard refresh. diff --git a/routes/scan.py b/routes/scan.py index 02db7e4..bc9ec3f 100644 --- a/routes/scan.py +++ b/routes/scan.py @@ -103,12 +103,10 @@ def scan_start(): return jsonify({"error": "scan already running"}), 409 options = request.get_json() or {} state._scan_abort.clear() + # Profiles are loaders: a scan must never write the sidebar state back into + # a profile (it overwrote user_ids, dropped Google-only users, and hit + # profiles[0] when no profile was active). profile_id is audit-only here. profile_id = options.pop("profile_id", None) - _save_settings({ - "sources": options.get("sources", []), - "user_ids": options.get("user_ids", []), - "options": options.get("options", {}), - }, profile_id=profile_id) def _run(): from scan_engine import run_scan try: diff --git a/routes/users.py b/routes/users.py index 19f4306..ebd6ed2 100644 --- a/routes/users.py +++ b/routes/users.py @@ -176,7 +176,7 @@ def lookup_user(): if not upn: return jsonify({"error": "upn required"}), 400 try: - data = state.connector._get(f"/users/{upn}", {"$select": "id,displayName,mail,userPrincipalName"}) + data = state.connector.lookup_user(upn) _email = data.get("mail") or data.get("userPrincipalName", upn) return jsonify({ "id": data["id"], diff --git a/scan_engine.py b/scan_engine.py index a56a33f..1bef525 100644 --- a/scan_engine.py +++ b/scan_engine.py @@ -41,6 +41,37 @@ except ImportError: from routes import state as _state +_MANUAL_PREFIX = "manual:" + + +def _resolve_manual_users(conn, user_ids: list[dict]) -> list[dict]: + """Resolve profile accounts saved as "manual:" to their directory id. + + Older profile-editor saves (and saves made while M365 was not connected) + store manual accounts by email only; Graph needs the user id. Accounts that + can't be resolved are reported and skipped — never widened to "all users". + """ + resolved, seen = [], set() + for u in user_ids: + uid = u.get("id", "") + if uid.startswith(_MANUAL_PREFIX): + upn = uid[len(_MANUAL_PREFIX):] + try: + data = conn.lookup_user(upn) + _email = data.get("mail") or data.get("userPrincipalName", upn) + u = {**u, "id": data["id"], + "displayName": _resolve_display_name(data.get("displayName", ""), _email, upn)} + except Exception as e: + logger.warning("[run_scan] manual account %r not found: %s", upn, e) + broadcast("scan_error", {"file": upn, "error": f"Manual account not found in M365: {e}"}) + continue + if u["id"] in seen: + continue + seen.add(u["id"]) + resolved.append(u) + return resolved + + def _get_scan_abort(): return _state._scan_abort @@ -645,6 +676,7 @@ def run_scan(options: dict): me_info.get("displayName", ""), me_info.get("mail") or me_info.get("userPrincipalName", "me"))}] else: + user_ids = _resolve_manual_users(conn, user_ids) sample = user_ids[0] if user_ids else None logger.info("[run_scan] user_ids: %d entries, type=%s, sample=%s", len(user_ids), type(user_ids).__name__, sample) diff --git a/static/js/CLAUDE.md b/static/js/CLAUDE.md index d7cbf57..058671e 100644 --- a/static/js/CLAUDE.md +++ b/static/js/CLAUDE.md @@ -80,7 +80,14 @@ Never revert to `!!window._googleConnected` / `_fileSources.length > 0` — thos - **`scheduler.js` strings must use `t()`** — frequency labels, "Next", "Running...", "Disabled", empty-job text, and empty-history text all have translation keys. Do not hard-code English strings in `schedLoad()` or `schedRenderJobs()`. - **Scheduler UI — `schedToggleReportOnly()`** — dims the Profile row, shows/hides `#schedReportOnlyHint`, and forces `#schedAutoEmail` checked. Called from the checkbox `onchange` handler and at the start of `schedAddJob()` / `schedEditJob()`. -- **Profile editor accounts** — default to unchecked. Only explicitly saved `user_ids` are checked. +- **Profile editor accounts** — default to unchecked. Only explicitly saved `user_ids` are checked. Saved IDs not in `S._allUsers` (`manual:`, users not loaded yet, users gone from the directory) are rendered by `_unresolvedAccountRows()` as checked rows so the next save keeps them — do not go back to building rows from `S._allUsers` alone (that silently dropped them on save). `user_ids: "all"` is kept as `"all"` when saved before users have loaded. +- **Manual accounts must be scannable** — they aren't in the `/api/users` directory list, so: + - `getSelectedUsers()` treats a missing `platform` as `'m365'`. Sidebar `addUserManually()` users historically had no platform and were silently excluded from every scan. Do not go back to a strict `u.platform === 'm365'` check. + - The profile editor's "+ Tilføj konto" looks the email up via `/api/users/lookup` and stores `{id: , displayName, email, manual: true}`. It falls back to `"manual:"` only when the lookup fails. Checkboxes with `data-manual` save as objects; everything else saves as plain id strings. + - `_applyProfile()` → `_addProfileManualUsers()` pushes manual accounts into `S._allUsers` (`manual: true`, `platform: 'm365'`). Legacy `manual:` entries are looked up and the pending ID is rewritten to the resolved one. Without this the account can never be ticked in the sidebar, so it's never scanned. + - `_userListFinal()` keys off `S._m365UsersLoaded`, not `S._allUsers.length`, because manual users can be pushed before the directory loads. + - Server side, `scan_engine._resolve_manual_users()` resolves any remaining `manual:` (e.g. from scheduled scans) via `conn.lookup_user()`. It skips and reports failures and never widens to "all users". +- **Pending profile users wait for the Google merge** — `_applyPendingProfileUsers()` keeps `window._pendingProfileUserIds` until every ID is matched or `_userListFinal()` (users loaded, and Google either not connected or `S._googleUsersMerged`). `_mergeGoogleUsers()` re-applies in its `finally`. A saved `google:` also matches an M365 user merged to `platform: 'both'` via `u.googleEmail`. Do not clear the pending IDs unconditionally — Google-only users would stay unticked. - **Date presets** — stored as `years * 365` (integer days). Do not use `* 365.25`. - **`copyTokenLink` is async** — called from `onclick` as fire-and-forget. Do not make it synchronous. - **Escape scan-derived strings with `esc()`** — `results.js` defines `esc()` (escapes `& < > " '`). Every value that originates from scanned content (`f.name`, `f.account_name`, `f.folder`, `f.source`, `f.modified`, `label`, image `alt`, and the same fields on `item`/related rows) must pass through `esc()` before going into `innerHTML` or a `title=`/`alt=` attribute. These are attacker-influenceable (e.g. a file named with markup), so an unescaped interpolation is stored XSS — including in shared read-only viewer sessions. Numeric counts (`cpr_count`, `size_kb`) don't need it. When embedding an object in an `onclick` payload, also `.replace(/"/g,'"')` the `JSON.stringify(...)`. diff --git a/static/js/profiles.js b/static/js/profiles.js index 18f9cff..87e1e94 100644 --- a/static/js/profiles.js +++ b/static/js/profiles.js @@ -215,6 +215,7 @@ function _applyProfile(profile) { } } else if (Array.isArray(profile.user_ids) && profile.user_ids.length) { window._pendingProfileUserIds = profile.user_ids.map(u => u.id || u); + _addProfileManualUsers(profile.user_ids); _applyPendingProfileUsers(); } else if (Array.isArray(profile.user_ids) && profile.user_ids.length === 0) { // Explicitly empty list — deselect everyone so previous sidebar state doesn't persist @@ -225,13 +226,71 @@ function _applyProfile(profile) { log(t('m365_profile_applied', 'Profile loaded') + ': ' + profile.name); } +// Google-only users (id "google:") arrive later than M365 users, via the +// un-awaited _mergeGoogleUsers(). Keep the pending IDs until every one is +// matched or the user list is final, else those users stay unticked. +function _userListFinal() { + return S._m365UsersLoaded && (!window._googleConnected || S._googleUsersMerged); +} + function _applyPendingProfileUsers() { const ids = window._pendingProfileUserIds; if (!ids || !ids.length || !S._allUsers.length) return; - // Select only the users listed in the profile - S._allUsers.forEach(u => { u.selected = ids.includes(u.id); }); + const wanted = new Set(ids); + // A saved google: may since have merged onto an M365 user (platform 'both') + const matches = u => wanted.has(u.id) || (u.googleEmail && wanted.has('google:' + u.googleEmail)); + S._allUsers.forEach(u => { u.selected = matches(u); }); renderAccountList(); - window._pendingProfileUserIds = null; + const found = S._allUsers.filter(matches).length; + if (found >= wanted.size || _userListFinal()) window._pendingProfileUserIds = null; +} + +function _isManualEntry(u) { + return typeof u === 'object' ? !!u.manual : String(u).startsWith('manual:'); +} + +// Manual accounts aren't in the /api/users directory list, so they must be added +// to S._allUsers (as sidebar manual users) or they can never be selected/scanned. +// Legacy "manual:" entries are looked up to get their directory id. +function _addProfileManualUsers(entries) { + entries.filter(_isManualEntry).forEach(async function(u) { + if (typeof u === 'object' && !String(u.id).startsWith('manual:')) { + if (!S._allUsers.some(x => x.id === u.id)) { + S._allUsers.push({id: u.id, displayName: u.displayName || u.email || u.id, + email: u.email || '', userRole: u.userRole || 'other', + platform: 'm365', manual: true, selected: false}); + } + return; // selection is handled by _applyPendingProfileUsers() + } + const legacyId = u.id || u; + try { + const email = legacyId.slice('manual:'.length); + const d = await _lookupM365User(email); + if (!d) { + // Not connected / not found — scheduled scans retry the lookup at scan time + log(t('m365_profile_manual_not_found', 'Manual account not found in M365') + ': ' + email, 'err'); + return; + } + let user = S._allUsers.find(x => x.id === d.id); + if (!user) { + user = {...d, platform: 'm365', manual: true}; + S._allUsers.push(user); + } + user.selected = true; + // Pending apply may re-run later — make it select the resolved id, not the legacy one + const pend = window._pendingProfileUserIds; + if (pend) window._pendingProfileUserIds = pend.map(x => x === legacyId ? d.id : x); + renderAccountList(); + } catch(e) { /* leave for the scan-time fallback */ } + }); +} + +// Returns {id, displayName, email} or null when M365 isn't connected / the user doesn't exist. +async function _lookupM365User(upn) { + const r = await fetch('/api/users/lookup?upn=' + encodeURIComponent(upn)); + if (!r.ok) return null; + const d = await r.json(); + return d && d.id ? d : null; } async function saveCurrentAsProfile() { @@ -365,19 +424,25 @@ function _openEditorForProfile(profile) { const srcCheck = (id) => allSources.includes(id) ? 'checked' : ''; // Build account list from S._allUsers - const savedIds = new Set((profile.user_ids || []).map(u => u.id || u)); - // If no saved IDs match current users, treat as all-selected (new profile or users changed) - const anyMatch = savedIds.size > 0 && S._allUsers.some(u => savedIds.has(u.id)); + const savedAll = profile.user_ids === 'all'; + const savedList = Array.isArray(profile.user_ids) ? profile.user_ids : []; + const savedIds = new Set(savedList.map(u => u.id || u)); + const isSaved = u => savedAll || savedIds.has(u.id) + || !!(u.googleEmail && savedIds.has('google:' + u.googleEmail)); + // Remember "all" so saving before the user list has loaded doesn't turn it into [] + window._pmgmtSavedAll = savedAll; const accountRows = S._allUsers.map(u => { // Only check if the user was explicitly saved — default to unchecked like the main window - const checked = anyMatch && savedIds.has(u.id) ? 'checked' : ''; + const checked = isSaved(u) ? 'checked' : ''; + const manualAttrs = u.manual + ? ` data-manual="1" data-name="${_esc(u.displayName || '')}" data-email="${_esc(u.email || '')}"` : ''; const platBadge = u.platform === 'both' ? 'M365+GWS' : (u.platform || 'm365') === 'google' ? 'GWS' : 'M365'; const roleBadge = u.userRole === 'student' ? t('role_student','Elev') : u.userRole === 'staff' ? t('role_staff','Ansat') : t('role_other','Anden'); const roleOverrideStyle = u.roleOverride ? 'color:var(--color-text-info);outline:1px solid var(--color-border-info);' : ''; - return ``; - }).join(''); + return ``; + }).join('') + _unresolvedAccountRows(savedList); body.innerHTML = `
@@ -464,6 +529,30 @@ function _openEditorForProfile(profile) { _renderEditorSources((profile.sources || []).concat(profile.google_sources || []).concat(profile.file_sources || [])); } +// Saved accounts that aren't in S._allUsers — manually added ("manual:"), +// not loaded yet, or gone from the directory. Rendered as checked rows so the +// next save keeps them instead of silently dropping them. +function _unresolvedAccountRows(savedList) { + const known = new Set(); + S._allUsers.forEach(u => { + known.add(u.id); + if (u.googleEmail) known.add('google:' + u.googleEmail); + }); + const seen = new Set(); + return savedList.map(u => { + const id = u.id || u; + if (!id || known.has(id) || seen.has(id)) return ''; + seen.add(id); + const isManual = _isManualEntry(u); + const label = (typeof u === 'object' && u.displayName) || id.replace(/^(manual|google):/, ''); + const badge = isManual ? t('m365_profile_acct_manual', 'Manual') + : t('m365_profile_acct_unresolved', 'Not loaded'); + const manualAttrs = _isManualEntry(u) && typeof u === 'object' + ? ` data-manual="1" data-name="${_esc(u.displayName || '')}" data-email="${_esc(u.email || '')}"` : ''; + return ``; + }).join(''); +} + function _peSetDate(val) { if (!val) return; const ms = new Date() - new Date(val); @@ -592,16 +681,24 @@ function _pmgmtRoleFilter(role) { _pmgmtFilterAccounts(document.getElementById('pmgmtAcctSearch')?.value || ''); } -function _pmgmtAddManual() { - const email = prompt('E-mail adresse:'); - if (!email || !email.trim()) return; +async function _pmgmtAddManual() { + const email = (prompt('E-mail adresse:') || '').trim(); + if (!email) return; const list = document.getElementById('pmgmtAcctList'); if (!list) return; - const id = 'manual:' + email.trim().toLowerCase(); - if (list.querySelector(`input[data-uid="${id}"]`)) return; // already exists + // Resolve to the directory id so the scan can use it. If M365 isn't reachable + // right now, fall back to "manual:" — the scan engine resolves that later. + let d = null; + try { d = await _lookupM365User(email); } catch(e) { /* offline — use fallback */ } + const id = d ? d.id : 'manual:' + email.toLowerCase(); + const label = d ? (d.displayName || email) : email; + const existing = [...list.querySelectorAll('input[data-uid]')].find(cb => cb.dataset.uid === id); + if (existing) { existing.checked = true; return; } // already listed — just tick it const lbl = document.createElement('label'); lbl.className = 'pmgmt-acct-row'; - lbl.innerHTML = `${_esc(email.trim())}Manuel`; + lbl.dataset.uid = id; + lbl.dataset.role = 'other'; + lbl.innerHTML = `${_esc(label)}${_esc(t('m365_profile_acct_manual', 'Manual'))}`; list.appendChild(lbl); } @@ -635,9 +732,13 @@ async function _pmgmtSaveFullEdit() { const effectiveGoogleSources = googleRendered ? googleSources : (profile.google_sources || []); const effectiveFileSources = fileRendered ? fileSources : (profile.file_sources || []); const allSources = m365Sources.concat(effectiveGoogleSources).concat(effectiveFileSources); - const user_ids = Array.from(document.querySelectorAll('#pmgmtAcctList input[type=checkbox]:checked')) - .map(cb => cb.dataset.uid) - .filter(Boolean); + let user_ids = Array.from(document.querySelectorAll('#pmgmtAcctList input[type=checkbox]:checked')) + .filter(cb => cb.dataset.uid) + .map(cb => cb.dataset.manual + ? {id: cb.dataset.uid, displayName: cb.dataset.name || '', email: cb.dataset.email || '', manual: true} + : cb.dataset.uid); + // "all" profile opened before users loaded: there were no rows to tick, so keep "all" + if (window._pmgmtSavedAll && !S._allUsers.length && !user_ids.length) user_ids = 'all'; const updated = { ...profile, name, @@ -769,6 +870,10 @@ window._esc = _esc; window._pmgmtUse = _pmgmtUse; window._pmgmtOpenEditor = _pmgmtOpenEditor; window._openEditorForProfile = _openEditorForProfile; +window._unresolvedAccountRows = _unresolvedAccountRows; +window._addProfileManualUsers = _addProfileManualUsers; +window._lookupM365User = _lookupM365User; +window._userListFinal = _userListFinal; window._peSetDate = _peSetDate; window._peSetYear = _peSetYear; window._renderEditorSources = _renderEditorSources; diff --git a/static/js/state.js b/static/js/state.js index 1050201..57f107d 100644 --- a/static/js/state.js +++ b/static/js/state.js @@ -19,6 +19,8 @@ export const S = { _progressCurrentUser: '', // Users _allUsers: [], + _m365UsersLoaded: false, // true once loadUsers() has fetched the directory + _googleUsersMerged: false, // true once _mergeGoogleUsers() has added Google users // Auth _currentAppMode: null, // Profiles diff --git a/static/js/users.js b/static/js/users.js index 2b54a6d..ae76c2d 100644 --- a/static/js/users.js +++ b/static/js/users.js @@ -28,6 +28,7 @@ async function loadUsers() { u.selected = prevSelected.has(u.id) ? prevSelected.get(u.id) : false; }); S._allUsers = [...fetched, ...toAdd]; + S._m365UsersLoaded = true; // Apply deferred "select all" from a profile chosen before users loaded if (window._pendingProfileAllUsers) { S._allUsers.forEach(u => { u.selected = true; }); @@ -107,6 +108,12 @@ async function _mergeGoogleUsers() { S._allUsers = S._allUsers.concat(googleUsers); renderAccountList(); } catch(e) { /* Google users unavailable */ } + finally { + // Google users are now as complete as they'll get (even on error/early + // return) — finish a profile apply that was waiting on google: IDs. + S._googleUsersMerged = true; + _applyPendingProfileUsers(); + } } let _activeRoleFilter = ''; // '' = all, 'staff', 'student' @@ -415,7 +422,7 @@ async function addUserManually() { const d = await r.json(); if (d.error) { alert('User not found: ' + d.error); return; } if (S._allUsers.find(u => u.id === d.id)) { alert('User already in list.'); return; } - S._allUsers.push({...d, manual: true}); + S._allUsers.push({...d, platform: 'm365', manual: true}); input.value = ''; renderAccountList(S._allUsers.filter(u => !u.manual).length <= 1); } catch(e) { @@ -441,16 +448,18 @@ function selectAllAccounts(checked) { function getSelectedUsers() { // Only return M365 users — Google users are handled separately via selectedGoogleEmails - let selected = S._allUsers.filter(u => u.selected !== false && (u.platform === 'm365' || u.platform === 'both')); + // Manually added users may have no platform — they are M365 lookups, so treat missing as 'm365' + let selected = S._allUsers.filter(u => u.selected !== false && ['m365', 'both'].includes(u.platform || 'm365')); // Respect the active role filter — hidden users must not sneak into the scan // even if they were checked before the filter was applied. if (_activeRoleFilter) { selected = selected.filter(u => (u.userRole || 'other') === _activeRoleFilter); } if (selected.length) { - return selected.map(u => ({ - id: u.id, displayName: u.displayName, userRole: u.userRole || 'other' - })); + // manual/email let a profile saved from this selection re-add manual users on apply + return selected.map(u => Object.assign( + { id: u.id, displayName: u.displayName, userRole: u.userRole || 'other' }, + u.manual ? { manual: true, email: u.email || '' } : {})); } // Fallback to DOM if S._allUsers not yet populated return Array.from(document.querySelectorAll('.account-check:checked')).map(cb => ({ diff --git a/tests/test_manual_users.py b/tests/test_manual_users.py new file mode 100644 index 0000000..94cf94c --- /dev/null +++ b/tests/test_manual_users.py @@ -0,0 +1,104 @@ +""" +Manual accounts in scan profiles. + +The profile editor stores manually added accounts either with their directory +id (looked up via GET /api/users/lookup) or, when M365 wasn't reachable / for +profiles saved by older versions, as "manual:". The scan engine must +resolve the latter to a Graph user id or they are never scanned. +""" +from __future__ import annotations +from unittest.mock import MagicMock + +import pytest + +import scan_engine + + +class _Conn: + def __init__(self, directory: dict[str, dict]): + self.directory = directory + self.lookups: list[str] = [] + + def lookup_user(self, upn: str) -> dict: + self.lookups.append(upn) + if upn not in self.directory: + raise RuntimeError("Request_ResourceNotFound") + return self.directory[upn] + + +@pytest.fixture() +def broadcasts(monkeypatch): + sent = [] + monkeypatch.setattr(scan_engine, "broadcast", lambda ev, data: sent.append((ev, data))) + return sent + + +class TestResolveManualUsers: + def test_manual_email_resolved_to_directory_id(self, broadcasts): + conn = _Conn({"ann@school.dk": {"id": "guid-ann", "displayName": "Ann Andersen", + "mail": "ann@school.dk"}}) + out = scan_engine._resolve_manual_users(conn, [ + {"id": "manual:ann@school.dk", "displayName": "manual:ann@school.dk", "userRole": "other"}, + ]) + assert out == [{"id": "guid-ann", "displayName": "Ann Andersen", "userRole": "other"}] + assert conn.lookups == ["ann@school.dk"] + assert broadcasts == [] + + def test_regular_and_resolved_manual_ids_pass_through_without_lookup(self, broadcasts): + conn = _Conn({}) + users = [{"id": "guid-1", "displayName": "One"}, + {"id": "guid-2", "displayName": "Two", "manual": True, "email": "two@school.dk"}] + assert scan_engine._resolve_manual_users(conn, users) == users + assert conn.lookups == [] + + def test_unknown_manual_account_skipped_and_reported(self, broadcasts): + conn = _Conn({}) + out = scan_engine._resolve_manual_users(conn, [ + {"id": "guid-1", "displayName": "One"}, + {"id": "manual:ghost@school.dk", "displayName": "ghost"}, + ]) + assert out == [{"id": "guid-1", "displayName": "One"}] + assert broadcasts and broadcasts[0][0] == "scan_error" + assert "ghost@school.dk" in broadcasts[0][1]["file"] + + def test_manual_account_already_selected_is_deduplicated(self, broadcasts): + conn = _Conn({"ann@school.dk": {"id": "guid-ann", "displayName": "Ann"}}) + out = scan_engine._resolve_manual_users(conn, [ + {"id": "guid-ann", "displayName": "Ann"}, + {"id": "manual:ann@school.dk", "displayName": "ann"}, + ]) + assert [u["id"] for u in out] == ["guid-ann"] + + def test_all_manual_unresolvable_gives_empty_list_not_all_users(self, broadcasts): + # run_scan resolves inside the non-empty branch, so an empty result must + # stay empty — it must never widen to "scan every tenant user". + conn = _Conn({}) + assert scan_engine._resolve_manual_users( + conn, [{"id": "manual:x@school.dk", "displayName": "x"}]) == [] + + +class TestLookupRoute: + @pytest.fixture() + def client(self): + import gdpr_scanner + gdpr_scanner.app.config["TESTING"] = True + with gdpr_scanner.app.test_client() as c: + yield c + + def test_lookup_uses_connector(self, client, monkeypatch): + from routes import state + conn = MagicMock() + conn.lookup_user.return_value = {"id": "guid-ann", "displayName": "Ann", + "mail": "ann@school.dk"} + monkeypatch.setattr(state, "connector", conn) + r = client.get("/api/users/lookup?upn=ann@school.dk") + assert r.status_code == 200 + assert r.get_json()["id"] == "guid-ann" + conn.lookup_user.assert_called_once_with("ann@school.dk") + + def test_lookup_not_found_is_404(self, client, monkeypatch): + from routes import state + conn = MagicMock() + conn.lookup_user.side_effect = RuntimeError("Request_ResourceNotFound") + monkeypatch.setattr(state, "connector", conn) + assert client.get("/api/users/lookup?upn=ghost@school.dk").status_code == 404 diff --git a/tests/test_route_integration.py b/tests/test_route_integration.py index 4cadfef..e48c5e9 100644 --- a/tests/test_route_integration.py +++ b/tests/test_route_integration.py @@ -661,3 +661,77 @@ class TestProfileRoutes: def test_delete_missing_key_returns_400(self, client): r = client.post("/api/profiles/delete", json={}) assert r.status_code == 400 + + +# --------------------------------------------------------------------------- +# Scan start must not write profiles +# --------------------------------------------------------------------------- + +class TestScanStartLeavesProfilesAlone: + """ + Regression: POST /api/scan/start used to call _save_settings() with the + sidebar state, overwriting the active profile's user_ids (dropping + Google-only users) — or profiles[0] when no profile was active. + """ + + @pytest.fixture(autouse=True) + def _isolate(self, tmp_path, monkeypatch): + import app_config, scan_engine + monkeypatch.setattr(app_config, "_SETTINGS_PATH", tmp_path / "settings.json") + monkeypatch.setattr(scan_engine, "run_scan", lambda opts: None) + + @staticmethod + def _wait_for_lock(): + from routes import state + deadline = time.time() + 2.0 + while not state._scan_lock.acquire(blocking=False): + assert time.time() < deadline, "scan lock was never released" + time.sleep(0.05) + state._scan_lock.release() + + def _save(self, client, name, user_ids): + r = client.post("/api/profiles/save", json={ + "name": name, "sources": ["email", "gmail"], + "google_sources": ["gmail"], "user_ids": user_ids, "options": {}, + }) + return r.get_json()["profile"] + + def _get(self, client, pid): + return client.get(f"/api/profiles/get?id={pid}").get_json()["profile"] + + def test_active_profile_users_unchanged(self, client, mock_connector): + users = ["m365-a", "google:b@school.dk", "manual:c@school.dk"] + p = self._save(client, "Staff", users) + r = client.post("/api/scan/start", json={ + "sources": ["email"], + "user_ids": [{"id": "m365-z", "displayName": "Z"}], + "options": {"older_than_days": 0}, + "profile_id": p["id"], + }) + assert r.status_code == 200 + self._wait_for_lock() + after = self._get(client, p["id"]) + assert after["user_ids"] == users + assert after["sources"] == ["email", "gmail"] + + def test_no_active_profile_leaves_first_profile_unchanged(self, client, mock_connector): + first = self._save(client, "First", ["m365-a", "m365-b"]) + self._save(client, "Second", ["m365-c"]) + r = client.post("/api/scan/start", json={ + "sources": ["email"], + "user_ids": [{"id": "m365-z", "displayName": "Z"}], + "options": {}, + "profile_id": None, + }) + assert r.status_code == 200 + self._wait_for_lock() + assert self._get(client, first["id"])["user_ids"] == ["m365-a", "m365-b"] + assert len(client.get("/api/profiles").get_json()["profiles"]) == 2 + + def test_scan_start_creates_no_profile(self, client, mock_connector): + r = client.post("/api/scan/start", json={ + "sources": ["email"], "user_ids": [{"id": "m365-z"}], "options": {}, + }) + assert r.status_code == 200 + self._wait_for_lock() + assert client.get("/api/profiles").get_json()["profiles"] == []