Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First Draft of the User Module #42

Merged
merged 47 commits into from
Apr 17, 2024
Merged

Conversation

KiLLuuuhh
Copy link
Contributor

@KiLLuuuhh KiLLuuuhh commented Feb 9, 2024

This pull request introduces the User Module with the following capabilities:

  • Create, update or delete User with multiple attributes including setting the password, otp_seed or api_key
  • Handles group memberships of users

While I welcome comprehensive feedback on the entire module, there are three functions in particular where I'd appreciate more focused and detailed review:

  1. set_apikeys
    I wanted to give the user the opportunity to set custom apikeys. However, to check the validity of the key, I only used a length validation. I hope to get some feedback from you to handle this case properly. Please consider also, how I handle the generation of the key, if None is provided. Regarding the apikey there is another uncertainty: If no key is provided, one is generated and could be returned to the user. I handled this using a new result attribute "apikeys" in the system_access_users.py file.
  2. Tests
    Please let me know, if additional tests must be implemented.

Note on Exclusion of "Certificates" Attribute:
I would like to point out that I consciously excluded the "certificates" attribute from this iteration of the user module. This decision was based on the fact, that when the checkbox is checked in the GUI, the User is redirected to the certificate page. The system_trust_certificates module is considered a seperate module in our Roadmap: #41

@KiLLuuuhh KiLLuuuhh self-assigned this Feb 9, 2024
@DonGiovanni83
Copy link
Contributor

In order to follow the naming convention (#13) used for modules, we should rename the new user module to system_access_users

@ombre8
Copy link
Member

ombre8 commented Mar 21, 2024

this resolves #50

molecule/system_access_users/converge.yml Outdated Show resolved Hide resolved
molecule/users/molecule.yml Outdated Show resolved Hide resolved
plugins/module_utils/system_access_users_utils.py Outdated Show resolved Hide resolved
plugins/module_utils/system_access_users_utils.py Outdated Show resolved Hide resolved
plugins/module_utils/system_access_users_utils.py Outdated Show resolved Hide resolved
plugins/modules/system_access_users.py Show resolved Hide resolved
plugins/modules/system_access_users.py Outdated Show resolved Hide resolved
@KiLLuuuhh KiLLuuuhh force-pushed the add/user_module branch 2 times, most recently from 78348ba to 043396b Compare April 12, 2024 14:24
@KiLLuuuhh KiLLuuuhh requested a review from rekup April 15, 2024 08:51
Copy link
Contributor

@rekup rekup left a comment

Choose a reason for hiding this comment

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

🥳

@KiLLuuuhh KiLLuuuhh merged commit e5bd046 into puzzle:main Apr 17, 2024
30 checks passed
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.

4 participants