nemiah / phpFinTS

PHP library to communicate with FinTS/HBCI servers
MIT License
131 stars 40 forks source link

HITANSv7 #309

Closed okj579 closed 1 year ago

okj579 commented 3 years ago

Es gibt mittlerweile die Segmentversion 7 für HITANS/HKTAN. Das wird mindestens von der KSK Göppingen verwendet und wirft den Fehler Unsupported segment type/version Fhp\Segment\HITANS\HITANSv7. Ich versuche die aktuelle Klassen darauf zu aktualisieren, aber ich finde keine Spezifikation für HITANS nach FinTS v3.0 oder HITANS#6. Vielleicht weißt du auf Anhieb wo die zu finden ist?

Ich habe einen Fix auf eine frühere Version gemacht: https://github.com/okj579/phpFinTS/commit/1c3f7bac90c2f2f48d86d1f141ef1867e9994611 Ich konnte die Felder nur als $unknown1-5 aufführen, aber das läuft mindestens.

Hier das Segment, wie ich es von der Bank bekomme. Die letzte 5 Felder sind seit v6 neu.

HITANS:175:7:4+1+1+1+N:N:0:922:2:pushTAN-dec:Decoupled::pushTAN 2.0:::Aufforderung:2048:J:2:N:0:0:N:N:00:2:N:2:180:1:1:J:J
Philipp91 commented 3 years ago

Also schickt die Bank nur HITANS:x:7 und kein HITANS:y:6 mehr? Für die Verhältnisse in der Bankenwelt käme ein solches Upgrade ohne Fallback dann doch ein bisschen plötzlich.

Die Spezifikation ist hier in Abschnitt B.5.2. Die Tabellen müsstest du 1:1 neben die jeweiligen Klassen legen und sie damit aktualisieren können.

Philipp91 commented 3 years ago

Und die VerfahrensparameterZweiSchrittVerfahrenV7 sind in Kapitel D auf Seite "173" (laut abgedruckter Seitenzahl) dokumentiert.

Wenn ich das richtig verstehe, kann $tanProzess jetzt "S" sein. Also ärgern wir uns im Nachhinein, dass das Feld ein integer und nicht string ist :-) Und wenn da ein S drin ist, dann ist die Challenge nicht sowas wie "Bitte gib das hier in dein TAN-Gerät ein und tipp dann das Ergebnis in die Anwendung" sondern eher sowas wie "Bitte drück auf dem Handy auf OK". Und während der Nutzer das tut, darf (oder nicht, je nach HITANS-Belegung) die Anwendung regelmäßig beim Server nachfragen, ob der Auftrag schon freigegeben wurde. Der Anwendungscode verwendet dann also nicht mehr FinTs::submitTan(), sondern eine neu einzubauende Funktion FinTs::checkTanSubmission() oder so.

Ich würde vorschlagen, eine neue Funktion TanRequest::requiresTanSubmission(): bool einzubauen und dort einige Dokumentation draufzupacken. In den bisher bekannten TAN-Verfahren würde sie true zurückgeben. Wenn sie das nicht tut, dann handelt es sich um ein Decoupled-Verfahren, und FinTs::submitTan($action) lehnt deshalb eine $action ab, deren $action->getTanRequest()->requiresTanSubmission()===true ist. Ebenso würde FinTs::checkTanSubmission() ablehnen, wenn es false ist.

Außerdem müssen neue Getter zu TanMode hinzugefügt werden, mit dem die Anwendung rausfinden kann, ob es sich um ein Decoupled-Verfahren handelt (vielleicht einfach auch wieder TanMode::requiresTanSubmission()?) und ob und wie häufig sie beim Server nachhaken darf.

ampaze commented 3 years ago

@Philipp91 Klingt schlüssig. Hast du eine Bank mit der du das Testen kannst, scheint ja wirklich hot new stuff zu sein.

aaukt commented 3 years ago

Ich stehe aktuell vor dem gleichen "Problem", die Sparkasse schickt (seit heute?) zusätzlich HITANS:x:7 zum bisherigen HITANS:y:6. Der Abruf schlägt wegen o.g. Fehlermeldung trotzdem fehl, da das neue Segment nicht erkannt wird.

Philipp91 commented 3 years ago

Mit allen meinen Banken funktioniert der Abruf momentan noch. Da @aaukt sagt, dass es fehlschlägt, sobald HITANSv7 in den BPD ist, gehe ich davon aus, dass keine meiner Banken bisher die neue Version unterstützt, sodass ich es mit denen nicht testen kann.

Die Fehlermeldung "Unsupported segment type/version ..." aus dem ersten Post hier finde ich im Code nicht, also kann ich nicht wirklich erraten, wo es schief läuft. Vielleicht könnte @okj579 einen Stack-Trace dazu posten?

Vielleicht könnte jemand einen Integration-Test machen, der den Fehler reproduziert?

Die neue Segmentversion zu ignorieren sollte nicht so schwer sein. Dann dürfte es erst einmal wieder gehen, solange die Bank noch die Version 6 mitschickt.

aaukt commented 3 years ago

Ich denke die Sparkassen sind gerade "dabei" PushTAN 2.0 vorzubereiten (siehe z.B. https://xs2a.sparkassen-hub.com/news).

Ich vermute @okj579 nutzt (wie wir auch) noch eine alte Version. Dort gab es hier: https://github.com/nemiah/phpFinTS/blob/1.6/lib/Fhp/Segment/BaseSegment.php#L98 den besagten Fehler.

Wie @Philipp91 schrieb haben wir erstmal den Weg gewählt die Version 7 zu ignorieren (grob gesagt Namespace überschreiben, Segment anlegen und Felder als optional markieren) damit funktioniert der Abruf erstmal wieder.

Philipp91 commented 3 years ago

Ich vermute @okj579 nutzt (wie wir auch) noch eine alte Version. Dort gab es hier: https://github.com/nemiah/phpFinTS/blob/1.6/lib/Fhp/Segment/BaseSegment.php#L98 den besagten Fehler.

Ah alles klar. Damals gab es noch keine anonymen Segmente. Seit phpFinTS die unterstützt, kann es die Version 7 (vermutlich) ignorieren.

aaukt commented 3 years ago

Danke für die Information, ein Grund mehr endlich auf die aktuelle Version zu gehen.

nemiah commented 3 years ago

Also ich kann bestätigen, dass die aktuelle Version auch mit HITANSv7 funktioniert.

okj579 commented 3 years ago

Ja, genau. Ich "pflege" eine alte Version, da wir noch auf PHP 5 angewiesen sind. Noch ein Grund zu upgraden :)

nemiah commented 3 years ago

Ich mache dieses Issue wieder auf, da ich einen Kunden habe, bei dem die Bank wohl gar kein v6 mehr anbietet.

Er bekommt folgende Meldung:

Fatal error: Uncaught FhpProtocolUnexpectedResponseException: The server does
not support any HITANS versions implemented in this library in
/var/www/html/20200522/open3A/Banking/fints-hbci-php/lib/Fhp/Protocol/BPD.php:104

Können wir v7 noch mit aufnehmen, damit es zumindest wieder funktioniert?

ampaze commented 3 years ago

Hab es mir noch nicht näher angeguckt, aber es klingt nicht so als wenn es damit getan wäre nur HITANSv7 zu implementieren, wenn da auch ein potentiell neuer TAN Prozess dran hängt. Ggf. dann ohne Support für "Auf Handy Ok drücken"?

nemiah commented 3 years ago

Ja, wäre doch schon super, auch wenn es erstmal ohne den neuen "decoupled" Modus funktioniert. :blush:

Philipp91 commented 3 years ago

Könnte mir jemand die BPD von so einer Bank zur Verfügung stellen? Am besten in Form eines Integration-Tests. Dann kann ich es mir mal anschauen.

nemiah commented 3 years ago

Wenn du mir sagst, wie ich das mache, dann klingle ich am Montag mal bei meinem Kunden durch :relaxed:

Philipp91 commented 3 years ago

Also ein Dump der ersten paar Nachrichten wäre gut (eben die, wo HITANS drin ist). Wenn das einfacher ist, würde auch ein Dump von $fints->bpd gehen (muss man evtl. auf public setzen).

Eventuell reicht es auch, wenn du mir die FinTsOptions für die Bank gibst, also die URL und BLZ, dann müsste ich die BPD ja selber abholen können.

So oder so kann ich aber nicht testen, ob der Login letztendlich funktioniert, weil ich ja keinen Account habe. Dafür müsste man (ggf. mit einer anderen Software wie Hibiscus, die vielleicht HITANSv7 schon unterstützt) alle Nachrichten mitschneiden bis und inklusive dem Login (also 1-2 mal HITAN).

nemiah commented 3 years ago

Ok, bitte probiers mal mit https://hbci11.fiducia.de/cgi-bin/hbciservlet BLZ 50190000

Wenn das nicht ausreicht, dann telefoniere ich mit dem Kunden und hole es mir aus seiner Installation.

Philipp91 commented 3 years ago

Da kriege ich das hier: ...HITANS:77:6:3+1+1+1+J:N:0:942:..., sprich Version 6 und sonst nix.

Philipp91 commented 3 years ago

Aber ich kann ja einfach mal stur die Spezifikation implementieren, vielleicht tut's ja.

Philipp91 commented 3 years ago

Eine Design-Frage: Wenn man bei einem Decoupled-Verfahren die Funktion $tanMode->getMaxTanLength(): int aufruft, dann gibt es natürlich keine richtige Antwort mehr. Wir könnten:

  1. den Rückgabewert auf : ?int ändern, oder
  2. eine Exception werfen, wenn man die Funktion auf einem TanMode aufruft, der TanMode::isDecoupled()==true zurückgibt.

In beiden Fällen funktioniert bestehender Code weiterhin, wenn man den Nutzern die neuen Tan-Modes nicht anbietet bzw. die Nutzer sie nicht auswählen.

Ich bin eher für die zweite Variante, weil dann für bestehenden Anwendungscode nicht plötzlich der Rückgabetyp ändert, d.h. auch bestehender Code mit Type-Checking "kompiliert" weiterhin. Außerdem muss man sich nicht mit Type Narrowing rumschlagen, wenn man es richtig macht:

if ($tanMode->isDecoupled()) {
  echo "Please press the button on your phone.";
} else {
  echo "Please enter a TAN of at most {$tanMode->getMaxTanLength()} digits.";
}

Bin mir aber nicht sicher, welche Exception die richtige wäre. Es muss eine RuntimeException sein, aber FailedPreconditionException gibt's in PHP irgendwie nicht.

nemiah commented 3 years ago

Da kriege ich das hier: ...HITANS:77:6:3+1+1+1+J:N:0:942:..., sprich Version 6 und sonst nix.

Hmm, seltsam. Ich schreibe ihm mal, er soll es nochmal probieren…

Philipp91 commented 3 years ago

Vielleicht ist die Bank ja zurückgerudert.

nemiah commented 3 years ago

Du kannst die neue Exception ja einfach erstellen:

FailedPreconditionException extends RuntimeException …

nemiah commented 3 years ago

Vielleicht ist die Bank ja zurückgerudert.

Ja, auch meine Vermutung :wink:

aaukt commented 3 years ago

Ich konnte noch etwas aus den Logs kramen:

HITANS:176:7:4+1+1+1+N:N:0:922:2:pushTAN-dec:Decoupled::pushTAN 2.0:::Aufforderung:2048:J:2:N:0:0:N:N:00:2:N:2:180:1:1:J:J

ampaze commented 3 years ago

eine Exception werfen, wenn man die Funktion auf einem TanMode aufruft, der TanMode::isDecoupled()==true zurückgibt.

Bin auch für Exception. RuntimeException ist auch ok, solange eine vernünftige Fehlermeldung drin steht.

nemiah commented 3 years ago

Ok, hat sich in Luft aufgelöst, er hatte noch die alte Version :facepalm:

karnimani commented 3 years ago

Ich bekomm auch einen Fehler und denke mal es hängt mit der Änderung zu v7 zusammen. Bin mir nicht ganz sicher, da ich 'Erstkontakt' mit der Bibliothek hab, aber folgendes bekomme ich bei meinem Test zurück:

ErrorException: Undefined offset: 921 nemiah\php-fints\lib\Fhp\FinTs.php:545 return $this->bpd->allTanModes[$tanModeId];

Die Bank liefert folgendes: HITANS:173:7:3+1+1+1+N:N:0:922:2:pushTAN-dec:Decoupled::pushTAN 2.0:::Aufforderung:2048:J:2:N:0:0:N:N:00:2:N:2:180:1:1:J:J und HITANS:172:6 mit TanModes 910, 911, 912, 913, 920, 921, 900

Ein Dump von $this->bpd->allTanModes zeigt mir, dass nur das Verfahren 922 enthalten ist, aber wenn ich es richtig verstanden habe, müssten laut Spezifikation (Abschnitt B.5.2) auch die v6 Verfahren enthalten sein:

Ein Kundenprodukt sollte – beginnend mit der höchsten Segmentversion – alle in der BPD enthaltenen HITANS-Segmente analysieren, um so dem Kunden alle vom Kreditinstitut unterstützten Sicherheitsverfahren anbieten zu können.

Leider konnte ich noch nicht herausfinden, wie man das ändern kann, vielleicht kann das jemand von euch prüfen?

Philipp91 commented 3 years ago

@karnimani Du hast Recht, das habe ich beim Implementieren überlesen.

Könntest du bitte:

  1. Diesen Fix testen? Ist nur eine Datei die du einfach lokal in deinem vendor-Verzeichnis patchen kannst.
  2. Mir sagen, mit welcher Bank das passiert (am besten in Form von BLZ und URL, so wie du sie in FinTsOptions verwendest). Dann kann ich einen Integration-Test machen, der sicherstellt, dass in FinTs::getBpd()->allTanModes die richtigen drin sind, wenn von der Bank anonym die BPD abgerufen werden -- in der Hoffnung, dass die Bank den anonymen Dialog unterstützt.
karnimani commented 3 years ago

Wow, danke @Philipp91, das war ja richtig schnell! Sieht gut aus:

array:8 [▼ 922 => Fhp\Segment\TAN\VerfahrensparameterZweiSchrittVerfahrenV7 {#2124 ▶} 910 => Fhp\Segment\TAN\VerfahrensparameterZweiSchrittVerfahrenV6 {#2091 ▶} 911 => Fhp\Segment\TAN\VerfahrensparameterZweiSchrittVerfahrenV6 {#2095 ▶} 912 => Fhp\Segment\TAN\VerfahrensparameterZweiSchrittVerfahrenV6 {#2130 ▶} 913 => Fhp\Segment\TAN\VerfahrensparameterZweiSchrittVerfahrenV6 {#2104 ▶} 920 => Fhp\Segment\TAN\VerfahrensparameterZweiSchrittVerfahrenV6 {#2096 ▶} 921 => Fhp\Segment\TAN\VerfahrensparameterZweiSchrittVerfahrenV6 {#2129 ▶} 900 => Fhp\Segment\TAN\VerfahrensparameterZweiSchrittVerfahrenV6 {#2128 ▶} ]

Url: https://banking-bw3.s-fints-pt-bw.de/fints30 BLZ: 65450070

Ohne jetzt zu recherchieren, wie macht man die anonyme Abfrage? Edit: Siehe Samples/bpd.php Und außerdem, Danke für die großartige Arbeit von euch allen!

danielr1996 commented 3 years ago

Ich bekomm auch einen Fehler und denke mal es hängt mit der Änderung zu v7 zusammen. Bin mir nicht ganz sicher, da ich 'Erstkontakt' mit der Bibliothek hab, aber folgendes bekomme ich bei meinem Test zurück:

ErrorException: Undefined offset: 921 nemiah\php-fints\lib\Fhp\FinTs.php:545 return $this->bpd->allTanModes[$tanModeId];

Die Bank liefert folgendes: HITANS:173:7:3+1+1+1+N:N:0:922:2:pushTAN-dec:Decoupled::pushTAN 2.0:::Aufforderung:2048:J:2:N:0:0:N:N:00:2:N:2:180:1:1:J:J und HITANS:172:6 mit TanModes 910, 911, 912, 913, 920, 921, 900

Ein Dump von $this->bpd->allTanModes zeigt mir, dass nur das Verfahren 922 enthalten ist, aber wenn ich es richtig verstanden habe, müssten laut Spezifikation (Abschnitt B.5.2) auch die v6 Verfahren enthalten sein:

Ein Kundenprodukt sollte – beginnend mit der höchsten Segmentversion – alle in der BPD enthaltenen HITANS-Segmente analysieren, um so dem Kunden alle vom Kreditinstitut unterstützten Sicherheitsverfahren anbieten zu können.

Leider konnte ich noch nicht herausfinden, wie man das ändern kann, vielleicht kann das jemand von euch prüfen?

Ich bekomme ebenfalls diesen Fehler, es handelt sich um die Sparkasse Nürnberg / 76050101, aus Experimenten mit node-fints weiß ich dass die Sparkasse HITANS#6 mit 921 als TanVerfahren liefert und HITANS#7 mit 922 (PushTan 2.0 / Decoupled).

Ich hab bei mir lokal die Methode FinTS::getTanModes() folgermaßen gepatcht:

    {
        $this->ensureTanModesAvailable();
        echo "<pre>";
        print_r($this->allowedTanModes);
        print_r($this->bpd->allTanModes);
        echo "</pre>";
        return array_combine($this->allowedTanModes, array_map(function ($tanModeId) {
            return $this->bpd->allTanModes[$tanModeId];
        }, $this->allowedTanModes));
    }

und bekomme dabei folgende Ausgabe:

Array
(
    [0] => 921
)
Array
(
    [922] => Fhp\Segment\TAN\VerfahrensparameterZweiSchrittVerfahrenV7 Object
        (
            [sicherheitsfunktion] => 922
            [tanProzess] => 2
            [technischeIdentifikationTanVerfahren] => pushTAN-dec
            [dkTanVerfahren] => Decoupled
            [nameDesZweiSchrittVerfahrens] => pushTAN 2.0
             ...
        )

)

Ohne jetzt in die Implementierung geschaut zu haben vermute ich dass $this->allowedTanModes HITANS#6 verwendet, $this->bpd->allTanModes aber schon HITANS#7, kann das so jemand bestätigen?

Philipp91 commented 3 years ago

Und verwendest du da schon den Fix aus #325? Der ist ja noch in keinem Release enthalten, d.h. man muss von dev-master installieren.

danielr1996 commented 3 years ago

Und verwendest du da schon den Fix aus #325? Der ist ja noch in keinem Release enthalten, d.h. man muss von dev-master installieren.

Nein ich verwende die offizielle 3.1, da hab ich wohl das Rad neu erfunden.

Mit dev-master funktioniert es, Danke für die schnelle Antwort, damit hat sich zumindest mein Problem gelöst

Philipp91 commented 3 years ago

@nemiah Vielleicht brauchen wir wieder mal ein Release.

nemiah commented 3 years ago

done :heavy_check_mark:

Philipp91 commented 1 year ago

Kann geschlossen werden.