bitwarden / android

Bitwarden mobile app for Android.
https://bitwarden.com
GNU General Public License v3.0
6.42k stars 808 forks source link

Login Response not fully deserialized in AuthService #661

Closed mastacheata closed 4 months ago

mastacheata commented 4 years ago

Hello,

I'm currently investigating and working on adding FIDO U2F support to the Android app of bitwarden. I've spent the last 10 or so hours going through the code and trying to figure out how things are tied together in there and seem to have found a way to pick up where your code stubs have left.

I did encounter a small problem, though: When I transfer the challenge information from AuthService to MainActivity using the message broadcaster, I always get a JSON string instead of an object.

I think this isn't deserialized because the object in Dictionary<string, object> is too broad of a type definition and will therefore not be deserialized.

I'm a bit confused as to what type would be better suited for the type in this case. If we assume that each provider in the TwoFactorProviders2 key is an object itself, I guess we could get away with nesting another string,string or strng,object Dictionary in place of the object.

But maybe I'm going down the wrong route alltogether here.

As a workaround for just this one case where I actually need the challenge Information, I could also import the JSON stuff into the Android subproject and deserialize the remainder over there. I don't think that's the "right" way to do it, though.

Any and all feedback on this issue would be much welciome.

Thanks in advance

kspearrin commented 4 years ago

Did you solve this? I see you opened a PR.

mastacheata commented 4 years ago

Yes and no. I started the PR as I ran into problems and thought maybe a second pair of eyes would be good to see where I might've done something wrong.

The Deserialization Problem wasn't really solved there, but I did a quick and dirty workaround. I further deserialized the object part of Dictionary<string, object> into another Dictionary<string, object> itself. Not sure if that's the best way to do so, but I haven't yet figured out if the Server's response is always structured the same or if it returns different stuff for different 2FA methods. If it's always the same structure, I think it would be better to create an Interface/Abstract Class to define the structure and map it explicitly.

For my purposes, the Dictionary is just fine, but I think that's not a clean way.

vvolkgang commented 4 months ago

Issue migrated to https://github.com/bitwarden/mobile/issues/661