Check nsAccountLock attribute for user status #89
Conversation
WalkthroughThe pull request introduces a new constant, Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client Code
participant Parser as parseUserStatus
Client->>Parser: Invoke parseUserStatus with flags (Microsoft & FreeIPA)
alt nsAccountLockFlag present
Parser->>Parser: Evaluate nsAccountLockFlag
Note right of Parser: "false" => STATUS_ENABLED<br>"true" => STATUS_DISABLED
else
Parser->>Parser: Evaluate UserAccountControlFlag
end
Parser-->>Client: Return user status
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (2)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/connector/user.go (1)
83-83: Enhance error handling and readability of FreeIPA status parsing.While the basic logic is correct, consider these improvements:
- Add error handling for unexpected
nsAccountLockvalues.- Simplify the if-else structure for better readability.
Consider this refactor:
nsAccountLockFlag := user.GetEqualFoldAttributeValue(attrNSAccountLock) if userAccountControlFlag != "" { // ... existing Microsoft logic ... } else if nsAccountLockFlag != "" { + // Handle FreeIPA's nsAccountLock attribute + switch strings.ToLower(nsAccountLockFlag) { + case "false": userStatus = v2.UserTrait_Status_STATUS_ENABLED - } else if nsAccountLockFlag == "true" { + case "true": userStatus = v2.UserTrait_Status_STATUS_DISABLED + default: + return userStatus, fmt.Errorf("unexpected nsAccountLock value: %s", nsAccountLockFlag) + } }Also applies to: 98-102
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/connector/user.go(3 hunks)
🔇 Additional comments (2)
pkg/connector/user.go (2)
44-45: LGTM! Clear and well-documented constant declaration.The constant follows the established naming convention and is appropriately documented as a FreeIPA-specific attribute.
81-81: LGTM! Clear documentation of supported status attributes.The comment effectively communicates that both Microsoft's
UserAccountControlFlagand FreeIPA'snsAccountLockare now supported for determining user status.
Co-authored-by: Geoff Greer <geoff@greer.fm>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/connector/user.go(3 hunks)
🧰 Additional context used
🪛 golangci-lint (1.62.2)
pkg/connector/user.go
107-107: expected ';', found 'else'
(typecheck)
🔇 Additional comments (2)
pkg/connector/user.go (2)
44-45: LGTM!The new constant follows the existing naming convention and is well-documented.
81-83: LGTM!The updated comment clearly documents both Microsoft and FreeIPA attribute support.
Yeah that looks better! I've added this |
This collects the nsAccountLock attribute, which is used by FreeIPA and Red Hat Identity to indicate whether an account is disabled
https://docs.redhat.com/en/documentation/red_hat_directory_server/11/html/administration_guide/user_account_management-inactivating_users_and_roles
The userStatus value will be updated accordingly
Summary by CodeRabbit