[3.2][3.3][CDB] Userreminder

[Mike-on-Tour]

In dem Forum, das ich betreue, gibt es einige Mitglieder, die irgendwann das Interesse verlieren und als "Karteileichen" die Users-Tabelle befüllen. Um solche Mitglieder identifizieren zu können, habe ich die Extension "User Reminder" geschrieben. Als ich sie bei phpbb.com zur Prüfung und Validierung einstellen wollte, wurde mir geschrieben, ich solle einen neuen Thread starten, was ich auch tat. Um auch die deutsch-sprachige Community beteiligen zu können, tue ich das in diesem Forum auch mal.

Extension Name: User Reminder for phpBB
Autor: Mike-on-Tour
Beschreibung: Diese Extension installiert eine Erweiterung im ACP, mit der neben einer Einstellungsseite auch drei Listen von Mitgliedern angezeigt werden.
Die erste Liste zeigt die sogenannten "Inaktiven Mitglieder", die sich eine in den Einstellungen vorgebbare Anzahl von Tagen nicht mehr im Forum eingeloggt haben. Sie können darauf mit einer Erinnerungs-Mail hingewiesen werden sowie nach einer ebenfalls einstellbaren Anzahl von Tagen erneut erinnert werden; diese zweite Erinnerung lässt sich auch abschalten. eine ebenfalls einstellbare Anzahl von Tagen nach der letzten Erinnerung können diese Mitglieder dann gelöscht werden. Sollte sich ein solches inaktives Mitglied zwischenzeitlich einloggen, wird es automatisch von der Liste genommen. Die Erinnerungen und das Löschen können auch automatisiert erfolgen.
In der zweiten Liste werden die "Schläfer" aufgelistet, das sind die Mitglieder, die sich nach Registrierung und Aktivierung noch nie eingeloggt haben. Einträge in dieser Liste können manuell zum Löschen ausgewählt werden.
In der dritten Liste werden die "Null-Poster" erfasst, das sind die Mitglieder, die zwar (mehr oder weniger) regelmäßig online sind, aber noch keine Beiträge gepostet haben. Auch die Einträge in dieser Liste lassen sich manuell zum Löschen auswählen.
WICHTIG:
Beim Löschen eines Users werden seine Beiträge beibehalten, um Lücken in den Themen zu vermeiden.
Bei aktiviertem automatischen Erinnern/Löschen wird während jedes Login-Vorganges neben dem Zurücksetzen der Erinnerungszeiten des jeweiligen Users (um ihn als "aktiv" zu kennzeichnen) auch geprüft, ob das automatische Erinnern oder Löschen aktiviert ist, im jeweiligen Fall wird dann festgestellt, welche User aktuell zum Erinnern oder Löschen heranstehen und die Erinnerungs-Mail versandt bzw. der Löschvorgang durchgeführt.
Voraussetzungen: phpBB >=3.3.0: PHP >= 7.4, < 8.5.0
Download: https://www.mike-on-tour.com/mot/mot_userreminder.php
Sprachen: en, de (formell und informell)
Styles: ACP
github repository: https://github.com/Mike-on-Tour/userreminder

Ich habe diese Extension mit phpBB 3.2.0, 3.2.7, 3.2.8, 3.2.9 und 3.2.10 unter PHP 5.4 und 7.2 und mit phpBB 3.3.0 und 3.3.1 unter PHP 7.2, 7.3 und 7.4 getestet und hoffentlich alle Fehler und Ungereimtheiten gefunden, aber mit der Zeit wird man ja betriebsblind.
Freue mich auf eure Kommentare.


Posting auf phpbb.com: https://www.phpbb.com/community/viewtop ... p=15410906

[LukeWCS]

Hallo Mike

Klingt interessant, das könnte ich auch brauchen. Danke für deine Erweiterung.

Als kleine Starthilfe kann ich anbieten dir einen Bericht von Ext Check zukommen zu lassen, quasi von Ext Coder zu Ext Coder. Es gibt einige Fehler hinsichtlich den Programmierrichtlinien laut CodeSniffer. Und EPV meldet auch ein paar Fehler, die du auf jeden Fall beheben solltest, weil auch kritische dabei sind.

Ich kann dir ausserdem schon sagen, das deine Ext über die gesamte Bandbreite von PHP kompatibel ist, also von 5.3-7.4, da ist alles grün.

[Dr.Death]

Gibt es dazu ein Git Repository ?

[Mike-on-Tour]

Hallo Luke,
an dem Bericht von ExtCheck bin ich sehr interessiert, will ja die bestehenden Fehler korrigieren. Kannst du den per PN schicken oder benötigst du dafür eine eMail-Adresse?

Automatisch dem Beitrag hinzugefügt nach 2 Minuten 13 Sekunden. Bitte Regel für „Topic-Bumping“ beachten. Danke.

Gibt es dazu ein Git Repository ?

Nein, gibt es (noch) nicht. Habe mich mit GitHub noch nicht näher beschäftigt, erscheint mir ein bisschen kompliziert und habe es deshalb bisher vor mir her geschoben, auch aus Zeitgründen.

[LukeWCS]

Hi Mike

Direkt per PN ist in dem Fall nicht möglich, weil der Bericht eine HTML Seite mit JS Funktionalität ist. E-Mail brauche ich nicht zwingend, ich setz das einfach in den Temp Ordner meiner Domain und schick dir nur den Link per PN sowie ein paar Infos zu EC.

[Mike-on-Tour]

Dank dem ExtCheck von LukeWCS konnte ich die Fehler alle beheben, nur in einer Sache komme ich nicht weiter. Mir werden unverändert in 2 Dateien insgesamt 6 Warnungen angezeigt, wonach eine potentielle SQL-Injection gefunden wurde. Wenn ich mir die Scripte ansehe, sind das die Zeilen, in denen ich Variablen zur Steuerung des DB-Abrufes in den Query aufnehme. Dass man darüber natürlich in die SQL-Abfrage einbrechen könnte, wenn diese Variablen durch den User selbst befüllt werden, ist mir klar, aber das ist hier nicht der Fall. In den Coding Guidelines habe ich dazu bisher auch nichts gefunden, deshalb versuche ich hier mal, eine Lösung zu erhalten.

Ein Beispiel ist folgender Code:

Code: Alles auswählen

$query = "UPDATE " . USERS_TABLE . " " .
		"SET mot_last_login = " . $row['user_lastvisit'] . " " .
		"WHERE user_id = " . $row['user_id'];

Was ich dabei gar nicht verstehe ist, dass der Code in Zeile 3 moniert wird, der in Zeile 2 jedoch nicht. Grundsätzlich passiert in beiden Zeilen das gleiche, also warum wird einmal eine Warnung ausgegeben und einmal nicht?
Und wie muss ich das umschreiben, damit die Warnung verschwindet?

Die überarbeitete Extension ist als Version 0.4.0 unter dem Link in meinem ersten Beitrag abrufbar.

[Dr.Death]

Verwende doch das DB System von phpBB und keine doppelten Anführungszeichen, sondern einfache:
https://area51.phpbb.com/docs/31x/codin ... s.html#sql
Dadurch werden auch SQL Injections vorgebeugt, da diese gefiltert werden können.

Anstelle von:

Code: Alles auswählen

$query = "UPDATE " . USERS_TABLE . " " .
		"SET mot_last_login = " . $row['user_lastvisit'] . " " .
		"WHERE user_id = " . $row['user_id'];

So in etwa:

Code: Alles auswählen

$sql_ary = array(
	'mot_last_login'	=> $row['user_lastvisit'],
);

$sql = 'UPDATE ' . USERS_TABLE . '
	SET ' . $this->db->sql_build_array('UPDATE', $sql_ary) . '
	WHERE user_id = ' . (int) $row['user_id'];

$this->db->sql_query($sql);

Dadurch kann man auch sauber und strukturiert mehrfach INSERTS hinbekommen wie z.B.:

Code: Alles auswählen

$sql_ary = array(
	'wm_race'	=> (int) $race_id,
	'wm_driver'	=> (int) $current_driver,
	'wm_team'	=> (int) $current_team,
	'wm_points'	=> $wm_points,
);

$this->db->sql_query('INSERT INTO ' . $table_wm . ' ' . $this->db->sql_build_array('INSERT', $sql_ary));

[Mike-on-Tour]

Danke für den Tipp und die Beispiele.
Nachdem ich jetzt damit rumexperimentiert habe, ist die Lösung aber nicht (nur) die Verwendung von einfachen Anführungszeichen, sondern die explizite Deklaration der beanstandeten Variable als Integer, denn (um beim Beispiel meines ursprünglichen Codes zu bleiben) mit

Code: Alles auswählen

$query = 'UPDATE ' . USERS_TABLE . ' ' .
		'SET mot_last_login = ' . $row['user_lastvisit'] . ' ' .
		'WHERE user_id = ' . $row['user_id'];

erhalte ich weiterhin die Warnung, mit

Code: Alles auswählen

$query = 'UPDATE ' . USERS_TABLE . ' ' .
		'SET mot_last_login = ' . $row['user_lastvisit'] . ' ' .
		'WHERE user_id = ' . (int) $row['user_id'];

nicht mehr.

Warum wird aber jetzt die 2. Zeile nicht beanstandet, die 3. Zeile aber schon? Das habe ich noch nicht verstanden.
Ansonsten hast du natürlich Recht, ich werde die Coding Guidelines entsprechend umsetzen.

[LukeWCS]

Hi Mike

Ergänzend zu Doc's Ausführungen:

Danke für den Tipp und die Beispiele.
Nachdem ich jetzt damit rumexperimentiert habe, ist die Lösung aber nicht (nur) die Verwendung von einfachen Anführungszeichen, sondern die explizite Deklaration der beanstandeten Variable als Integer, denn (um beim Beispiel meines ursprünglichen Codes zu bleiben) mit

Was die EPV Meldung angeht: genau das ist die Lösung in dem Fall, sprich explizite Typdeklaration. Denn $row['user_id'] kann an dieser Stelle "sonstwas" sein. Klar ist das in diesem Fall eine User ID, also INTEGER. Aber EPV prüft eine Variable nicht zurück bis zur Quelle um festzustellen, ob $row['user_id'] wirklich "sauber" ist. EPV sieht nur, das $row['user_id'] nicht explizit deklariert wurde und meldet das entsprechend.

Wenn EPV eine "potentielle Sicherheitslücke" meldet, heisst das noch nicht das es sich dabei auch tatsächlich um eine Sicherheitslücke handelt. Darum auch der Zusatz "potentiell". Aber um jegliche "potentielle" Probleme zu vermeiden, meldet EPV das. Es könnte ja z.B. auch durch einen Fehler, den man später mal im Code selber verursacht, dazu kommen, das eine potentielle Sicherheitslücke nicht mehr "potentiell" sondern "effektiv" wird. Das vermeidet man von vorneherein, indem man MySQL Anweisungen mit expliziter Typdeklaration verwendet.

Warum wird aber jetzt die 2. Zeile nicht beanstandet, die 3. Zeile aber schon? Das habe ich noch nicht verstanden.
Ansonsten hast du natürlich Recht, ich werde die Coding Guidelines entsprechend umsetzen.

Ich bin kein MySQL Experte, weil ich das auch nur selten brauche und wenn dann nur einzelne Funktionalitäten. Soweit ich es aber verstanden habe, liegt die potentielle Sicherheitslücke bei der SQL Klausel WHERE und darum wird dir auch diese Zeile gemeldet.

[Dr.Death]

Bei dem Beispiel:

Code: Alles auswählen

$query = 'UPDATE ' . USERS_TABLE . ' ' .
	'SET mot_last_login = ' . $row['user_lastvisit'] . ' ' .
	'WHERE user_id = ' . (int) $row['user_id'];

Kannst du den Teil ' .
' weglassen da ein Zeilenumbruch verwendet wird.

Code: Alles auswählen

$query = 'UPDATE ' . USERS_TABLE . '
	SET mot_last_login = ' . $row['user_lastvisit'] . '
	WHERE user_id = ' . (int) $row['user_id'];