ptsochantaris/trailer

Store GitHub API tokens more securely

Closed this issue · 5 comments

mkantor commented

I noticed that the API tokens are stored in plaintext in user defaults. It'd be nice if they were encrypted on disk, perhaps using Keychain Services?

mkantor commented

Actually, since updating to 1.1.0 I don't see the token anymore when running $ defaults read com.housetrip.Trailer, so maybe this has been resolved. How are tokens stored now?

ptsochantaris commented

Hi @mkantor, yes, tokens are now stored in the server settings, in the database.

1.1.0 and up will auto-migrate your existing token and API settings from defaults and create an entry for a server based on them, removing the settings in the process.

However, they the settings the DB are still clear-text and easily readable by anyone with access to the SQLite store in ~/Library/Application Support/com.housetrip.Trailer

I like your suggestion about possibly using Keychain for auth tokens, and will investigate this more, but I'm worried about reducing the simplicity of handling tokens, which currently makes Trailer very easy and simple to set up initially. But I will check it out first, because it's an API I'm not well aquatinted with, and check back with you.

(And then of course there is the slippery slope of wondering about expanding auth for servers, things like full OAuth instead of the simple API tokens currently used, etc :))

mkantor commented

Sounds good. If you do decide to go this route, UICKeyChainStore may come in handy.

ptsochantaris commented

That's good advice, I've had a chance to use this recently and it's awesome how simple it makes everything 👍 However one part of building Trailer for me is to try and keep it clear of any pods, mostly to nudge me to investigate more aspects of Cocoa's API rather than simply relying on (the excellent and well tested functionality of) external pods.

Still thinking about this issue, and I still agree it's a good idea to keep the token better hidden, so stay tuned :)

ptsochantaris commented

Please follow the link to the issue above (#470) which contains a test build with this feature enabled. Will close this (erm... 9 year old 🤦) issue now. You're welcome! <64 ton weight, splat>