[Fixes #14177] Implement storage and handling of authentications for external resources#14192
[Fixes #14177] Implement storage and handling of authentications for external resources#14192
Conversation
|
Warning Gemini is experiencing higher than usual traffic and was unable to create the review. Please try again in a few hours by commenting |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #14192 +/- ##
==========================================
+ Coverage 64.82% 64.96% +0.13%
==========================================
Files 960 967 +7
Lines 58865 59208 +343
Branches 8069 8104 +35
==========================================
+ Hits 38159 38462 +303
- Misses 19083 19115 +32
- Partials 1623 1631 +8 🚀 New features to boost your workflow:
|
| return custom_services_types | ||
|
|
||
|
|
||
| def get_basic_auth_credentials(auth_config): |
There was a problem hiding this comment.
@sijandh35 I would move these utils under the security module
| def clean(self): | ||
| cleaned_data = super().clean() | ||
|
|
||
| if cleaned_data.get("type") == "basic" and not self.instance.pk and not cleaned_data.get("password"): |
There was a problem hiding this comment.
Call the registry to obtain the handler for the type, and deledate payload validation to t handler classmethod, like AuthHandler.validate(cleaned_data)
| def save(self, commit=True): | ||
| instance = super().save(commit=False) | ||
|
|
||
| if instance.type == "basic": |
There was a problem hiding this comment.
Let's avoid conditions on specific types. This couples the Admin to the implemented handlers.
For safety and simplicity, let's encrypt/decrypt the entire payload.
| auth_handler_registry = AuthHandlerRegistry() | ||
|
|
||
|
|
||
| def build_auth_handler(config): |
There was a problem hiding this comment.
we don't need this util. The caller can just use auth_handler_registry.build(config) itself.
| return (auth_handler.username, auth_handler.password) | ||
|
|
||
|
|
||
| def create_basic_auth_config(username, password): |
There was a problem hiding this comment.
This can be a class method on the AuthBasicHandler
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| def get_basic_auth_credentials(auth_config): |
There was a problem hiding this comment.
This can be a class methos on BasicAuthHandler.
However, see my comment inside services...
| service.id, | ||
| username=service.username if service.needs_authentication else None, | ||
| password=service.get_password() if service.needs_authentication else None, | ||
| username=username, |
There was a problem hiding this comment.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 32555648 | Triggered | Authentication Tuple | eebd47c | geonode/security/tests.py | View secret |
| 32555648 | Triggered | Authentication Tuple | eebd47c | geonode/services/tests.py | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Fixes #14177
Checklist
For all pull requests:
The following are required only for core and extension modules (they are welcomed, but not required, for contrib modules):
Submitting the PR does not require you to check all items, but by the time it gets merged, they should be either satisfied or inapplicable.