fix: hws console user two-factor authentication rule #96
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdd OPA policy to detect HUAWEI Cloud console users without two-factor authentication, complemented by corresponding input schema, metadata, and relation files. Entity relationship diagram for input schema in new OPA policyerDiagram
INPUT {
string name
string domain_id
string access_mode
object LoginProtects
}
USER_ATTRIBUTE {
string name
string domain_id
string access_mode
}
INPUT ||--o| USER_ATTRIBUTE : contains
Class diagram for new OPA policy: hws_console_account_disable_mfa_39classDiagram
class input {
UserAttribute: object
LoginProtects: object | null
}
class UserAttribute {
name: string
domain_id: string
access_mode: string
}
class hws_console_account_disable_mfa_39 {
risk: bool
login_without_mfa_rule: bool
user_name: string
user_id: string
}
input --> UserAttribute
hws_console_account_disable_mfa_39 --> input
hws_console_account_disable_mfa_39 : risk if login_without_mfa_rule
hws_console_account_disable_mfa_39 : login_without_mfa_rule if access_mode == "console" and LoginProtects == null
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary of ChangesHello @Lyc-heng, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new security rule for Huawei Cloud Identity and Access Management (IAM) to enhance account security. The rule specifically identifies console users who have not enabled two-factor authentication (MFA), flagging them as a medium-level risk. This helps ensure that critical access points are protected with an additional layer of security, aligning with best practices for cloud resource management. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
Blocking issues:
- Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (link)
- Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (link)
- Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (link)
- Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (link)
- Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (link)
General comments:
- The local vars user_name and user_id aren’t actually used by any rule—either drop them or wire them into your output/reporting logic.
- Consider renaming the package and login_without_mfa_rule to match your other policy naming conventions for better consistency and clarity.
- Double-check that input.LoginProtects is the correct path and covers both null and empty-array cases in your event schema so no console users slip through untested.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The local vars user_name and user_id aren’t actually used by any rule—either drop them or wire them into your output/reporting logic.
- Consider renaming the package and login_without_mfa_rule to match your other policy naming conventions for better consistency and clarity.
- Double-check that input.LoginProtects is the correct path and covers both null and empty-array cases in your event schema so no console users slip through untested.
## Individual Comments
### Comment 1
<location> `rules/HUAWEI_CLOUD/HUAWEI_CLOUD_IAM User_202511060951_877031/input.json:1` </location>
<code_context>
8DM44qIu6MCtkv1ytefO
</code_context>
<issue_to_address>
**security (generic-api-key):** Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
*Source: gitleaks*
</issue_to_address>
### Comment 2
<location> `rules/HUAWEI_CLOUD/HUAWEI_CLOUD_IAM User_202511060951_877031/input.json:1` </location>
<code_context>
gMT6Jdf0RD0TFcnCeY8U
</code_context>
<issue_to_address>
**security (generic-api-key):** Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
*Source: gitleaks*
</issue_to_address>
### Comment 3
<location> `rules/HUAWEI_CLOUD/HUAWEI_CLOUD_IAM User_202511060951_877031/input.json:1` </location>
<code_context>
YLLvBEJ6h4SRvCY22Li2Jo9gyFVj1jTJI7QmauOKmeYhV9YQRluvREqv96lWVSCTWsUfLNdAQuV7fJB3c7lIzrK9K6HYKTHQsgUu6D6XNxkvoAV4XMrKz6DUeWVe
</code_context>
<issue_to_address>
**security (generic-api-key):** Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
*Source: gitleaks*
</issue_to_address>
### Comment 4
<location> `rules/HUAWEI_CLOUD/HUAWEI_CLOUD_IAM User_202511060951_877031/input.json:1` </location>
<code_context>
IGFMPX7bIq2ClRDPd7wJ
</code_context>
<issue_to_address>
**security (generic-api-key):** Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
*Source: gitleaks*
</issue_to_address>
### Comment 5
<location> `rules/HUAWEI_CLOUD/HUAWEI_CLOUD_IAM User_202511060951_877031/input.json:1` </location>
<code_context>
1bZ9i4Gf7sVIVkPCNNmK
</code_context>
<issue_to_address>
**security (generic-api-key):** Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
*Source: gitleaks*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new OPA rule to detect Huawei Cloud IAM console users that do not have multi-factor authentication (MFA) enabled. The changes include the Rego policy file, along with its metadata and sample input data.
My review focuses on the correctness and maintainability of the new rule. I've identified a couple of areas for improvement:
- The Rego policy contains unused variables which should be removed to improve code clarity.
- The sample input data seems to have an inconsistency in user IDs between different objects, which should be corrected to ensure the test data is representative and reliable for this and future rules.
Overall, the core logic of the rule is sound and addresses the intended security check. The suggested changes will improve the quality of the codebase.
| @@ -0,0 +1 @@ | |||
| {"CredentialsDetail":[{"last_use_time":"ob6lkVULJqS2Aflt6eGNu19Oijv","access":"8DM44qIu6MCtkv1ytefO","create_time":"4vUbQ1P0CpAzWItRS7N4QpFYZvR","user_id":"rzCg5YULCLYpRKRigi8fYxLSodhjoaLa","description":"5lm4h7rf3RYlrJHo","status":"4OBnOh"},{"last_use_time":"bXX5k0NFc9tv10A18qljs4m8Rco","access":"gMT6Jdf0RD0TFcnCeY8U","create_time":"XGhAeUJZ7Fh0I4B51cs6am21tZd","user_id":"5w3wL6XKLtgvqzx4yvDJR23PZaIw43pp","description":"s5advgGc7XF0l62S","status":"CmY8Ql"}],"User":{"domain_id":"tnPFBTUPDBKjqvrNhoe1yEclZxqDLLbk","pwd_status":false,"name":"QLzj7U4vbond3Op3s","links":{"next":"","previous":"","self":"mSo4NHKxgfj91P9El2I9UVMX9EBBxoWcPiXWl7IUwoyrHK62s3frBGCbb0SJo6OYpd6irq3BBt4a6peWU"},"password_expires_at":"","id":"xzmYe08vYH68aPTljptq7Q3rTuiRvtCA","enabled":true},"LoginProtects":null,"UserAttribute":{"pwd_strength":"tB2c","create_time":"o0dmHCLxSdr5h8Y2AE5FW","last_login_time":"7R3Lru4uvbpOruCGQIo","description":"","access_mode":"Ib7znGK","areacode":"","enabled":true,"domain_id":"zhCme2KA8anfJWSrczczJrXF2ZPdAUV6","pwd_status":false,"xuser_id":"","update_time":"hzd8muxfAHmTFN80rK3V2","phone":"w","is_domain_owner":false,"name":"4ZVEvh36kfuWQbhP9","links":{"next":"","previous":"","self":"puui3sv9LQqOkoK2U6O9XhrGMrifTscqqlOSoebT7XjastVHV8WQWH3aTfuTIoBIIK5AhsbNozLbRfBHBN8WvRChN8q"},"id":"4qSCghh2GeXMORBbVMVYzQDIN27VYpkR","xuser_type":"","email":""},"DomainPasswordPolicy":{"password_not_username_or_invert":true,"password_char_combination":3,"password_validity_period":90,"minimum_password_age":0,"maximum_password_length":32,"password_requirements":"YLLvBEJ6h4SRvCY22Li2Jo9gyFVj1jTJI7QmauOKmeYhV9YQRluvREqv96lWVSCTWsUfLNdAQuV7fJB3c7lIzrK9K6HYKTHQsgUu6D6XNxkvoAV4XMrKz6DUeWVe","maximum_consecutive_identical_chars":0,"minimum_password_length":12,"number_of_recent_passwords_disallowed":4},"Credentials":[{"access":"IGFMPX7bIq2ClRDPd7wJ","create_time":"SkttDNS0weCBn4ngj5ytlkySnQt","user_id":"RvA8MSqBezx3TaoVHfKzrz4hNMwhMvga","description":"KOs4l6MruUYqjl7T","status":"NplZzi"},{"access":"1bZ9i4Gf7sVIVkPCNNmK","create_time":"yd7DvzFBgiiHRrk8vgZ1C4J3iaQ","user_id":"9EQ4XISDt2gj6z0fJ706PGvg8MjprtkK","description":"Zzq79ES98LUSRoou","status":"FlcLkG"}],"UserGroups":null} No newline at end of file | |||
There was a problem hiding this comment.
The test data in this file appears to be inconsistent. The User.id (xzmYe08vYH68aPTljptq7Q3rTuiRvtCA) is different from UserAttribute.id (4qSCghh2GeXMORBbVMVYzQDIN27VYpkR). According to the collector logic in collector/hws/collector/iam/user.go, the UserAttribute details are fetched for the user from the User object, so their IDs should be the same. While this doesn't break the current rule, incorrect test data can cause issues with testing and future rule development. Please correct the test data to be consistent.
| user_name := input.UserAttribute.name | ||
| user_id := input.UserAttribute.domain_id |
There was a problem hiding this comment.
The variables user_name and user_id are defined but are not used anywhere in the policy. This adds unnecessary clutter and can be confusing for future readers. It's best to remove this dead code.
1 participant
Thank you for your contribution to CloudRec!
What About:
java)go)opa)Description:
hws adds an IAM rule to check whether console users have enabled two-factor authentication
Summary by Sourcery
Add an OPA policy rule to flag Huawei Cloud console users without two-factor authentication enabled
New Features: