OXID 4.9: kein Admin-Login mehr möglich über SSL-Proxy!?

Moin Leute,
wollte nur mal kurz fragen, ob es sonst noch jmd. auffiel, dass man seit OXID 4.9 (rc2 + final) nicht mehr in den Admin kommt, wenn der Shop-SSL über einen Proxy läuft? Ist dies ein Bug oder gar gewollt? Ist zwar nicht die beste Lösung für Shops, klar, aber trotzdem weit verbreitet. Bisher ging es auch immer reibungslos, da es ja in OXID explizit vorgesehen ist, siehe oxconfig, function _checkSsl():

//additional special handling for profihost customers
        if (isset($aServerVars['HTTP_X_FORWARDED_SERVER']) &&
            (strpos($aServerVars['HTTP_X_FORWARDED_SERVER'], 'ssl') !== false ||
             strpos($aServerVars['HTTP_X_FORWARDED_SERVER'], 'secure-online-shopping.de') !== false)
        ) {
            $this->setIsSsl(true);
        }

Der Login selbst scheint zu klappen, aber sobald “navigation” ins Spiel kommt (Top-Frame), dann wird man sofort wieder ausgeloggt und zum Login zurückgeleitet mit “redirected=1”. Weder im Server- noch im exception-Log findet sich ein Eintrag dazu, also tappe ich gerade ziemlich im Dunkeln… :frowning:

Edit/PS:
Vorhin dämmerte es allmählich, aber nun bin ich fast noch verwunderter als vorher. Und zwar ist es so, dass in oxutilsurl die Funktion _getHosts() geleert wurde und keine Alternative existiert in Form von _addHost() (wie in OXID 4.8), die das interne Array füllt. Somit schlägt “isCurrentShopHost()” immer fehl, da nur der aktuelle Shop-Host geprüft wird, aber eben nicht der SSL-Proxy und die Language-URLs, etc.
Gibt es also ein trifftigen Grund, warum dieser vorherige Code aus 4.8 gelöscht wurde, ich sehe nicht, was dagegen spricht, und im Changelog ist auch nichts dazu vermerkt:

protected function _getHosts()
    {
        if ( $this->_aHosts === null ) {
            $this->_aHosts = array();
            $oConfig = $this->getConfig();

            // mall (ssl) url
            $this->_addHost( $oConfig->getConfigParam( "sMallShopURL" ), $this->_aHosts );
            $this->_addHost( $oConfig->getConfigParam( "sMallSSLShopURL" ), $this->_aHosts );

            // language url
            $this->_addHost( $oConfig->getConfigParam( 'aLanguageURLs' ), $this->_aHosts );
            $this->_addHost( $oConfig->getConfigParam( 'aLanguageSSLURLs' ), $this->_aHosts );

            if ( !count( $aHosts ) && !$oConfig->mustAddShopIdToRequest() ) {
                // current url
                $this->_addHost( $oConfig->getConfigParam( "sShopURL" ), $this->_aHosts );
                $this->_addHost( $oConfig->getConfigParam( "sSSLShopURL" ), $this->_aHosts );
            }

            if ( $this->isAdmin() ) {
                $this->_addHost( $oConfig->getConfigParam( "sAdminSSLURL" ), $this->_aHosts );
            }
        }

        return $this->_aHosts;
    }

Soll man das nun per Hotix-Modul wieder hinzufügen (schnell gemacht), oder was wäre der beste Weg, dies zu “fixen” (ist ja immer noch nicht klar, ob es ein Fehler oder gewollt ist)?

Hat wohl irgendwas mit dem Fix für Bug #5809 zu tun: https://github.com/OXID-eSales/oxideshop_ce/blame/v4.9.0/source/core/oxutilsurl.php#L454
Willst du vielleicht mal auf der dev-general-Mailingliste nachfragen?

Ah prima, danke für den Hinweis! Dann macht es alles also halbwegs Sinn, nur wurde wahrscheinlich ein wenig übers Ziel hinausgeschossen. Das könnte/sollte man dann ja doch als Bug eintragen. Verwirrend ist halt folgendes:
Der oben erwähnte Code war ja bereits vorhanden, wurde aber nie genutzt. Nun würde er Sinn machen, wurde aber gelöscht. Und es betrifft evtl. nicht nur SSL-Proxies, sondern auch Mallshop-URLs, die Language-URLs (wenn vorhanden), usw.
Ich habe mir als temporäre Lösung per Modul die Logik der alten _getHosts() in die neue Version migriert, dann klappt alles wie gewünscht und auch Bug #5809 wird berücksichtigt. Ich denke mal, so war es ursprünglich auch gedacht gewesen… :wink:

Hallo Mitmacher,

habe auch einen SSL-Proxy. Im Modul schreiben bin ich nicht fit. Wäre nett, wenn du dein Hotfix Modul öffentlich machen würdest.

LG
Frank

Äh klar, sorry, war vorhin sogar angedacht, aber dann wurde ich abgelenkt. Ich habe es mal gezippt angehängt, da eXchange länger dauert und evtl. auch übertrieben wäre. Erstmal in Kürze eine Debatte im Bugtracker anregen…
Noch ein Hinweis zum Modul: man muss es natürlich ohne SSL installieren, und erst danach in der config.inc.php den Proxy eintragen, und dann komplett neu einloggen. Frohes Schaffen damit! :slight_smile:

PS: da ist noch ein anderer kleiner Part im Modul, der auf SSL umlenkt, falls man den Admin ohne aufruft. Das erspart bei der ersten Cookie-Erstellung eine Warnung, bzw. den Auto-Logout. Kommt selten vor, aber schadet nichts und ist eher Kosmetik, also nicht wundern… :wink:

Suuuper!

Vielen Dank dafür Mitmacher!

Hallo,

ich denke, das hängt mit dem Change von MD5 auf SHA-2 zusammen:

Der zugehörige Bug ist dieser hier:
https://bugs.oxid-esales.com/view.php?id=5901

Es ist zwar aufgefallen, dass es mit Subshops Probleme geben könnte aber ich vermute, SSL-Proxies wurden noch nicht bedacht (weil man’s auch - ehrlich gesagt - nicht wirklich macht). Schreibt das trotzdem ruhig bitte mit in den Bug-Eintrag rein, damit die dort Bescheid wissen.

Gruß

aber ich vermute, SSL-Proxies wurden noch nicht bedacht (weil man’s auch - ehrlich gesagt - nicht wirklich macht)

Hi Marco, ich habe fast auf eine ähnliche Antwort gewartet, du enttäuscht mich nicht! :smiley:
Deswegen hatte ich extra euren Code zitiert, weil die Entscheidung SSL-Proxies zu supporten kam ja nicht von mir, wenngleich ich sie durchaus unterstütze. Aber die Debatte lassen wir hier lieber, zumal es ja ein scheinbar weiterreichendes Problem ist, ich kam nur zufällig wg. meines lokalen Testproxys drauf…

Ansonsten danke für deine weitere Idee, aber ich gaube, den Zusammenhang kann man ausschließen. Außerdem existiert der Bugeintrag 5901 gar nicht mehr :confused:. Ich denke aber, es ist relativ eindeutig, was hier fehlt und ich habe es einfach mal als neuen Bug mit Bezug auf #5809 eingetragen: 0005905: Missing checks for SSL, mallshop and language-hosts - OXID eShop bugtrack
Okay?

Klar okay. Danke :slight_smile:

Hi,

kann mir jemand eine PM schicken sobald das wieder geht.

Nutze normal auch einen SSL-Proxy und suche jetzt seit einer Stunde im Forum warum ich nicht mehr in den Admin konnte.

Hab das komplette Update mittlerweile 2 Mal gemacht und und und…

Jetzt SSL deaktiviert und schon geht’s. Würde nur gern SSL nutzen.

Danke im Voraus.

Kai

Kai, du kannst auch den Bug beobachten :wink:

…und bis zu einer offiziellen Lösung eben mein Hotfix-Modul nutzen, dafür ist es ja gedacht gewesen… :cool:

Ja genau. @Mitmacher, hast Du vielleicht Lust, das als Pull-Request einzureichen? Ich bin mir jetzt nicht sicher, ob das für den Standard taugt aber im positiven Fall wird es gemerged.

Gruß

Ojeh, github hatte ich schon wieder fast verdrängt, aber sollte ich hinbekommen (wohl morgen). Stellt sich nur die Frage, ob es Sinn macht, auch den anderen Aspekt des Moduls gleichzeitig einzureichen, oder immer möglichst “atomar” (also beides getrennt)? Es passt ja zum Thema, ist allerdings ein permanentes altes OXID-Problemchen:
Wenn man nämlich einen SSL-Proxy einsetzt, aber z.b. einen Admin-Bookmark auf die No-SSL-Seite hat, dann wird beim ersten Aufruf das falsche Cookie gesetzt und der erste Admin-Login schlägt somit fehl. Also ist es imho doch konsequent, wenn man einen redirect (auf SSL) setzt, falls die Admin-URL ohne SSL aufgerufen wird. Rein kosmetisch gilt dies natürlich auch für “echte” SSL-Domains, nur hat man dort das Cookie-Problem nicht, da man wohl meistens dieselbe SLD benutzt. Aber egal, damit es in jeder Konstellation funktioniert, wären konkret doch nur 2 Zeilen Code zu ergänzen in controllers/admin/login.php, function render(), hinter “$myConfig = $this->getConfig();”:

if ( !$myConfig->isSsl() && strpos( $myConfig->getConfigParam( 'sAdminSSLURL' ), 'https://' ) === 0 )
            oxRegistry::getUtils()->redirect( $myConfig->getConfigParam( 'sAdminSSLURL' ), false, 302 );

Also wie jetzt am besten verfahren? Braucht dieser Kosmetik-Patch evtl. erstmal eine eigene Bugmeldung? Ich habe mich jahrelang gescheut, warum auch immer, aber empfand es wohl nicht als wichtig genug. :wink:

Ich finde das ist eine sinnvolle Verbeserung. Man könnte diese zusätzliche Änderung bei dem Pull-Request evtl. in einem separaten Commit “servieren”, damit es bei Bedarf leicht zu trennen ist. Müssen dafür evtl. auch Unit-Tests geändert werden? Oder gibt es für den Fall noch gar keine?
Ein Bugeintrag erleichtert vermutlich die Akzeptanz, ist meiner Meinung nach aber nicht zwingend notwendig.

Tja, von Unit-Tests habe ich kaum Ahnung und von Github scheinbar auch nicht, sorry! :o
habe es eben zweimal versucht, mir den aktuellen branch b-5.2-ce in meinen Online-Fork zu ziehen, aber das wurden plötzlich Pullrequests in die falsche Richtung, hilfe. Geht das überhaupt per Browser-GUI, oder muss ich das 5.1-Repo erst ganz löschen, damit ich ein neues für 5.2 forken kann? Weil zusätzlich geht wohl nicht, da es ja immer gleich heißt (titel: oxidshop_ce), puh, ziemlich verwirrend…

PS: ich hab mich einfach hier dran orientiert und es klappte auch prima, um den 5.1-branch auf den neuesten Stand zu bringen. Nur ein Update/Merge auf 5.2 geht damit wohl nicht(?):
http://stackoverflow.com/questions/20984802/how-can-i-keep-my-fork-in-sync-without-adding-a-separate-remote/21131381

PPS: so, alles gut, nachdem ich das Ganze mal reforkt habe! Zwar unnötig umständlich, aber mag ja sein, dass die GUI nicht mehr hergibt. Auf Dauer werde ich wohl auch einem ordentlichen Client nutzen, aber momentan fehlt mir einfach die Zeit dafür. Solch kleine Patches müssen auch online gehen, finde ich. Also habe ich nun endlich mal einen korrekten PR erzeugt und bin gespannt, was nun passiert… :wink:

Sieht gut aus, #176, gell? Mach Dich drauf gefasst, dass immer nach coding standards und nach PhpUnit-Tests gefragt wird. Auch wäre es immer cool, wenn Du in den Masterbranch (dev-ce) rein committen könntest.

Gruß

Sieht gut aus, #176, gell?

Yep, habe auch gerade den Titel noch gerade gerückt! :slight_smile:
PhpUnit-Tests dürften hier wohl nicht nötig sein? Werde ich mich beizeiten mal mit beschäftigen müssen. Aber die Coding-Standards versuche ich eh immer einzuhalten und habe mich die letzten Jahre vermehrt nach euch gerichtet. Dummerweise hatte ich mir dadurch auch angewöhnt, innerhalb ( Klammern ) die Leerzeichen einzubauen und genau dies räumt ihr nun wieder rigoros auf, wie ich schon gesehen habe! :eek:
Aber stimmt schon, hat so auch kaum wer sonst gemacht, nur fand ich es irgendwie luftiger zu lesen, auch wenn die Zeilen etwas länger wurden, egal nun halt wieder zurück…

PS: ach so, NUR in den dev-ce-Branch?

[QUOTE=Mitmacher;151252]
PS: ach so, NUR in den dev-ce-Branch?[/QUOTE]

Ja. In die anderen Branches wird übernommen, sofern das sinnvoll ist.

Gruß

Okay, aber muss/kann ich das noch korrigieren, oder war es nur für die Zukunft gemeint? Jedenfalls hätte ich dann ja auch nicht reforken müssen, da ich den 5.1/5.2-branch gar nicht benötige scheinbar? Steht das eigentlich irgendwo mal alles gut zusammengefasst, damit ich nicht jedesmal dieselben Fehler mache? :wink: