learningequality/ka-lite — pull request #807 by bcipolli (2013-10-10 21:23:35+00:00)
Title: Do not include new fields in _hashable_representation

This is a major bug in the current logic. FORTUNATELY, we haven't hit it--but we were about to!

Background:
* We use all "acceptable" fields (_hashable_fields) to create a string representation of an object (_hashable_representation) that we use for signing.
* After transmitting over the wire, the receiver computes the received object's _hashable_representation, then verifies that the signed value matches the computed _hashable_representation from the received object.

Issue:
* By default, a field on a model is included in the _hashable_fields. ***This includes fields that exist in one version of KA Lite and not in others***
* Across versions, if different fields are used, then the _hashable_representations will differ, and the objects won't verify when sent over the wire.

Current change:
* Only include fields that were defined in the original model definition (i.e. no "minversion" property value)
* For pre-existing models (i.e. 0.9.2 and earlier), this means having no "minversion" field..
* For models itself with a minversion, this means the same thing--only include fields with no minversion (i.e. the original object definition)

Why this isn't an issue now:
* We only have defined two *fields* with a minversion: Device.version and SyncSession.errors. Both of these objects override the default _hashable_representation method, and so this bug was not hit during previous testing, and is not an issue in production code.

Why this is an issue now:
* Because the "language" field was added, but ***cannot*** be included in _hashable_fields

Testing:
* Tested that the code works, but didn't test any cross-version syncing.

Notes:
* @aronasorman, please read this over, read through the code, and ask any questions about the method
* @jamalex please review the issue here and check that this is the right fix. I think it's pretty straightforward, but great to have your confirmation
* @jamalex This reasoning here is why I was explaining we can't add a field to DeviceZone--it would break the hashable_representation for DeviceZone. But since it's a permissions-related field, we can't simply exclude it--then it could be reset on any device, and nobody would be able to tell.