Skip to content

Add IAM console access entitlement and grants#118

Open
c1-dev-bot[bot] wants to merge 3 commits into
mainfrom
feat/console-access-entitlement
Open

Add IAM console access entitlement and grants#118
c1-dev-bot[bot] wants to merge 3 commits into
mainfrom
feat/console-access-entitlement

Conversation

@c1-dev-bot

@c1-dev-bot c1-dev-bot Bot commented May 21, 2026

Copy link
Copy Markdown

Summary

  • Add console_access entitlement on AWS account resources (account and account_iam) to represent IAM console login capability
  • Emit grants for IAM users whose credential report shows password_enabled=true
  • Enrich IAM user profiles with console_access_enabled, console_password_last_used, and console_password_last_changed attributes
  • Fetch the credential report once per account per sync via GenerateCredentialReport/GetCredentialReport APIs with graceful degradation if permissions are missing

Details

New file: credential_report.go

Core module for fetching and parsing the AWS IAM credential report CSV. Polls GenerateCredentialReport with 2-second intervals (up to 2 minutes), then parses the CSV into per-user entries. fetchCredentialReportBestEffort() wraps the fetch with graceful degradation — logs a warning and returns nil if the report cannot be generated.

Modified: iam_user.go

  • Added per-account credential report caching (sync.Mutex + map keyed by parent account ID) to avoid redundant API calls during multi-account Orgs syncs
  • iamUserProfile() now enriches the user profile with console access attributes from the credential report

Modified: account_iam.go and account.go

  • Both account resource types now emit a console_access assignment entitlement (grantable to IAM users)
  • Grants() fetches the credential report and emits grants for users with passwords enabled
  • Cross-account access uses AWSClientFactory.GetIAMClient() with fallback to the default IAM client

Modified: resource_types.go

  • Removed SkipEntitlementsAndGrants from resourceTypeAccountIam
  • Added iam:GenerateCredentialReport and iam:GetCredentialReport permissions to relevant resource types

Modified: connector.go

  • Updated builder calls to pass the IAM client and client factory to account resource types

Test plan

  • Verify single-account mode: IAM users with console passwords get console_access grants
  • Verify Orgs mode (without SSO): each account emits console_access entitlement and grants
  • Verify Orgs mode (with SSO): console access phase runs after permission set grants
  • Verify graceful degradation when IAM credentials lack iam:GenerateCredentialReport permission
  • Verify IAM user profiles contain console_access_enabled, console_password_last_used, console_password_last_changed

Fixes: CXH-1523


Note

Automated PR - This PR was created by an automated process. Please review the changes carefully before merging.
If you have any questions or concerns, please reach out to the team for assistance.

…ibute

Add console_access entitlement on AWS account resources (both account and
account_iam) so that IAM users with AWS Management Console access
(LoginProfile) are surfaced as grants in ConductorOne.

Implementation:
- Fetch IAM credential report (GenerateCredentialReport + GetCredentialReport)
  once per account per sync to determine console access status
- Emit console_access assignment entitlement on account resources, grantable
  to iam_user principals
- Emit grants for each IAM user with password_enabled=true in the report
- Enrich IAM user profiles with console_access_enabled,
  console_password_last_used, and console_password_last_changed attributes
- Add iam:GenerateCredentialReport and iam:GetCredentialReport to required
  permissions for account, account_iam, and iam_user resource types
- Remove SkipEntitlementsAndGrants from account_iam resource type
- Credential report fetch degrades gracefully (warns and skips) if permissions
  are missing

Fixes: CXH-1523
@c1-dev-bot c1-dev-bot Bot requested a review from a team May 21, 2026 15:16
@linear-code

linear-code Bot commented May 21, 2026

Copy link
Copy Markdown

CXH-1523

Comment on lines +29 to +33
PasswordLastUsed string
PasswordLastChanged string
}

func (e *credentialReportEntry) IsPasswordEnabled() bool {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Bug: The credentialReportEntry struct does not capture the arn column from the credential report CSV. Both account.go:442 and account_iam.go:138 construct the user ARN as fmt.Sprintf("arn:aws:iam::%s:user/%s", accountID, username), but this is incorrect for IAM users with non-default paths (e.g., a user john under path /admins/ has ARN arn:aws:iam::ACCOUNT:user/admins/john, not arn:aws:iam::ACCOUNT:user/john).

The credential report CSV has an arn column with the full ARN. Add an ARN field here, parse it from the CSV, and use it in grant construction instead of building the ARN from the username. Without this fix, console access grants for IAM users with paths won't match the resources created in iam_user.go:List (which uses user.Arn from the API).

Comment thread pkg/connector/account.go
Comment on lines +442 to +443
userARN := fmt.Sprintf("arn:aws:iam::%s:user/%s", accountID, username)
uID, err := resourceSdk.NewResourceID(resourceTypeIAMUser, userARN)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Bug: This constructs the ARN using only the username from the credential report, omitting the IAM user path. IAM users created under non-default paths (e.g., /admins/) will produce an incorrect ARN that doesn't match the resource ID set in iam_user.go:List. Use the arn column from the credential report CSV instead of constructing it manually. See the comment on credential_report.go for the full fix.

Comment on lines +138 to +139
userARN := fmt.Sprintf("arn:aws:iam::%s:user/%s", accountID, username)
uID, err := resourceSdk.NewResourceID(resourceTypeIAMUser, userARN)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Bug: Same ARN construction issue as account.go — IAM users with non-default paths will produce incorrect ARNs. Use the arn column from the credential report entry instead.

Comment thread pkg/connector/iam_user.go
Comment on lines +135 to 153
cacheKey = parentId.Resource
}

o.credReportMu.Lock()
defer o.credReportMu.Unlock()

if o.credReportCache == nil {
o.credReportCache = make(map[string]map[string]*credentialReportEntry)
}
if report, ok := o.credReportCache[cacheKey]; ok {
return report
}

report := fetchCredentialReportBestEffort(ctx, iamClient)
o.credReportCache[cacheKey] = report
return report
}

func userTagsToMap(u iamTypes.User) map[string]interface{} {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: The mutex is held for the entire duration of fetchCredentialReportBestEffort, which polls AWS for up to 2 minutes. Consider releasing the lock after the cache check, performing the API call without the lock, then re-acquiring to store the result. This avoids blocking concurrent callers unnecessarily. A double-check pattern (check cache → unlock → fetch → lock → check again → store) would prevent redundant fetches while keeping the critical section short.

@github-actions

github-actions Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

Connector PR Review: Add IAM console access entitlement and grants

Blocking Issues: 1 | Suggestions: 1 | Threads Resolved: 0
Review mode: incremental since 0470598
View review run

Review Summary

The new commits simplify getIAMClientForAccount in account_iam.go to return *iam.Client instead of (*iam.Client, error) (logging warnings internally and falling back to the default client), and fix credential_report.go to use errors.Is(err, io.EOF) instead of a direct comparison. Both are clean improvements with no new issues. The prior blocking finding (ARN construction missing IAM path) and suggestion (mutex held during credential report API call) remain unaddressed.

Security Issues

None found.

Correctness Issues

  • credential_report.go:27-31, account.go:442, account_iam.go:138: User ARN constructed from username alone (arn:aws:iam::ACCOUNT:user/USERNAME), missing the IAM path component. Grants for users with non-default paths won't match synced resources.

Suggestions

  • iam_user.go:135-153: Mutex held during credential report API call (up to 2-minute poll) — consider a double-check locking pattern to keep the critical section short.
Prompt for AI agents
Verify each finding against the current code and only fix it if needed.

## Correctness Issues

In `pkg/connector/credential_report.go`:
- Around line 27-31: The `credentialReportEntry` struct is missing an `ARN` field. Add `ARN string` to the struct. In `parseCredentialReportCSV`, parse the `arn` column from the CSV (it exists in the AWS credential report) and populate this field. Then update the map key from username to ARN, or keep the username key but store the ARN in the entry.

In `pkg/connector/account.go`:
- Around line 442-443: Replace `userARN := fmt.Sprintf("arn:aws:iam::%s:user/%s", accountID, username)` with `entry.ARN` (using the ARN field from the credential report entry). This ensures IAM users with non-default paths get the correct ARN that matches the resource ID set during sync.

In `pkg/connector/account_iam.go`:
- Around line 138-139: Same fix as account.go — replace the `fmt.Sprintf` ARN construction with `entry.ARN` from the credential report entry.

## Suggestions

In `pkg/connector/iam_user.go`:
- Around line 135-153: Refactor `getCredentialReport` to release the mutex before calling `fetchCredentialReportBestEffort`. Use a double-check pattern: lock, check cache, unlock, fetch report, lock again, check cache again (another goroutine may have populated it), store result, unlock.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking issues found — see review comments.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking issues found — see review comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants