tmux-1password icon indicating copy to clipboard operation
tmux-1password copied to clipboard

Feature: Local cache for op items

Open delucca opened this issue 4 years ago • 17 comments

☕ Purpose

After I started using #13, I've noticed that it takes too long if every time I fetch passwords my computer needs to go to the 1Password server to fetch them.

To fix this, I've created this PR, which saves a local cache containing the result of op list-items. The cache is available for 6 hours :)

This PR is based on #13, so some of the code here may looks different but it is actually the implementation of #13. As soon as that PR is merged, it would be more clear to see the diffs.

🧐 Checklist

  • [x] Adds a function to save the cache
  • [x] Adds a condition to use cache instead of fetch, if available
  • [x] Adds a condition to remove the cache if it is older than 6hours

delucca avatar Feb 01 '20 01:02 delucca

While I love the idea of caching, putting all of my incredibly sensitive passwords in an unencrypted file in a directory that every program running on my computer has read access to seems like a really bad idea, and not something I would be comfortable with from a security perspective.

camspiers avatar Feb 01 '20 04:02 camspiers

@camspiers the file contains only the title and ID of the account :) it does not contain the password itself.

The password is fetched by the plug-in afterwards, when you select which account you want to get

delucca avatar Feb 01 '20 06:02 delucca

Just to give more details, the data I'm caching is the return of op list-items after being filtered by jq, which results only in tuples containing: (Account title, Account ID)

This is not sensitive data, so there is no need to encrypt it.

Even if someone stole your file, they would need your 1Password password to fetch the actual data for your account. So, in my opinion, there is no need for any extra security step here

delucca avatar Feb 01 '20 06:02 delucca

Oh right. Excellent!

camspiers avatar Feb 01 '20 12:02 camspiers

@yardnsm can you take a look at this please? :D thanks!

delucca avatar Feb 01 '20 16:02 delucca

btw, apologies for the false assumption above. This feature is going to be really valuable. Thanks!

camspiers avatar Feb 01 '20 16:02 camspiers

@yardnsm nice idea! I'm going to search for a proper keybinding to use!

About 1pass, in my opinion it is an awesome tool, but too hard to setup (I tried to setup it many times and there is a bunch of issues with it) and it doesn't adds too much value to op (in my opinion).

So, this could be a good alternative to those who won't use 1pass

delucca avatar Feb 01 '20 21:02 delucca

@yardnsm I'm going to take a look on your suggestions on the following days.

Also, I've noticed another problem. Since we're just asking for user password during the fetch_items, if the user session has expired after selecting a cached item the user would not be able to fetch the password itself.

I must also fix this by asking for the password during afterwards if the session has expired

delucca avatar Feb 02 '20 17:02 delucca

@yardnsm I've updated the code :) can you please review it and merge with master?

I've also updated the tmux-token path (from $HOME to /tmp), to match the pattern of storing temporary data on the right place ($HOME should not be used for tmp data)

The only thing that I wasn't able to do was add a condition to login the user if the session expired. op doesn't provide a command to validate the current session, so I reduced the CACHE_TTL to 30 minutos in order to match the session token TTL.

delucca avatar Feb 07 '20 05:02 delucca

@yardnsm thanks for the review, I'm going to take a look on your comments.

delucca avatar Feb 11 '20 00:02 delucca

@yardnsm I've fixed your comments, and also fixed a bug.

The cache was being automatically cleared before fetch_items. Now it automatically clears the cache, before fetching items, if the cache is old enought. :)

Waiting for you review!

delucca avatar Feb 11 '20 00:02 delucca

@yardnsm done :) Waiting for you approval

delucca avatar Feb 12 '20 01:02 delucca

I've merged #18, which continues #13. You can revert the changes from #13 and it should be fine 👌

yardnsm avatar Feb 24 '20 21:02 yardnsm

Great! Thanks.

I'm going to revert it in the next few days and get in touch with you ;)

delucca avatar Feb 25 '20 03:02 delucca

@yardnsm sorry for the late reply. I've just finished it and you can merge right now ;) Thanks!

delucca avatar Mar 19 '20 21:03 delucca

@yardnsm will this branch ever be merged? :)

delucca avatar Apr 17 '20 16:04 delucca

This looks good, great work @odelucca! Any chance of this being merged @yardnsm?

Also, do you think it's worth updating the permissions of the temporary files so they can't be read by other system users?

tapayne88 avatar Jun 15 '20 11:06 tapayne88