Keep Google-only accounts out of the M365 scan
Graph returned 404 for /users/google:<email>/mailFolders: Google- Workspace-only accounts (id "google:<email>") reached the M365 scan. - getSelectedUsers() fell back to "every ticked checkbox in the DOM" whenever no M365 account was ticked — including Google-only rows. The fallback is now only used before the user list has loaded, and never includes google: rows. - startScan() skips the M365 part (with a log line) when M365 sources are ticked but no M365 accounts are: the server treats an empty account list as "every user in the tenant". - run_scan: _resolve_manual_users -> _resolve_scan_users, which now also drops google: ids (reported as a phase message). Covers scheduled scans and profiles that already contain Google-only accounts. Never widens to all users. Tests: 3 new in tests/test_manual_users.py; the run_scan one fails on the old engine with exactly the reported Graph call. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
b60c4a5c47
commit
b5476228ba
8 files changed
+97
-17
No files matched your search
@@ -260,6 +260,7 @@
|
||||
"m365_admin_note": "Viser kun din konto. For at liste alle brugere skal en administrator give samtykke til <strong>User.Read.All</strong> i Azure Portal, eller tilføj konti manuelt nedenfor.",
|
||||
"m365_btn_scan": "Scan",
|
||||
"m365_btn_stop": "Stop",
|
||||
"m365_no_m365_accounts": "Ingen Microsoft 365-konti valgt — Microsoft 365-kilderne springes over.",
|
||||
"m365_btn_stopping": "Stopper…",
|
||||
"m365_scan_stopping": "Stopper scanning — færdiggør det aktuelle element…",
|
||||
"m365_scan_stopped": "Scanning stoppet.",
|
||||
@@ -517,6 +518,7 @@
|
||||
"m365_filter_special_only": "Art. 9 følsomme data",
|
||||
"m365_badge_special": "Art.9",
|
||||
"m365_phase_scanning": "Scanner…",
|
||||
"m365_skipped_google_accounts": "Sprang {n} konto(er) over, der kun findes i Google Workspace — ikke i Microsoft 365",
|
||||
"a30_special_cat": "Særlige kategorier (artikel 9)",
|
||||
"a30_special_cat_note": "Disse elementer indeholder helbredsoplysninger, straffeoplysninger, biometriske data, religiøse, etniske, fagforeningsmæssige, politiske eller seksuelle oplysninger. Et eksplicit retsgrundlag (art. 9, stk. 2) og eventuelt en konsekvensanalyse (art. 35) er påkrævet.",
|
||||
"a30_col_special": "Art. 9",
|
||||
|
||||
@@ -260,6 +260,7 @@
|
||||
"m365_admin_note": "Es wird nur Ihr Konto angezeigt. Um alle Benutzer aufzulisten, muss ein Administrator <strong>User.Read.All</strong> im Azure Portal genehmigen oder Konten manuell unten hinzufügen.",
|
||||
"m365_btn_scan": "Scannen",
|
||||
"m365_btn_stop": "Stop",
|
||||
"m365_no_m365_accounts": "Keine Microsoft 365-Konten ausgewählt — Microsoft 365-Quellen werden übersprungen.",
|
||||
"m365_btn_stopping": "Wird gestoppt…",
|
||||
"m365_scan_stopping": "Scan wird gestoppt — aktuelles Element wird abgeschlossen…",
|
||||
"m365_scan_stopped": "Scan gestoppt.",
|
||||
@@ -517,6 +518,7 @@
|
||||
"m365_filter_special_only": "Art. 9 besondere Kategorien",
|
||||
"m365_badge_special": "Art.9",
|
||||
"m365_phase_scanning": "Scanne…",
|
||||
"m365_skipped_google_accounts": "{n} reine Google Workspace-Konten übersprungen — nicht in Microsoft 365",
|
||||
"a30_special_cat": "Besondere Kategorien (Artikel 9)",
|
||||
"a30_special_cat_note": "Diese Elemente enthalten Gesundheits-, Straf-, biometrische, religiöse, ethnische, gewerkschaftliche, politische oder sexuelle Daten. Eine ausdrückliche Rechtsgrundlage (Art. 9 Abs. 2) und ggf. eine DSFA (Art. 35) sind erforderlich.",
|
||||
"a30_col_special": "Art. 9",
|
||||
|
||||
@@ -260,6 +260,7 @@
|
||||
"m365_admin_note": "Only showing your account. To list all users, an admin must grant <strong>User.Read.All</strong> consent in Azure Portal, or add accounts manually below.",
|
||||
"m365_btn_scan": "Scan",
|
||||
"m365_btn_stop": "Stop",
|
||||
"m365_no_m365_accounts": "No Microsoft 365 accounts selected — skipping the Microsoft 365 sources.",
|
||||
"m365_btn_stopping": "Stopping…",
|
||||
"m365_scan_stopping": "Stopping scan — finishing the current item…",
|
||||
"m365_scan_stopped": "Scan stopped.",
|
||||
@@ -517,6 +518,7 @@
|
||||
"m365_filter_special_only": "Art. 9 special category",
|
||||
"m365_badge_special": "Art.9",
|
||||
"m365_phase_scanning": "Scanning…",
|
||||
"m365_skipped_google_accounts": "Skipped {n} Google Workspace-only account(s) — not in Microsoft 365",
|
||||
"a30_special_cat": "Art. 9 special category items",
|
||||
"a30_special_cat_note": "These items contain health, criminal, biometric, religious, ethnic, trade union, political, or sexual orientation data. An explicit legal basis (Art. 9(2)) and possibly a DPIA (Art. 35) is required.",
|
||||
"a30_col_special": "Art. 9",
|
||||
|
||||
+21
-6
@@ -42,18 +42,27 @@ except ImportError:
|
||||
from routes import state as _state
|
||||
|
||||
_MANUAL_PREFIX = "manual:"
|
||||
_GOOGLE_PREFIX = "google:"
|
||||
|
||||
|
||||
def _resolve_manual_users(conn, user_ids: list[dict]) -> list[dict]:
|
||||
"""Resolve profile accounts saved as "manual:<email>" to their directory id.
|
||||
def _resolve_scan_users(conn, user_ids: list[dict]) -> list[dict]:
|
||||
"""Turn the requested accounts into Graph user ids for the M365 scan.
|
||||
|
||||
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".
|
||||
- "manual:<email>" (older profile-editor saves, or saves made while M365
|
||||
was not connected) is looked up to get the directory id.
|
||||
- "google:<email>" is a Google-Workspace-only account: it has no M365
|
||||
mailbox/drive and Graph 404s on it. Profiles and older UI selections can
|
||||
contain them, so they are dropped here (the Google scan covers them).
|
||||
Accounts that can't be used are reported and skipped — never widened to
|
||||
"all users".
|
||||
"""
|
||||
resolved, seen = [], set()
|
||||
skipped_google = []
|
||||
for u in user_ids:
|
||||
uid = u.get("id", "")
|
||||
if uid.startswith(_GOOGLE_PREFIX):
|
||||
skipped_google.append(uid[len(_GOOGLE_PREFIX):])
|
||||
continue
|
||||
if uid.startswith(_MANUAL_PREFIX):
|
||||
upn = uid[len(_MANUAL_PREFIX):]
|
||||
try:
|
||||
@@ -69,6 +78,12 @@ def _resolve_manual_users(conn, user_ids: list[dict]) -> list[dict]:
|
||||
continue
|
||||
seen.add(u["id"])
|
||||
resolved.append(u)
|
||||
if skipped_google:
|
||||
logger.info("[run_scan] skipped %d Google-only account(s) in M365 scan", len(skipped_google))
|
||||
broadcast("scan_phase", {"phase": LANG.get(
|
||||
"m365_skipped_google_accounts",
|
||||
"Skipped {n} Google Workspace-only account(s) — not in Microsoft 365").replace(
|
||||
"{n}", str(len(skipped_google)))})
|
||||
return resolved
|
||||
|
||||
|
||||
@@ -670,7 +685,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)
|
||||
user_ids = _resolve_scan_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)
|
||||
|
||||
+1
-1
@@ -91,7 +91,7 @@ Never revert to `!!window._googleConnected` / `_fileSources.length > 0` — thos
|
||||
- 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".
|
||||
- Server side, `scan_engine._resolve_scan_users()` resolves any remaining `manual:<email>` (e.g. from scheduled scans) via `conn.lookup_user()`, and drops `google:<email>` (Google-only accounts, which Graph 404s on). 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`.
|
||||
- **`copyTokenLink` is async** — called from `onclick` as fire-and-forget. Do not make it synchronous.
|
||||
|
||||
+9
-3
@@ -569,7 +569,13 @@ function startScan(resume) {
|
||||
document.getElementById('resumeBanner').style.display = 'none';
|
||||
document.getElementById('logPanel').innerHTML = '<div class="log-line log-live" id="logLive" style="display:none"></div>';
|
||||
try { sessionStorage.removeItem(_LOG_SESSION_KEY); } catch(e) {}
|
||||
S._m365ScanRunning = sources.length > 0;
|
||||
// The M365 scan needs explicit accounts — the server treats an empty list as
|
||||
// "every user in the tenant". With only Google accounts ticked, skip it.
|
||||
const runM365 = sources.length > 0 && user_ids.length > 0;
|
||||
if (sources.length && !runM365) {
|
||||
log(t('m365_no_m365_accounts', 'No Microsoft 365 accounts selected — skipping the Microsoft 365 sources.'), 'warn');
|
||||
}
|
||||
S._m365ScanRunning = runM365;
|
||||
S._googleScanRunning = googleSources.length > 0;
|
||||
S._fileScanRunning = fileSources.length > 0;
|
||||
S._srcPct = { m365: 0, google: 0, file: 0 };
|
||||
@@ -613,8 +619,8 @@ function startScan(resume) {
|
||||
}
|
||||
|
||||
setTimeout(() => {
|
||||
// Fire M365 scan if any M365 sources are selected
|
||||
if (sources.length > 0) {
|
||||
// Fire M365 scan if any M365 sources and accounts are selected
|
||||
if (runM365) {
|
||||
fetch('/api/scan/start', {
|
||||
method: 'POST', headers: {'Content-Type':'application/json'},
|
||||
body: JSON.stringify({sources, user_ids, options, resume: !!resume,
|
||||
|
||||
+9
-2
@@ -461,8 +461,15 @@ function getSelectedUsers() {
|
||||
{ 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 => ({
|
||||
// Once the user list is loaded it is authoritative: no ticked M365 users means
|
||||
// none. (Falling back to the DOM here collected every ticked checkbox —
|
||||
// including Google-only "google:<email>" rows — and sent them to the M365
|
||||
// scan, where Graph 404s on them.)
|
||||
if (S._allUsers.length) return [];
|
||||
// Fallback to DOM only while S._allUsers is not yet populated
|
||||
return Array.from(document.querySelectorAll('.account-check:checked'))
|
||||
.filter(cb => !String(cb.dataset.id || '').startsWith('google:'))
|
||||
.map(cb => ({
|
||||
id: cb.dataset.id, displayName: cb.dataset.name, userRole: cb.dataset.role || 'other'
|
||||
}));
|
||||
}
|
||||
|
||||
@@ -37,7 +37,7 @@ 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, [
|
||||
out = scan_engine._resolve_scan_users(conn, [
|
||||
{"id": "manual:ann@school.dk", "displayName": "manual:ann@school.dk", "userRole": "other"},
|
||||
])
|
||||
assert out == [{"id": "guid-ann", "displayName": "Ann Andersen", "userRole": "other"}]
|
||||
@@ -48,12 +48,12 @@ class TestResolveManualUsers:
|
||||
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 scan_engine._resolve_scan_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, [
|
||||
out = scan_engine._resolve_scan_users(conn, [
|
||||
{"id": "guid-1", "displayName": "One"},
|
||||
{"id": "manual:ghost@school.dk", "displayName": "ghost"},
|
||||
])
|
||||
@@ -63,7 +63,7 @@ class TestResolveManualUsers:
|
||||
|
||||
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, [
|
||||
out = scan_engine._resolve_scan_users(conn, [
|
||||
{"id": "guid-ann", "displayName": "Ann"},
|
||||
{"id": "manual:ann@school.dk", "displayName": "ann"},
|
||||
])
|
||||
@@ -73,10 +73,56 @@ class TestResolveManualUsers:
|
||||
# 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(
|
||||
assert scan_engine._resolve_scan_users(
|
||||
conn, [{"id": "manual:x@school.dk", "displayName": "x"}]) == []
|
||||
|
||||
|
||||
class TestGoogleOnlyAccountsDropped:
|
||||
"""Regression: google:<email> ids reached the M365 scan and Graph 404'd
|
||||
("/users/google:anne@school.dk/mailFolders")."""
|
||||
|
||||
def test_google_only_accounts_are_dropped_and_reported(self, broadcasts):
|
||||
conn = _Conn({})
|
||||
out = scan_engine._resolve_scan_users(conn, [
|
||||
{"id": "guid-1", "displayName": "One"},
|
||||
{"id": "google:anne@school.dk", "displayName": "Anne"},
|
||||
{"id": "google:appleadmin@adm.school.dk", "displayName": "Apple Admin"},
|
||||
])
|
||||
assert out == [{"id": "guid-1", "displayName": "One"}]
|
||||
assert conn.lookups == []
|
||||
phases = [d["phase"] for e, d in broadcasts if e == "scan_phase"]
|
||||
assert phases and "2" in phases[0]
|
||||
|
||||
def test_only_google_accounts_gives_empty_list_not_all_users(self, broadcasts):
|
||||
assert scan_engine._resolve_scan_users(_Conn({}), [
|
||||
{"id": "google:anne@school.dk", "displayName": "Anne"}]) == []
|
||||
|
||||
def test_run_scan_never_calls_graph_for_google_ids(self, broadcasts, monkeypatch):
|
||||
from routes import state
|
||||
monkeypatch.setattr(scan_engine, "DB_OK", False)
|
||||
monkeypatch.setattr(scan_engine, "_load_checkpoint", lambda key, **kw: None)
|
||||
monkeypatch.setattr(scan_engine, "_save_checkpoint", lambda *a, **kw: None)
|
||||
monkeypatch.setattr(scan_engine, "_clear_checkpoint", lambda **kw: None)
|
||||
monkeypatch.setattr(state, "flagged_items", [])
|
||||
conn = MagicMock()
|
||||
conn.is_app_mode = True
|
||||
conn.list_all_mail_folders_for.return_value = []
|
||||
monkeypatch.setattr(state, "connector", conn)
|
||||
import sse
|
||||
saved = sse._current_scan_id
|
||||
try:
|
||||
scan_engine.run_scan({"sources": ["email"], "options": {}, "user_ids": [
|
||||
{"id": "google:anne@school.dk", "displayName": "Anne"},
|
||||
{"id": "guid-1", "displayName": "One"},
|
||||
]})
|
||||
finally:
|
||||
sse._current_scan_id = saved
|
||||
called_ids = {c.args[0] for c in conn.list_all_mail_folders_for.call_args_list}
|
||||
assert "guid-1" in called_ids
|
||||
assert not any(str(i).startswith("google:") for i in called_ids)
|
||||
conn.list_users.assert_not_called() # never widened to the whole tenant
|
||||
|
||||
|
||||
class TestLookupRoute:
|
||||
@pytest.fixture()
|
||||
def client(self):
|
||||
|
||||
Reference in new issue
Block a user