-
Notifications
You must be signed in to change notification settings - Fork 171
feat: managed connection inclusion #1242
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
base: master
Are you sure you want to change the base?
feat: managed connection inclusion #1242
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1242 +/- ##
==========================================
+ Coverage 80.42% 80.46% +0.03%
==========================================
Files 146 146
Lines 5772 5794 +22
Branches 1174 1183 +9
==========================================
+ Hits 4642 4662 +20
- Misses 645 646 +1
- Partials 485 486 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
docs/configuring-the-deploy-cli.md
Outdated
|
|
||
| ```json | ||
| { | ||
| "AUTH0_INCLUDED_CONNECTIONS": ["github", "google-oauth2", "Username-Password-Authentication"] |
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.
"Username-Password-Authentication" comes under database resource type on deploy cli
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.
Revised the documentation.
| }; | ||
|
|
||
| const managedConnectionNames = this.config('AUTH0_INCLUDED_CONNECTIONS'); | ||
| const filteredConnections = managedConnectionNames |
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.
Please check the logic on src/context/index.ts , similar logic AUTH0_EXCLUDED_CONNECTIONS, AUTH0_INCLUDED_CONNECTIONS can be applied
| conflicts: [], | ||
| }; | ||
|
|
||
| const managedConnectionNames = this.config('AUTH0_INCLUDED_CONNECTIONS'); |
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.
Instead of using from config object, please use better to use similar approach AUTH0_EXCLUDED_CONNECTIONS
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.
Used the same logic as AUTH0_EXCLUDED_CONNECTIONS.
|
Please mark it ready to review once the unit tests pass |
kushalshit27
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.
Can you please point to where we are adding the validation restriction when AUTH0_INCLUDED_CONNECTIONS and AUTH0_EXCLUDED_CONNECTIONS together?
| if (includedConnections.length > 0 && filteredConnections.length !== connections.length) { | ||
| const excludedCount = connections.length - filteredConnections.length; | ||
| log.info( | ||
| `AUTH0_INCLUDED_CONNECTIONS is configured. Managing ${filteredConnections.length} connection(s), ignoring ${excludedCount} connection(s) not in the managed list.` |
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.
No ignoring, please add validation to ensure not to use both in the config 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.
Implemented a validation check ensuring that AUTH0_INCLUDED_CONNECTIONS and AUTH0_EXCLUDED_CONNECTIONS are mutually exclusive.
| if (includedConnections.length > 0 && filteredConnections.length !== connections.length) { | ||
| const excludedCount = connections.length - filteredConnections.length; | ||
| log.info( | ||
| `AUTH0_INCLUDED_CONNECTIONS is configured. Managing ${filteredConnections.length} connection(s), ignoring ${excludedCount} connection(s) not in the managed list.` | ||
| ); | ||
| } |
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.
should not be part of src/tools/auth0/handlers/connections.ts
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.
Implemented the validation check within the global context.
…and AUTH0_EXCLUDED_CONNECTIONS
kushalshit27
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.
Current code changes are not working. Please add instructions on how to test, ensuring that the changes are thoroughly tested before moving to 'PR ready to review'.
- Adjusted logic to ensure 'enterprise-saml' is not marked for deletion when not in the include list. - Modified tests to reflect that 'google-oauth2' should not be deleted if it's not in the include list. - Updated behavior to only delete managed connections when assets are empty, preserving unmanaged ones. - Ensured that unmanaged connections are ignored during updates, preventing unintended deletions.
🔧 Changes
This PR introduces the
AUTH0_INCLUDED_CONNECTIONSconfiguration option to the Auth0 Deploy CLI. This allows for a "managed-only" approach to connection management, which is essential for environments where self-service or dynamic connections exist alongside managed infrastructure.Key Improvements:
exportandimport.JSON Format
Environment Variable
export AUTH0_INCLUDED_CONNECTIONS='["github","google-oauth2"]'📚 References
Closes #1163
Enterprise Connections
🔬 Testing
Unit test added
E2E added
📝 Checklist