reinteractive / secretlink

TopSekr.it - Share with impunity
https://topsekr.it/
Other
4 stars 0 forks source link

Fix/virus checker breaks secret #30

Closed richardcrichardc closed 7 years ago

richardcrichardc commented 7 years ago

Bit of a minefield...

First commit updates Opscare gem so I could work on it locally - bug in old version of Skylight

Second commit updates schema.rb with a pending schema update - please check this is right - I'm new to Rails.

Third commit adds a unit test to detect the bug.

Fourth commit fixes the bug in as minimal way as I could. I started going down a rabbit hole here then pulled myself back from the brink. Looks like sending a secret is being conflated with receiving a secret. Could do with a bit of a tidy up.

Fifth commit fixes a broken link I could not help.

Also line four of secrets controller looks wrong:

before_filter :check_session, only: [:new, :create, :edit]

Surely these filters should apply to :update as well. (Better still move show method to another controller for receiving secret, remove line 3 and remove only: clause entirely, but that is down the rabbit hole)

richardcrichardc commented 7 years ago

@mikel do you want me to delete auth tokens properly:

  1. Add checks when fetching auth_tokens that they have not expired
  2. Add a cron, not sure how do do this with rails, to delete old tokens.

Would also delete old secrets in cron - currently these are checked for expiry when loaded.

Alternative to cron (how easy are crons in rails?), roll a d1000 and run a delete where expires_at < now for both secrets and auth_tokens. With the healthcheck hitting it every 5 seconds, the deletes will run every hour or two.

mikel commented 7 years ago

@richardcrichardc I think ship this as is and then we'll look at adding more functionality later.