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

Improve modify_user function #347

Merged
merged 14 commits into from
Nov 26, 2020
Merged

Conversation

ArnoStiefvater
Copy link
Member

@ArnoStiefvater ArnoStiefvater commented Nov 25, 2020

What:

The comment, auth_source and group_ids parameters got added for gmpv7. Now its possible to change those fields too. auth_source has to be used explicitly if not the default source allowed for authentication for the user is to be used.
modify_user now only takes an ID for identifying the user which is to be modified for gmpv214.
The name parameter replaces the old new_name parameter for gmpv214 and is not used for identifying a user anymore.

Why:

To be able to set the appropriate fields.

How:

Added tests.

Checklist:

@codecov
Copy link

codecov bot commented Nov 25, 2020

Codecov Report

Merging #347 (7407d81) into master (4f5a72a) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #347      +/-   ##
==========================================
+ Coverage   97.49%   97.51%   +0.02%     
==========================================
  Files          20       20              
  Lines        4264     4311      +47     
==========================================
+ Hits         4157     4204      +47     
  Misses        107      107              
Impacted Files Coverage Δ
gvm/protocols/gmpv208/types.py 100.00% <ø> (ø)
gvm/protocols/gmpv214/types.py 100.00% <ø> (ø)
gvm/protocols/gmpv8/types.py 100.00% <ø> (ø)
gvm/protocols/gmpv9/types.py 97.15% <ø> (ø)
gvm/protocols/latest.py 100.00% <ø> (ø)
gvm/protocols/gmpv214/gmpv214.py 100.00% <100.00%> (ø)
gvm/protocols/gmpv7/gmpv7.py 99.15% <100.00%> (+<0.01%) ⬆️
gvm/protocols/gmpv7/types.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f5a72a...7407d81. Read the comment docs.

@y0urself
Copy link
Member

As mentioned before: I think this should be in an earlier version of gmp ...?!

@ArnoStiefvater
Copy link
Member Author

ArnoStiefvater commented Nov 25, 2020

As mentioned before: I think this should be in an earlier version of gmp ...?!

But this change completely breaks the API. For example it is not possible to identifying a user by name anymore. Its not only adding stuff. I can modify the 208 function to include the comment, user_auth and group_ids params, but the name change breaks the API.

Modify user now only takes an ID for identifying
the user which is to be modified.
The name parameter is now the new name and not an
identifier anymore.
The comment, auth_source and group_ids parameters
got added. Now its possible to change those fields
too.
auth_source has to be used explicitly if not the
default Source allowed for authentication for
the user is to be used.
Add parameters comment, auth_source and group_ids.
Make documentation about hosts_allow and
ifaces_allow clearer.
@ArnoStiefvater ArnoStiefvater changed the title Modify user Improve modify_user function Nov 26, 2020
gvm/protocols/gmpv7/gmpv7.py Outdated Show resolved Hide resolved
@ArnoStiefvater ArnoStiefvater marked this pull request as ready for review November 26, 2020 11:26
@ArnoStiefvater ArnoStiefvater requested a review from a team as a code owner November 26, 2020 11:26
@y0urself
Copy link
Member

you may add a test for the new Enum UserAuthType ? In testtypes?

@y0urself y0urself merged commit b4bcf92 into greenbone:master Nov 26, 2020
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.

3 participants