search

jcasbin / jdbc-adapter

JDBC adapter for Casbin
https://github.com/casbin/jcasbin
Apache License 2.0
34 stars 38 forks source link

Potential for partial/complete dataloss #14

Closed bpatters closed 4 years ago

bpatters commented 4 years ago

Failure in the savePolicy function can lead to a complete loss of all data because savePolicy doesn't use transactions AND:

  1. First drops the table
  2. Recreates the table.
  3. Executes a batch of inserts to save the policy.

If 1 succeed and a failure occurs anytime after it can lead to a partial or complete data loss.

It'd be much safer if it:

  1. Started a transaction
  2. Deleted everything in the table (not drop table, mysql doesn't support DML transactions)
  3. Inserted the new policy.
  4. Committed the transaction.

I actually don't see any usage of transactions in this Adapter, which is worrisome, but maybe I'm missing something?

hsluoyz commented 4 years ago

@fangzhengjin @nodece @gopherJ

nodece commented 4 years ago

@hsluoyz I will take up it.

nodece commented 4 years ago

Hi @bpatters , I submitted a PR, Could you review the #15 ?

hsluoyz commented 4 years ago

Fixed by: https://github.com/jcasbin/jdbc-adapter/pull/15

hsluoyz commented 4 years ago

v1.1.4 has been released on Maven and GitHub: https://github.com/jcasbin/jdbc-adapter

hsluoyz commented 4 years ago

@nodece v2.0.0 has been released. You can edit the release text: https://github.com/jcasbin/jdbc-adapter/releases/tag/v2.0.0