googlemaps / google-maps-services-java

Java client library for Google Maps API Web Services
Apache License 2.0
1.71k stars 947 forks source link

Depending on `java.time.Instant` is problematic on Android #559

Closed syslogic closed 5 years ago

syslogic commented 5 years ago

These two methods, which depend on java.time.Instant:

https://github.com/googlemaps/google-maps-services-java/blob/b56d71c89a32a40640d1eb895c4bcb3ef192f1b5/src/main/java/com/google/maps/DirectionsApiRequest.java#L172

https://github.com/googlemaps/google-maps-services-java/blob/b56d71c89a32a40640d1eb895c4bcb3ef192f1b5/src/main/java/com/google/maps/DirectionsApiRequest.java#L185

May cause in this warning on Android devices with an API level < 26:

Call requires API level 26 (current min is 23): java.time.Instant#now

Which means, that one can currently only set value now on Android devices with an API level < 26:

https://github.com/googlemaps/google-maps-services-java/blob/b56d71c89a32a40640d1eb895c4bcb3ef192f1b5/src/main/java/com/google/maps/DirectionsApiRequest.java#L196


Suggested backwards-compatibility fix:

Please add two alternative methods, which permit setting UNIX epoch time - alike the API expects it.


For reference: https://stackoverflow.com/questions/55550936/call-requires-api-level-26-current-min-is-23-java-time-instantnow/55556222

domesticmouse commented 5 years ago

Per the README, Android isn’t a supported platform.

syslogic commented 5 years ago

That's a rather limited way of thinking - which vastly limits possible application. I'd say, that the API is just way to abstracted, to use it properly - which only leads to the demand to fork it and fix it for JVM 7 targets (this is not even an Android specific issue)... and even when building with JDK 8 this applies.

domesticmouse commented 5 years ago

The issue is that including an API key in an Android app opens a developer up to a denial of service attack on their credit card through API key theft.

domesticmouse commented 5 years ago

As to Java 7 support, if you look at the release notes and other closed bugs you will see that moving to Java 8 minimum was a community decision. With that said, I'm happy with people forking the codebase and adapting it for their requirements. I'm optimising for getting developers started quickly and securely.

syslogic commented 5 years ago

The issue is that including an API key in an Android app opens a developer up to a denial of service attack on their credit card through API key theft.

Not really, because Maps API keys should be restricted by package-name and signature fingerprint... which is by far more (secure) difficult to reproduce than a simple 3-digit CCV or a 4-digit PIN code. Not restricting an API key is indeed quite alike carelessly pushing a magnetic-strip card into a skimmer device & disclosing the PIN or CCV. I'd rather have the impression, that credit-cards by themselves are the weakest link in the chain... because their security may be based upon 16 digits + 3 or 4 for the "security", which in general is a joke. Here in Europe they've meanwhile introduced EMV, which is a tad better, but nevertheless flawed.

The current situation is that Android is supported - even if unofficially - but Android API >= 26 is required to set these timestamps, while the Directions API does accept UNIX epoch time. This is based upon the JVM version on the target device and this affects a huge percentage of devices (don't know for certain, but that may be 50%), which are being limited to the current time "now". Even when java.time.Instant is available to the JVM, being able to alternatively set int values would be more flexible, because that's what a SQLite or mySQL database commonly provides. java.time.Instant might come handy - but it's not exactly a replacement for the established standards. A method which permits setting raw key/value URL parameters would also help to get around the requirement for java.time.Instant - which is a class perfectly fine for an application, but quite limiting for a library.

domesticmouse commented 5 years ago

The problem is that Google Maps API keys can't be protected by package name and secure fingerprint.

syslogic commented 5 years ago

Of course this is possible; you might be referring to outdated information. Just go to the Credentials Panel, select such an API key for an Android application... there it reads Restrict usage to your Android apps > Add your package name and SHA-1 signing-certificate fingerprint to restrict usage to your Android apps. It wasn't always alike this, but this is the current situation... it's about the same as for web-sites, which can be restricted to HTTP referrers - where the key is even published in plain-text to the world wide web. Would rather assume, that regular Java applications have more of a security problem there - because I cannot see how one would restrict the API key for them (except setting up IP restriction for the range of a network).

Or see this example of mine, which demonstrates how not to publish API keys to GitHub. This is being accomplished by a .properties file, which holds the API key and is being parsed when building - and placeholders in the Manifest.xml. The intention is to provide them with an easy way to supply their own API key - and even if I'd publish my key, they could not use it, because fingerprint collisions are unlikely.

This all is rather irrelevant for the problem, that even when building with Java 8 - it won't run properly on JVM 7, unless the suggested methods for backwards-compatibility would be added - the basic idea is to provide a way to set any desired departure_time (as one would expect it). The API key's security barely is the issue on Android - and adding the Gradle dependency is simple (and so starting quickly is possible), but one might face a limitation, which the Directions API otherwise does not have. Would you accept a PR?

domesticmouse commented 5 years ago

I understand what the console says. However, the Maps API backends do not honour that restriction as this wasn't designed as an API to be used from mobile. It was always meant for usage from servers. We have APIs for Android, iOS and the Web for client usage scenarios.