Model inconsistency when sending a new password profile from the Android client

[ogarcia]

There is a model inconsistency when sending a new password profile from the Android client (and maybe from iOS) in the numbers field.

When a profile is sent from Android it looks like this:

{
  "id": null,
  "site": "a",
  "login": "v",
  "lowercase": true,
  "uppercase": true,
  "number": true,
  "digits": true,
  "symbols": true,
  "length": 16,
  "counter": 1
}

But when it is sent from the extension or from the web it has this other scheme

{
  "login": "user@example.com",
  "site": "example.com",
  "uppercase": true,
  "lowercase": true,
  "numbers": true,
  "symbols": true,
  "length": 16,
  "counter": 1,
  "version": 2
}

As you can see there is a difference. Mobile clients send the number field and the rest send the numbers field which is an inconsistency in the model.

This is affecting Rockpass, I can fix it, but I think it would be more "elegant" to use the same model in all requests to the server.

UPDATE: Not only is there inconsistency in numbers, the mobile application sends a digits field (which I understand is deprecated) and does not send the version field.

[edouard-lopez]

@ogarcia thanks for spotting this,
If you want to take a shot at it, here are the relevant tests and code files. I think having @guillaumevincent insight will be best.

mobile

• lesspass/mobile/src/password/passwordGenerator.test.js

  Line 54 in 96c147f

  number: true,

• lesspass/mobile/src/password/PasswordGeneratorScreen.js

  Line 54 in 394f16f

  number: state.digits,

backend

• lesspass/containers/backend/api/tests/tests_passwords.py

  Line 54 in 96c147f

  "number": True,

[ogarcia]

@edouard-lopez thank you. Finally I have solved the problem on my side accepting the inconsistency of the model because I understand that it will take @guillaumevincent more time to solve it.

In Rockpass it now works correctly, but with time this inconsistency should be fixed.

[guillaumevincent]

Oh nice spot, yes I should fix this

[guillaumevincent]

@ogarcia I can't find where number was used in the android version.
Maybe an obsolete version.

The client API (javascript and mobile apps) are using digits and not numbers. I pushed a patch to change numbers into digits and make it consistent everywhere.

I added some retro compatibility test on the server.

lesspass/containers/backend/api/tests/tests_passwords.py

Lines 48 to 88 in 8c19c8d

	def test_create_password_old_api(self):
	password = {
	"site": "lesspass.com",
	"login": "test@lesspass.com",
	"lowercase": False,
	"uppercase": True,
	"numbers": False,
	"symbols": False,
	"counter": 2,
	"length": 12,
	}
	self.assertEqual(0, models.Password.objects.count())
	self.client.post("/api/passwords/", password)
	self.assertEqual(1, models.Password.objects.count())
	profile = models.Password.objects.first()
	self.assertEqual(profile.site, "lesspass.com")
	self.assertEqual(profile.login, "test@lesspass.com")
	self.assertFalse(profile.lowercase)
	self.assertTrue(profile.uppercase)
	self.assertFalse(profile.digits)
	self.assertFalse(profile.symbols)
	self.assertEqual(profile.counter, 2)
	self.assertEqual(profile.length, 12)
	self.assertEqual(profile.version, 2)
	def test_create_password_with_missing_s_old_api(self):
	password = {
	"site": "lesspass.com",
	"login": "test@lesspass.com",
	"lowercase": True,
	"uppercase": True,
	"number": False,
	"symbol": False,
	"counter": 1,
	"length": 16,
	"version": 2,
	}
	self.client.post("/api/passwords/", password)
	profile = models.Password.objects.first()
	self.assertFalse(profile.digits)
	self.assertFalse(profile.symbols)

You may want to change this on your server implementation before I push the new client.
I will test the clients (mobile and old browser with the old API) and it should works also.

[guillaumevincent]

oh I'm sorry,

@edouard-lopez shows the error in the mobile app

Edit: we are sending number and digits, I removed number that is incorrect.

[ogarcia]

@guillaumevincent IMHO to me it makes more sense that the valid field is numbers instead of digits, after all we are talking about numbers from 0 to 9.

In fact I see that the confusion has spread throughout the different implementations:

• https://github.com/tuxlinuxien/lesspassgo/blob/master/core/v2/core.go#L21
• https://github.com/71/lesspass.rs/blob/master/src/lib.rs#L51
• https://github.com/mevdschee/lesspass.go/blob/master/lesspass/lesspass.go#L19
• https://github.com/ameuret/cli/blob/master/cli.js#L15
• https://github.com/mevdschee/lesspass.php/blob/master/lesspass.php#L5
• https://github.com/mevdschee/lesspass.py/blob/master/lesspass/lesspass.py#L9
• https://github.com/ogarcia/lesspass-client/blob/master/src/main.rs#L81 (this is a bad example because it is the CLI that I have made)

Now, if you decide that the model instead of using numbers uses digits I respect your decision, but in that case let me change it because my server implementation expects to receive number or numbers. Let me know your final decision.

[ogarcia]

@guillaumevincent The preference to use numbers instead of digits in my case (and possibly in many others) is because in Spanish the word digits (translated as digitos) is used more to refer to fingers or, as a verb to indicate entering data with the fingers (digitar). The same thing happens in Italian. The word digits (dito) refers only to the fingers.

That is why I believe that in many implementations they have preferred to use the word numbers to avoid confusion.

[guillaumevincent]

I started with numbers. This is why everybody are using numbers.

But numbers are not from 0 to 9. A number from 0 to 9 is a digit.

I will update the client to send numbers and digits in the POST and PUT payload. Like that you don't have to change your implementation. And If I have numbers in the GET payload, I will update the payload. Today it's already like that.

[ogarcia]

I started with numbers. This is why everybody are using numbers.

I understand.

No, I can also change the implementation if the correct model is to use digits, in fact I can deserialize numbers, number and digits as digits. The only thing that worries me is that I can only serialize (without doing a church arc) to a key. If right now the server only sent digits would all the clients work correctly?

[guillaumevincent]

No I'm not sure.

For now don't touch anything. I will make the client work with { "numbers" : boolean } or { "digits": boolean }.
The clients will send { "numbers" : boolean, "digits": boolean } in the POST and PUT requests.

Today I think only the mobile application send { "number" : boolean, "digits": boolean } < see the missing s in number. I will fix this.

[guillaumevincent]

Thanks to this patch, clients should become compatible soon
c92c0e3

[ogarcia]

Perfect. I will wait a while (a few months) before updating the server and change numbers to digits to make sure everyone has updated their clients and that they have no problems.

Also, by testing I have discovered that in my case I would have to modify the serializer to be able to accept indistinctly numbers and digits (Rust is very strict about this), so the best thing to do is simply to make the change when everyone is updated.