Forum PHP.pl

Hej, zaprojektowałem proste logowanie 2-etapowe (na maila przychodzi kod do logowania po wpisaniu hasła).
Logowanie to jest używane tylko przez 2 osoby ale do bardzo wrażliwych danych więc mam następujące pytanie: czy jest bezpieczne? Może popełniłem jakiś błąd który może powodować "włam"? Dzięki z góry za odpowiedź


login.php

[PHP] pobierz, plaintext 
<?php
 
session_start();
 
	require 'dbsoft.php';
 
    $password = $_POST['password'];
 
    $email = htmlentities($_POST['email'], ENT_QUOTES, "UTF-8");
 
    $sekret = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx";
 
    $sprawdz = file_get_contents('https://www.google.com/recaptcha/api/siteverify?secret='.$sekret.'&response='.$_POST['g-recaptcha-response']);
 
    $odpowiedz = json_decode($sprawdz);
 
    if ($odpowiedz->success==false)
    {
        $_SESSION['e_bot']="Potwierdź, że nie jesteś botem!";
        header('Location: ../login.php');
        exit();
    }
        $sql = 'SELECT * FROM account WHERE email=?';
                        $statement = $connection->prepare($sql);
                        $statement->execute([$email]);
                        $oz = $statement->rowCount();
 
                if($oz>0){
 
                $row = $statement->fetchAll(PDO::FETCH_OBJ);
 
                foreach($row as $rows):
                $password_base = $rows->password;
                $email = $rows->email;
                $id_base = $rows->id;
                endforeach;
 
                if (password_verify($password, $password_base))
                {
 
                    $_SESSION['verify'] = true;
                    $_SESSION['id_session'] = $id_base;
 
                    function random(int $i)
                    {
                        return substr(str_shuffle(str_repeat($x='0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ', ceil($i/strlen($x)) )),1,$i);
                    }
 
                    $code = random(35);
                    $now = time() + 900;
                    $time = date("Y-m-d H:i:s");
 
                    $sql = "INSERT INTO verify_code (id_user, code, date, status_code) VALUES (?, ?, ?, ?)";
                    $connection->prepare($sql)->execute([$id_base, $code, $now, 0]);	
 
                    $message="Kod do weryfikacji:\n\n$code\n\nWiadomosc zostala wygenerowana automatycznie: $time";
                    $sender="From: domena.pl";
                    @mail($email, "Kod weryfikacyjny", $message, $sender);
 
                    unset($_SESSION['error']);
 
                    $statement = null;
                    $connection = null;
 
                    header('Location: verify.php');
 
                }
                else 
                {
 
                    $_SESSION['error'] = '<span style="color:red">Nieprawidłowy login lub hasło!</span>';
                    $statement = null;
                    $connection = null;                    
                    header('Location: ../login.php');
 
                }
 
            } else {
 
                $_SESSION['error'] = '<span style="color:red">Nieprawidłowy login lub hasło!</span>';
                    $statement = null;
                    $connection = null;
                header('Location: ../login.php');
 
            }
 
    $statement = null;
    $connection = null;
 
?>
[PHP] pobierz, plaintext 

verify_check.php:

[PHP] pobierz, plaintext 
<?php
 
session_start();
 
if (!isset($_SESSION['verify']))
{
  header('Location: ../login.php');
  exit();
}
 
 
if(empty($_POST["code"]) || !isset($_POST["code"])){
 
  $_SESSION['error'] = '<span style="color:red">Nie podano kodu</span>';             
  header('Location: verify.php');
 
}
 
$code = htmlentities($_POST["code"], ENT_QUOTES, "UTF-8");
 
require '../database_connect/dbsoft.php';
 
$sql = 'SELECT * FROM verify_code WHERE code=?';
$statement = $connection->prepare($sql);
$statement->execute([$code]);
 
$row = $statement->fetchAll(PDO::FETCH_OBJ);
 
foreach($row as $rows):
$id_user = $rows->id_user;
$date = $rows->date;
$code_base = $rows->code;
$status_base = $rows->status_code;
$ide = $rows->id;
endforeach;
 
$now = time();
 
if($status_base == 0){
  if($code_base == $code){
 
    if($_SESSION['id_session'] == $id_user){
 
      if($now <= $date){
 
        $_SESSION['admin'] = true;
 
        $st = 1;
 
        $sql = "UPDATE verify_code SET status_code=? WHERE id=?";
        $statement = $connection->prepare($sql);
        $statement->execute([$st, $ide]);
 
        unset($_SESSION['verify']);
 
        $statement = null;
        $connection = null;
 
        header('Location: ../../');
        exit();
 
      }else{
        echo 'Czas minał';
      }
 
    }else{
      echo 'To nie ty!';
    }
 
  }else{
   echo 'Nieprawidłowy kod';
  }  
}else{
  echo 'Kod został zużyty';
}
[PHP] pobierz, plaintext 

Jeśli jest dobrze to proszę też o odpowiedź

Ten post edytował gloweres 9.11.2020, 03:00:50

Popraw wcięcia.

jedynie bym usuwał użyte kody, nie ma sensu ich trzymać. Poza tym da się żyć, ale bez composera co to za życie

Postanowiłem przechowywać je w bazie żeby mieć logi kto kiedy się logował (bo zapisuje id użytkownika, datę zalogowania oraz czy kod został wykorzystany)

można by się przyczepić jeszcze do generowania samych kodów, istnieje prawdopodobieństwo powtórzenia się kodu, więc albo jak Pyton pisał usuwać, albo jakiś mocniejszy algorytm np GUID

Ja jeszcze przed samym przypisaniem sesji wygenerowalbym nowe id sesyjne

Sam kod powinien mieć możliwy krótki czas życia np. 15min.
Do logowania możesz mieć oddzielną tabelkę z historią i tam możesz sobie zapisywać ID usera, datę, i czy udane logowanie czy nie. Wtedy możesz nawet wykrywać że ktoś chce sobie powłamywać się.

Co do kolizji kodów wspominany przez @gino to prawda. jest libka do php do generowania UUIDv4 (najbardziej randomowy) https://github.com/ramsey/uuid
Ale jesli to jest taki prosty kod jak go pokazujesz to nie ma sensu szaleć.

Z takim ciagiem praktycznie niemozliwe

Włącz sobie jeszcze pełne raportowanie błędów i popraw wszystko. Tak logicznie robisz też insert kodu który za chwilę może się wywalić na mail() i nic z tym nie robisz np rollback transakcji.