Repository navigation
v4.0 fixes (aggregate) - #190
Closed
aschumann-virtualcable wants to merge 86 commits into
Closed
aschumann-virtualcable wants to merge 86 commits into
aschumann-virtualcable wants to merge 86 commits into
Conversation
Updates the import logic to accept a CSV-formatted string instead of a JSON array, improving compatibility and aligning with common data exchange formats. Enhances input handling by leveraging the CSV parser for more robust processing and easier integration.
Changes the lookup for the X2GO client executable to pass a list of paths, ensuring compatibility with the expected argument type and preventing runtime errors if the path is not found. Updates signature to reflect the code adjustment.
Refreshes IP and MAC from the server model before triggering wake-on-LAN, ensuring accurate MAC usage for user-services migrated from older versions where the MAC might be missing. Also prevents WOL attempts with unknown MAC addresses to improve reliability.
Unifies method naming across services and tests for consistency and clarity by renaming all occurrences of the misspelled 'is_avaliable' to the correct 'is_available'. Prevents confusion and potential integration issues stemming from inconsistent method names.
…_logout Concurrent logout events (duplicate actor logouts, or the assigned_unused worker racing an interactive logout) could hit the same userservice. The second event saw a machine a prior logout had already returned to cache (cache_level != NONE) and called release() on it, destroying a valid L1 cache element and causing the observed "Creating cache" <-> "Removing" churn. - Serialize concurrent logouts by locking and re-reading the row (select_for_update inside transaction.atomic()); guard DoesNotExist for rows a prior event already released and cleaned. - When the machine was already handled (back in cache or no longer usable), early-return without releasing instead of destroying the cache element. - Save the cloned userservice before logging in forced_move_assigned_to_cache_l1: logging a not-yet-saved copy inserts an uds_log row with a NULL owner_id, silently swallowed before but poisoning the enclosing atomic transaction and breaking the following save(). Add regression tests covering no-cache release, already-in-cache skip, missing-row no-op, L1 overflow release, and L1 growth move-to-cache. Backport of 0cc1988 + 6a28eb9 from 5.0 (master-fix-logout-cache-race).
Some IdPs and copy-paste sources embed invisible code points in the username attribute (BOM U+FEFF, ZWSP, ZWNJ, ZWJ, soft hyphen, etc.). The result: visually identical strings reach the DB as different rows, creating duplicate user objects under the same authenticator. The chokepoint is Authenticator.get_or_create_user, which stored the username as-is via get_or_create(name=...). Strip invisible / control chars, NFKC-normalize, and trim there. SAML, OAuth2, LDAP, ticket auth, REST and web login all funnel through it. Includes a minimal test for the helper covering BOM in three positions, ZWSP / ZWNJ / ZWJ, soft hyphen, fullwidth -> ASCII via NFKC, NUL / tab, case preservation, and empty input.
… it is ready
Backport of 6c22b8b74 from 5.0 ("Fixed back to cache timing with snapshot recovery").
forced_move_assigned_to_cache_l1() marked the user service USABLE and only then
called move_to_level(), which runs the move-to-cache queue and, for snapshot pools,
the snapshot recovery operation. For the few seconds that recovery took, the machine
was already listed as a valid cache entry and could be handed out to a user.
Leave it PREPARING and let move_to_level() set the final state from the outcome of
the operation, so the machine only becomes usable once it really is.
Applies on top of dev/v4.0-new-feature-back-to-cache-snapshot-recovery: plain v4.0
does not call move_to_level() there, so the window this closes does not exist yet.
…zation Follow-up on the username normalization, fixing three problems: * Existing rows were left polluted, so normalizing only on write meant the next login no longer matched them and created a new, empty user (losing groups, permissions and assigned user services). A data migration now normalizes those names and merges the duplicates into a single row, moving user services, permissions, group memberships, logs, properties and child users to the surviving one. * The normalization ran before the invisible chars were stripped. Removing a zero width joiner between a letter and its combining mark leaves a decomposed string, which no longer matches the precomposed form the next login sends. Strip first, compose after. * NFKC folded compatibility code points into ASCII, so two distinct directory identities collapsed into a single UDS user, and one inherited the other's groups and services. Use NFC.
The normalization added to Authenticator.get_or_create_user was not applied on the other paths that look users up (or create them) by name, so a username carrying invisible code points (BOM, zero-width, soft hyphen, ...) still failed to match the -- already clean -- stored row: * Authenticator.is_user_allowed: an existing user was reported as non existing. * Handler.get_user: the REST session username did not match its user row. * Users.save_item: a name pasted from the admin UI created a polluted row again (and, as a side effect of normalizing first, a name made *only* of invisible chars is now rejected by the existing "Username cannot be empty" check). Also reduces samples/uds_dedupe_users.sql to its diagnostic query. The apply part relied on FK CASCADE to clean up the merged rows, which silently dropped their assigned user services; the 0048_dedupe_user_names migration already does the merge properly on upgrade.
Previously, get_attributes_regex_field returned the prefix as the attribute name to request from LDAP, and process_regex_field only used the prefix as a key — the value was never prefixed. For UCA's 'alumno:alumno' / 'pas:pas' / 'pdi:pdi' mappings this meant the wrong attribute was fetched and the role string was missing its prefix, so group matching silently failed. - get_attributes_regex_field: return the right-hand side of ':' as the attribute to look up. - process_regex_field: when 'prefix:attr' is used, look up 'attr' and prepend 'prefix' to each value (alumno + 'S' -> 'alumnoS'). Existing '+' concat and '**' prepend syntax are unchanged. Adds unit tests covering the colon prefix, the self-rename case, and the pre-existing syntaxes.
The group membership check ran an unbounded search, so directories with more entries than their own page size (1000 by default on Active Directory) answered sizeLimitExceeded. python-ldap raises on that result and discards the partial entries, so the test reported a raw LDAP error dict while the authenticator itself worked fine. Bound that search to LDAP_RESULT_LIMIT and treat the size limit as a pass, as the other checks already do: reaching the limit proves matching groups exist.
- Introduced `consts.py` for OpenShift constants including token validity and cache durations. - Created `exceptions.py` to define custom exceptions for OpenShift errors. - Developed `types.py` to manage OpenShift VM states, interfaces, volumes, and VM definitions. - Added `provider.py` to implement the OpenShift provider with necessary configurations and connection handling. - Implemented `publication.py` for managing OpenShift template publications and VM creation processes. - Created `service.py` to define the OpenShift service for cloning VMs and managing their lifecycle. - Added `service_fixed.py` for handling fixed VM pools in OpenShift. - Included PNG icons for provider and service representations.
…es/v4.0-snapshot-recovery
A rejected TOTP code left no trace, so a clock drift between the client and the server could not be told apart from a wrong or a replayed code. Traces the result, the valid window and the server time; the code itself is never traced.
The multiple IPs fixture included a 'localhost' entry that the test expected to be dropped because it does not resolve, and asserted a hostname obtained from a reverse lookup of 172.27.1.25-27. Both hold only on a machine whose resolver behaves like ours, so the test failed elsewhere. Same fix already applied on master.
…to v4.0-fixes # Conflicts: # server/src/uds/core/managers/userservice.py
…t-dns' into v4.0-fixes
…t' into v4.0-fixes
…il-open' into v4.0-fixes
…elete' into v4.0-fixes # Conflicts: # server/src/uds/services/Proxmox/service.py # server/tests/services/proxmox/test_client.py # server/tests/services/proxmox/test_service_linked.py
…ng' into v4.0-fixes
…re moving to cache
…o fix" This reverts commit ba4289a.
This reverts commit 1dae131.
…ability override The is_avaliable -> is_available rename is not part of v4.0, so the OpenShift services must override the base method under its v4.0 name.
Contributor
Author
|
Cierro. Vuelvo a abrir limpio sin reverts. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Aggregate branch with the v4.0 fixes, each original branch merged individually (no squash). Enterprise counterpart: VirtualCable/enterprise
dev/v4.0-enterprise-fixes.Contents
Authentication
auth-regex-colon-prefix: honour the prefix inprefix:attributeregex fields and accept the**syntax.duplicate-users: normalise user names (NFC, invisible characters) to avoid duplicated rows; migration0048merges existing duplicates.auth-exception-error-mapping: authenticator errors map to access denied.oauth2-logout-redirect: logout redirects to the OAuth2 provider logout URL.simpleldap-sizelimit: SimpleLDAP connection test no longer fails onsizeLimitExceeded.totp-validation-logging: trace TOTP validation, log only rejected codes.importcsv,importcsv-federated: user import from CSV (REST and admin GUI, rebuilt bundle).Cache, snapshot and logout
back-to-cache-timing,snapshot-recovery: a machine going back to cache stays PREPARING until the move-to-cache queue (snapshot recovery included) finishes; backport of 6c22b8b74 from 5.0.logout-cache-race: concurrent logouts on the same user service no longer destroy the valid cache element (select_for_update+ re-read).Proxmox
proxmox-publication-delete: do not delete a publication template while machines still depend on it; wait for disks to disappear before considering a VM deleted.proxmox-is-deleted-fail-open: onlyProxmoxNotFoundcounts as deleted; any other error propagates.Other
Not included (Adolfo, review 2026-10-16)
is_avaliable->is_availablerename was reverted on Adolfo's call: v4.0 keeps the old name.fixes/v4.0-openshift(the OpenShift service: provider, publication, service, deployment, types, client, icons; 2090 lines, 14 files) was reverted: it is a feature, not a fix.backport-openshift-v4.0(the 28-line follow-up to the OpenShift backport, onopenshift/client.py) was reverted: backports without a fix on top do not go into v4.0.Pending decision (AzureAD, enterprise PR)
entraid-common-endpointpins the login URL to the configured tenant, andazuread-multi-tenantaddsallowed_tenants, which needs/common. As merged, the pinned URL wins, so users from other tenants cannot log in whenallowed_tenantsis set. Options: use/commononly whenallowed_tenantshas a value, or leave both branches out of v4.0.Tests
server/testswith the enterprise links pointing atdev/v4.0-enterprise-fixes: 1035 passed, 0 failed, 175 skipped. Unit tests only; nothing has been run against a live platform (Proxmox, HyperV, AzureAD).