-
Notifications
You must be signed in to change notification settings - Fork 5
Feat/create app client generation script #921
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
Feat/create app client generation script #921
Conversation
WalkthroughThe changes introduce a new Python script for interactive AWS Cognito app client creation, update documentation to reflect new procedures and external registry usage, add new fields to onboarding instructions, and remove legacy YAML configuration and related tests. The workflow now centers on automation and centralized tracking, replacing manual YAML-based management. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant create_app_client.py
participant AWS Cognito
participant Google Sheet Registry
User->>create_app_client.py: Run script and provide input (name, compact, state, scopes)
create_app_client.py->>AWS Cognito: Create user pool client via boto3
AWS Cognito-->>create_app_client.py: Return client ID and secret
create_app_client.py->>User: Output credentials and URLs as JSON
User->>Google Sheet Registry: Register new app client info
Assessment against linked issues
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (4)
backend/compact-connect/app_clients/create_app_client.py (2)
239-261
: Consider renamingstate
parameter tojurisdiction
for consistency.The function parameter is named
state
, but the JSON output field isjurisdiction
. Consider renaming the parameter to match the output field name for clarity.-def print_credentials(client_id, client_secret, environment, compact, state): +def print_credentials(client_id, client_secret, environment, compact, jurisdiction): """Print the credentials and API information in JSON format for secure copy/paste.""" # Get environment-specific URLs auth_urls = { 'beta': 'https://compact-connect-staff-beta.auth.us-east-1.amazoncognito.com/oauth2/token', 'prod': 'https://compact-connect-staff.auth.us-east-1.amazoncognito.com/oauth2/token', 'test': 'https://compact-connect-staff-test.auth.us-east-1.amazoncognito.com/oauth2/token', } api_base_urls = { 'beta': 'https://api.beta.compactconnect.org', 'prod': 'https://api.compactconnect.org', 'test': 'https://api.test.compactconnect.org', } credentials = { 'clientId': client_id, 'clientSecret': client_secret, 'compact': compact, - 'state': state, + 'jurisdiction': jurisdiction, 'authUrl': auth_urls.get(environment), - 'licenseUploadUrl': f'{api_base_urls.get(environment)}/v1/compacts/{compact}/jurisdictions/{state}/licenses', + 'licenseUploadUrl': f'{api_base_urls.get(environment)}/v1/compacts/{compact}/jurisdictions/{jurisdiction}/licenses', }
311-313
: Avoid catch-all exception handling.The bare except clause on line 311 catches all exceptions, which could hide important errors. Consider catching specific exceptions or letting unexpected errors propagate.
- except Exception as e: # noqa: BLE001 - print(f'Error: {e}') + except (ValueError, KeyboardInterrupt) as e: + print(f'Error: {e}') sys.exit(1)backend/compact-connect/app_clients/README.md (2)
4-4
: Consider adding a link to the Google Sheet registry.Since the Google Sheet is mentioned as critical for disaster recovery, consider adding a placeholder or instruction for where to find the link to this registry.
56-64
: Fix markdown list indentation.The unordered list items have inconsistent indentation.
- - App client name (e.g., "example-ky-app-client-v1") - - Compact (aslp, octp, coun) - - State postal abbreviation (e.g., "ky", "la") - - Additional scopes (optional) +- App client name (e.g., "example-ky-app-client-v1") +- Compact (aslp, octp, coun) +- State postal abbreviation (e.g., "ky", "la") +- Additional scopes (optional) **Automatic Scope Generation:** The script automatically creates these standard scopes: - - `{compact}/readGeneral` - General read access for the compact - - `{state}/{compact}.write` - Write access for the specific state/compact combination +- `{compact}/readGeneral` - General read access for the compact +- `{state}/{compact}.write` - Write access for the specific state/compact combination
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/compact-connect/app_clients/README.md
(2 hunks)backend/compact-connect/app_clients/create_app_client.py
(1 hunks)backend/compact-connect/app_clients/it_staff_onboarding_instructions/README.md
(4 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#848
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/registration.py:111-117
Timestamp: 2025-06-17T19:05:36.255Z
Learning: In CompactConnect PR #848, the user landonshumway-ia decided to leave timezone handling code in _should_allow_reregistration function as-is after testing in sandbox environment confirmed it works correctly. The user's reasoning was that reregistration is an edge case, the code has been tested and verified, and AWS is unlikely to change behavior that would break many clients. This represents a pragmatic engineering decision based on testing and risk assessment.
backend/compact-connect/app_clients/it_staff_onboarding_instructions/README.md (5)
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#798
File: backend/compact-connect/docs/onboarding/JURISDICTION_COMPACT_ONBOARDING.md:117-117
Timestamp: 2025-05-13T22:33:53.986Z
Learning: The app_clients directory is located at the root of the compact-connect directory, so the correct relative path from docs/onboarding is ../../app_clients/README.md
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#852
File: backend/compact-connect/stacks/persistent_stack/__init__.py:545-549
Timestamp: 2025-06-09T22:03:03.232Z
Learning: In the CompactConnect codebase, production active_compact_member_jurisdictions configurations are very stable and unlikely to be removed. The vast majority of configuration errors in get_list_of_active_jurisdictions_for_compact_environment() will occur when developers are deploying sandbox environments, so error messages that primarily reference sandbox configuration are appropriate for the typical use case.
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#882
File: backend/compact-connect/lambdas/python/migration/compact_configured_states_871/main.py:127-130
Timestamp: 2025-06-26T16:42:00.781Z
Learning: In the compact_configured_states_871 migration, existing jurisdiction configurations that have licenseeRegistrationEnabled: true are migrated with isLive: true for backwards compatibility. This prevents breaking existing live functionality since those states are already operational. The migration preserves the current live state of jurisdictions to maintain service continuity, while new states added after migration can start with isLive: false and be controlled by compact admins.
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#798
File: backend/compact-connect/docs/api-specification/latest-oas30.json:55-127
Timestamp: 2025-05-14T17:00:51.255Z
Learning: The mixed format of security scope names in OpenAPI definitions (slash `/` for compact-level scopes like "aslp/admin" and dot `.` for jurisdiction-specific scopes like "al/aslp.admin") is defined by AWS Cognito's constraints and is not an issue that needs standardization.
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#769
File: backend/compact-connect/lambdas/python/provider-data-v1/tests/function/test_handlers/test_encumbrance.py:138-147
Timestamp: 2025-04-29T02:52:40.532Z
Learning: In CompactConnect tests, hardcoded values (like license type abbreviations 'slp') in test queries are sometimes used intentionally rather than using dynamic lookups. This is a deliberate design decision to make tests fail if default test data changes, requiring developers to consciously update related tests.
backend/compact-connect/app_clients/README.md (1)
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#798
File: backend/compact-connect/docs/onboarding/JURISDICTION_COMPACT_ONBOARDING.md:117-117
Timestamp: 2025-05-13T22:33:53.986Z
Learning: The app_clients directory is located at the root of the compact-connect directory, so the correct relative path from docs/onboarding is ../../app_clients/README.md
backend/compact-connect/app_clients/create_app_client.py (1)
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#798
File: backend/compact-connect/docs/onboarding/JURISDICTION_COMPACT_ONBOARDING.md:117-117
Timestamp: 2025-05-13T22:33:53.986Z
Learning: The app_clients directory is located at the root of the compact-connect directory, so the correct relative path from docs/onboarding is ../../app_clients/README.md
🪛 markdownlint-cli2 (0.17.2)
backend/compact-connect/app_clients/README.md
56-56: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
57-57: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
58-58: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
59-59: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
63-63: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
64-64: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: TestPython
🔇 Additional comments (5)
backend/compact-connect/app_clients/create_app_client.py (2)
263-270
: Good security awareness in credential output.The warnings about copying credentials securely and not leaving them in terminal history or logs demonstrate good security practices.
216-226
: Well-configured Cognito app client settings.The app client configuration correctly uses:
- Client credentials flow for M2M authentication
- 15-minute token validity as documented
- Security best practice with
PreventUserExistenceErrors
backend/compact-connect/app_clients/it_staff_onboarding_instructions/README.md (2)
39-46
: Good alignment with script output format.The updated JSON format with
compact
,jurisdiction
,authUrl
, andlicenseUploadUrl
fields matches the output from the newcreate_app_client.py
script.
184-185
: Important security reminder appropriately emphasized.The bold formatting for the warning about not storing credentials in application code adds appropriate emphasis to this critical security practice.
backend/compact-connect/app_clients/README.md (1)
47-65
: Well-documented interactive script process.The instructions clearly explain the interactive process and automatic scope generation, making it easy for users to understand what the script does.
@jlkravitz This is ready for your review. Thanks |
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.
a couple nits but looks great!
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
backend/compact-connect/app_clients/README.md (1)
56-59
: Fix markdown list indentation for consistency.The unordered list items have inconsistent indentation that should be corrected according to markdown standards.
- The script will prompt you for: - - App client name (e.g., "example-ky-app-client-v1") - - Compact (aslp, octp, coun) - - State postal abbreviation (e.g., "ky", "la") - - Additional scopes (optional) + The script will prompt you for: + - App client name (e.g., "example-ky-app-client-v1") + - Compact (aslp, octp, coun) + - State postal abbreviation (e.g., "ky", "la") + - Additional scopes (optional)- The script automatically creates these standard scopes: - - `{compact}/readGeneral` - General read access for the compact - - `{state}/{compact}.write` - Write access for the specific state/compact combination + The script automatically creates these standard scopes: + - `{compact}/readGeneral` - General read access for the compact + - `{state}/{compact}.write` - Write access for the specific state/compact combinationAlso applies to: 63-64
backend/compact-connect/app_clients/bin/create_app_client.py (1)
298-298
: Consider more specific exception handling.The broad exception catch with
# noqa: BLE001
suppresses the linting warning, but consider if more specific exception types could be handled differently to provide better error messages to users.- except Exception as e: # noqa: BLE001 + except KeyboardInterrupt: + print('\nOperation cancelled by user.') + sys.exit(0) + except Exception as e: print(f'Error: {e}') sys.exit(1)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/compact-connect/app_clients/README.md
(2 hunks)backend/compact-connect/app_clients/bin/create_app_client.py
(1 hunks)backend/compact-connect/app_clients/example_app_client.yml
(0 hunks)backend/compact-connect/tests/unit/test_app_client_yaml.py
(0 hunks)
💤 Files with no reviewable changes (2)
- backend/compact-connect/app_clients/example_app_client.yml
- backend/compact-connect/tests/unit/test_app_client_yaml.py
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#848
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/registration.py:111-117
Timestamp: 2025-06-17T19:05:36.255Z
Learning: In CompactConnect PR #848, the user landonshumway-ia decided to leave timezone handling code in _should_allow_reregistration function as-is after testing in sandbox environment confirmed it works correctly. The user's reasoning was that reregistration is an edge case, the code has been tested and verified, and AWS is unlikely to change behavior that would break many clients. This represents a pragmatic engineering decision based on testing and risk assessment.
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#848
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/registration.py:111-117
Timestamp: 2025-06-17T18:50:23.464Z
Learning: AWS Cognito's AdminGetUser API returns UserLastModifiedDate as a UNIX epoch timestamp in the raw API response, but boto3 (Python SDK) automatically converts this to a Python datetime object for easier use. The datetime object may be timezone-naive and require timezone information to be added for proper comparison with timezone-aware datetime objects.
backend/compact-connect/app_clients/bin/create_app_client.py (1)
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#798
File: backend/compact-connect/docs/onboarding/JURISDICTION_COMPACT_ONBOARDING.md:117-117
Timestamp: 2025-05-13T22:33:53.986Z
Learning: The app_clients directory is located at the root of the compact-connect directory, so the correct relative path from docs/onboarding is ../../app_clients/README.md
backend/compact-connect/app_clients/README.md (3)
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#798
File: backend/compact-connect/docs/onboarding/JURISDICTION_COMPACT_ONBOARDING.md:117-117
Timestamp: 2025-05-13T22:33:53.986Z
Learning: The app_clients directory is located at the root of the compact-connect directory, so the correct relative path from docs/onboarding is ../../app_clients/README.md
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#848
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/registration.py:111-117
Timestamp: 2025-06-17T19:05:36.255Z
Learning: In CompactConnect PR #848, the user landonshumway-ia decided to leave timezone handling code in _should_allow_reregistration function as-is after testing in sandbox environment confirmed it works correctly. The user's reasoning was that reregistration is an edge case, the code has been tested and verified, and AWS is unlikely to change behavior that would break many clients. This represents a pragmatic engineering decision based on testing and risk assessment.
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#798
File: backend/compact-connect/docs/api-specification/latest-oas30.json:55-127
Timestamp: 2025-05-14T17:00:51.255Z
Learning: The mixed format of security scope names in OpenAPI definitions (slash `/` for compact-level scopes like "aslp/admin" and dot `.` for jurisdiction-specific scopes like "al/aslp.admin") is defined by AWS Cognito's constraints and is not an issue that needs standardization.
🪛 markdownlint-cli2 (0.17.2)
backend/compact-connect/app_clients/README.md
56-56: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
57-57: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
58-58: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
59-59: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
63-63: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
64-64: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: TestPython
🔇 Additional comments (9)
backend/compact-connect/app_clients/README.md (2)
47-65
: Excellent documentation of the new automated workflow.The documentation clearly explains the transition from manual AWS CLI commands to the interactive Python script, including the automatic scope generation and user prompts. This provides clear guidance for technical staff.
69-81
: Strong security guidance for credential handling.The documentation appropriately emphasizes secure transmission of credentials and includes practical instructions for using the script output with one-time secret generators. The reminder to remove credentials after transmission is a good security practice.
backend/compact-connect/app_clients/bin/create_app_client.py (7)
22-47
: Robust configuration loading with proper error handling.The function correctly traverses the directory tree to locate
cdk.json
and handles the case where the file is not found. The configuration extraction for compacts and jurisdictions is well-implemented.
49-66
: Comprehensive scope validation patterns.The regex patterns correctly match both compact-level and jurisdiction-level scopes according to the documented format. The pattern coverage appears complete for the described scope types.
68-122
: Thorough input validation with clear error messages.The validation functions properly check compacts against the loaded configuration, validate states for specific compacts, and provide detailed error messages for invalid scopes. The scope validation includes helpful examples of valid patterns.
124-188
: User-friendly interactive input collection.The function provides clear prompts, validates input in loops with retry logic, and shows a comprehensive confirmation summary before proceeding. The automatic scope generation and deduplication logic is correct.
190-224
: Proper AWS client creation with comprehensive error handling.The boto3 client creation includes appropriate error handling for credential issues and AWS API errors. The Cognito client configuration parameters are correctly set for OAuth client credentials flow.
226-258
: Secure credential output with appropriate warnings.The function generates environment-specific URLs correctly and outputs credentials in a structured JSON format. The security warnings about not leaving credentials in terminal history are important and well-placed.
1-305
: Script correctly locates cdk.json in its parent directoriesThe
load_cdk_config()
function increate_app_client.py
begins its search from the script’s directory (backend/compact-connect/app_clients/bin
) and traverses up—so it will successfully findbackend/compact-connect/cdk.json
. No changes are needed for loading the CDK configuration.
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.
@isabeleliassen good to merge!
As we onboard multiple states to API access in the beta environment, we need a more streamlined way to generate the app client credentials for IT staff. This adds a script and updates the related documentation to improve this process.
Closes #916
Summary by CodeRabbit