PSA: change authentication from user/password to token

[hurzhurz]

Fixes #11735
Refs #11589 (comment)

Die Umsetzung könnte sicherlich noch verbessert werden, aber zumindest lässt sich die direkte PSA Integration so schonmal wieder nutzen.

[RTTTC]

This is such a good news for my Citroen!
I hope it gets into the upcoming release 😎

[andig]

Super PR! Wenn ich es richtig verstehe, wird hier die Lösung von Flobz portiert?

[Sillium007]

@hurzhurz erstmal: super cool! Und danke für Deine Mühe!

Funktioniert das den auch noch? Weil bei flobz gibt es mit dem refresh token seit kurzem Probleme: flobz/psa_car_controller#851 (comment)

[hurzhurz]

Super PR! Wenn ich es richtig verstehe, wird hier die Lösung von Flobz portiert?

Im Prinzip ja, also ich habe mir die Lösung nicht komplett angeschaut, aber ich glaube Flobz hatte das evtl. schon immer so gemacht mit der Verwendung vom refresh_token. Zumindest ist der Teil nichts neues an der API. Für die Umsetzung habe ich hier nun vorallem Code von der Mercedes-Integration verwendet und angepasst, dort ist das ziemlich ähnlich.
Den Teil mit der Token-Generierung/Beschaffung per evcc token hatte ich mir etwas mehr von Flobz abgeschaut und dann so ähnlich gebaut, wobei der zweite Teil davon, die eigentliche Anforderung vom Token per erhaltenem Code, übrigens wieder sehr ähnlich zum Token-Refresh ist.

Funktioniert das den auch noch? Weil bei flobz gibt es mit dem refresh token seit kurzem Probleme: flobz/psa_car_controller#851 (comment)

Also ich hatte zumindest beim Basteln in den letzten Tagen keine Probleme, da hat alles funktioniert.
Allerdings scheint es da um den remote refresh_token, der für die "Steuerung" von Klima&co per MQTT verwendet wird, zu gehen. Der ist glaube ich unabhängig hiervon.

[andig]

Was noch auffällt: die Pflege von Client ID/Secret ist jetzt doppelt. Vorschlag: die sollte komplett in psa.OAuthConfig(brand, realm, scheme, country) landen? Dann könnte evtl. auch die Erzeugung der authorize_url nochmal etwas durch oc.AuthCodeURL() vereinfacht werden. In jedem Fall entfällt die Doppelpflege. Ich kanns leider selbst nicht umbauen weil mit die Testmöglichkeit fehlt.

[hurzhurz]

Was noch auffällt: die Pflege von Client ID/Secret ist jetzt doppelt. Vorschlag: die sollte komplett in psa.OAuthConfig(brand, realm, scheme, country) landen? Dann könnte evtl. auch die Erzeugung der authorize_url nochmal etwas durch oc.AuthCodeURL() vereinfacht werden. In jedem Fall entfällt die Doppelpflege. Ich kanns leider selbst nicht umbauen weil mit die Testmöglichkeit fehlt.

Ja, das doppelt zu Pflegen ist definitiv höchstens eine Interimslösung.
Ich dachte mir auch schon dass es vermutlich besser wäre die OAuth Config auszulagern. Da ich aber absoluter Go und EVCC Neuling bin und da noch nicht überall zu 100% durchblicke, wollte ich noch nicht direkt mit einem größeren Umbau anfangen ;)

[andig]

@hurzhurz let me try to simplify this. It would be helpful if anybody could send credentials to info@evcc.io. Seems we might be able to automate this entirely ;)

[andig]

I probably broke it, but it may be worth retrying from here.

[RTTTC]

Citroen credentials sent

[andig]

@hurzhurz wenns ok ist würde ich auch noch den Refresh umbauen- immer in der Hoffnung, dass wir die zwischenzeitlich eingebauten Fehler beheben können?

[hurzhurz]

@andig klar, nur zu :)
Ich war übrigens auch schon dabei das ganze umzubauen und ziemlich weit, aber du warst schneller und dein Code ist schöner ;)

Größtenteils scheint es zu funktionieren, aber hier ist das Realm noch ein Problem:
api := psa.NewAPI(log, identity, "realm", oc.ClientID)
Da muss sowas wie clientsB2COpel rein. Ich hatte überlegt dazu ein erweitertes oauth config struct zu bauen und es da mit zu speichern...

[andig]

Ok, wir brauchen das also noch. Dann lass uns doch noch eine psa.Realm(brand) Funktion machen- lasse gerne Dir den Vortritt :)

[andig]

dein Code ist schöner ;)

Mach Dir nix draus- ich hab ja genau das schon 5000x gemacht. Lösch dann gerne raus was ich auskommentiert habe.

[andig]

Ok, wir brauchen das also noch. Dann lass uns doch noch eine psa.Realm(brand) Funktion machen- lasse gerne Dir den Vortritt :)

Oder noch besser: wir übergeben das wieder so wie es am Anfang war. Moment.

[andig]

Ich habe jetzt einen Testaccount für Citroen, bekomme aber keinen Code. Mir ist nicht klar welches Country angegeben werden muss (wovon hängt das ab- Auto ist deutsch, Anwender aus LT). Die Citroen Seite sagt dann immer "unbekannter Account" auf fronzösisch :O.

[hurzhurz]

Das finde ich unglücklich. Der eigentliche Parameter ist der Username und den haben wir bereits in der Config. Da das Token nur in der identity gespeichert wird ist schon alles abgedeckt. Die Änderung braucht es m.E. nicht.

Ok, also ich hab es nun von "account" auf "user" geändert, kein Problem :)
Eigentlich muss es wie gesagt nicht der Username sein, aber irgendwas brauchen wir halt damit die Identity eindeutig referenziert werden kann, da hier der Token ja kein Subject oder sowas hat.

Da beisst sich die Katze jetzt in den Schwanz. Denn dafür brauchts den Settingskey des jeweiligen Fahrzeugs. Ein zusätzlicher Account Parameter ist dafür keine gute Lösung.

Mit dem User als Parameter geht es genauso. Da der in der config steht kann man den SettingsKey problemlos jederzeit ermitteln.
Bei Tesla mit dem Subject ist das natürlich ein anderes Thema, da hat man (ohne Token) ja nichts in der Config stehen...

Yep, das ist doof. Dann müssen wir das aber auch konsequent für die anderen Fahrzeuge machen wo wir das brauchen. Aus dieser Sicht: separater PR, lass uns den Scope hier lieber klein halten.

Ok, also kann man dann noch entfernen/herauslösen. Wobei es vermutlich auch nicht stören würde das für PSA drin zu lassen?

Ich habe jetzt einen Testaccount für Citroen, bekomme aber keinen Code. Mir ist nicht klar welches Country angegeben werden muss (wovon hängt das ab- Auto ist deutsch, Anwender aus LT). Die Citroen Seite sagt dann immer "unbekannter Account" auf fronzösisch :O.

Also bei mir ist die Login-Formular aktuell auch immer auf französisch, warum auch immer. Erst die Bestätigung ist dann auf deutsch.
Der Country Code scheint nur so halb wichtig zu sein, ich kann auch "fr" statt "de" verwenden ohne dass es einen Unterschied zu machen scheint. Aber es muss wohl ein gültiger Code sein, weil das Teil sonst sagt dass quasi die Redirect URL unbekannt/ungültig ist.
Der Fehler "unbekannter Account" klingt eher so als wäre das die falsche Marke für den Account... seltsam...

[hurzhurz]

Nochmal wegen Identity, SettingsKey, User/Subject/... :
Wäre es vielleicht grundsätzlich eine Überlegung konfigurationstechnisch die Identitys von den Vehicles zu trennen?
Also dass man beides separat definiert und im Vehicle dann die zugehörige Identity angibt.
Dann könnte man für den SettingsKey generell den "config name" der entsprechenden Identity verwenden und es wäre evtl. auch besser zu erkennen wenn mehrere Vehicle über einen Account laufen bzw. mit den gleichen Token abgefragt werden.
Wobei das natürlich eine größere Änderung wäre, die eher nicht mal eben umzusetzen ist...

[RTTTC]

So for me the login form is currently always in French, for whatever reason. Only the confirmation is then in German.
The country code only seems to be half important, I can also use "fr" instead of "de" without it seeming to make a difference. But it must be a valid code, otherwise the part says that the redirect URL is unknown/invalid.
The "unknown account" error sounds more like it's the wrong brand for the account... strange...

Do you have to do any additional verification at any point? (PIN or SMS verification)

(Login data provided is taken from evcc yaml where it used to work fine before the PSA changed things, also works fine with Citroen app now)

[andig]

Eigentlich muss es wie gesagt nicht der Username sein, aber irgendwas brauchen wir halt damit die Identity eindeutig referenziert werden kann, da hier der Token ja kein Subject oder sowas hat.

Das braucht es nur wenn Du das Token speichern willst. Wie gesagt -> separater PR da es das für andere Fahrzeuge auch braucht. Lass uns das hier einfach nicht machen.

Ok, also kann man dann noch entfernen/herauslösen. Wobei es vermutlich auch nicht stören würde das für PSA drin zu lassen?

Ein Parameter und damit eine Fehlerquelle weniger den wir ohnehin schon kennen und der v.a. zusammen passen muss.

[andig]

Reverted last changes re session storage and aligned with Tesla implementation. We also need to re-add the opel etc templates or application start will break if anyone still has these in his config. Or (maybe better) add them as covers to psa.yaml.

[hurzhurz]

Das braucht es nur wenn Du das Token speichern willst. Wie gesagt -> separater PR da es das für andere Fahrzeuge auch braucht. Lass uns das hier einfach nicht machen.

Da hatten wir evtl. etwas aneinander vorbeigeredet :)
Also ich meinte vorallem dass man das in der NewIdentity braucht, so wie im jetzt zusammengesetzen subject hier:

evcc/vehicle/psa/identity.go

Line 31 in 5c31b76

subject := "psa." + strings.ToLower(brand) + "." + strings.ToLower(user)

Aber egal, so passt es ja dann

Ein Parameter und damit eine Fehlerquelle weniger den wir ohnehin schon kennen und der v.a. zusammen passen muss.

Alles klar. Ich hab noch eine weitere kleine Änderung, die fürs speichern aus der token_psa.go heraus nötig war, entfernt.
Soll ich denn mal schauen dass ich da mal in ein einem separaten Branch/PR was vorbereite, oder willst du das lieber selbst machen?

[hurzhurz]

Do you have to do any additional verification at any point? (PIN or SMS verification)

No, just the login with username/email and password, nothing else

[hurzhurz]

We also need to re-add the opel etc templates or application start will break if anyone still has these in his config. Or (maybe better) add them as covers to psa.yaml.

I have used as base and adapted it for new individual template files.
I'm not sure how covers is supposed to work exactly. In the generated doc files it would still look like this for all brands:

render:
  - default: |
      type: template
      template: psa

[andig]

Sieht gut aus! Verbleibendes Problem sind noch die Templates (und dass ich nix testen kann...). Templates muss ich nochmal nachgrübeln.

[deadrabbit87]

@andig was bräuchtest zum Testen?

Würde meinem Peugeot gerne zur Verfügung stellen.

[andig]

Zugangsdaten für info@evcc.io

[hurzhurz]

Sieht gut aus! Verbleibendes Problem sind noch die Templates (und dass ich nix testen kann...). Templates muss ich nochmal nachgrübeln.

Also abgesehen davon dass es etwas unschön/redundant ist für jede Marke ein eigenes Template (mit minimalsten Unterschieden) zu haben, bin ich mir gerade nicht sicher was für ein Problem damit noch übrig bliebe bei den Templates?
Also ich konnte bei mir mit der letzten Änderung die config umbauen von:

type: opel

auf

type: template
template: opel

Und das hat geklappt. Wobei evtl. zu erwähnen wäre dass ich im Bezug auf den/die Token Parameter vom Tesla Template etwas abweichen musste um kein Fehler zu bekommen.

EDIT: ok doch, ein kleines Problem gab es noch beim Generieren von Tokens:
vehicle type 'template' does not support token authentication
Ich hab mal einen schnelle Interims-Fix gemacht, den willst du vielleicht aber noch verbessern?

[andig]

Klasse PR, vielen Dank!

[hurzhurz]

Dir vielen Dank für deine Mühe das ganze nochmal so kräftig zu überarbeiten! :)

Nur eine kleine Anmerkung noch zum letzten commit:
In den Templates hatte ich das Passwort schon als deprecated Parameter drin (weiter unten), ist nun doppelt. Ich hoffe mal das stört nicht...

[andig]

Und weg, danke. So ist das wenn mans überall identisch haben will ;)

[stan23]

Echt prima dass ihr einen Weg gefunden habt das in evcc einzubauen.

Hier gibt es eine Diskussion zur Anwendung. Machen wir etwas falsch? Fehlt ein Schritt?
#13685