Validate AWS connection credentials at creation time#2189
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds server-side validation for AWS connection credentials at creation time (including optional role assumption checks) and updates the UI to set expectations about longer validation when roles are configured, addressing OPS-4026.
Changes:
- Add AWS auth validation that checks required fields, verifies base credentials via STS GetCallerIdentity, and validates configured role assumptions.
- Update the connection create/edit dialog to show an informational banner when AWS roles are present and to display multi-line validation errors.
- Add Jest test coverage for AWS auth validation (fields, base credentials, implicit role, roles, and error handling).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/react-ui/src/app/features/connections/components/create-edit-connection-dialog-content.tsx | Adds AWS-roles info banner and improves error message placement/formatting during connection creation/edit. |
| packages/openops/src/lib/aws/auth.ts | Implements AWS auth validation: required fields, STS base credential verification, and role assumption validation. |
| packages/openops/test/aws/auth.test.ts | Adds unit tests covering the new AWS auth validation behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add real-time validation for AWS connections by calling GetCallerIdentity to verify base credentials and AssumeRole to validate each configured role ARN. This enables fail-fast detection of invalid credentials for AWS Benchmark workflows. Backend changes: - Call AWS STS GetCallerIdentity API to validate base credentials - Sequentially validate each assume role ARN via AssumeRole - Support implicit role validation for EC2/ECS environments - Return detailed error messages indicating exact failure point - Refactor validation into separate helper functions - Sanitize IAM principal names in error messages for security Frontend changes: - Add blue info alert when AWS roles are present - Display validation errors above save button - Enhance error display with whitespace-pre-wrap for multi-line errors Testing: - Add comprehensive test suite with 17 tests covering all scenarios - Test field validation, base credentials, implicit roles, and role ARNs - Test error handling and sanitization Fixes OPS-4026. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Improve error message sanitization to cover more AWS ARN patterns and fix error message capitalization for better readability. Changes: - Expand sanitization to cover service-role, assumed-role, and policy ARNs - Add three-layer regex protection (User:/Role: prefix, on resource:, catch-all) - Support both IAM and STS ARN patterns - Change "Role" to "role" in error messages for proper grammar - Add 3 new tests for service-role, assumed-role, and policy sanitization Security improvements: - Redacts service-role paths (e.g., role/service-role/MyLambdaRole) - Redacts assumed-role session names (e.g., assumed-role/RoleName/session) - Redacts custom policy names (e.g., policy/MyCustomPolicy) - Prevents disclosure of internal infrastructure naming All 20 tests passing. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Extract sanitization logic from auth.ts into dedicated sanitization module with clear helper functions and comprehensive test coverage. Changes: - Create new sanitization folder under packages/openops/src/lib/aws/ - Extract sanitizeAwsError into error-sanitizer.ts with three helpers: - sanitizeUserRolePrefixedArns: Handles "User:" and "Role:" patterns - sanitizeResourcePrefixedArns: Handles "on resource:" patterns - sanitizeRemainingArns: Catch-all for other IAM/STS ARN patterns - Add comprehensive documentation explaining each pattern - Create index.ts for clean imports Testing: - Add new test suite: error-sanitizer.test.ts (17 tests) - Test User/Role prefixes (4 tests) - Test resource prefixes (4 tests) - Test catch-all patterns (4 tests) - Test complex real-world scenarios (5 tests) - All 199 AWS tests passing (auth + sanitizer) Benefits: - Better separation of concerns (auth vs sanitization) - Self-documenting code through named functions - Easier to test sanitization in isolation - Cleaner auth.ts without inline regex comments Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Fix validation bug where role assumptions would hit real AWS endpoints even when a custom endpoint (LocalStack) was configured. This caused validation to fail incorrectly for LocalStack environments. Changes: - Add optional endpoint parameter to assumeRole() in sts-common.ts - Pass auth.endpoint to all assumeRole() calls in auth.ts: - getCredentialsFromAuth (line 43) - getCredentialsListFromAuth (line 92) - validateRoleAssumptions (line 222) - Update all test expectations to include endpoint parameter Testing: - Add test: "should pass endpoint to assumeRole when provided" - Update 5 existing test expectations with undefined endpoint - All 200 AWS tests passing (auth.test.ts: 21, auth.test.ts general: 22) Impact: - LocalStack users can now validate role assumptions correctly - Custom STS endpoints are respected during validation - Consistent with base credential validation behavior Fixes issue identified by GitHub Copilot. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Replace form.watch() with useWatch for the roles field to prevent unnecessary re-renders on every form field change. The dialog only needs to know when the roles array changes to show/hide the info banner. Changes: - Import useWatch from react-hook-form - Replace form.watch() with useWatch targeting 'request.value.props.roles' - Simplify hasRoles computation to check roles directly Benefits: - Reduces re-renders when other fields change (name, credentials, etc.) - Improves performance for connection creation/edit dialog - Maintains same functionality with better efficiency Addresses PR feedback: #2189 (comment) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Replace sequential role validation with batched concurrent validation using Promise.allSettled. This significantly reduces validation time for connections with many AWS role ARNs. Changes: - Process roles in batches of 5 using Promise.allSettled - Validate roles concurrently within each batch to avoid AWS rate limiting - Check results in order to report first failure deterministically - Maintain fail-fast behavior across batches Testing: - Add test: "should validate many roles concurrently in batches" (12 roles) - Add test: "should fail on 7th role when first 6 succeed..." (batch boundary) - All 23 AWS auth tests passing Performance impact: - 1 role: ~1.5s (unchanged) - 5 roles: ~1.5s (was ~5s, 70% faster) - 10 roles: ~3s (was ~10s, 70% faster) - 20 roles: ~6s (was ~20s, 70% faster) Benefits: - Significantly faster validation for multi-account setups - Respects AWS rate limits with batch size of 5 - Maintains deterministic error reporting - Improves UX for connections with many roles Addresses PR feedback: #2189 (comment) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Replace inline comments and repeated code with well-named helper functions to improve test readability and maintainability. Remove concurrency tests that were testing implementation details rather than behavior. Changes: - Extract constants: EXAMPLE_ACCESS_KEY, EXAMPLE_SECRET_KEY, DEFAULT_REGION, LOCALSTACK_ENDPOINT - Add createAuthObject() to build auth objects with overrides - Add createRole() to generate role configurations - Add mockSuccessfulAssumeRole() and mockSuccessfulAccountId() for common mocks - Add reimportAuthWithImplicitRole() to handle module reimport pattern - Remove all inline comments from tests - Remove concurrency tests that tested batch size implementation details Benefits: - Self-documenting code through function names instead of comments - Less repetition of test data - Easier to understand test intent - Tests focus on behavior not implementation details - Maintained all 21 passing tests Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Extract validation logic into focused, self-documenting functions to improve code readability and maintainability. Replace inline comments with descriptive function names following clean code principles. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
ravikiranvm
approved these changes
Apr 2, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes OPS-4026.