Do not serialize access tokens #1267
Conversation
| * @return a new instance of this class defined by the values passed inside this static factory | ||
| * method | ||
| * @see #fromUrl(URL) | ||
| */ | ||
| @NonNull | ||
| public static RouteOptions fromJson(String json) { | ||
| public static RouteOptions fromJson(@NonNull String json, @Nullable String accessToken) { |
There was a problem hiding this comment.
just to clarify, is this method used anywhere except tests?
will we always have accessToken=null in the DirectionsResponse ?
There was a problem hiding this comment.
This is a utility so that devs don't need to do fromJson.toBuilder.accessToken.build and it's clear that accessToken needs to be provided again, it's not serialized.
@rharter First of all, thanks for the great library! Do you know off the top of your head if above is possible with some workaround or if not if would be feasible and how big of a lift would be? We're going to create a ticket in https://github.com/rharter/auto-value-gson but wanted to hear your initial thoughts first as creator of the extension. Looking forward to hearing from you. Thanks again! 🙇♂️ |
LukasPaczos
force-pushed
the
lp-dont-serialize-access-tokens
branch
from
24c9a90
to
6c54ab3
Compare
Ensures that access tokens are not serialized in order to avoid often unintentional leaks of the data when paired with other tools and utilities that serialize and store either the whole route response, route, or only the options. Unfortunately, due the limitations of the code generation tools used we need to make the token field nullable until we find a better solution.
LukasPaczos
force-pushed
the
lp-dont-serialize-access-tokens
branch
from
6c54ab3
to
24f64a2
Compare
| Gson gson = new Gson(); | ||
| JsonObject object = gson.fromJson(json, JsonObject.class); | ||
|
|
||
| Assert.assertNull(object.get("access_token")); |
There was a problem hiding this comment.
NIT
| Assert.assertNull(object.get("access_token")); | |
| assertNull(object.get("access_token")); |
|
I don't have any specific approaches to solve the problem above. It sounds like the desire is really to have AVG deserialize a populated builder that the user can then provide an access token to, since you wouldn't be able to deserialize a valid object with a missing, but non-nullable, property. Offhand that would be my first inclination. The above suggested API looks good to me, internally the fromJson method could just deserialize a package private object like a builder and construct the route options by combining that with the provided token. |
|
Thanks for chiming in @rharter! The idea of deserializing into an intermediate, private structure gave me the idea to deserialize into a generic |
|
Thanks a lot @rharter for your input. Really appreciate it 🙇♂️ As @LukasPaczos mentioned take a look at #1270 if you're curious about the solution we opted for 🚀 Thanks again! |
Ensures that access tokens are not serialized in order to avoid often unintentional leaks of the data when paired with other tools and utilities that serialize and store either the whole route response, route, or only the options. Unfortunately, due to the limitations of the code generation tools used we need to make the token field nullable until we find a better solution.
I also tried upgrading the https://github.com/rharter/auto-value-gson library that is used in this project to the latest version but it didn't seem to have the features needed to support a non-nullable property that is also not serializable and initialize it externally. We can cut a feature request, contribute, or think about a different solution in the future.