On 02/11/2012 12:36 AM, Endi Sukma Dewata wrote:
On 2/6/2012 8:15 AM, Petr Vobornik wrote:
ipasshpubkey attribute was added to user and host details pages.

New widget for ssh public keys was created.

Static preview:
http://pvoborni.fedorapeople.org/ssh/#identity=user&navigation=identity&user-facet=default&user-pkey=aortiz


https://fedorahosted.org/freeipa/ticket/2340

Everything works, so it's ACKed. I have some comments:

Pushed to master, ipa-2-2.


1. When you click Add to add a new SSH public key the UI creates a new
space for the key. The space is still empty, but it shows an undo
button, implying there's a change that can be saved. However, clicking
Update will generate an error "no modifications to be performed". Some
possible solutions:

(a) Show the "Set SSH key" dialog when you click Add. Once you entered
the key, it creates the new space that already contains the new key.

(b) When you click Update, remove all empty new space and do not
generate the modify operation unless there are other changes.

This should be addressed in save and test_dirty_method (omit empty keys in save). 1a is a nice idea but user can still set an empty key - it doesn't solve the problem.

2. If we decided to do #1a, the dialog title would be "Add SSH Public
Key" and the button would be Add. For edit the title would be "Edit SSH
Public Key" and the button would be Update.

3. Instead of showing "New: key set" or "New: key not set" we probably
can show the first few characters of the key itself. This way if you're
entering multiple keys you know which ones are already added.

Like it.

4. It's also possible to use a table widget, which will work like
solution #1a except that all changes are saved immediately so we don't
need to show the partial key like in #3.

I don't think table fits in the facet. Table would be better candidate for attribute with more than one displayed column.

5. Instead of using a "Show/Set key" link, we probably can link the
fingerprint (for existing keys) or the partial key like in #3 (for new
keys). If we did the table like in #4, it can be an Edit link.

Might work.

6. When modifying a key, changing the fingerprint to "Modified" is
probably not necessary because the undo button will imply that the key
has been modified and we can't see the original fingerprint anymore.

Maybe. I didn't want to show the fingerprint because it is not valid for the modified value.

7. When you edit an existing key and set it into empty, it will say
"Modified: key not set". I think we can just strikeout the fingerprint
because it's the same as deleting the key.

Agree.

8. This is a minor server issue, not sure whether it's already fixed in
the latest patch. When adding a new user with a key, the output of the
user-add command contains the key too. In user-show the key is only
shown with the --all option.


1-7 - I'll address them later.


--
Petr Vobornik

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to