[MSSQL][PHP]Niedziałający warunek sprawdzania uprawnień użytkownika Opcje

[Adym]

Cześć,

Mam problem z warunkiem spawdzającym uprawnienia użytkownika do wykonania funkcji usunięcia zamówienia, który ma realizować poniższy kod:

[PHP] pobierz, plaintext 
public function delete(){
		if($this->order_id) {	
		if($_SESSION['user_type'] == 1 || $_SESSION['userid'] == $order['user_id']) {			
				$stmt_A = $this->conn->prepare("
					DELETE FROM ".$this->orderTable." 				
					WHERE order_id = ?");
 
				$order_id = htmlspecialchars(strip_tags($this->order_id));
 
				$stmt_A->bind_param("i", $order_id);
 
					if ($stmt_A->execute()) {
						$stmt_B = $this->conn->prepare("
						DELETE FROM ".$this->orderProductsTable." 				
						WHERE order_id = ?");
 
						$stmt_B->bind_param("i", $order_id);
 
						if ($stmt_B->execute()) {
						$stmt_C = $this->conn->prepare("
						DELETE FROM ".$this->orderRelationsTable." 				
						WHERE relation_order_id = ?");
 
						$stmt_C->bind_param("i", $order_id);
											}
						if ($stmt_C->execute()) {
							return true;
						}
					}
				}
			}
			else {return false;}
		}
[PHP] pobierz, plaintext 

Na początku sprawdzam czy funkcja została "nakarmiona" numerem id zamówienia (order_id) a później czy zalogowany użytkownik jest adminem ($_SESSION['user_type'] == 1) albo czy jest autorem kasowanego zamówienia ($_SESSION['userid'] == $order['user_id']). Jeżeli któryś z tych warunków jest prawdziwy funkcja powinna przejść dalej. W moim przypadku mimo tego warunku użytkownik albo kasuje wszystko albo admin nie może kasować. Nie wiem co jest przyczyną takiego działania. Czy ktoś może mi wytłumaczyć co robię źle?

Pozdrawiam

Ten post edytował Adym 8.02.2021, 10:25:58

[nospor]

Nigdzie w tej funkcji nie widze by $order byla ustawiana. Zmienne w magiczny sposob nie pojawiaja sie w funkcjach.

Tak ciezko sprawdzic przy uzyciu VAR_DUMP czy zmienne zawieraja to co myslisz ze zawieraja?
Tak ciezko wlaczyc wyswietlanie wszystkich bledow by na ekranie tudziez w logach zobaczyc od razu co jest nie tak?

[trueblue]

A swoją drogą, dlaczego nie masz pozakładanych relacji pomiędzy tabelą orderTable, a orderProductsTable oraz orderRelationsTable? Wystarczyłoby wtedy usuwać wyłączenie rekord z tabeli orderTable.

[Adym]

@Nospor, nie bez przyczyny piszę w dziale przedszkole. Proszę więc się nie spinać. Sprawdzę co mi ten var_dump wyrzuca (nie znałem go wcześniej).


@trueblue masz rację, rzeczywiście mogę tak zrobić.



Ten post edytował Adym 8.02.2021, 12:37:18

[nospor]

Sie nie spinam. Tez kiedys zaczynalem. Jednak zamiast leciec na forum z kazda pierda, najpierw sam sprawdzalem co zawieraja zmienne. Znajomosc var_dump jest ti nie potrzebna. moze byc zwykle ECHO czy PRINT.

[gino]

Nigdzie w tej funkcji nie widze by $order byla ustawiana. Zmienne w magiczny sposob nie pojawiaja sie w funkcjach.

Chyba, że to jest OOP i $this->order_id to jakaś właściwość klasy, bo jeszcze to public function...

edit: error mihi @nospor, bo świtało już, jak się kładłem, ale to nie zienia faktu, że to wycinek jakiejś klasy, dziwny trochę u człowieka który początkuje jak sam pisze (IMG:style_emoticons/default/smile.gif)

Ten post edytował gino 8.02.2021, 13:27:50

[nospor]

gino ja mowilem o zmiennej $order a nie $this->order_id. Wyraznie napisalem (IMG:style_emoticons/default/tongue.gif)

[Adym]

Tak jak małe dziecko uczy sie mówić naśladując zasłyszane słowa od rodziców tak ja uczę się php wykorzystując fragmenty kodu bardziej doświadczonych kolegów.

Rzeczywiście nie mam z czym porównywać drugiego warunku. Pytanie czy lepiej zrobić teraz zapytanie do bazy i przez SELECTa pobrać user_id autora danego zapotrzebowania (mając order_id) i porównać czy jest on taki sam jak zalogowanego użytkownika czy też jest jakieś inne prostsze rozwiązanie?

[nospor]

Tak jak małe dziecko uczy sie mówić naśladując zasłyszane słowa od rodziców tak ja uczę się php wykorzystując fragmenty kodu bardziej doświadczonych kolegów.

Tylko ze male dziecko, z racji faktu ze nie ma wybory, to musi tak robic. Ty zdaje sie umiesz czytac i jakies podstawy wpierw bys mogl ogarnac sam, nie sadzisz?

Skoro to $order gdzies tam przeciez masz, to nie mozesz go przekazac do funkcji delete() jako parametr? Ewentualnie jako wlasciwosc klasy na podobnej zasadzie co $this->order_id

[Adym]

Kliknięcie przycisku usuń wyzwala skrypt:

[PHP] pobierz, plaintext 
$(document).on('click', '.delete', function(){
		var orderId = $(this).attr("id");		
		var action = "orderDelete";
		if(confirm("Czy jesteś pewien, że chcesz usunąć to zapotrzebowanie?")) {
			$.ajax({
				url:"manage_orders.php",
				method:"POST",
				data:{orderId:orderId, action:action},
				success:function(data) {					
					orderData.ajax.reload();
				}
			})
		} else {
			return false;
		}
	});	
});
[PHP] pobierz, plaintext 

Potwierdzenie powoduje:

[PHP] pobierz, plaintext 
<?php
include_once 'config/Database.php';
include_once 'class/Order.php';
$database = new Database();
$db = $database->getConnection();
 
$order = new Order($db);
 
if (!empty($_POST['action']) && $_POST['action'] == 'orderDelete') {
    $order->order_id = (isset($_POST['orderId']) && $_POST['orderId']) ? $_POST['orderId'] : '0';
    $order->delete();
}
?>
[PHP] pobierz, plaintext 

i jak tu wcisnąć jeszcze id autora zamówienia?

Ten post edytował Adym 8.02.2021, 13:55:21

[nospor]

Czyli jednak nigdzie nie masz rekordu $order calego. Z racji faktu ze w funkcji uzywales $order sadzilem ze jednak takowy masz. W takim wypadku musisz niestety pobrac go z bazy wpierw.

[Adym]

Cześć,

Warunek już działa (IMG:style_emoticons/default/smile.gif) . Rzeczywiście zmienna z którą porównywałem $_SESSION['userid'] nie była zdefiniowana. Trochę to trwało ale się udało i frustracja zamieniła się w satysfakcję (IMG:style_emoticons/default/smile.gif) . Czy coś jeszcze jest do poprawienia wg was?:

[PHP] pobierz, plaintext 
	public function delete(){
		if($this->order_id) {
			$sqlQuery = "
				SELECT user_id
				FROM ". $this->orderTable ." 
				WHERE order_id = '". $this->order_id ."' ";
 
				$stmt = $this->conn->prepare($sqlQuery);
				$stmt->execute();
				$result = $stmt->get_result();
				$order_row = $result->fetch_assoc();
 
				if($_SESSION['user_type'] == 1 || $_SESSION['userid'] == $order_row['user_id']) {
					$order_id = htmlspecialchars(strip_tags($this->order_id));
 
					$stmt_A = $this->conn->prepare("
						DELETE FROM ".$this->orderTable." 				
						WHERE order_id = ?");
 
					$stmt_A->bind_param("i", $order_id);
 
						if ($stmt_A->execute()) {
							$stmt_B = $this->conn->prepare("
							DELETE FROM ".$this->orderProductsTable." 				
							WHERE order_id = ?");
 
							$stmt_B->bind_param("i", $order_id);
 
							if ($stmt_B->execute()) {
							$stmt_C = $this->conn->prepare("
							DELETE FROM ".$this->orderRelationsTable." 				
							WHERE relation_order_id = ?");
 
							$stmt_C->bind_param("i", $order_id);
												}
							if ($stmt_C->execute()) {
								return true;
							}
						}
				}
			}
		}
[PHP] pobierz, plaintext 

Ten post edytował Adym 10.02.2021, 12:41:57

[nospor]

Cos do poprawy? Hm... wszystko?
Pomijajac fakt, ze mega mieszasz logike i odpowiedzialnosc klas, to:

nie
$order_id = htmlspecialchars(strip_tags($this->order_id));

a:
$order_id = (int) $this->order_id);
Id to liczba i jako liczba masz to przedstawiac a zadne inne bezsensowne filtrowania nie beda potrzebne.

No i czemu do jednego zapytania to zabezpieczasz a do innego juz nie?
Czemu w jednym bindujesz a w innym juz walisz jako string?

nie ==
a ===

Jak juz zwracasz TRUE gdy sie powiedzie, to badz konsekwentny i zwracaj FALSE gdy sie nie powiedzie

[Adym]

Czy teraz lepiej czy coś jeszcze razi Cię w oczy (IMG:style_emoticons/default/wink.gif) ?

[PHP] pobierz, plaintext 
	public function delete(){
		if($this->order_id) {
			$order_id = $this->order_id;
 
			$stmt = $this->conn->prepare(" SELECT user_id FROM ". $this->orderTable ." WHERE order_id = ? ");
			$stmt->bind_param("i", $order_id);
			$stmt->execute();
			$result = $stmt->get_result();
			$order_row = $result->fetch_assoc();
 
				if($_SESSION['user_type'] === 1 || $_SESSION['userid'] === $order_row['user_id']) {
					$stmt_A = $this->conn->prepare("DELETE FROM ".$this->orderTable." WHERE order_id = ?");
					$stmt_A->bind_param("i", $order_id);
 
					if ($stmt_A->execute()) {
						$stmt_B = $this->conn->prepare("DELETE FROM ".$this->orderProductsTable." WHERE order_id = ?");
						$stmt_B->bind_param("i", $order_id);
 
						if ($stmt_B->execute()) {
						$stmt_C = $this->conn->prepare("
						DELETE FROM ".$this->orderRelationsTable." 				
						WHERE relation_order_id = ?");
 
						$stmt_C->bind_param("i", $order_id);
											}
						if ($stmt_C->execute()) {
							return true;
						} else {return false;}
					} 
				} 
			}
		}
[PHP] pobierz, plaintext 

Ten post edytował Adym 10.02.2021, 13:01:53

[nospor]

to jest zbedne
$order_id = $this->order_id;

Dzialaja bezposrednio na $this->order_id


return false leci tylko w okreslonym wypadku. Po drodze jest cala masa IFow, ktore nie zwracaja nic. Zamiast ten else co dodales, daj poprostu
return false
na samym koncu funkcji

Unikaj zagniezdzen.

Zamiast kobylastdego IF
if($this->order_id) {
.......
}


daj tak

if(!$this->order_id) {
return false;
}
......

[Adym]

Dzięki za pomoc NOSPOR. Zdaję sobie oczywiście sprawę, że kod można pewnie napisać lepiej, wydajniej, ładniej ale do tego będę dochodził małymi kroczkami.

Pozdrawiam!