Fix profiles losing users and scan manually added accounts #1

Merged
Netadmin merged 2 commits from fix/profile-user-persistence into main 2026-09-28 07:40:41 +00:00
14 changed files with 376 additions and 31 deletions

No files matched your search

+3
View File
@@ -532,6 +532,9 @@
"m365_profile_save_tip": "Gem aktuelle indstillinger som profil", "m365_profile_save_tip": "Gem aktuelle indstillinger som profil",
"m365_profile_save_prompt": "Profilnavn:", "m365_profile_save_prompt": "Profilnavn:",
"m365_profile_applied": "Profil indlæst", "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_saved": "Profil gemt",
"m365_profile_manage_btn": "Profiler", "m365_profile_manage_btn": "Profiler",
"m365_profile_clear_btn": "Ryd", "m365_profile_clear_btn": "Ryd",
+3
View File
@@ -532,6 +532,9 @@
"m365_profile_save_tip": "Aktuelle Einstellungen als Profil speichern", "m365_profile_save_tip": "Aktuelle Einstellungen als Profil speichern",
"m365_profile_save_prompt": "Profilname:", "m365_profile_save_prompt": "Profilname:",
"m365_profile_applied": "Profil geladen", "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_saved": "Profil gespeichert",
"m365_profile_manage_btn": "Profile", "m365_profile_manage_btn": "Profile",
"m365_profile_clear_btn": "Löschen", "m365_profile_clear_btn": "Löschen",
+3
View File
@@ -532,6 +532,9 @@
"m365_profile_save_tip": "Save current settings as a profile", "m365_profile_save_tip": "Save current settings as a profile",
"m365_profile_save_prompt": "Profile name:", "m365_profile_save_prompt": "Profile name:",
"m365_profile_applied": "Profile loaded", "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_saved": "Profile saved",
"m365_profile_manage_btn": "Profiles", "m365_profile_manage_btn": "Profiles",
"m365_profile_clear_btn": "Clear", "m365_profile_clear_btn": "Clear",
+4
View File
@@ -224,6 +224,10 @@ class M365Connector:
return {"displayName": "App (service account)", "id": "", "mail": ""} return {"displayName": "App (service account)", "id": "", "mail": ""}
return self._get("/me") 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: def list_users(self, top: int = 999) -> list:
"""List licensed domain users in the tenant (requires User.Read.All). """List licensed domain users in the tenant (requires User.Read.All).
+2 -1
View File
@@ -115,5 +115,6 @@ Optional AI-powered NER replacing spaCy. Activated via `config.json` keys `claud
## Gotchas ## Gotchas
- **`_load_settings()` return** — does NOT include `file_sources`. Returns only: sources, user_ids, options, retention_years, fiscal_year_end, email_to. - **`_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. - **`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.
+3 -5
View File
@@ -103,12 +103,10 @@ def scan_start():
return jsonify({"error": "scan already running"}), 409 return jsonify({"error": "scan already running"}), 409
options = request.get_json() or {} options = request.get_json() or {}
state._scan_abort.clear() 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) 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(): def _run():
from scan_engine import run_scan from scan_engine import run_scan
try: try:
+1 -1
View File
@@ -176,7 +176,7 @@ def lookup_user():
if not upn: if not upn:
return jsonify({"error": "upn required"}), 400 return jsonify({"error": "upn required"}), 400
try: 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) _email = data.get("mail") or data.get("userPrincipalName", upn)
return jsonify({ return jsonify({
"id": data["id"], "id": data["id"],
+32
View File
@@ -41,6 +41,37 @@ except ImportError:
from routes import state as _state 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:<email>" 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(): def _get_scan_abort():
return _state._scan_abort return _state._scan_abort
@@ -645,6 +676,7 @@ def run_scan(options: dict):
me_info.get("displayName", ""), me_info.get("displayName", ""),
me_info.get("mail") or me_info.get("userPrincipalName", "me"))}] me_info.get("mail") or me_info.get("userPrincipalName", "me"))}]
else: else:
user_ids = _resolve_manual_users(conn, user_ids)
sample = user_ids[0] if user_ids else None sample = user_ids[0] if user_ids else None
logger.info("[run_scan] user_ids: %d entries, type=%s, sample=%s", logger.info("[run_scan] user_ids: %d entries, type=%s, sample=%s",
len(user_ids), type(user_ids).__name__, sample) len(user_ids), type(user_ids).__name__, sample)
+8 -1
View File
@@ -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.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()`. - **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:<email>`, 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: <directory id>, displayName, email, manual: true}`. It falls back to `"manual:<email>"` 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:<email>` 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:<email>` (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:<email>` 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`. - **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. - **`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,'&quot;')` the `JSON.stringify(...)`. - **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,'&quot;')` the `JSON.stringify(...)`.
+123 -18
View File
@@ -215,6 +215,7 @@ function _applyProfile(profile) {
} }
} else if (Array.isArray(profile.user_ids) && profile.user_ids.length) { } else if (Array.isArray(profile.user_ids) && profile.user_ids.length) {
window._pendingProfileUserIds = profile.user_ids.map(u => u.id || u); window._pendingProfileUserIds = profile.user_ids.map(u => u.id || u);
_addProfileManualUsers(profile.user_ids);
_applyPendingProfileUsers(); _applyPendingProfileUsers();
} else if (Array.isArray(profile.user_ids) && profile.user_ids.length === 0) { } else if (Array.isArray(profile.user_ids) && profile.user_ids.length === 0) {
// Explicitly empty list — deselect everyone so previous sidebar state doesn't persist // 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); log(t('m365_profile_applied', 'Profile loaded') + ': ' + profile.name);
} }
// Google-only users (id "google:<email>") 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() { function _applyPendingProfileUsers() {
const ids = window._pendingProfileUserIds; const ids = window._pendingProfileUserIds;
if (!ids || !ids.length || !S._allUsers.length) return; if (!ids || !ids.length || !S._allUsers.length) return;
// Select only the users listed in the profile const wanted = new Set(ids);
S._allUsers.forEach(u => { u.selected = ids.includes(u.id); }); // A saved google:<email> 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(); 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:<email>" 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() { async function saveCurrentAsProfile() {
@@ -365,19 +424,25 @@ function _openEditorForProfile(profile) {
const srcCheck = (id) => allSources.includes(id) ? 'checked' : ''; const srcCheck = (id) => allSources.includes(id) ? 'checked' : '';
// Build account list from S._allUsers // Build account list from S._allUsers
const savedIds = new Set((profile.user_ids || []).map(u => u.id || u)); const savedAll = profile.user_ids === 'all';
// If no saved IDs match current users, treat as all-selected (new profile or users changed) const savedList = Array.isArray(profile.user_ids) ? profile.user_ids : [];
const anyMatch = savedIds.size > 0 && S._allUsers.some(u => savedIds.has(u.id)); 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 => { const accountRows = S._allUsers.map(u => {
// Only check if the user was explicitly saved — default to unchecked like the main window // 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' ? '<span style="font-size:9px;padding:1px 5px;border-radius:10px;background:linear-gradient(90deg,#E6F1FB 50%,#EAF3DE 50%);color:#1a4a1a;font-weight:500;border:0.5px solid #b5d4b5">M365+GWS</span>' const platBadge = u.platform === 'both' ? '<span style="font-size:9px;padding:1px 5px;border-radius:10px;background:linear-gradient(90deg,#E6F1FB 50%,#EAF3DE 50%);color:#1a4a1a;font-weight:500;border:0.5px solid #b5d4b5">M365+GWS</span>'
: (u.platform || 'm365') === 'google' ? '<span style="font-size:9px;padding:1px 5px;border-radius:10px;background:#EAF3DE;color:#3B6D11;font-weight:500">GWS</span>' : (u.platform || 'm365') === 'google' ? '<span style="font-size:9px;padding:1px 5px;border-radius:10px;background:#EAF3DE;color:#3B6D11;font-weight:500">GWS</span>'
: '<span style="font-size:9px;padding:1px 5px;border-radius:10px;background:#E6F1FB;color:#185FA5;font-weight:500">M365</span>'; : '<span style="font-size:9px;padding:1px 5px;border-radius:10px;background:#E6F1FB;color:#185FA5;font-weight:500">M365</span>';
const roleBadge = u.userRole === 'student' ? t('role_student','Elev') : u.userRole === 'staff' ? t('role_staff','Ansat') : t('role_other','Anden'); 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);' : ''; const roleOverrideStyle = u.roleOverride ? 'color:var(--color-text-info);outline:1px solid var(--color-border-info);' : '';
return `<label class="pmgmt-acct-row" data-uid="${_esc(u.id)}" data-role="${_esc(u.userRole || 'other')}"><input type="checkbox" ${checked} data-uid="${_esc(u.id)}"><span style="flex:1;color:var(--color-text-primary);overflow:hidden;text-overflow:ellipsis;white-space:nowrap">${_esc(u.displayName)}</span>${platBadge}<button type="button" class="pmgmt-role-badge" data-uid="${_esc(u.id)}" onclick="_pmgmtCycleRole(this.getAttribute('data-uid'),event)" style="font-size:9px;padding:1px 5px;border-radius:10px;background:#D3D1C7;border:none;cursor:pointer;${roleOverrideStyle}">${roleBadge}</button></label>`; return `<label class="pmgmt-acct-row" data-uid="${_esc(u.id)}" data-role="${_esc(u.userRole || 'other')}"><input type="checkbox" ${checked} data-uid="${_esc(u.id)}"${manualAttrs}><span style="flex:1;color:var(--color-text-primary);overflow:hidden;text-overflow:ellipsis;white-space:nowrap">${_esc(u.displayName)}</span>${platBadge}<button type="button" class="pmgmt-role-badge" data-uid="${_esc(u.id)}" onclick="_pmgmtCycleRole(this.getAttribute('data-uid'),event)" style="font-size:9px;padding:1px 5px;border-radius:10px;background:#D3D1C7;border:none;cursor:pointer;${roleOverrideStyle}">${roleBadge}</button></label>`;
}).join(''); }).join('') + _unresolvedAccountRows(savedList);
body.innerHTML = ` body.innerHTML = `
<div> <div>
@@ -464,6 +529,30 @@ function _openEditorForProfile(profile) {
_renderEditorSources((profile.sources || []).concat(profile.google_sources || []).concat(profile.file_sources || [])); _renderEditorSources((profile.sources || []).concat(profile.google_sources || []).concat(profile.file_sources || []));
} }
// Saved accounts that aren't in S._allUsers — manually added ("manual:<email>"),
// 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 `<label class="pmgmt-acct-row" data-uid="${_esc(id)}" data-role="${_esc((typeof u === 'object' && u.userRole) || 'other')}"><input type="checkbox" checked data-uid="${_esc(id)}"${manualAttrs}><span style="flex:1;color:var(--text);overflow:hidden;text-overflow:ellipsis;white-space:nowrap" title="${_esc(id)}">${_esc(label)}</span><span style="font-size:9px;padding:1px 5px;border-radius:10px;background:#D3D1C7;color:#444441">${_esc(badge)}</span></label>`;
}).join('');
}
function _peSetDate(val) { function _peSetDate(val) {
if (!val) return; if (!val) return;
const ms = new Date() - new Date(val); const ms = new Date() - new Date(val);
@@ -592,16 +681,24 @@ function _pmgmtRoleFilter(role) {
_pmgmtFilterAccounts(document.getElementById('pmgmtAcctSearch')?.value || ''); _pmgmtFilterAccounts(document.getElementById('pmgmtAcctSearch')?.value || '');
} }
function _pmgmtAddManual() { async function _pmgmtAddManual() {
const email = prompt('E-mail adresse:'); const email = (prompt('E-mail adresse:') || '').trim();
if (!email || !email.trim()) return; if (!email) return;
const list = document.getElementById('pmgmtAcctList'); const list = document.getElementById('pmgmtAcctList');
if (!list) return; if (!list) return;
const id = 'manual:' + email.trim().toLowerCase(); // Resolve to the directory id so the scan can use it. If M365 isn't reachable
if (list.querySelector(`input[data-uid="${id}"]`)) return; // already exists // right now, fall back to "manual:<email>" — 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'); const lbl = document.createElement('label');
lbl.className = 'pmgmt-acct-row'; lbl.className = 'pmgmt-acct-row';
lbl.innerHTML = `<input type="checkbox" checked data-uid="${_esc(id)}"><span style="flex:1;color:var(--text);overflow:hidden;text-overflow:ellipsis;white-space:nowrap">${_esc(email.trim())}</span><span style="font-size:9px;padding:1px 5px;border-radius:10px;background:#D3D1C7;color:#444441">Manuel</span>`; lbl.dataset.uid = id;
lbl.dataset.role = 'other';
lbl.innerHTML = `<input type="checkbox" checked data-uid="${_esc(id)}" data-manual="1" data-name="${_esc(label)}" data-email="${_esc(d ? (d.email || email) : email)}"><span style="flex:1;color:var(--text);overflow:hidden;text-overflow:ellipsis;white-space:nowrap" title="${_esc(email)}">${_esc(label)}</span><span style="font-size:9px;padding:1px 5px;border-radius:10px;background:#D3D1C7;color:#444441">${_esc(t('m365_profile_acct_manual', 'Manual'))}</span>`;
list.appendChild(lbl); list.appendChild(lbl);
} }
@@ -635,9 +732,13 @@ async function _pmgmtSaveFullEdit() {
const effectiveGoogleSources = googleRendered ? googleSources : (profile.google_sources || []); const effectiveGoogleSources = googleRendered ? googleSources : (profile.google_sources || []);
const effectiveFileSources = fileRendered ? fileSources : (profile.file_sources || []); const effectiveFileSources = fileRendered ? fileSources : (profile.file_sources || []);
const allSources = m365Sources.concat(effectiveGoogleSources).concat(effectiveFileSources); const allSources = m365Sources.concat(effectiveGoogleSources).concat(effectiveFileSources);
const user_ids = Array.from(document.querySelectorAll('#pmgmtAcctList input[type=checkbox]:checked')) let user_ids = Array.from(document.querySelectorAll('#pmgmtAcctList input[type=checkbox]:checked'))
.map(cb => cb.dataset.uid) .filter(cb => cb.dataset.uid)
.filter(Boolean); .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 = { const updated = {
...profile, ...profile,
name, name,
@@ -769,6 +870,10 @@ window._esc = _esc;
window._pmgmtUse = _pmgmtUse; window._pmgmtUse = _pmgmtUse;
window._pmgmtOpenEditor = _pmgmtOpenEditor; window._pmgmtOpenEditor = _pmgmtOpenEditor;
window._openEditorForProfile = _openEditorForProfile; window._openEditorForProfile = _openEditorForProfile;
window._unresolvedAccountRows = _unresolvedAccountRows;
window._addProfileManualUsers = _addProfileManualUsers;
window._lookupM365User = _lookupM365User;
window._userListFinal = _userListFinal;
window._peSetDate = _peSetDate; window._peSetDate = _peSetDate;
window._peSetYear = _peSetYear; window._peSetYear = _peSetYear;
window._renderEditorSources = _renderEditorSources; window._renderEditorSources = _renderEditorSources;
+2
View File
@@ -19,6 +19,8 @@ export const S = {
_progressCurrentUser: '', _progressCurrentUser: '',
// Users // Users
_allUsers: [], _allUsers: [],
_m365UsersLoaded: false, // true once loadUsers() has fetched the directory
_googleUsersMerged: false, // true once _mergeGoogleUsers() has added Google users
// Auth // Auth
_currentAppMode: null, _currentAppMode: null,
// Profiles // Profiles
+14 -5
View File
@@ -28,6 +28,7 @@ async function loadUsers() {
u.selected = prevSelected.has(u.id) ? prevSelected.get(u.id) : false; u.selected = prevSelected.has(u.id) ? prevSelected.get(u.id) : false;
}); });
S._allUsers = [...fetched, ...toAdd]; S._allUsers = [...fetched, ...toAdd];
S._m365UsersLoaded = true;
// Apply deferred "select all" from a profile chosen before users loaded // Apply deferred "select all" from a profile chosen before users loaded
if (window._pendingProfileAllUsers) { if (window._pendingProfileAllUsers) {
S._allUsers.forEach(u => { u.selected = true; }); S._allUsers.forEach(u => { u.selected = true; });
@@ -107,6 +108,12 @@ async function _mergeGoogleUsers() {
S._allUsers = S._allUsers.concat(googleUsers); S._allUsers = S._allUsers.concat(googleUsers);
renderAccountList(); renderAccountList();
} catch(e) { /* Google users unavailable */ } } 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:<email> IDs.
S._googleUsersMerged = true;
_applyPendingProfileUsers();
}
} }
let _activeRoleFilter = ''; // '' = all, 'staff', 'student' let _activeRoleFilter = ''; // '' = all, 'staff', 'student'
@@ -415,7 +422,7 @@ async function addUserManually() {
const d = await r.json(); const d = await r.json();
if (d.error) { alert('User not found: ' + d.error); return; } 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; } 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 = ''; input.value = '';
renderAccountList(S._allUsers.filter(u => !u.manual).length <= 1); renderAccountList(S._allUsers.filter(u => !u.manual).length <= 1);
} catch(e) { } catch(e) {
@@ -441,16 +448,18 @@ function selectAllAccounts(checked) {
function getSelectedUsers() { function getSelectedUsers() {
// Only return M365 users — Google users are handled separately via selectedGoogleEmails // 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 // Respect the active role filter — hidden users must not sneak into the scan
// even if they were checked before the filter was applied. // even if they were checked before the filter was applied.
if (_activeRoleFilter) { if (_activeRoleFilter) {
selected = selected.filter(u => (u.userRole || 'other') === _activeRoleFilter); selected = selected.filter(u => (u.userRole || 'other') === _activeRoleFilter);
} }
if (selected.length) { if (selected.length) {
return selected.map(u => ({ // manual/email let a profile saved from this selection re-add manual users on apply
id: u.id, displayName: u.displayName, userRole: u.userRole || 'other' 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 // Fallback to DOM if S._allUsers not yet populated
return Array.from(document.querySelectorAll('.account-check:checked')).map(cb => ({ return Array.from(document.querySelectorAll('.account-check:checked')).map(cb => ({
+104
View File
@@ -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:<email>". 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
+74
View File
@@ -661,3 +661,77 @@ class TestProfileRoutes:
def test_delete_missing_key_returns_400(self, client): def test_delete_missing_key_returns_400(self, client):
r = client.post("/api/profiles/delete", json={}) r = client.post("/api/profiles/delete", json={})
assert r.status_code == 400 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"] == []