-
Notifications
You must be signed in to change notification settings - Fork 294
[User secret] Validate stored token belongs to authenticated user [feature/ig4-authentication] #8823
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…tication] (mlrun#8345) We are installing it directly from Test PyPI in the Dockerfile, as a temporary solution until the official version is released. Once released, it will be moved to the requirements.txt file. https://iguazio.atlassian.net/browse/ML-10486 The rest will be handled in the future as part of this story: https://iguazio.atlassian.net/browse/ML-10626.
…K method [feature/ig4-authentication] (mlrun#8370) The previous `/user-secrets` POST endpoint was never implemented and always returns HTTP 400. It will be removed and replaced with a scoped API under `/api/v1/user-secrets/tokens`, which is purpose-built for token management. removing also the related SDK method `db.create_user_secrets(...)` https://iguazio.atlassian.net/browse/ML-10452
…thentication] (mlrun#8366) In this PR: 1. Introduced a new client for IG4 on the server side. 2. Created a `BaseClient` to hold shared logic between clients. 3. Added `iguazio.v3.Client` (for IG3) and `iguazio.v4.Client` (for IG4), with the appropriate client selected based on `mlrun.mlconf.httpdb.authentication.mode`. 4. Introduced a new `AuthenticationMode` enum to represent the available authentication modes. 5. Removed the `verify_request_session` method from the sync client, as it is no longer used. 6. Moved all HTTP header keys to the `HeaderNames` constants module. 7. Introduced a new `AuthorizationHeaderPrefixes` enum for commonly used header value prefixes (e.g., Bearer, Basic). **Notes:** 1. This PR does not yet support sending requests using the Iguazio SDK package, this will be handled in a separate story/PR - https://iguazio.atlassian.net/browse/ML-10644. 2. The `iguazio.v4.Client` implementation is not yet complete, this PR only includes the initial file restructuring. https://iguazio.atlassian.net/browse/ML-10294
…tication] (mlrun#8388) This PR implements the functionality for `iguazio.v4.Client`, including: - Ensuring requests include either an `Authorization` header or an` _oauth2_proxy` cookie - Sending an async request directly to Orca to retrieve user info - Extracting the username and group IDs - Creating an AuthInfo object to pass to OPA for verification - Adding a CookieNames constant - Updating the `iguazio_client` pytest fixture to support both IG3 and IG4 clients Verification is covered by unit tests, as well as by testing on IG4 and IG3 systems https://iguazio.atlassian.net/browse/ML-10294
…-authentication] (mlrun#8440) <!-- A short summary of what this PR does. --> <!-- Include any relevant context or background information. --> This PR adds new HTTPDB SDK methods to store or update one or more token secrets in MLRun. The new methods allow sending offline token JWTs to the MLRun API, which stores each token as a user secret and returns a structured response indicating whether each token was created, updated, or skipped. --- <!-- - Key changes (e.g., added feature X, refactored Y, fixed Z) --> Added store_secret_token method to store or update a single token: `db.store_secret_token(secret_token: mlrun.common.schemas.SecretToken, log_warning: bool = True)` Added store_secret_tokens method to store or update multiple tokens: `db.store_secret_tokens(secret_tokens: list[mlrun.common.schemas.SecretToken], log_warning: bool = True)` --- - [x] I updated the documentation (if applicable) - [x] I have tested the changes in this PR --- <!-- - How it was tested (unit tests, manual, integration) --> <!-- - Any special cases covered. --> --- - Ticket link: https://iguazio.atlassian.net/browse/ML-10501 - Design docs links: https://iguazio.atlassian.net/wiki/spaces/MLRUN/pages/404521061/BE+Secret+Token+Support+HLD - External links: https://iguazio.atlassian.net/wiki/spaces/ARC/pages/361103361/MLRun+Secret+Tokens+in+IG4 --- - [ ] Yes (explain below) - [x] No <!-- If yes, describe what needs to be changed downstream: --> --- <!-- Anything else reviewers should know (follow-up tasks, known issues, affected areas etc.). --> <!-- ### 📸 Screenshots / Logs --> API side PR - mlrun#8408
…feature/ig4-authentication] (mlrun#8408) This PR introduces a new API endpoint to store or update multiple offline tokens: `PUT /api/v1/user-secrets/tokens` For each token in the request, the following validations and operations are performed: 1. Token Name Validation – Ensure each token has a non-empty and unique name within the request. 2. JWT Decoding – Decode the offline token and verify it contains the required claims: sub (user ID) and exp (expiration). 3. User Ownership Validation – Confirm that the sub in the offline token matches the authenticated user ID 4. Token Verification – Use the Iguazio SDK to validate all tokens via the `refresh_access_tokens` call. This will be enabled in a separate PR (mlrun#8443). 5. Secret Storage – (Not implemented in this PR) Creation and updating of Kubernetes secrets will be handled in a separate PR. Response Structure – Return lists of token names categorized by action: created_tokens, updated_tokens, and skipped_tokens. This functionality has been verified only via unit tests for now Additional Changes 1. Renamed `api/endpoints/secrets.py` → `api/endpoints/project_secrets.py`. 2. Extracting the user_id from Orca’s session verification endpoint and saving it in the AuthInfo https://iguazio.atlassian.net/browse/ML-10487
…ntication] (mlrun#8466) ### 📝 Description <!-- A short summary of what this PR does. --> <!-- Include any relevant context or background information. --> This PR updates the HTTP session retry logic to prevent retries for PUT requests to `/user-secrets/tokens`. --- ### 🛠️ Changes Made <!-- - Key changes (e.g., added feature X, refactored Y, fixed Z) --> 1. Added `_is_retry_put_allowed` method to determine whether retries are allowed for PUT requests. 2. Defined `NON_RETRIABLE_PATHS` to include `/user-secrets/tokens` 3. Updated HTTP session initialization to respect `retry_on_put` based on the requested path. --- ### ✅ Checklist - [x] I updated the documentation (if applicable) - [x] I have tested the changes in this PR --- ### 🧪 Testing <!-- - How it was tested (unit tests, manual, integration) --> <!-- - Any special cases covered. --> Ran unit tests to verify retry logic. Tested on an IG4 system by sending a request to `/user-secrets/tokens` and confirmed that it was sent only once (no retries). --- ### 🔗 References - Ticket link: https://iguazio.atlassian.net/browse/ML-10847 --- ### 🚨 Breaking Changes? - [ ] Yes (explain below) - [x] No <!-- If yes, describe what needs to be changed downstream: --> --- ### 🔍️ Additional Notes <!-- Anything else reviewers should know (follow-up tasks, known issues, affected areas etc.). --> <!-- ### 📸 Screenshots / Logs -->
…de [feature/ig4-authentication] (mlrun#8471) <!-- A short summary of what this PR does. --> <!-- Include any relevant context or background information. --> Enable conditional support for secret tokens based on `mlrun.httpdb.authentication.mode`. This PR restricts certain SDK methods and API endpoints so they can only be used when authentication.mode is set to `iguazio-v4`. --- <!-- - Key changes (e.g., added feature X, refactored Y, fixed Z) --> 1. Moved AuthenticationMode enum to a shared location. 2. Created a `iguazio_v4_only ` wrapper for SDK methods to control execution based on the authentication mode. 3. Added a FastAPI dependency for `user-secrets` endpoints to enforce mode-based access control. --- - [ ] I updated the documentation (if applicable) - [x] I have tested the changes in this PR --- <!-- - How it was tested (unit tests, manual, integration) --> <!-- - Any special cases covered. --> 1. Verified SDK methods fail when called outside iguazio-v4 mode. 2. Verified API user-secrets endpoints return 400 when the mode is not iguazio-v4. --- - Ticket link: https://iguazio.atlassian.net/browse/ML-10494 - Design docs links: https://iguazio.atlassian.net/wiki/spaces/MLRUN/pages/404521061/BE+Secret+Token+Support+HLD#2.1.9-Configurable-Authentication-Mode --- - [ ] Yes (explain below) - [x] No <!-- If yes, describe what needs to be changed downstream: --> --- <!-- Anything else reviewers should know (follow-up tasks, known issues, affected areas etc.). --> <!-- ### 📸 Screenshots / Logs -->
…tion] (mlrun#8443) <!-- A short summary of what this PR does. --> <!-- Include any relevant context or background information. --> Implement token verification when storing tokens by using the Iguazio SDK to validate all tokens via the `refresh_access_token` call. --- <!-- - Key changes (e.g., added feature X, refactored Y, fixed Z) --> 1. Install iguazio version 0.0.1a9 from Test PyPI. 2. The iguazio package is only supported on Python ≥ 3.11, so run `test_iguazio_v4.py` only on Python 3.11. 3. Update the test/Dockerfile to install iguazio only when Python ≥ 3.11. 4. Import `iguazio` only on Python ≥ 3.11. 5. Initialize the Iguazio client with `auto_login=False` (will not attempt to login on unauthenticated requests). 6. Implement the `refresh_access_token` method. 7. Catch errors and handle failures by raising an `Unauthorized `error. --- - [x] I updated the documentation (if applicable) - [x] I have tested the changes in this PR --- <!-- - How it was tested (unit tests, manual, integration) --> <!-- - Any special cases covered. --> Tested in the system with valid and invalid access tokens, including tokens that: - Do not belong to the user. - Are expired. --- - Ticket link: https://iguazio.atlassian.net/browse/ML-10814 - Design docs links: https://iguazio.atlassian.net/wiki/spaces/MLRUN/pages/404521061/BE+Secret+Token+Support+HLD - External links: https://iguazio.atlassian.net/wiki/spaces/ARC/pages/361103361/MLRun+Secret+Tokens+in+IG4 --- - [ ] Yes (explain below) - [x] No <!-- If yes, describe what needs to be changed downstream: --> --- <!-- Anything else reviewers should know (follow-up tasks, known issues, affected areas etc.). --> <!-- ### 📸 Screenshots / Logs -->
…-authentication] (mlrun#8484) <!-- A short summary of what this PR does. --> <!-- Include any relevant context or background information. --> This PR implements storing user tokens in Kubernetes secrets by first attempting to read the secret (by name and labels). - If the secret does not exist, it is created. - If the secret exists, the token’s expiration is checked, and the secret is updated only if the new token has a later expiration. --- <!-- - Key changes (e.g., added feature X, refactored Y, fixed Z) --> - Implemented store_user_token_secret to handle secret creation and conditional update based on expiration. - Updated k8s.read_secret to find secrets by name and verify labels. - Fixed SDK response methods to correctly report created, updated, and skipped tokens. - Adjusted SDK warnings to only log when tokens are actually stored or updated. --- - [x] I updated the documentation (if applicable) - [x] I have tested the changes in this PR --- <!-- - How it was tested (unit tests, manual, integration) --> <!-- - Any special cases covered. --> Unit tests for secret creation, update, and skip scenarios. Verified manually on IG4 system. --- - Ticket link: https://iguazio.atlassian.net/browse/ML-10492 - Design docs links: https://iguazio.atlassian.net/wiki/spaces/MLRUN/pages/404521061/BE+Secret+Token+Support+HLD#2.2.3-Token-Store-or-Update-Flow - External links: https://iguazio.atlassian.net/wiki/spaces/ARC/pages/361103361/MLRun+Secret+Tokens+in+IG4 --- - [ ] Yes (explain below) - [x] No <!-- If yes, describe what needs to be changed downstream: -->
…ication] (mlrun#8498) <!-- A short summary of what this PR does. --> <!-- Include any relevant context or background information. --> This PR implements listing user token secrets from Kubernetes for the authenticated user. The implementation uses label selectors with the username label to find relevant secrets and then validates that each secret name matches the expected format --- <!-- - Key changes (e.g., added feature X, refactored Y, fixed Z) --> 1. Added SDK method: `list_secret_tokens` 2. Added API endpoint: `GET /user-secrets/tokens` 3. Implemented `list_secrets` and `list_user_token_secrets` in Kubernetes secrets store --- - [x] I updated the documentation (if applicable) - [x] I have tested the changes in this PR --- <!-- - How it was tested (unit tests, manual, integration) --> <!-- - Any special cases covered. --> Unit tests for listing secrets. Verified manually on IG4 system. --- - Ticket link: https://iguazio.atlassian.net/browse/ML-10495, https://iguazio.atlassian.net/browse/ML-10497 - Design docs links: https://iguazio.atlassian.net/wiki/spaces/MLRUN/pages/404521061/BE+Secret+Token+Support+HLD#2.2.5-Token-Listing-Flow - External links: https://iguazio.atlassian.net/wiki/spaces/ARC/pages/361103361/MLRun+Secret+Tokens+in+IG4 --- - [ ] Yes (explain below) - [x] No <!-- If yes, describe what needs to be changed downstream: -->
…g4-authentication] (mlrun#8574) <!-- A short summary of what this PR does. --> <!-- Include any relevant context or background information. --> This PR introduces new configuration methods to better define the authentication mode in MLRun. It also relocates the `iguazio_v4_only` function wrapper to a shared location for reuse. --- <!-- - Key changes (e.g., added feature X, refactored Y, fixed Z) --> Create new configuration methods to define the authentication mode - `mlrun.mlconf.is_iguazio_mode()` - `mlrun.mlconf.is_iguazio_v4_mode()` Move the iguazio_v4_only function wrapper to a common location --- - [ ] I updated the documentation (if applicable) - [x] I have tested the changes in this PR --- <!-- - How it was tested (unit tests, manual, integration) --> <!-- - Any special cases covered. --> Existing unit tests --- - Ticket link: https://iguazio.atlassian.net/browse/ML-10983 --- - [ ] Yes (explain below) - [x] No <!-- If yes, describe what needs to be changed downstream: --> ---
…8529) Implements IGTokenProvider, so that --- 1. added new values to mlrun config: ``` "auth_with_oauth_token": { "enabled": False, "request_timeout": 5, "refresh_threshold": 0.75, "auth_token_file": "~/.igz.yml", "auth_token_name": "" }, "auth_token_endpoint": "" ``` 2. Offline Token Resolution The IGTokenProvider resolves an offline token through one of two mechanisms: a. **From Environment Variable** If `MLRUN_AUTH_OFFLINE_TOKEN` is set, it is used as the Offline Token. b. **From YAML Token File** If the environment variable is not set, the SDK attempts to load a file from the path specified `config.auth_with_oauth_token.auth_token_file` (default: `~/.igz.yml`). 2. Getting token from file get either `auth_token_name` (dynamically enriched to `default` if empty), if not found, get the first one. --- - [ ] I updated the documentation (if applicable) - [x] I have tested the changes in this PR --- Unit tests (all new functionality is covered): ``` mlrun/db/auth_utils.py 182 67 63% 34, 38, 43, 46, 49, 56-73, 76-77, 80, 83, 86-97, 100-137, 142-154, 185, 206, 250, 263, 266-267, 281, 320 ``` --- - Ticket link:https://iguazio.atlassian.net/browse/ML-10609 - Design docs links:https://iguazio.atlassian.net/wiki/spaces/MLRUN/pages/411960071/Support+sdk-side+IG4+authentication+-+token+usage+and+management+HLD --- - [ ] Yes (explain below) - [x] No <!-- If yes, describe what needs to be changed downstream: --> --- Open question: Should we fail if file has more than one tokens with the same name or should we get the first one?
…4-authentication] (mlrun#8584) ### 📝 Description <!-- A short summary of what this PR does. --> <!-- Include any relevant context or background information. --> Fix token request handling in DynamicTokenProvider. Previously, all requests used `data=request_body`, which broke flows that require `application/json`. Now, each subclass specifies the correct request format, and requests respect the configured SSL verification. --- ### 🛠️ Changes Made <!-- - Key changes (e.g., added feature X, refactored Y, fixed Z) --> Refactored `DynamicTokenProvider.fetch_token` to support both data and json payloads. Added `mlrun.mlconf.httpdb.http.verify` to enforce SSL verification for all token requests. --- ### ✅ Checklist - [ ] I updated the documentation (if applicable) - [x] I have tested the changes in this PR --- ### 🧪 Testing <!-- - How it was tested (unit tests, manual, integration) --> <!-- - Any special cases covered. --> Tested on IG4 system --- ### 🔗 References - Ticket link: - Design docs links: - External links: --- ### 🚨 Breaking Changes? - [ ] Yes (explain below) - [x] No <!-- If yes, describe what needs to be changed downstream: -->
…[feature/ig4-authentication] (mlrun#8588) ### 📝 Description <!-- A short summary of what this PR does. --> <!-- Include any relevant context or background information. --> Support both .yml and .yaml file extensions when loading secret tokens. If the .yml file is not found, the code now attempts to load a .yaml file instead. This issue occurred because the Iguazio SDK saved the file with a .yaml extension instead of .yml. Additionally, `os.path.expanduser` is now used to resolve the file path correctly. --- ### 🛠️ Changes Made <!-- - Key changes (e.g., added feature X, refactored Y, fixed Z) --> 1. Updated `read_secret_tokens_file` to support both .yml and .yaml file extensions. 2. Added `os.path.expanduse`r to expand ~ in file paths. --- ### ✅ Checklist - [x] I updated the documentation (if applicable) - [x] I have tested the changes in this PR --- ### 🧪 Testing <!-- - How it was tested (unit tests, manual, integration) --> <!-- - Any special cases covered. --> Added unit tests Tested on the IG4 system. --- ### 🔗 References - Ticket link: https://iguazio.atlassian.net/browse/ML-10609 - Design docs links: https://iguazio.atlassian.net/wiki/spaces/MLRUN/pages/411960071/Support+sdk-side+IG4+authentication+-+token+usage+and+management+HLD - External links: https://iguazio.atlassian.net/wiki/spaces/ARC/pages/361103361/MLRun+Secret+Tokens+in+IG4 --- ### 🚨 Breaking Changes? - [ ] Yes (explain below) - [x] No <!-- If yes, describe what needs to be changed downstream: --> --- ### 🔍️ Additional Notes <!-- Anything else reviewers should know (follow-up tasks, known issues, affected areas etc.). --> <!-- ### 📸 Screenshots / Logs -->
…lrun#8589) ### 📝 Description <!-- A short summary of what this PR does. --> <!-- Include any relevant context or background information. --> The SDK needs to be aware when authentication is set to Iguazio v4 in order to enable certain SDK methods (e.g., storing secrets, listing secrets, revoking, and syncing). To support this, we now pass the authentication mode value, configured on the server, to the client. --- ### 🛠️ Changes Made <!-- - Key changes (e.g., added feature X, refactored Y, fixed Z) --> Added `httpdb_authentication_mode` to the client spec schema. --- ### ✅ Checklist - [ ] I updated the documentation (if applicable) - [x] I have tested the changes in this PR --- ### 🧪 Testing <!-- - How it was tested (unit tests, manual, integration) --> <!-- - Any special cases covered. --> Verified changes on an IG4 system --- ### 🔗 References - Ticket link: https://iguazio.atlassian.net/browse/ML-10494 - Design docs links: https://iguazio.atlassian.net/wiki/spaces/MLRUN/pages/404521061/BE+Secret+Token+Support+HLD#2.1.9-Configurable-Authentication-Mode --- ### 🚨 Breaking Changes? - [ ] Yes (explain below) - [x] No <!-- If yes, describe what needs to be changed downstream: --> --- ### 🔍️ Additional Notes <!-- Anything else reviewers should know (follow-up tasks, known issues, affected areas etc.). --> <!-- ### 📸 Screenshots / Logs -->
…g4-authentication] (mlrun#8567) ### 📝 Description <!-- A short summary of what this PR does. --> <!-- Include any relevant context or background information. --> This PR implements the `mlrun.sync_secret_tokens()` method in the MLRun SDK, allowing users to synchronize local secret tokens with the backend. It reads the local token file (~/.igz.yml), validates the tokens, detects duplicates, and uploads them to the backend. Warnings are logged if any tokens are updated due to newer expiration times found locally. --- ### 🛠️ Changes Made <!-- - Key changes (e.g., added feature X, refactored Y, fixed Z) --> 1. Added sync_secret_tokens() function to the SDK. 2. `load_secret_tokens_from_file` to only read token data. 3. `validate_secret_tokens` to validate fields, detect duplicates, and return SecretToken objects. --- ### ✅ Checklist - [x] I updated the documentation (if applicable) - [x] I have tested the changes in this PR --- ### 🧪 Testing <!-- - How it was tested (unit tests, manual, integration) --> <!-- - Any special cases covered. --> Added unit tests covering valid, invalid, and duplicate token scenarios. Tested on IG4 system --- ### 🔗 References - Ticket link: https://iguazio.atlassian.net/browse/ML-10502 - Design docs links: https://iguazio.atlassian.net/wiki/spaces/MLRUN/pages/404521061/BE+Secret+Token+Support+HLD#2.2.4-Token-Sync-Flow - External links: https://iguazio.atlassian.net/wiki/spaces/ARC/pages/361103361/MLRun+Secret+Tokens+in+IG4 --- ### 🚨 Breaking Changes? - [ ] Yes (explain below) - [x] No <!-- If yes, describe what needs to be changed downstream: --> --- ### 🔍️ Additional Notes <!-- Anything else reviewers should know (follow-up tasks, known issues, affected areas etc.). --> <!-- ### 📸 Screenshots / Logs -->
…un#8623) ### 📝 Description <!-- A short summary of what this PR does. --> <!-- Include any relevant context or background information. --> Bump iguazio package to `0.0.1a10` --- ### 🛠️ Changes Made <!-- - Key changes (e.g., added feature X, refactored Y, fixed Z) --> --- ### ✅ Checklist - [ ] I updated the documentation (if applicable) - [ ] I have tested the changes in this PR --- ### 🧪 Testing <!-- - How it was tested (unit tests, manual, integration) --> <!-- - Any special cases covered. --> --- ### 🔗 References - Ticket link: - Design docs links: - External links: --- ### 🚨 Breaking Changes? - [ ] Yes (explain below) - [ ] No <!-- If yes, describe what needs to be changed downstream: --> --- ### 🔍️ Additional Notes <!-- Anything else reviewers should know (follow-up tasks, known issues, affected areas etc.). --> <!-- ### 📸 Screenshots / Logs -->
…feature/ig4-authentication] (mlrun#8612) ### 📝 Description Refresh Fallback Flow: 0. If the token refresh fails (i.e., `fetch_token()` raises an exception), retry the 1 and 2 steps up to 3 attempts: 1. Re-read the offline token from the configured file (to capture potential external updates such as rotation). 2. Retry exchanging the offline token at the token_endpoint. 3. If all refresh attempts fail, delegate handling to _post_fetch_hook: 3.1 If a cached access token is still valid (not fully expired), it will continue to be used. 3.2 If the token is expired, raise an error. --- ### ✅ Checklist - [ ] I updated the documentation (if applicable) - [x] I have tested the changes in this PR --- ### 🧪 Testing UT --- ### 🔗 References - Ticket link: https://iguazio.atlassian.net/browse/ML-10780 - Design docs links: - External links: --- ### 🚨 Breaking Changes? - [ ] Yes (explain below) - [x] No
…tication] (mlrun#8514) ### 📝 Description <!-- A short summary of what this PR does. --> <!-- Include any relevant context or background information. --> This PR introduces support for revoking user offline tokens in the Iguazio client. Revoked tokens can no longer be used to obtain access tokens. The feature includes both SDK and API support for deleting tokens and automatically deleting the associated Kubernetes secret. --- ### 🛠️ Changes Made <!-- - Key changes (e.g., added feature X, refactored Y, fixed Z) --> 1. Added SDK method: `revoke_secret_token` to revoke a user token. 2. Added API endpoint: `DELETE /user-secrets/tokens/{name}` for token revocation. 3. Integrated token revocation with Iguazio backend. 4. Retrieves and deletes the corresponding Kubernetes secret after revocation. 5. Updated `create_secret` and `update_secret` methods to accept encoded values. --- ### ✅ Checklist - [x] I updated the documentation (if applicable) - [x] I have tested the changes in this PR --- ### 🧪 Testing <!-- - How it was tested (unit tests, manual, integration) --> <!-- - Any special cases covered. --> --- ### 🔗 References - Ticket link: https://iguazio.atlassian.net/browse/ML-10499, https://iguazio.atlassian.net/browse/ML-10498 - Design docs links: https://iguazio.atlassian.net/wiki/spaces/MLRUN/pages/404521061/BE+Secret+Token+Support+HLD#2.2.6-Token-Revocation-Flow - External links: https://iguazio.atlassian.net/wiki/spaces/ARC/pages/361103361/MLRun+Secret+Tokens+in+IG4 --- ### 🚨 Breaking Changes? - [ ] Yes (explain below) - [x] No <!-- If yes, describe what needs to be changed downstream: --> ---
### 📝 Description Sync tokens after IGTokenProvider creation --- ### ✅ Checklist - [ ] I updated the documentation (if applicable) - [ ] I have tested the changes in this PR --- ### 🧪 Testing Both provider and sync flows were tested in previous PRs --- ### 🔗 References - Ticket link: https://iguazio.atlassian.net/browse/ML-11066 - Design docs links: - External links: --- ### 🚨 Breaking Changes? - [ ] Yes (explain below) - [x] No
…run#8632) ### 📝 Description <!-- A short summary of what this PR does. --> <!-- Include any relevant context or background information. --> --- ### 🛠️ Changes Made <!-- - Key changes (e.g., added feature X, refactored Y, fixed Z) --> --- ### ✅ Checklist - [ ] I updated the documentation (if applicable) - [ ] I have tested the changes in this PR --- ### 🧪 Testing <!-- - How it was tested (unit tests, manual, integration) --> <!-- - Any special cases covered. --> --- ### 🔗 References - Ticket link: - Design docs links: - External links: --- ### 🚨 Breaking Changes? - [ ] Yes (explain below) - [ ] No <!-- If yes, describe what needs to be changed downstream: --> --- ### 🔍️ Additional Notes <!-- Anything else reviewers should know (follow-up tasks, known issues, affected areas etc.). --> <!-- ### 📸 Screenshots / Logs -->
…8633) ### 📝 Description <!-- A short summary of what this PR does. --> <!-- Include any relevant context or background information. --> Sync tokens must be called only after retrieving the client spec, since the sync method is available only in IG4 mode --- ### 🛠️ Changes Made <!-- - Key changes (e.g., added feature X, refactored Y, fixed Z) --> move `mlrun.secrets.sync_secret_tokens()` after we get the client spec --- ### ✅ Checklist - [ ] I updated the documentation (if applicable) - [ ] I have tested the changes in this PR --- ### 🧪 Testing <!-- - How it was tested (unit tests, manual, integration) --> <!-- - Any special cases covered. --> --- ### 🔗 References - Ticket link: - Design docs links: - External links: --- ### 🚨 Breaking Changes? - [ ] Yes (explain below) - [ ] No <!-- If yes, describe what needs to be changed downstream: --> --- ### 🔍️ Additional Notes <!-- Anything else reviewers should know (follow-up tasks, known issues, affected areas etc.). --> <!-- ### 📸 Screenshots / Logs -->
…4-authentication] (mlrun#8667) ### 📝 Description <!-- A short summary of what this PR does. --> <!-- Include any relevant context or background information. --> When attempting to call `revoke_offline_token` in Iguazio, it fails with the error: `No authentication is available, login required when trying to revoke token.` This happens because the request requires passing the access token from the request headers to Iguazio. --- ### 🛠️ Changes Made <!-- - Key changes (e.g., added feature X, refactored Y, fixed Z) --> 1. Call `set_override_auth_headers` before revoking the token in Iguazio using the request headers. 2. Bump the Iguazio package to 0.0.1a12. --- ### ✅ Checklist - [x] I updated the documentation (if applicable) - [x] I have tested the changes in this PR --- ### 🧪 Testing <!-- - How it was tested (unit tests, manual, integration) --> <!-- - Any special cases covered. --> Tested on my ig4 system --- ### 🔗 References - Ticket link: https://iguazio.atlassian.net/browse/ML-11075 - Design docs links: - External links: --- ### 🚨 Breaking Changes? - [ ] Yes (explain below) - [x] No <!-- If yes, describe what needs to be changed downstream: --> --- ### 🔍️ Additional Notes <!-- Anything else reviewers should know (follow-up tasks, known issues, affected areas etc.). --> <!-- ### 📸 Screenshots / Logs -->
…on] (mlrun#8668) ### 📝 Description <!-- A short summary of what this PR does. --> <!-- Include any relevant context or background information. --> Adding CODEOWNERS coverage for the `mlrun/auth` folder --- ### 🛠️ Changes Made <!-- - Key changes (e.g., added feature X, refactored Y, fixed Z) --> --- ### ✅ Checklist - [ ] I updated the documentation (if applicable) - [ ] I have tested the changes in this PR --- ### 🧪 Testing <!-- - How it was tested (unit tests, manual, integration) --> <!-- - Any special cases covered. --> --- ### 🔗 References - Ticket link: - Design docs links: - External links: --- ### 🚨 Breaking Changes? - [ ] Yes (explain below) - [ ] No <!-- If yes, describe what needs to be changed downstream: --> --- ### 🔍️ Additional Notes <!-- Anything else reviewers should know (follow-up tasks, known issues, affected areas etc.). --> <!-- ### 📸 Screenshots / Logs -->
…n [feature/ig4-authentication] (mlrun#8674) ### 📝 Description <!-- A short summary of what this PR does. --> <!-- Include any relevant context or background information. --> Fix an issue when running tests for the v4 Iguazio client. Previously, instantiating the v4 client would initialize the real iguazio.Client, which reads the local igz.yaml file and attempts to refresh tokens. If this file exists on the machine, it could trigger real network requests during tests. This PR mocks the iguazio.Client using unittest.mock.MagicMock to prevent actual initialization and external calls, allowing tests to run safely in isolation. --- ### 🛠️ Changes Made <!-- - Key changes (e.g., added feature X, refactored Y, fixed Z) --> - Mocked `self._client` in the v4 client to avoid real Iguazio client initialization. - Updated the `iguazio_client` pytest fixture to apply the mock for v4 clients. --- ### ✅ Checklist - [ ] I updated the documentation (if applicable) - [x] I have tested the changes in this PR --- ### 🧪 Testing <!-- - How it was tested (unit tests, manual, integration) --> <!-- - Any special cases covered. --> --- ### 🔗 References - Ticket link: - Design docs links: - External links: --- ### 🚨 Breaking Changes? - [ ] Yes (explain below) - [x] No <!-- If yes, describe what needs to be changed downstream: --> --- ### 🔍️ Additional Notes <!-- Anything else reviewers should know (follow-up tasks, known issues, affected areas etc.). --> <!-- ### 📸 Screenshots / Logs -->
liranbg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor comment
| f"Offline token '{token_name}' subject '{token_sub}' does not match the authenticated user ID. " | ||
| "Stored tokens can only belong to the authenticated user." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont think that revealing the subject makes sense from client POV. you may log it with "warning" and raise the exception
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you say log with "warning" and then raise the exception, do you mean like this:
| f"Offline token '{token_name}' subject '{token_sub}' does not match the authenticated user ID. " | |
| "Stored tokens can only belong to the authenticated user." | |
| if token_sub != authenticated_id: | |
| mlrun.utils.logger.warning(f"Offline token '{token_name}' subject {token_sub} does not match the authenticated user ID: {authenticated_id}.") | |
| raise mlrun.errors.MLRunInvalidArgumentError( | |
| f"Offline token '{token_name}' does not match the authenticated user ID. " | |
| "Stored tokens can only belong to the authenticated user." | |
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@liranbg Please do not commit this suggestion as it will require changes in some tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@liranbg I've committed this change for now. Let me know if you think it needs to be different
…dation in single function
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
moranbental
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job!
📝 Description
During
store_secret_tokensCRUD function, validate that each token'ssubis equal to the authenticated user's id. This will prevent from user A to store user B's secret token.🛠️ Changes Made
store_secret_tokenscrud function,_validate_token_namehas been refactored to_validate_token_name_and_userthat checks if the token's sub = authenticated user id✅ Checklist
🧪 Testing
Unit test for
_validate_secret_token_nameAdded a new unit test for
store_secret_tokensfor when authenticated user is not equal to token's subRefactored existing unit tests for
store_secret_tokensto accept auth_info instead of just username🔗 References
🚨 Breaking Changes?
🔍️ Additional Notes