2 Commits
Author SHA1 Message Date
StyxX65andClaude Opus 5.5 1c79ac15b7 Scan manually added accounts
Build — Windows, Linux & macOS / GDPRScanner / linux (push) Has been cancelled
Build — Windows, Linux & macOS / GDPRScanner / macos (push) Has been cancelled
Build — Windows, Linux & macOS / GDPRScanner / windows (push) Has been cancelled
Build — Windows, Linux & macOS / Create GitHub Release (push) Has been cancelled
Manual accounts were never scanned:
- Sidebar "add user" entries had no platform, and getSelectedUsers()
  required platform === 'm365', so they were dropped from every scan.
- Profile-editor manual accounts were stored as "manual:<email>", which
  is not a Graph user id, and applying the profile never put them in the
  sidebar list, so they could not be selected.

Now the editor looks the email up via /api/users/lookup and stores the
directory id plus name/email (falling back to manual:<email> only when the
lookup fails). Applying a profile adds its manual accounts to the sidebar
and resolves legacy manual:<email> entries, logging any that can't be
found. The scan engine resolves remaining manual:<email> ids (e.g. from
scheduled scans) via the new M365Connector.lookup_user(), skipping and
reporting failures without widening to all users.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-09-28 09:27:56 +02:00
StyxX65andClaude Opus 5.5 bbe9c62ff9 Fix profiles losing their selected users
Starting an M365 scan wrote the sidebar state back into the active profile
via _save_settings(), replacing its user_ids with the sidebar selection
(minus Google-only users and anything hidden by the role filter). With no
profile active it overwrote profiles[0]. Profiles are loaders, so scan
start no longer writes them; profile_id is audit-only.

The profile editor built its account list from S._allUsers only, so
manually added accounts, users not yet loaded, and users gone from the
directory were not rendered and got dropped on the next save. They now
render as checked "Manual" / "Not loaded" rows, and user_ids "all" is kept
when saved before users load.

Applying a profile cleared the pending user IDs before the un-awaited
Google merge had added google:<email> users, leaving them unticked.
Pending IDs are now kept until all are matched or the user list is final,
and _mergeGoogleUsers() re-applies them. A saved google:<email> also
matches an M365 user merged to platform 'both'.

Adds regression tests for scan start leaving profiles untouched.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-09-28 09:22:32 +02: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"] == []