feat(plugins): config profiles, credential_process, SSO, AssumeRole, ElastiCache, Keyspaces (#1567)#1572
Conversation
…dential_process (#1567)
datlechin
changed the title
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
TablePro/Plugins/TableProPluginKit/AWS/AWSCredentialResolver.swift
Lines 165 to 166 in 0fa10f0
Drain credential_process stderr without blocking
For a credential_process that fails while writing a large amount to stderr, this reads stdout to EOF before draining stderr, so the child can block on a full stderr pipe and never close stdout, hanging the connection attempt indefinitely. Drain both pipes concurrently (or merge/discard stderr while the process runs) before waiting for termination.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } catch let error as AWSAuthError { | ||
| throw DynamoDBError.authFailed(error.localizedDescription) | ||
| } catch let error as AWSSSOError { | ||
| throw DynamoDBError.authFailed(error.localizedDescription) |
There was a problem hiding this comment.
When a DynamoDB SSO profile has a missing or expired token cache, AWSCredentialResolver throws AWSSSOError, but this catch converts it into DynamoDBError.authFailed. The test-connection flow only offers the browser sign-in path when AWSSSOLoginService.isSSOExpired(error) receives an AWSSSOError directly, so DynamoDB SSO users hit the generic connection failure instead of the new login prompt. Preserve the underlying SSO error (or teach the detector about this wrapper) for the awsAuthMethod == "sso" path.
Useful? React with 👍 / 👎.
|
Addressed the review and the failing PluginKit ABI Gate:
Build green, all AWS/ConnectionField suites pass, SwiftLint |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b07814978
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return try await runCredentialProcess(command, profileName: profileName) | ||
| } | ||
|
|
||
| throw AWSAuthError.profileIncomplete(profileName) |
There was a problem hiding this comment.
When an AssumeRole profile uses source_profile that points at an SSO profile, the recursive resolveProfileChain call loads settings containing sso_account_id/sso_role_name but no static keys or credential_process, so it falls through here with profileIncomplete instead of fetching the SSO role credentials to use as the STS base credentials. This breaks a supported shared-config shape such as [profile app-admin] role_arn=... source_profile=sso-base with [profile sso-base] sso_session=..., even though both SSO and AssumeRole are otherwise implemented in this change.
Useful? React with 👍 / 👎.
…stead of an RDS token (#1567)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 528052a849
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) -> String { | ||
| let formatter = DateFormatter() | ||
| formatter.locale = Locale(identifier: "en_US_POSIX") | ||
| formatter.timeZone = TimeZone(identifier: "UTC") |
There was a problem hiding this comment.
When awsAuth is enabled for Amazon Keyspaces, this timestamp is placed in both the amzdate response field and the string-to-sign; the AWS Cassandra SigV4 plugins generate YYYY-MM-DDTHH:mm:ss.SSSZ, not the normal compact SigV4 YYYYMMDDTHHmmssZ format used here. Because the server verifies the challenge response against that Keyspaces-specific value, IAM authentication for Keyspaces will fail even with valid credentials.
Useful? React with 👍 / 👎.
| section: .advanced | ||
| ), | ||
| ] | ||
| ] + AWSAuthFields.standard() + [AWSAuthFields.elastiCacheReplicationGroupField()] |
There was a problem hiding this comment.
Selecting one of these new Redis AWS IAM options hides the password and shows the replication group field, but RedisPluginDriver.connect() still constructs RedisPluginConnection with only config.password and never calls AWSCredentialResolver or ElastiCacheAuthTokenGenerator. In that IAM-auth configuration the password is empty, so the connection skips AUTH entirely and ElastiCache IAM users cannot connect despite the UI offering the mode.
Useful? React with 👍 / 👎.
1 participant
Fixes #1567. AWS-backed connections could not use profiles defined in
~/.aws/config(the screenshot in the issue: a DynamoDB "AWS Profile" connection failing with "Profile not found or incomplete in ~/.aws/credentials"). The root cause was that each AWS plugin reimplemented credential resolution, and the copies only read~/.aws/credentials. This unifies AWS credential resolution into one shared layer inTableProPluginKitand uses it everywhere, then extends it to full AWS-CLI parity across all AWS-backed connections.What changed, by stage
AWSCredentials,AWSCredentialResolver,AWSSSO,AWSAuthError, plus newAWSConfigFile/AWSSigV4/AWSSTSintoTableProPluginKit/AWS/as public API so the app and every plugin share one implementation. Additive and ABI-safe (nocurrentPluginKitVersionbump).credential_process(which usesProcess) is gated to macOS so the iOS PluginKit target still builds. Credentials now carry anexpirationfor refresh.~/.aws/config+~/.aws/credentials, supportscredential_processand SSO, re-resolves on expiry, and shows accurate errors.~/.aws/config+~/.aws/credentials(normalizing[profile X]/[X]), with a free-text fallback. Implemented as an additiveConnectionField.dynamicOptions(no@frozenchange).role_arn+source_profilechaining via STS AssumeRole (withexternal_id,duration_seconds,credential_source = Environment). When an SSO session has expired, a prompt runs the OIDC device-login flow in the browser and refreshes the cached token.Tests
Unit tests for every deterministic/protocol piece: dual-file profile resolution (incl. the config-only case that reproduces the bug), SSO parsing/token-cache/role-fetch, STS AssumeRole response parsing, SSO device-login parsing/polling/cache, and the RDS/ElastiCache/Keyspaces token generators. All AWS suites pass; SwiftLint
--strictclean on changed lines.Needs live-AWS verification (please test before release)
The deterministic logic is unit-tested against AWS's documented algorithms, but these live paths can't be verified in CI and need real AWS:
Release
DynamoDB and Cassandra are registry-only plugins and need a registry re-release after merge; Redis is bundled. The PluginKit changes are additive (no version bump).