[Freeipa-devel] [freeipa PR#139][opened] WebUI: Vault Management

[pvomacka]

URL: https://github.com/freeipa/freeipa/pull/139
Author: pvomacka
Title: #139: WebUI: Vault Management
Action: opened

PR body:
"""
Add vault management into WebUI, there are some constraints:
- There is no crypto library so Symmetric and Assymetric vaults
are not supported in WebUI. Also retrieving or archiving data
is not supported.
- There aren't any container support right now

Supported is:
- Browsing vaults
- Adding Standard vaults (users, service, shared)
- Removing vaults
- Adding and removing owners
- Adding and removing members
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/139/head:pr139
git checkout pr139

[pvomacka]

URL: https://github.com/freeipa/freeipa/pull/139
Author: pvomacka
Title: #139: WebUI: Vault Management
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/139/head:pr139
git checkout pr139

[mbasti-rh]

URL: https://github.com/freeipa/freeipa/pull/139
Title: #139: WebUI: Vault Management

mbasti-rh commented:
"""
I'm not sure if this is done on purpose, but Vault section is shown there even I have no KRA installed in topology, and I'm getting error

```
An error has occurred (IPA Error 3000: InvocationError)

KRA service is not enabled
```

It is not nice, IMO some placeholder pointing to ipa-kra-install could be better
"""

See the full comment at https://github.com/freeipa/freeipa/pull/139#issuecomment-251978110

[pvoborni]

URL: https://github.com/freeipa/freeipa/pull/139
Title: #139: WebUI: Vault Management

pvoborni commented:
"""
For other optional UIs like CA/Trusts or DNS, Web UI checks on UI start if the component is installed by batch command with:

```JavaScript
{method: "env", params: [[], {}]}
{method: "dns_is_enabled", params: [[], {}]}
{method: "trustconfig_show", params: [[], {}]}
{method: "domainlevel_get", params: [[], {}]}
{method: "ca_is_enabled", params: [[], {}]}
```
For KRA, it can add kra_is_enabled command.

Traditionally, UI is hidden if component is not installed.



"""

See the full comment at https://github.com/freeipa/freeipa/pull/139#issuecomment-251997063

[mbasti-rh]

URL: https://github.com/freeipa/freeipa/pull/139
Title: #139: WebUI: Vault Management

mbasti-rh commented:
"""
1)
I created shared vault, but I cannot see it in 'Shared Vaults', it is show only in 'My Vaults'
i.e. it was created ad user vault according CLI

2)
'My Vaults' I expected that it will show all vaults created by me, but it is not true, it shows only my user vaults. Can we set name to more explicit, like 'My User Vaults' or is it too much and only I'm dumb?

3)
I broke it, I cannot add vault, adder dialog just show and instantly disappears

Steps to reproduce:
a. click on add vault
b. click on vault user, mark empty line (dont click)
c. press ESC (dialog should disappear)
d. click on add Vault again, it should not work
e. dialog suddenly shows when you click on My Vaults

No errors in browser console. What could cause this?

4)
Can you please add tests for this?

5) Nitpick
If you add vault from Service vault, then predefined value should be service vault in adder dialog.
Same for shared vault

6)
Missing 'type' column in my vaults

7)
For symmetric vault, there is 'salt' shown in CLI, and I can change this in CLI. IMO this should be supported in webUI too

8)
For asymetric vault, public key is show in CLI, and user can also change this public key, IMO this should work in webUI too.

9)
I would like to see big fat warning in adder dialog that content of 'standard' vaults can be seen by users with higher privileges (admins). This is the reason why we set symmetric vault as default in CLI. But because in webUI the standard vault is the only one vault that can be added, we should inform users to use rather CLI and create symmetric vault

* IMO we should add this warning into CLI too

10)
Vaultconfig-show shows transport certificate, should we shown this in webUI as well?

"""

See the full comment at https://github.com/freeipa/freeipa/pull/139#issuecomment-252016356

[mbasti-rh]

URL: https://github.com/freeipa/freeipa/pull/139
Title: #139: WebUI: Vault Management

mbasti-rh commented:
"""
Yeah, and I forgot to write:

11)
There should be an information in webUI, that secrets can be added/retrieved to vault only by using vault-archive and vault-retrieve from CLI
"""

See the full comment at https://github.com/freeipa/freeipa/pull/139#issuecomment-252018655

[mbasti-rh]

URL: https://github.com/freeipa/freeipa/pull/139
Title: #139: WebUI: Vault Management

mbasti-rh commented:
"""
Please disregard comment 1), it works (sorry :( my fault)
"""

See the full comment at https://github.com/freeipa/freeipa/pull/139#issuecomment-252020032

[pvomacka]

URL: https://github.com/freeipa/freeipa/pull/139
Author: pvomacka
Title: #139: WebUI: Vault Management
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/139/head:pr139
git checkout pr139

[pvomacka]

URL: https://github.com/freeipa/freeipa/pull/139
Title: #139: WebUI: Vault Management

pvomacka commented:
"""
@mbasti-rh
2) fixed
3) I filled a ticket: https://fedorahosted.org/freeipa/ticket/6388
4) Tests added
5) Fixed
6) Fixed
7) Salt added
8) Field for public key added
9) Warning added
10) Transport certificate is now visible in WebUI
11) Information added into adder dialog

The issue with showing error in case that KRA is not installed is also fixed.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/139#issuecomment-256062716

[pvomacka]

URL: https://github.com/freeipa/freeipa/pull/139
Author: pvomacka
Title: #139: WebUI: Vault Management
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/139/head:pr139
git checkout pr139

[pvomacka]

URL: https://github.com/freeipa/freeipa/pull/139
Title: #139: WebUI: Vault Management

pvomacka commented:
"""
Fixed PEP8 errors.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/139#issuecomment-256271405

[mbasti-rh]

URL: https://github.com/freeipa/freeipa/pull/139
Title: #139: WebUI: Vault Management

mbasti-rh commented:
"""
NACK

1)
view My User Vaults/Add vault
There is no marked radio button and all fields are shown which are mutual exclusive. A one option from radio group should be marked. It is doing fancy things when no radiobutton is marked and you fill other fields and press add.

2)
Vault config view shows only one server, not list of all KRA servers installed

3)
I'm quite puzzled wit behavior `User Vaults` and `My User Vaults` executes following commands
```
User Vaults: vault-find --users --pkey-only
My Users Vaults: vault-find
```

So how does it actually works? What `My User Vault` do then? I would expect a filter in that command and users flag. Also why in one case was called command with --pkey-only and not in second time?
"""

See the full comment at https://github.com/freeipa/freeipa/pull/139#issuecomment-262308679

[pvomacka]

URL: https://github.com/freeipa/freeipa/pull/139
Author: pvomacka
Title: #139: WebUI: Vault Management
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/139/head:pr139
git checkout pr139

[pvomacka]

URL: https://github.com/freeipa/freeipa/pull/139
Title: #139: WebUI: Vault Management

pvomacka commented:
"""
@mbasti-rh Thank you for review.

1. Fixed
2. Fixed
Additionally I fixed two more issues which I found during testing. The tables on User Vaults or Service Vaults didn't show different (different users/services) vaults which have the same name - FIXED now.
The second issue was that when i.e. admin add user/share vault on service page, the search page with new vault was not refreshed - FIXED now.

3. My User Vaults calls vault-find which returns all Vaults which have currently logged user is in Owners or Members group. It is called without --pkey-only, because we want to get also information about vault type in response.
User Vaults shows all user vaults (of all users) and there is --pkey-only because we call vault-show for each user vault which is returned and in each vault-show response we get all (and several more) information which are also in vault-find (without --pkey-only). So, we don't need to transfer data (those parts which are in both responses) twice.

I understand that the difference between those two sections could not be very clear. If you have any idea on how to improve this feel free to put a comment here or open a ticket.

"""

See the full comment at https://github.com/freeipa/freeipa/pull/139#issuecomment-266378821

[mbasti-rh]

URL: https://github.com/freeipa/freeipa/pull/139
Title: #139: WebUI: Vault Management

mbasti-rh commented:
"""

Post by pvomacka
I understand that the difference between those two sections could not be very clear. If you have any idea on how to improve this feel free to put a comment here or open a ticket.

I have, you can extend vault-find command :)
"""

See the full comment at https://github.com/freeipa/freeipa/pull/139#issuecomment-266413702

[mbasti-rh]

URL: https://github.com/freeipa/freeipa/pull/139
Title: #139: WebUI: Vault Management

mbasti-rh commented:
"""
NACK: DNS records page is broken
"""

See the full comment at https://github.com/freeipa/freeipa/pull/139#issuecomment-267106705

[mbasti-rh]

URL: https://github.com/freeipa/freeipa/pull/139
Title: #139: WebUI: Vault Management

mbasti-rh commented:
"""
Server roles page is broken too, or at least it looks weird, probably server names are missing
"""

See the full comment at https://github.com/freeipa/freeipa/pull/139#issuecomment-267114366

[pvomacka]

URL: https://github.com/freeipa/freeipa/pull/139
Title: #139: WebUI: Vault Management

pvomacka commented:
"""
@mbasti-rh Both bugs fixed, thank you.

Back to the difference between My User Vault and User Vault. I forgot to mention that My User Vault shows only vaults which are created for the user (who is logged in) and where that user is in Member or Owner group. I think that it is consistent with CLI, or not?
"""

See the full comment at https://github.com/freeipa/freeipa/pull/139#issuecomment-268269736

[pvomacka]

URL: https://github.com/freeipa/freeipa/pull/139
Author: pvomacka
Title: #139: WebUI: Vault Management
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/139/head:pr139
git checkout pr139

[MartinBasti]

URL: https://github.com/freeipa/freeipa/pull/139
Title: #139: WebUI: Vault Management

MartinBasti commented:
"""
Works for me
"""

See the full comment at https://github.com/freeipa/freeipa/pull/139#issuecomment-273845743

[tiran]

URL: https://github.com/freeipa/freeipa/pull/139
Title: #139: WebUI: Vault Management

tiran commented:
"""
@MartinBasti you approved this PR a month ago but it has neither the ACK flag nor was it merged.

@pvomacka Your work would be useful for my Custodia Vault work. Can you rebase this PR to master to verify it still works?
"""

See the full comment at https://github.com/freeipa/freeipa/pull/139#issuecomment-282053796

[pvomacka]

URL: https://github.com/freeipa/freeipa/pull/139
Author: pvomacka
Title: #139: WebUI: Vault Management
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/139/head:pr139
git checkout pr139

[pvomacka]

URL: https://github.com/freeipa/freeipa/pull/139
Title: #139: WebUI: Vault Management

pvomacka commented:
"""
@tiran Yes, rebased.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/139#issuecomment-282060928

[pvomacka]

URL: https://github.com/freeipa/freeipa/pull/139
Author: pvomacka
Title: #139: WebUI: Vault Management
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/139/head:pr139
git checkout pr139

[pvomacka]

URL: https://github.com/freeipa/freeipa/pull/139
Author: pvomacka
Title: #139: WebUI: Vault Management
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/139/head:pr139
git checkout pr139

[pvoborni]

URL: https://github.com/freeipa/freeipa/pull/139
Title: #139: WebUI: Vault Management

Label: +ack

[MartinBasti]

URL: https://github.com/freeipa/freeipa/pull/139
Author: pvomacka
Title: #139: WebUI: Vault Management
Action: closed

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/139/head:pr139
git checkout pr139

[MartinBasti]

URL: https://github.com/freeipa/freeipa/pull/139
Title: #139: WebUI: Vault Management

MartinBasti commented:
"""
master:

* c3115fa617fb049ba48d356d280fdb23c312ebca Additional option to add and del operations can be set
* ec63456b7c1fba6bd8d9073e63c27ef685f08c60 Allow to set another other_entity name
* 93a7f4c88db159664664bd82d1d00e5e0033ac22 Possibility to skip checking writable according to metadata
* 6d1374f7f82d144b8aa361e9e637c5388f8f7edb Added optional option in refreshing after modifying association table
* bbca1d9219bfab9f204cb0217495cbd94b7098be Add property which allows refresh command to use url value
* 042e113db9bc66dcd0da0d5e8b8d025212695705 Add possibility to pass url parameter to update command of details page
* 2e6e0698865e7d530c6ebf87a12e46f990ac1d87 Extend _show command after _find command in table facets
* 039a6f7b4ff392974408cb9e274f8a3777e009fd Possibility to set list of table attributes which will be added to _del command
* 8dfe692251d38934a21ad3bc648d839d83e27caa Add possibility to hide only one tab in sidebar
* de4d4a51b542b8e473919dbc14f7a0810944b544 WebUI: search facet's default actions might be overriden
* 587b7324fb1f6899deb151c30662362c18c5258e WebUI: allow to show rows with same pkey in tables
* 39d7ef3de4b0345274b4b8e8f6918e3b714879ad WebUI: add vault management
* ab8c69f4c602c0eaefbb058c108428ca30a80e98 TESTS: Add support for KRA in ui_driver
* 0808504ba1ab743acdf4231876d49c26dbae6621 TESTS: Add support for sidebar with facets
* f95275748465ffacecfbf55ca2cd2fc54f3860b7 TESTS WebUI: Vaults management
"""

See the full comment at https://github.com/freeipa/freeipa/pull/139#issuecomment-286369632

[MartinBasti]

URL: https://github.com/freeipa/freeipa/pull/139
Title: #139: WebUI: Vault Management

Label: +pushed