Update REST API, RBAC, and metadata handling for improved security and clarity#82
Merged
jlegrand62 merged 48 commits intodevfrom Feb 3, 2026
Merged
Update REST API, RBAC, and metadata handling for improved security and clarity#82jlegrand62 merged 48 commits intodevfrom
jlegrand62 merged 48 commits intodevfrom
Conversation
- Update `session.headers` to include Authorization header with JWT token upon successful login.
- Added `size` and `base64` query parameters for image endpoints, allowing thumbnail, large, or original images with optional Base64 JSON output. - Extended point cloud and ground‑truth endpoints with `size` (preview/orig/voxel) and `coords` flags to return point coordinates as JSON. - Added mesh endpoint support for `coords` flag to expose vertices and triangles in JSON. - Updated MIME type handling with `mimetypes` and incorporated `pybase64` for Base64 encoding. - Refactored documentation and cleaned up deprecated comments.
- Include `pybase64` in `src/server/pyproject.toml` to support Base64 image encoding.
- Update POST handler signatures to accept `**kwargs` and forward them to underlying DB operations. - Pass `**kwargs` to `scan.set_metadata`, `scan.create_fileset`, `fileset.create_file`, `fileset.delete_file`, and `file.set_metadata` calls. - Adjust method definitions for file‑level and scan‑level metadata updates to propagate `**kwargs` to the appropriate underlying functions.
- Implement `has_role` to verify a user’s role membership - Add explanatory docstring to `get_guest_user` - Introduce `can_create_user` that checks the `MANAGE_USERS` permission for user creation privileges
- Update comment to correctly describe username availability check instead of “login” in user creation logic.
- Always set owner, timestamps, and creator on new scans/filesets - Add authentication context and permission checks to user‑creation, group deletion, and metadata modification - Enrich error messages with resource identifiers - Wrap all metadata updates, file creations, deletions, and imports in exclusive locks - Log detailed operation summaries including user and resource IDs - Propagate **kwargs to REST‑API helpers for proper user lookup and permissions - Update logging and error handling to be consistent across scan, fileset, and file APIs
- Standardize docstring formatting for all methods in `rbac.py`. - Remove redundant and inconsistent comments for better clarity. - Reorganize `can_create_group` logic to align with `can_manage_groups` permission. - Simplify admin role checks in group management functions (`can_add_to_group`, `can_delete_group`). - Ensure `create_group` uses `can_manage_groups` for permission validation. - Update role/permission summaries to include additional context like ownership and shared groups.
- Standardize references throughout `session.py` from `JWT` to `JSON Web Token` for clarity. - Update variable names, comments, log messages, and docstrings to reflect the terminology change. - Refine function signatures and parameters, replacing `jwt_token` with `token` for consistency. - Correct minor grammatical inconsistencies in comments for enhanced readability and uniformity.
…EST API methods - Add `requires_jwt` decorator to `post` methods for enhanced security and token validation. - Update method signatures in `rest_api.py` to include `**kwargs` and propagate them to relevant database operations (`create_user`, `logout`, `get_user_data`). - Replace `jwt_token` with `token` in function calls for consistency. - Refactor logout logic to rely on `kwargs` and use `db.logout` for session invalidation. - Clean up redundant comments and improve clarity in error handling and docstrings.
- Updated `can_modify_scan_owner` to require ``Permission.MANAGE_USERS`` instead of admin‑only. - Removed legacy `can_access_scan_by_owner` method that lacked group sharing support. - Fixed grammar and clarified docstrings for `can_modify_scan_sharing`, `can_modify_scan`, and related methods.
- In `src/commons/plantdb/commons/fsdb/metadata.py`, the `metadata` helper now logs a warning when a key `data` is set to `None` instead of raising an `IOError`.
- The warning message is generated by `logger.warning(f"Metadata key '{data}' was set to `None`!")`, preserving the operation flow while alerting developers to the missing value.
- Import `Any` from `typing` and add it to relevant type annotations. - Annotate all metadata loader and storer helpers with explicit return types. - Update the warning logic for `None` values remains unchanged, but the function signatures now reflect their return types.
- Introduced improved error messaging with resource-specific identifiers for better debugging. - Updated permission validation logic for scan creation and deletion, ensuring stricter checks on user roles. - Made docstrings consistent with refined terminology and updated type hints across core functions. - Improved locking mechanisms by wrapping metadata updates, scans, and fileset operations in exclusive locks. - Refactored RBAC methods for better clarity and alignment with security best practices.
- In **`src/server/plantdb/server/rest_api.py`** replace all `plantdb_url()` calls in docstring examples with `plantdb_url('localhost', port=5000)`.
- Add a login request example that obtains an `access_token` and stores it in a `token` variable.
- Show how to include `headers={'Authorization': 'Bearer ' + token}` in the file upload example.
- Update calls that previously used `self.db.get_scan(scan_id)` and `file.write_raw(...)` to pass `**kwargs` (e.g., `self.db.get_scan(scan_id, **kwargs)` and `file.write_raw(..., **kwargs)`).
- Adjust related example URLs for metadata retrieval and updates to use the new host/port form.
- Introduced `rank` property on `Role` to expose a numeric hierarchy (READER=1, CONTRIBUTOR=2, ADMIN=3) - Added `can_assign(target_role)` method to determine if a role can assign another role based on rank comparison - Updated docstrings in `src/commons/plantdb/commons/auth/models.py` with usage examples for `rank` and `can_assign`
- Add tests for `Permission` constants and string values - Add tests for `Role` constants, permissions set, rank ordering, and `can_assign` logic - Add tests for `User` serialization (`to_dict`, `from_dict`), JSON conversion, lock state checks, and failed‑attempt tracking - Add tests for `Group` add/remove user functionality, duplicate prevention, and `has_user` checks - Create new test file `src/commons/tests/test_auth_models.py` containing all tests above.
- Delete extensive test suite for `Permission`, `Role`, `User`, and `Group` that was previously in `src/commons/tests/test_auth.py`. - Consolidate all auth model tests into `src/commons/tests/test_auth_models.py`.
- Simplify active‑session check by using a truthy `username` instead of `is not None`. - Expand the docstring for `session_username` to explain validation, parameters, and return value. - Introduce new method `session_token(username)` that cleans up expired sessions and returns the active session id for the given `username` or `None` if no active session exists.
- Revise `User` class docstring for clarity and consistency. - Add new optional attributes ``last_failed_attempt`` and ``password_last_change`` with defaults. - Update type hints for ``permissions``, ``last_login``, ``is_active``, ``failed_attempts``, and ``locked_until``. - Update docstrings for ``__eq__``, ``to_dict``, ``from_dict``, ``to_json``, and ``from_json`` methods. - Adjust `Group` class docstring formatting and role mention.
- Convert multiline docstrings in `src/commons/plantdb/commons/auth/manager.py` to single‑line format for consistency.
- Update the welcome log message to ``f"Welcome {fullname}, please log in...'``.
- Adjust the wording in the `_ensure_admin_user` docstring to read “used internally in the class”.
- Remove extraneous blank lines and normalize comment spacing throughout the file.
- No changes to runtime behavior, only documentation formatting.
- In `FSDB.disconnect` remove the temporary database directory when the instance was created by `dummy_db` (flagged by `_is_dummy`). - Add a cleanup block that uses `shutil.rmtree` and logs success or warning. - In `test_database.py` set `db._is_dummy = True` for dummy DBs so the cleanup logic is exercised. - Update `dummy_db` documentation to show that it logs in as the `admin` user and to verify that `db.path().exists()` is `False` after disconnect.
- Rename `create_group` signature from `name: str` to `group_name: str` and update all docstring references accordingly. - Add informative logs for permission checks and actions: error logs when permission is denied, info logs for successful creations, and warning logs for removals and deletions. - Update return‑value documentation to use code fences and clarify that `None` indicates a denied permission. - Adjust `add_user_to_group` and `remove_user_from_group` docstrings to list ``True`` and ``False`` outcomes with proper formatting. - Ensure all log messages reference the correct `group_name` variable and user names consistently.
- Added `get_logged_username` function to determine the current user from `SingleSessionManager`, `JWTSessionManager`, or `SessionManager`. - Updated `require_authentication` decorator to call the new helper and inject the username into the wrapped method. - Refactored `login` to block concurrent logins under a `SingleSessionManager`, log informative messages, and use clearer wording. - Renamed `create_user` parameter from `username` to ``new_username`` to avoid shadowing. - Adjusted related docstrings and log messages in `src/commons/plantdb/commons/fsdb/core.py`.
- Add doctest-style examples to `validate_user`, `login`, `logout`, `create_user`, `get_guest_user`, `get_username`, `get_user_data`. - Add examples to group‑management functions `create_group`, `add_user_to_group`, `remove_user_from_group`, `delete_group`, `list_groups`. - Show how to use `dummy_db()` for testing, including login/logout logs and expected return values.
- Cleaned up and tightened docstring comments, adding backticks around variable names and file references
- Re‑formatted error messages and permission checks for consistency and clarity
- Adjusted directory structure description to use the correct `${FSDB.basedir}` reference
- Fixed minor punctuation and spacing issues in the `require_connected_db` decorator and related methods
- Updated logger messages to use multi‑line strings where appropriate and to correctly reference usernames and IDs
- Minor text corrections in `FSDB.disconnect` and other permission‑related error messages
- Overall: no functional changes, only cosmetic and readability improvements.
No functional changes.
- Allow `session_manager` keyword argument in `dummy_db` and forward it to `FSDB` constructor. - Default to `None` to preserve backward compatibility with existing calls.
- Pass a `JWTSessionManager` initialized with `jwt_key` to the `test_database` function for both empty and populated test database scenarios. - Update `test_database` calls to include the new `session_manager` keyword argument, enabling JWT authenticated sessions during testing.
- Replace “JWT token” with “JSON Web Token” in `rest_api.py` and CLI documentation. - Minor wording fixes in docstrings and log messages (e.g., “A logger instance…” and “The path to the created test database.”). - No functional changes.
- Change login doctest to use `admin`/`admin` credentials instead of `anonymous`/`AlanMoore`. - Update missing credentials example to reflect the new default user `admin`. - Adjust metadata examples to show owner as `guest` instead of `anonymous`. - Reflect the updated owner in the POST metadata doctest.
- Update `src/server/plantdb/server/rest_api.py` to call `self.db.create_user` with `new_username`, `fullname`, and `password` extracted via `data.pop`. - Adjust the inline comment to match the new wording for clarity.
…idation - Update doctest examples in `src/server/plantdb/server/rest_api.py` for registering a user, logging in, and checking user existence. - Add a step‑by‑step example for admin login, user creation, and subsequent login. - Extend the `Logout` endpoint docstring with a full usage example. - Enhance the `TokenValidation` endpoint docstring to show how to validate a JWT. - Adjust inline comments and wording for clarity across the auth‑related endpoints.
- Update `TokenRefresh.__init__` in `rest_api.py` to accept a `db` parameter and assign it to `self.db` (previously `None`). - Extend the `post` method docstring to describe refreshing a **JSON Web Token** and add doctest examples demonstrating a full token‑refresh workflow. - Replace the brief “Refresh JWT token.” comment with a detailed description and example usage section. - Preserve the `requires_jwt` decorator behavior and token extraction logic.
…refresh - In `src/commons/plantdb/commons/auth/session.py`, the guard that logged a warning and returned `None` when `self._user_has_session(username)` was true has been removed from `create_session`. - The same guard has been removed from `refresh_session` (`_validate_jwt`), allowing token refresh attempts without an early exit. - The code now relies on the existing concurrency limit check to enforce session policy.
No functional changes.
No functional changes.
- Introduce `create_user` function in `src/client/plantdb/client/api_endpoints.py` that returns the registration URL path `/register`. - Include comprehensive docstring and example usage.
- Implemented `create_user` in `plantdb_client.py` to post user data to the `/register` endpoint, returning a boolean success flag and detailed error logging. - Replaced explicit `status_code == 200` checks with the more idiomatic `response.ok` in authentication, logout, and token refresh methods. - Updated imports to place `api_endpoints` after other imports for clarity. - Added comprehensive docstrings, error handling for non‑OK responses, and exception logging in `create_user`. - Adjusted file `plantdb_client.py` accordingly.
|
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
- Update logger in `manager.py` to reference the `username` variable instead of the `user` object when reporting a locked account.
- Apply `@requires_jwt` to all `get` and `post` handlers that access or modify scans. - Extend method signatures to accept `**kwargs` and forward them to `self.db.get_scan`, `self.db.get_scan`, and `self.db.create_scan`. - Re‑order decorators for the ZIP‑upload `post` method to keep `@requires_jwt` after `@rate_limit`.
…ples - Change return types of `request_login`, `request_check_username`, `request_logout`, `request_new_user`, `request_scan_names_list`, and `request_scans_info` from `dict`/`bool` to `requests.Response`. - Update docstrings and examples to show callers using `.json()` or `.ok` on the returned `Response`. - Remove redundant `.json()` and `.ok` checks from the functions; the caller now handles the response. - Adjust example code in `rest_api.py` to match new behavior and demonstrate correct usage.
Include a `session_token` argument and description in `request_scan_data`, `request_scans_info`, `request_scans_meta`, `request_scans_download`, and `request_scans_data`.
- Introduce `default_user` keyword argument in the wrapper for `get_logged_username` - Pass `default_user` and `token` to `get_logged_username` in the correct order - Update the `core.py` docstring to show usage of `default_user='guest'` - Minor refactor to kwargs handling for clarity.
- Added new decorator `add_jwt_from_header` that extracts the JWT from the request header and passes it via `kwargs['token']`. - Replaced all `@requires_jwt` annotations in `rest_api.py` with `@add_jwt_from_header`. - Simplified decorator logic: token presence and validity are now handled by the database layer. - Updated docstrings and example code to use `response.json()` instead of manual `json.loads` and removed redundant `import json` statements.
ArthurLuciani2
approved these changes
Feb 2, 2026
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
2 participants
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.
Summary of Changes
Improved REST API functionality
requires_jwtdecorator for enhanced token validation.size,base64, andcoordsquery parameters for image, mesh, and point cloud endpoints.**kwargsacross REST API methods, ensuring better parameter handling and user context.Refined RBAC logic and documentation
can_modify_scan_ownerand removed outdated methods.Enhanced metadata handling
None), added exclusive locks for updates to scans, filesets, and metadata.General improvements
pybase64for Base64 encoding in server dependencies.Additional Notes
These changes aim to enhance security, improve clarity, and standardize functionality across the codebase. Future work could further extend testing and optimize specific API operations.