add script to update users country in keycloak - #172
Conversation
39773af to
fbd07ee
Compare
01558f7 to
cf549d8
Compare
cf549d8 to
9d32f37
Compare
poetry audit reportLoading... No vulnerabilities found ✨✨ |
pgetta
left a comment
pgetta
left a comment
There was a problem hiding this comment.
Approving — the two scripts are well-structured (dry-run/live with double confirmation, attribute merge follows the existing billing.py pattern, and the psycopg2 ANY(CAST(... AS uuid[])) list binding is correct). Left 4 non-blocking inline comments worth addressing before relying on these in a live run, two of which are silent data-correctness issues in write tools.
| for row in result.mappings(): | ||
| row_dict = dict(row) | ||
| oid = str(row_dict["owner_id"]) | ||
| if oid in result_map: |
There was a problem hiding this comment.
Mixed/upper-case CSV UUIDs silently skip labs. result_map is keyed by the raw CSV id (line 193), but matched against oid = str(row_dict["owner_id"]) where owner_id::text is always rendered in lowercase canonical form by Postgres. The query's = ANY(CAST(... AS uuid[])) is case-insensitive and validate_user_ids uses re.IGNORECASE, so an uppercase UUID is accepted and its lab is fetched — then dropped here because the lowercase oid doesn't match the uppercase key.
Failure: CSV row id = 5F9D…A1 (a common uppercase export format) → lab exists and is returned, but reports as NO_VLAB and is never updated — a silent skip in a data-write tool.
Fix: lowercase both the result_map keys and oid (or normalize CSV ids on load).
| return [] | ||
|
|
||
| # Strategy 1: Use regex to find all UUID-shaped strings in the input | ||
| uuid_pattern = re.compile( |
There was a problem hiding this comment.
Unanchored UUID regex truncates malformed IDs and silently drops junk tokens. Strategy 1 uses an unanchored findall and returns immediately on any match. A token with an extra trailing hex char is matched only up to 36 chars, then passes the anchored validate_user_ids.
Failure: pasting 12345678-1234-1234-1234-1234567890123 (13-digit final group, a typo) → captured as …123456789012 and accepted as valid; if that truncated UUID belongs to a different real user, the wrong user's country is overwritten in live mode.
The same early return found also discards non-UUID tokens (e.g. bad-id-typo, <valid-uuid>) before validate_user_ids runs, so the ⚠️ N invalid UUID(s) will be skipped warning never fires and the operator never learns a token was dropped. Consider anchoring the match per-token (split first, then validate each) instead of scanning the whole blob.
| ORDER BY created_at | ||
| """) | ||
|
|
||
| result_map: Dict[str, List[Dict[str, Any]]] = {uid: [] for uid in owner_ids} |
There was a problem hiding this comment.
Duplicate CSV rows cause repeated updates and inflated counts. load_csv doesn't dedupe and analyze_users iterates per row, so a repeated owner id emits the same vlab_id multiple times; apply_updates then runs one UPDATE per result.
Failure: CSV has the same id twice with a@x.com then b@x.com → the lab is updated to a@x.com then b@x.com (non-deterministic last-write-wins), and the UPDATE N confirmation prompt plus the "X updated" summary double-count the single lab. Consider deduping by id (or by resolved vlab_id) before analysis.
| padding=(1, 2), | ||
| ) | ||
| ) | ||
| scope = UpdateScope.BOTH |
There was a problem hiding this comment.
Dry-run preview doesn't match the default live run. Dry run hardcodes scope = UpdateScope.BOTH, while live mode prompts and defaults to missing_only.
Failure: operator dry-runs and sees "12 virtual labs would be updated" (missing + mismatches), then runs live and accepts the default missing_only, which changes only the 3 missing ones — the preview overstates the default action, undermining dry-run as a verification step. Consider running the analysis with the same scope the live run will use (or letting the user pick scope in dry-run too).
2 participants
Uh oh!
There was an error while loading. Please reload this page.