🔒️ Enhance security of API keys 🗃️ #7085
Merged
Conversation
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
giancarloromeo
added
t:maintenance
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7085 +/- ##
=======================================
Coverage 87.60% 87.60%
=======================================
Files 1758 1758
Lines 68099 68116 +17
Branches 1124 1124
=======================================
+ Hits 59660 59676 +16
- Misses 8130 8131 +1
Partials 309 309
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the security of API keys by hashing API secrets using Blowfish (with 10 iterations) and refactors API key creation/deletion flows across multiple services and tests.
- Updates API key creation and deletion RPC interfaces and repository functions.
- Adjusts related unit tests and migration scripts to support the new security standards.
- Refactors director-v2 and dynamic sidecar modules to use the updated API key logic.
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| services/web/server/tests/unit/with_dbs/01/test_api_keys_rpc.py | Update test calls to use the new delete_api_key_by_key and convert api_key ids with IDStr. |
| services/web/server/tests/unit/with_dbs/01/test_api_keys.py | Remove legacy test for get_or_create_api_key. |
| services/web/server/src/simcore_service_webserver/api_keys/_service.py | Refactor API key creation/deletion functions to use new helper functions. |
| services/web/server/src/simcore_service_webserver/api_keys/_repository.py | Update API key insertion to hash the secret and add new deletion and listing functions. |
| services/web/server/src/simcore_service_webserver/api_keys/_controller_rpc.py | Update exposed RPC endpoints to reflect new method names. |
| services/director-v2/** | Refactor API key creation and deletion logic to use new functions in substitutions, _api_auth, and _api_auth_rpc. |
| services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_events_utils.py | Adjust dynamic sidecar cleanup to generate and delete API keys using new helpers. |
| services/api-server/tests/unit/_with_db/conftest.py | Update fake API key creation to hash API secrets; use new column filtering. |
| packages/service-library/, packages/postgres-database/, packages/models-library/** | Update models, migration scripts, and RPC interfaces for consistent API key/security handling. |
...-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_events_utils.py
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR improves the security of API keys by switching to Blowfish-based hashing with 10 iterations and refactoring key creation and deletion flows across multiple services and tests. Key changes include:
- Updating API key creation to use a new hashing function and refactoring the corresponding RPC and REST endpoints.
- Modifying repository operations and migration scripts to store hashed API secrets and add an index on the api_key column.
- Adjusting related test fixtures and director-v2 modules to use the updated API key logic.
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| services/web/server/tests/unit/with_dbs/01/test_api_keys.py | Removed redundant test for idempotent API key creation. |
| services/web/server/src/simcore_service_webserver/api_keys/plugin.py | Updated API key controller route imports. |
| services/web/server/src/simcore_service_webserver/api_keys/_service.py | Refactored API key creation/deletion flows and replaced legacy key generation. |
| services/web/server/src/simcore_service_webserver/api_keys/_repository.py | Changed API secret storage to use Blowfish hashing and updated conflict handling. |
| services/director-v2/... | Renamed API auth functions and updated tests to reflect the new behavior. |
| services/api-server/... | Adjusted test fixtures and repository queries to match the updated API key schema. |
| packages/postgres-database/... | Added migration scripts to index api_key and hash existing API secrets. |
| packages/models-library/... | Introduced functions for generating unique API keys and secrets. |
Comments suppressed due to low confidence (1)
services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_events_utils.py:330
- The comment contains a typo ('debuug'); please correct it to 'debug'.
# used for debuug, normally sleeps 0
...rc/simcore_postgres_database/migration/versions/742123f0933a_hash_exising_api_secret_data.py
Outdated
Show resolved
Hide resolved
…tion/versions/742123f0933a_hash_exising_api_secret_data.py Co-authored-by: Copilot <[email protected]>
…o/osparc-simcore into is6880/hash-api-key-secret
pcrespov
approved these changes
Apr 22, 2025
packages/models-library/src/models_library/rpc/webserver/auth/api_keys.py
Show resolved
Hide resolved
pcrespov
reviewed
Apr 22, 2025
services/director-v2/src/simcore_service_director_v2/modules/osparc_variables/_api_auth.py
Show resolved
Hide resolved
services/director-v2/src/simcore_service_director_v2/modules/osparc_variables/_api_auth.py
Outdated
Show resolved
Hide resolved
services/director-v2/src/simcore_service_director_v2/modules/osparc_variables/_api_auth_rpc.py
Show resolved
Hide resolved
services/director-v2/src/simcore_service_director_v2/modules/osparc_variables/_api_auth_rpc.py
Show resolved
Hide resolved
services/web/server/src/simcore_service_webserver/api_keys/_repository.py
Show resolved
Hide resolved
…o/osparc-simcore into is6880/hash-api-key-secret
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 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.
What do these changes do?
This PR enhances the security of API keys by hashing API secrets using Blowfish (with 10 iterations) and refactors API key creation/deletion flows across multiple services and tests.
Store a hashed version (actually
blowfishwith10iterations) of theapi_secret.When storing:
When verifying:
All existing API keys already stored in the DB must be hashed & updated.
Related issue/s
How to test
Dev-ops checklist