Add provision_server management command
#4975
There are no files selected for viewing
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,184 @@ | ||
| import json | ||
| import os | ||
|
|
||
| from constance import config | ||
| from django.core.management.base import BaseCommand | ||
|
|
||
| from allauth.socialaccount.models import SocialApp | ||
| from kobo.apps.accounts.models import SocialAppCustomData | ||
|
|
||
|
|
||
| class Command(BaseCommand): | ||
| help = ( | ||
| 'Provision server settings including social apps and constance configs' | ||
| ) | ||
|
|
||
| def add_arguments(self, parser): | ||
| subparsers = parser.add_subparsers(dest='command') | ||
|
|
||
| # Subcommand for managing social apps | ||
| socialapp_parser = subparsers.add_parser( | ||
| 'socialapp', help='Insert a social app and related custom data' | ||
| ) | ||
|
|
||
| socialapp_parser.add_argument( | ||
| 'provider', type=str, help='Either openid_connect or microsoft' | ||
| ) | ||
| socialapp_parser.add_argument( | ||
| 'provider_id', type=str, help='Provider ID used in the login url' | ||
| ) | ||
| socialapp_parser.add_argument( | ||
| 'name', | ||
| type=str, | ||
| help='Name of the organization displayed on the login page', | ||
| ) | ||
| socialapp_parser.add_argument( | ||
| 'client_id', type=str, help='App ID, or consumer key' | ||
| ) | ||
| socialapp_parser.add_argument( | ||
| 'secret', | ||
| nargs='?', | ||
| type=str, | ||
| default='', | ||
| help='API secret, client secret, or consumer secret', | ||
| ) | ||
| socialapp_parser.add_argument( | ||
| 'key', | ||
| nargs='?', | ||
| type=str, | ||
| default='', | ||
| help='Key provided by client', | ||
| ) | ||
| socialapp_parser.add_argument( | ||
| 'settings', | ||
bufke marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| type=str, | ||
| help='Settings in json format enclosed with single quotes', | ||
| ) | ||
|
|
||
| # Subcommand for managing constance configurations | ||
| config_parser = subparsers.add_parser( | ||
| 'config', help='Update application configuration' | ||
| ) | ||
| config_parser.add_argument( | ||
| 'config_kv_pairs', | ||
| nargs='+', | ||
| type=str, | ||
| help='Constance configuration values as key=value pairs', | ||
| ) | ||
|
|
||
| def handle(self, *args, **kwargs): | ||
| command = kwargs.get('command') | ||
|
|
||
| if command == 'socialapp': | ||
| self.handle_socialapp(kwargs) | ||
| elif command == 'config': | ||
| self.handle_config(kwargs) | ||
| else: | ||
| self.stdout.write(self.style.ERROR('No valid subcommand provided')) | ||
|
|
||
| def handle_socialapp(self, kwargs): | ||
| provider = kwargs['provider'] | ||
| provider_id = kwargs['provider_id'] | ||
| name = kwargs['name'] | ||
| client_id = kwargs['client_id'] | ||
| secret = os.getenv('SOCIAL_APP_SECRET') or kwargs['secret'] | ||
| key = kwargs['key'] | ||
| settings_json = kwargs['settings'] | ||
|
|
||
| try: | ||
| settings = json.loads(settings_json) | ||
| except TypeError: | ||
| raise json.JSONDecodeError | ||
|
There was a problem hiding this comment. I think this is catching the same error and then re-raising it? Why not allow the original exception to be raised? If you want to catch it and add more context, do that. Or leave it alone. Catching and raising without any benefit is obfuscating the error. |
||
|
|
||
| social_app_data = { | ||
| 'provider': provider, | ||
| 'provider_id': provider_id, | ||
| 'name': name, | ||
|
There was a problem hiding this comment. This line can be omitted, you don't need to repeat the args in get_or_create again in defaults. The usage of get_or_create makes a lot of sense though. |
||
| 'client_id': client_id, | ||
| 'secret': secret, | ||
| 'key': key, | ||
| 'settings': settings, | ||
| } | ||
|
|
||
| social_app, created = SocialApp.objects.get_or_create( | ||
| defaults=social_app_data | ||
|
There was a problem hiding this comment. This doesn't work, you need to have a lookup based on a unique field - maybe name? The first time it works. Then with a different name, I still get "Social app for my_first_name already exists" even though I changed the name. |
||
| ) | ||
|
|
||
| if not created: | ||
| self.stdout.write( | ||
| f'Social app for {social_app.name} already exists' | ||
| ) | ||
| else: | ||
| self.stdout.write( | ||
| f'Successfully created social app for {social_app.name}' | ||
| ) | ||
|
|
||
| social_app_custom_data_exists = SocialAppCustomData.objects.filter( | ||
| social_app=social_app | ||
| ).exists() | ||
|
|
||
| if not social_app_custom_data_exists: | ||
|
There was a problem hiding this comment. I'd probably inline this as it's setting a variable that is used exactly once. Minor, fine-to-ignore, comment. |
||
| SocialAppCustomData.objects.create( | ||
| social_app=social_app, | ||
| is_public=False, | ||
| ) | ||
| self.stdout.write( | ||
| f'Successfully created custom data for {social_app.name}' | ||
| ) | ||
| else: | ||
| self.stdout.write( | ||
| f'Custom data for {social_app.name} already exists' | ||
| ) | ||
|
|
||
| def handle_config(self, kwargs): | ||
| config_kv_pairs = kwargs['config_kv_pairs'] | ||
| for pair in config_kv_pairs: | ||
| key, value = pair.split('=') | ||
| if hasattr(config, key): | ||
| try: | ||
| if value.lower() == 'true': | ||
| value = True | ||
| elif value.lower() == 'false': | ||
| value = False | ||
| else: | ||
| # Specific handling for fields taking JSON arrays | ||
| if key in [ | ||
| 'PROJECT_METADATA_FIELDS', | ||
| 'USER_METADATA_FIELDS', | ||
| ]: | ||
| if value.startswith('[') and value.endswith(']'): | ||
|
There was a problem hiding this comment. Is this needed? I know that json technically isn't supposed to start with an array. But this works for me >>> data = "[1, 2]"
>>> import json
>>> json.loads(data)
[1, 2]
>>> json.dumps(json.loads(data))
'[1, 2]' |
||
| try: | ||
| value = json.loads(value) | ||
| value = json.dumps(value) | ||
|
There was a problem hiding this comment. Interesting way to validate json, I imagine it works fine. I don't think it's possible to error in the dumps line, so that can be moved outside of the try, except. Or actually I think you could achieve the validation with you don't really need the value, right? You're just converting to and then from json to validate it I assume. If you end up keeping this code - I might try to break it out so it's not so nested. I don't typically say one must "Don't repeat yourself" when repeating just twice. But in this case it might help reduce the nesting. |
||
| except json.JSONDecodeError as e: | ||
| self.stdout.write( | ||
| f'Invalid JSON array value for key {key}. {e}' | ||
| ) | ||
| continue | ||
| else: | ||
| self.stdout.write( | ||
| f'Invalid JSON array format for key {key}. Should start and end with "[" and "]".' | ||
| ) | ||
| continue | ||
| # Handling for other keys that should be JSON objects | ||
| else: | ||
| if value.startswith('{') and value.endswith('}'): | ||
| try: | ||
| value = json.loads(value) | ||
| value = json.dumps(value) | ||
| except json.JSONDecodeError as e: | ||
| self.stdout.write( | ||
| f'Invalid JSON object value for key {key}. {e}' | ||
| ) | ||
| continue | ||
|
|
||
| setattr(config, key, value) | ||
| self.stdout.write( | ||
| f'Successfully updated configuration for {key}' | ||
| ) | ||
| except Exception as e: | ||
| self.stdout.write( | ||
| f'Error setting configuration for {key}: {e}' | ||
| ) | ||
| else: | ||
| self.stdout.write(f'Configuration key {key} does not exist') | ||
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,161 @@ | ||
| import json | ||
| from unittest.mock import patch | ||
|
|
||
| from django.core.management import call_command | ||
| from django.test import TestCase | ||
|
|
||
| from allauth.socialaccount.models import SocialApp | ||
| from kobo.apps.accounts.models import SocialAppCustomData | ||
|
|
||
|
|
||
| class ProvisionServerCommandTest(TestCase): | ||
|
|
||
| @patch('os.getenv') | ||
| def test_handle_successful_creation(self, mock_getenv): | ||
| mock_getenv.return_value = None | ||
|
|
||
| self.assertEqual(SocialApp.objects.count(), 0) | ||
| self.assertEqual(SocialAppCustomData.objects.count(), 0) | ||
|
|
||
| call_command( | ||
| 'provision_server', | ||
| 'socialapp', | ||
| 'openid_connect', | ||
| 'test_provider_id', | ||
| 'Test Organization', | ||
| 'test_client_id', | ||
| 'test_secret', | ||
| '', | ||
| '{"key": "value"}', | ||
| ) | ||
|
|
||
| self.assertEqual(SocialApp.objects.count(), 1) | ||
| self.assertEqual(SocialAppCustomData.objects.count(), 1) | ||
|
|
||
| social_app = SocialApp.objects.first() | ||
| self.assertEqual(social_app.provider, 'openid_connect') | ||
| self.assertEqual(social_app.provider_id, 'test_provider_id') | ||
| self.assertEqual(social_app.name, 'Test Organization') | ||
| self.assertEqual(social_app.client_id, 'test_client_id') | ||
| self.assertEqual(social_app.secret, 'test_secret') | ||
| self.assertEqual(social_app.settings, {'key': 'value'}) | ||
|
|
||
| @patch('os.getenv') | ||
| def test_handle_existing_social_app(self, mock_getenv): | ||
| mock_getenv.return_value = None | ||
|
|
||
| SocialApp.objects.create( | ||
| provider='openid_connect', | ||
| provider_id='test_provider_id', | ||
| name='Test Organization', | ||
| client_id='test_client_id', | ||
| secret='test_secret', | ||
| key='', | ||
| settings={'key': 'value'}, | ||
| ) | ||
|
|
||
| self.assertEqual(SocialApp.objects.count(), 1) | ||
|
|
||
| call_command( | ||
| 'provision_server', | ||
| 'socialapp', | ||
| 'openid_connect', | ||
| 'test_provider_id', | ||
| 'Test Organization', | ||
| 'test_client_id', | ||
| 'test_secret', | ||
| '', | ||
| '{"key": "value"}', | ||
| ) | ||
|
|
||
| self.assertEqual(SocialApp.objects.count(), 1) | ||
| self.assertEqual(SocialAppCustomData.objects.count(), 1) | ||
|
|
||
| @patch('os.getenv') | ||
| def test_handle_invalid_json(self, mock_getenv): | ||
| mock_getenv.return_value = None | ||
|
|
||
| with self.assertRaises(json.JSONDecodeError): | ||
| call_command( | ||
| 'provision_server', | ||
| 'socialapp', | ||
| 'openid_connect', | ||
| 'test_provider_id', | ||
| 'Test Organization', | ||
| 'test_client_id', | ||
| 'test_secret', | ||
| '', | ||
| '{"invalid_json"}', | ||
| ) | ||
|
|
||
| @patch('os.getenv') | ||
| def test_handle_with_env_secret(self, mock_getenv): | ||
| mock_getenv.return_value = 'env_secret' | ||
|
|
||
| self.assertEqual(SocialApp.objects.count(), 0) | ||
|
|
||
| call_command( | ||
| 'provision_server', | ||
| 'socialapp', | ||
| 'openid_connect', | ||
| 'test_provider_id', | ||
| 'Test Organization', | ||
| 'test_client_id', | ||
| 'test_secret', | ||
| '', | ||
| '{"key": "value"}', | ||
| ) | ||
|
|
||
| self.assertEqual(SocialApp.objects.count(), 1) | ||
| social_app = SocialApp.objects.first() | ||
| self.assertEqual(social_app.secret, 'env_secret') | ||
|
|
||
| @patch('kpi.management.commands.provision_server.config') | ||
| def test_update_existing_config_key(self, mock_config): | ||
| setattr(mock_config, 'TEST_CONFIG_KEY', 'old_value') | ||
| call_command( | ||
| 'provision_server', | ||
| 'config', | ||
| 'TEST_CONFIG_KEY=new_value', | ||
| ) | ||
| self.assertEqual(getattr(mock_config, 'TEST_CONFIG_KEY'), 'new_value') | ||
|
|
||
| @patch('kpi.management.commands.provision_server.config') | ||
| def test_update_non_existing_config_key(self, mock_config): | ||
| delattr(mock_config, 'NON_EXISTENT_KEY') | ||
| call_command( | ||
| 'provision_server', | ||
| 'config', | ||
| 'NON_EXISTENT_KEY=new_value', | ||
| ) | ||
|
|
||
| self.assertFalse(hasattr(mock_config, 'NON_EXISTENT_KEY')) | ||
|
|
||
| @patch('kpi.management.commands.provision_server.config') | ||
| def test_update_config_key_with_different_data_types(self, mock_config): | ||
|
|
||
| call_command( | ||
| 'provision_server', | ||
| 'config', | ||
| 'TEST_BOOL_KEY=True', | ||
| ) | ||
| self.assertEqual(getattr(mock_config, 'TEST_BOOL_KEY'), True) | ||
|
|
||
| call_command( | ||
| 'provision_server', | ||
| 'config', | ||
| 'TEST_JSON_KEY={"key": "value"}', | ||
| ) | ||
| self.assertEqual( | ||
| getattr(mock_config, 'TEST_JSON_KEY'), '{"key": "value"}' | ||
| ) | ||
|
|
||
| call_command( | ||
| 'provision_server', | ||
| 'config', | ||
| 'PROJECT_METADATA_FIELDS=[{"key": "value"}]', | ||
| ) | ||
| self.assertEqual( | ||
| getattr(mock_config, 'PROJECT_METADATA_FIELDS'), | ||
| '[{"key": "value"}]', | ||
| ) |
Oops, something went wrong.
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.
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 like the documentation in general, good work. Especially the argument docs. I would add an example command here too, for quick learning.