Implement C_CopyObject for DB backend #540
Conversation
|
Ignore the CI warning, something appears broken there (it is on our radar). Thanks for the PR, this seems like a somewhat more major change, any objections to scheduling this for merging after we release 2.6.0 and tagging it for a follow-up release? |
|
No objections at all, in fact I'm using this code for a few months and held it for the next release window myself (there were multiple requests to make a release and I did not want to postpone the release further). After 2.6.0 we can discuss and develop a more generic approach to token versioning, from which more issues will profit (#403). I do not consider these commits absolutely final, as the approach of attempting migration in every single opening is not perfect. |
|
I don't think we should include this in 2.6.1 but in 2.7.0. This change will perform a database change. Not only should minor patch releases contain no schema changes (or even significant functional changes), but moreover this makes it impossible to downgrade to the previous patch version, or easily migrate a token storage from one installation to another (or back). This is used and will be unexpected between minor patch versions. |
|
Actually, the change should be both forward and backwards compatible (though only forward compatibility was tested, and just once :) ). The view is created automatically, and is ignored by older versions (AFAIK). Also, from my POV no new feature is added - just that now C_CopyObject is broken, and can be fixed in a patch release. The db schema change is unfortunate, but I can't do better than this. As I said though, no pressure from my side - we already need to use patched version of SoftHSM. |
|
Please rebase on develop and mark as ready when ready. |
This helps to debug backends. Signed-off-by: Ondřej Hlavatý <[email protected]>
This view can be used to look up attributes. Signed-off-by: Ondřej Hlavatý <[email protected]>
Uses previously added view to select the attribute with lowest id larger than the last one. Signed-off-by: Ondřej Hlavatý <[email protected]>
These are commonly present and missing them causes inconsistent behavior - attributes can be stored successfully but not retrieved later. Signed-off-by: Ondřej Hlavatý <[email protected]>
Aearsis
force-pushed
the
feature/db-copy-object
branch
from
2f382e2 to
e8d542e
Compare
Formerly there was no way to list all attributes, as they are scattered in different tables. This PR implements it using a view, through which all attributes can be seen. This view is then used for attribute enumeration, allowing C_CopyObject to work.
The view is added in
DBObject::migrateTables(), which is called when opening existing token. It should be compatible with tokens created in previous version.If we did not insist on minimalizing changes, it might be even better to throw all attributes in a single table - SQLite is fine with varying types of table cells.