swlkr / allyourpasswords Goto Github PK

I would suggest not quitting the app when the app window is closed (also please remember to make it possible to spawn the window again when the app is clicked on in the Dock)

Also, currently allyoupasswords doesn't remember window position/size after quitting and opening.

Keep up the cool work! 🚫☁️

It would be really nice to add TOTP secrets support.

Type: Bug report

Description

The listing of login items does not allow a user to immediately select the top item in the listing for viewing in the right-hand detail container.

Information

macOS: 10.14
Xcode: 10.2

This issue was present in my forked version of the project at 383946f.

Expected results

When logged into the application and the list of logins view is presented, immediately selecting the top item in the list (the default pre-highlighted item) should then be rendered in the right-hand window port by the containerViewController member of TableViewController, and the item should be selected in the tableView as to highlight that it is the currently selected item in another colour.

Actual results

When logged into the application and the list of logins view is presented, immediately selecting the top item in the list (the default pre-highlighted item) has no effect on the application and does not pass the clicked upon item index to the containerViewController member of TableViewController and so the item is not viewable.

However, should you first click onto another list item (eg. index > 0 == true) you will see that item displayed in the containerViewController. If you then re-select index 0 by clicking it, it will then be displayed in the containerViewController.

Analysis

It seems to be that by using the following lines in reloadTableView(), when the window first loads, the top item in the list is assigned as tableView.selectedRow as it's at index 0. This applies the default, darker highlighting on the item in the left-hand sidebar TableView.

  tableView.selectRowIndexes(NSIndexSet(index: index) as IndexSet, byExtendingSelection: false)
  tableView.scrollRowToVisible(index)

When clicking any other login item, the function tableViewSelectionDidChange is triggered due to tableView.selectedRow changing value. However, if we select the 0th login item (topmost), this event is not triggered as I suspect that tableView.selectedRow is already set to 0 and so does not change.

Hey!

Really enjoying your blog series and this project. I'm tinkering in Swift/Objective-C with your project to learn (and hopefully contribute).

Sadly, I've encountered a few issues with the database communication. I'm not 100% where the problem lies (my environment, allyourpasswords, SQLite.swift).

Information

Issue type: bug

macOS: 10.14
Xcode: 10.2
SQLite.swift: 0.11.5
SQLCipher: 4.1.0

This issue was present in my forked version of the project at 383946f.

Description and Analysis

The following error occurs when running the application and entering the master password, then hitting ‘Enter’.

CREATE TABLE IF NOT EXISTS “prop” (“id” INTEGER PRIMARY KEY NOT NULL, “login” INTEGER NOT NULL, “name” TEXT, “value” TEXT)**
**SELECT * FROM “login” ORDER BY “name”, “url”**
**Fatal error: ‘try!’ expression unexpectedly raised an error: unrecognized token: “:” (code: 1): file ../allyourpasswords/allyourpasswords/ContainerViewController.swift, line 30**
**2019-04-13 16:19:30.081910+0100 allyourpasswords[16167:112381] Fatal error: ‘try!’ expression unexpectedly raised an error: unrecognized token: “:” (code: 1): file ../allyourpasswords/allyourpasswords/ContainerViewController.swift, line 30

As this is the first time I’ve run the application, my suspicion is it’s caused by the following line when it receives a 0 value as I’ve no logins saved yet:

let rowCount = try! db?.scalar(login.table.count)
...
if (rowCount ?? 0 > 0) {
    row = firstRow
    showDetailViewController()
} else {
    showEmptyViewController()
}

I'm new to Swift/Objective-C but my research/experimentation shows that the following guard protects against this unhandled exception by allowing the nil-coalescing operator in the if statement to take the (now optional due to the ?) rowCount as nil thus using the default value 0 when logging in for the first time (no logins yet). This obviously doesn't fix the bug itself as this just returns from the function immediately, however, it will allow us to log in to our development versions of the app to find a proper solution.

guard let rowCount = try? db?.scalar(login.table.count) else {
    return
}

A very similar bug occurs when creating or editing a login, clicking the Cancel button will throw the following unhandled exception:

CREATE TABLE IF NOT EXISTS "prop" ("id" INTEGER PRIMARY KEY NOT NULL, "login" INTEGER NOT NULL, "name" TEXT, "value" TEXT)
SELECT * FROM "login" ORDER BY "name", "url"
Fatal error: 'try!' expression unexpectedly raised an error: unrecognized token: ":" (code: 1): file ../allyourpasswords/allyourpasswords/EditViewController.swift, line 140
2019-04-13 16:31:36.558772+0100 allyourpasswords[16433:116736] Fatal error: 'try!' expression unexpectedly raised an error: unrecognized token: ":" (code: 1): file ../allyourpasswords/allyourpasswords/EditViewController.swift, line 140

This stems from the cancelButtonClicked function and appears to be for the same reason as the ContainerViewController::viewWillAppear() function.

    @IBAction func cancelButtonClicked(_ sender: NSButton) {
        let container = self.parent as! ContainerViewController
        let rowCount = try! db?.scalar(login.table.count)

        if rowCount ?? 0 > 0 {
            container.row = self.row
            container.showDetailViewController()
        } else {
            container.showEmptyViewController()
        }
    }

Upon investigation, let rowCount = try! db?.scalar(“SELECT COUNT(*) FROM login”) will return the correct number of rows, but login.table.count always returns 0 if using a guard, and throws an unhandled exception if not using an optional try! (in other words, this throws: let rowCount = try! db?.scalar(login.table.count)). Suggests to me an issue within SQLite.swift.

Thanks!

i'can't run this code because xcode gives me some errors with SQLite.

Hello, saw this on PH and wanted to check out the source. Disclosure: I work for 1Password (I assume 1Password is what you refer to when you referred to your old pwd mgr 😜)

Now, I just want to ask what sorts of security architecture you have here, and maybe I can suggest some improvements:

I see that you store the MP in the keychain. That's a good start, probably better than some. My main issue is that you do a string comparison to check if the password is correct; this is actually too fast. If an attacker wanted to brute force the password, they could do millions of guesses per second this way. Additionally, if a keychain vuln were to happen (like the super bad one last year), the encryption key is a sitting duck. I would research "slow hash functions" and choose a cryptographically secure one, then store the hashed version in the keychain so that 1) each guess requires a long operation to do the comparison and 2) the encryption key isn't stored in the keychain.

I have other thoughts but I don't want to dump them all at once, just wanted to make some friendly suggestions :) Password security is everyone's responsibility.