Qt-SESAM icon indicating copy to clipboard operation
Qt-SESAM copied to clipboard

Let the user group domains

Open 607011 opened this issue 9 years ago • 31 comments

607011 avatar Sep 13 '15 06:09 607011

Should be merged with issue #5

607011 avatar Sep 21 '15 06:09 607011

Heavily working on this issue on branch Issue-31

607011 avatar Nov 04 '15 16:11 607011

TODO

  • Changes in domain settings should be reflected in viewer instantaneously. Currently, the tree is built from scratch (and this is buggy).
  • Allow sorting (ascending/descending) by group name or domain name.
  • Let the user add groups.
  • Let the user rename groups.
  • Let the user move entries between groups via drag'n'drop and via "move to list ►" shown in context menu when right-clicking on an entry.
  • Let the user delete an entry in the domain viewer.
  • After merging with master:
    • merge with #81,
    • then implement 1-click login when double-clicking on an entry in the domain viewer.

607011 avatar Nov 04 '15 16:11 607011

Merged in 'master'.

607011 avatar Dec 08 '15 05:12 607011

Currently working on drag'n'drop of items in tree view …

607011 avatar Jan 23 '16 07:01 607011

I'm stuck in the implementation of the above TODO list. @egbrt I'd appreciate your collaboration on this issue. Feel free to do whatever you want.

Only one caveat: Please do not touch the code that (de)serializes DomainSettings (DomainSettings::fromVariantMap(), DomainSettings::toVariantMap()) because changing the format of the JSON-encoded data will break compatibility with other implementations of c't SESAM, e.g. the Android App or the Python version.

607011 avatar Jan 25 '16 10:01 607011

I need to get access to d->treeModel in DomainSettings MainWindow::collectedDomainSettings(void) const, but Qt won't let me add Q_D(MainWindow). It says 'invalid conversion...in expansion of macro Q_D'. What should I do?

egbrt avatar Jan 25 '16 15:01 egbrt

@egbrt You can't access members via D-pointers in const methods.

Instead of d->treeModel use d_ptr->treeModel in const methods.

If this doesn't suit your needs remove const from method declaration, but only as a last resort.

607011 avatar Jan 25 '16 15:01 607011

@egbrt You might want to have a look at GitHub Flavored Markdown to improve the legibility of your comments ;-)

607011 avatar Jan 25 '16 15:01 607011

At the moment a domain name must be unique. This is a problem when you have multiple logins at the same 'domain' with different user names and/or have similar accounts at a different 'domain' with the same user name. For example:

  • Bank
    • DeutscheBank
      • Sparkasse [user=name1]
      • Sparkasse [user=name2]
    • BerlinerBank
      • Sparkasse [user=name1]

I think the unique identifier for a domain should be a combination of the hierarchy, domain name and user name.

egbrt avatar Jan 27 '16 16:01 egbrt

That's a good idea. However: the new system has to work with a dropdown list. The dropdown might get removed from Qt-SESAM in the future but it stays in the app and the python version.

pinae avatar Jan 27 '16 20:01 pinae

It's up to the user to select a unique domain name. I don't see any problem with it.

607011 avatar Jan 27 '16 20:01 607011

As long as you have all domain names in one place (for example a combobox) it may be okay to insist that domain names (or preferably the combination domain name + user name) should be unique. When you support a hierarchy it is different. A user will compare that with a directory structure on his computer where it is allowed to have files with the same name in different directories. Besides when you are in a specific group you don't see the domain names that are present in other groups, so how should you know what a unique name is.

egbrt avatar Jan 28 '16 08:01 egbrt

@egbrt Thanks for clarification.

Feel free to change how the combobox shows the domains and whatever is needed to implement the desired behavior. But please bear in mind that the domain name written to the (encrypted) settings must not change for already existing data sets. That's because the domain name is weaved into the generated passwords, i.e. the generated passwords will change if the corresponding domain name changes.

607011 avatar Jan 28 '16 08:01 607011

fyi I just pushed a new version of Issue-31 to my fork. I'm not yet ready for a pull request but perhaps you like to see how it is coming along.

egbrt avatar Jan 28 '16 09:01 egbrt

@egbrt I like how you are streamlining the code. E.g. it looks like the combobox is getting obsolete :+1:

607011 avatar Jan 28 '16 10:01 607011

I'm trying to install the server application in my local lampp installation to check if my changes effect the sync process. I've already changed the location of the database file to a writable directory, but I'm getting the following error:

Fatal error: Uncaught exception 'PDOException' with message 'SQLSTATE[HY000] [14] unable to open database file' in /opt/lampp/htdocs/sesam/ajax/globals.php:64 Stack trace: #0 /opt/lampp/htdocs/sesam/ajax/globals.php(64): PDO->__construct('sqlite:/home/eg...', NULL, NULL, Array) #1 /opt/lampp/htdocs/sesam/ajax/install.php(2): require_once('/opt/lampp/htdo...') #2 {main} thrown in /opt/lampp/htdocs/sesam/ajax/globals.php on line 64

egbrt avatar Feb 01 '16 16:02 egbrt

Are you sure the configured SQLite file path is writable by the webserver process? Or is it only writable by you (which isn't necessarily the same)?

607011 avatar Feb 01 '16 16:02 607011

Oops, sorry.

egbrt avatar Feb 02 '16 08:02 egbrt

The database is created now, but I'm having trouble creating a self-signed certificate. I've installed the server code into htdocs/sesam and created a self-signed certificate with openssl, but I keep getting the SSL error messages 22 (host name did not match...) and 9 (certificate is self-signed..).

egbrt avatar Feb 02 '16 10:02 egbrt

@egbrt, Generating a certificate chain with a new CA worked for me.

607011 avatar Feb 02 '16 10:02 607011

Thanks to @egbrt's great commitment to this feature it will soon be merged into the master branch. Stay tuned!

607011 avatar Feb 10 '16 07:02 607011

How is it going? Do you need any help?

egbrt avatar Feb 22 '16 09:02 egbrt

Sorry. I'm currently busy with other things.

607011 avatar Feb 22 '16 09:02 607011

@egbrt, an idea you may want to follow: A text field for incremental search in DomainTreeModel would be useful to resemble the behavior of the now gone combobox. I'd like to have it placed directly above the tree. What do you think?

I guess it can be implemented with a little help of QAbstractProxyModel

607011 avatar Feb 22 '16 09:02 607011

@ola-ct, I'm not sure how useful that would be. Currently, I'm (still) using PasswordSafe with 140 accounts divided into 20 (sub)groups and I've never needed to search for an account... I suspect the hierarchy eliminates the need for a search box.

egbrt avatar Feb 23 '16 09:02 egbrt

@egbrt Be glad that you have the memory of an elephant ;-)

For oblivious people like me the feature would be of great use …

607011 avatar Feb 23 '16 09:02 607011

Actually, you don't need the memory of an elephant. The hierarchy is like an index on your collection of passwords. You don't even have to remember the groups because those are visible as soon as you open you open the password manager. After that it is just expanding the right group, then optionally one or more subgroups and then you end up with a small list of accounts. For example, 20 (sub)groups with each 10 accounts gives access to 200 passwords.

egbrt avatar Feb 23 '16 13:02 egbrt

No problem, I'm going to implement the incremental search on my own. Because I need it more than you needed a tree viewer ;-)

607011 avatar Feb 23 '16 14:02 607011

Let me suggest a deal ;-) When you have finished merging the tree viewer into the master branch, I'm going to implement the incremental search. I'm waiting for the tree viewer to get merged into the master branch because I also have an idea about the user interface. I'd like to make the tree viewer the main user interface and move the 'right part' of the interface into a separate dialogue window.

egbrt avatar Feb 23 '16 14:02 egbrt