plugin-update-checker icon indicating copy to clipboard operation
plugin-update-checker copied to clipboard

Secret key or token should not be hardcoded

Open Streamlinelv opened this issue 6 years ago • 8 comments

Hi Janis, have been using your product for more than a year now and I am really happy with your awesome work and want to thank you for that.

Now I am going through the integration and I see that in the plugin I must add either the secret_key (Bitbucket) or token (Github). However both Bitbucket and Github strongly suggest not to hard code them into any products: "Warning: Treat your tokens like passwords and keep them secret. When working with the API, use tokens as environment variables instead of hardcoding them into your programs."

So my question is what would be the best way to still keep the updates coming but without hard coding them into the plugin?

Thanks

Streamlinelv avatar May 27 '19 07:05 Streamlinelv

Ok, I found this article: https://w-shadow.com/blog/2013/03/19/plugin-updates-securing-download-links/ And I get the idea of it but I am hosting my plugin on Bitbucket. I have already integrated license checking part. Now my idea would be to send a request over to license server before each update check to validate the license and if it is valid return consumer_key and consumer_secret. Could you give me a direction which function is responsible for running updates that I should take a look at to tweak and add this additional validation step just before sending the download request over to Bitbucket with the received keys?

Streamlinelv avatar May 27 '19 08:05 Streamlinelv

There are a couple of different ways you could go about that. At a high level, the function that checks for updates is Puc_v4p6_UpdateChecker::checkForUpdates(). You could create your own subclass of Puc_v4p6_Vcs_PluginUpdateChecker and override that function.

Aside from checking for updates, plugins (but not themes) also need to display plugin information when the user clicks the "View details" link. The checkForUpdates() function is not called in this case. Instead, the more low-level requestInfo() function is used. checkForUpdates() also indirectly calls this function. You could override requestInfo() instead.

Personally, I would do this slightly differently. I would do license validation separately and then cache the token for some limited time (e.g. 10 days). Then I would pass the token to the update checker upon initialization. This has the benefit of fewer network requests, and it could be simpler to set up.

YahnisElsts avatar May 27 '19 10:05 YahnisElsts

Thanks for the direction, Janis, will look further into this. Have a great week!

Streamlinelv avatar May 27 '19 12:05 Streamlinelv

Hey Janis,

So I have already setup the license validation and am doing it separately from the update checker, the license key is stored for 15 days so that the there would be less traffic to license server.

Now the question I'm having is how to pass back the token of the download consumer_key and consumer_secret? You mentioned "Then I would pass the token to the update checker upon initialization." - as I see this, is once I validate the license, I should also pass back these tokens to get the updates that I should also store in the database. Is this how you would suggest it to do or there is a better way with added security? Would really love to hear your thoughts since I understand you already have implemented something like this in another Premium product of yours.

Kind thanks

Streamlinelv avatar May 28 '19 06:05 Streamlinelv

Yes, that's basically what I had in mind. Store the token(s) in the database. After calling buildUpdateChecker(), load the stored token and call setAuthentication().

I do use PUC in a paid plugin, but that implementation is somewhat different since I use my own update server and not BitBucket/GitHub. When a customer activates their license on a site, my license management script generates a unique authentication token for that specific site. The plugin then saves this token in the database and uses it to retrieve updates. I think site-specific tokens are better for security because it allows me to delete compromised or expired tokens without affecting other users.

I don't know if creating a new token for each user/site would be practical with BitBucket. Even if it's possible, it might take a lot of work to set up and manage.

YahnisElsts avatar May 28 '19 09:05 YahnisElsts

Thanks for your thoughts and experience. Really great that you are sharing. Right now I guess my idea was to use one update token for all users and to manually change it once or twice a year but I am guessing it could not be the best option.. Also storing the product on my own server sounds unsafe since WP is not very famous for its security holes :) so there clearly is a dilemma I'm facing here.

Streamlinelv avatar May 28 '19 11:05 Streamlinelv

Sure, GitHub probably has better security than WordPress, but in most cases I'd be more worried about someone leaking license keys/access credentials on a pirate forum than about the possibility that someone might hack into my site. Hacking something takes a lot of work. It's probably much easier to for one person to legitimately purchase a license and then distribute the keys and/or files to everyone who wants them.

YahnisElsts avatar May 30 '19 17:05 YahnisElsts

This is exactly what I have been looking for & why I have not used this repo in production

On Thu, May 30, 2019, 1:11 PM Yahnis Elsts [email protected] wrote:

Sure, GitHub probably has better security than WordPress, but in most cases I'd be more worried about someone leaking license keys/access credentials on a pirate forum than about the possibility that someone might hack into my site. Hacking something takes a lot of work. It's probably much easier to for one person to legitimately purchase a license and then distribute the keys and/or files to everyone who wants them.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/YahnisElsts/plugin-update-checker/issues/288?email_source=notifications&email_token=ACG6WW5AXG4TSWK6XK54DPLPYADCTA5CNFSM4HPZD2U2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWS5I2A#issuecomment-497407080, or mute the thread https://github.com/notifications/unsubscribe-auth/ACG6WW6IM5QMACKO6QN4M7TPYADCTANCNFSM4HPZD2UQ .

kwcjr avatar May 31 '19 09:05 kwcjr