Skip to content

Instantly share code, notes, and snippets.

@jeyemwey

jeyemwey/luca-android.md

Last active Mar 31, 2021
Embed
What would you like to do?

re: Luca Android App

Huhu, ich wollte mal den Aufschlag machen, was den Code der Luca-App für Android angeht. Seht es als "persönliche, nicht-kommerzielle Betrachtung", oder so.

Kurz zu mir: Ich bin kein IT-Security-Mensch und ich habe keine formale Vorbildung in Informatik. Aber ich habe schon mal die eine oder andere Java-Anwendung von innen gesehen und weiß ungefähr, was möglich ist und worauf man achten sollte. Wenn ich was vergessen haben sollte, haut mich gerne an, und ich ergänze es.

Der Übersicht halber, in mehreren Abschnitten:

Das Repository

Das Repository ist unter https://gitlab.com/lucaapp/android zu finden.

  • Aktuell ist nur die Android-App online. Leute beschweren sich auf der Issue-Seite darüber.
  • Es sind keine Commits zu finden, stattdessen ein bloßer Codedump. Das ist immer ein Zeichen dafür, dass da schludrig gearbeitet wurde. "bug fixes", "everything i did today" und sowas. Oder den Entwickler*innen wollen nicht mit ihrem Namen dran stehen.
  • Die Lizenz ist an anderer Stelle schon auseinander genommen worden.
  • Sie geben in der Readme an, dass der Code "nicht notwendigerweise" dem Entwicklungsstand oder der Play Store-Version entspricht. AHJA!

Code-Aufteilung

  • Es gibt zwei Orte für Tests [1] [2], was ich ein wenig komisch finde, aber dafür kenne ich mich nicht genug mit Android-Apps aus.
  • Die Tests vertesten nicht die komplette Anwendung, sondern nur einzelne Teile, und dann auch nicht konsequent.
  • Die eigentliche Anwendung ist hier. Sie verzichten auf Frameworks wie Spring für Autowiring, sondern erstellen eine Reihe an Singletons in der Main-Methode und geben sie dann mit Dependency Injection rein. Kann man machen, fällt dir aber irgendwann auf den Fuß, wenn die Anwendung älter wird.
  • Die Klassen sind nach Tätigkeitsbereich und nicht nach Struktur getrennt, was es schwieriger macht, alles auf einen Blick zu sehen (z.B. alle Exception-Klassen).

Code-Nitpicks

  • An allen Ecken und Enden findet man offensichtlich-generierten Code der IDE. Mit Lombok hätte man sich das ersparen können und insgesamt cleaneren Code.
  • Debugging by System.out.println darf natrürlich nicht fehlen. Ich glaube aber, dass der Code wie die Z85.java selbst, übernommen wurde, also kein Punkt dafür.
  • Do you even enum? In der HistoryItem-Klasse werden ab L28 ein Haufen Identifier definiert, die über eine Annotation geprüft werden. Mit Enums hätte man das einfacher und schöner hinbekommen können.
  • Hier und da (z.B. hier) loggen sie, dass sie etwas verschlüsselt haben und loggen den Klartext direkt daneben raus. Bin mir nicht sicher, wie restriktiv der Zugriff auf die Logs ist, aber Sinn der Übung kann das ja nicht sein.
  • Der NetworkManager erzählt ein wenig darüber, wie die Anwendung mit dem Server spricht.
    • Prod, Staging und auch die Info-Webseite scheinen hinter dem selben Load Balancer zu liegen, jedenfalls gibts Cert-Pinning auf *.luca-app.de und der Fingerprint des Zertifikats ist als String im Code. Hätte man sicherlich hübscher lösen können, über eine Properties-File z.B.
    • Die Staging-Environment ist aus dem Internet zu erreichen. Sie scheint es aber noch nicht so lange zu geben, die keyId bei https://staging.luca-app.de/api/v3/keys/daily/current ist mit 2 ziemlich klein, wobei sie bei app.luca-app.de schon bei 23 ist. Hat da erst gestern jemand dran gedacht, dass ein seperates Staging eine gute Idee ist?
    • Mit jedem Request wird der Modellname deines Handys, deine Android-Version und die installierte Luca-Version mitgeschickt. Yay, Datensparsamkeit. Macht bestimmt nachher hübsche Graphen für Smudo.
    • Wer mag, kann mit den Request-Modellen auch mal ein Backend bauen, solange das noch nicht publik ist.

Fazit

Wie oben beschrieben, habe ich relativ wenig Vorerfahrung mit Cryptographie. Darum habe ich mich auch aus „anything crypto“ rausgehalten.

Insgesamt ist der Code "so lala", man versteht schon was es tut, aber man merkt auch an allen Enden, dass er mit der heißen Nadel gestrickt wurde. Einige Fehler wären sicherlich durch ehrliches Code-Reviews oder Pairprogramming schon intern aufgefallen.

Bitte: Wenn ich was vergessen haben sollte, schreib mir einen Tweet an @confuzd_, einen Toot an @jannik@uelfte.club oder eine Matrix-Nachricht an @jannik:introverts.xyz. Zäänks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment