Pomoc - Szukaj - Użytkownicy - Kalendarz
Pełna wersja: [skrypt] bezpieczne logowanie
Forum PHP.pl > Inne > Oceny
kiamil
Link do dema: DEMO
Link do kodu: CODE

Zapraszam do oceniania, autoryzacja oparta na jednej zmiennej sesyjnej, IMHO unikalny skrypt. Potrzebwałbym też porady dotyczące bezpieczeństwa.
Babcia@Stefa
Rejestracja i logowanie zwykłe jak widać, może jakiś graficzny captha?

http://www.di3d.boo.pl/logowanie/code.php?file=config.php axesmiley.png

Cytat
$cfg['salt'] = '_babia_stefa_lubi_ogorki_xD';


Co to ma znaczyć?! blinksmiley.gif

Kod jest brzydki (brak odstępów), mógłbyś próbować zrobić go obiektowo skoro "rozdajesz" go publicznie.


Ale "$cfg['salt'] = '_babia_stefa_lubi_ogorki_xD';" mnie zaskoczyło, siedziałem zamurowany przed komputerem na dobre pare minut ... axesmiley.png

Pozdrawiam, WebNuLL...
kiamil
Odstępów nigdy nie robiłem, nie jestem tak zaawansowany by obiektówką walić ;]
Z tym saltem to jakoś tak spontanicznie mi przyszło, nie ma tam żadnego innego znaczenia..
SzamanGN
Dla mnie ciekawe rozwiązanie
kiamil
Coś więcej? Porady dot. bezpieczeństwa? Jeszcze zrobie ochrone przed BF w wersji 0.2 winksmiley.jpg
pyro
BF?
.radex
Cytat(pyro @ 23.12.2008, 20:54:00 ) *
BF?


bruteforce.
pyro
a sorry tongue.gif
erix
Cytat
Odstępów nigdy nie robiłem

Pora zacząć. tongue.gif

Kod
global $_SESSION; global $cfg;

Blah, na chorobę globalizować $_SESSION? Skoro piszesz jakieś skrypty zdobądź chociaż podstawy... dry.gif

Kod
return sha1(time().$nick);

IMHO trochę za słaby ten token... Masz funkcje generowania liczb pseudolosowych, /dev/urandom, uniqid" title="Zobacz w manualu PHP" target="_manual...
Poza tym, nie wiem, po co podajesz użytkownikowi token...

Kod
<? session_start(); session_regenerate_id(); ?>

A na szto tak?

Kod
md5('adm'.$cfg['salt'])

MD5 za bezpieczne nie jest.

Ogólnie: kod nieczytelny, nic rewelacyjnego.
Babcia@Stefa
Cytat(kiamil @ 23.12.2008, 20:17:02 ) *
Odstępów nigdy nie robiłem, nie jestem tak zaawansowany by obiektówką walić ;]
Z tym saltem to jakoś tak spontanicznie mi przyszło, nie ma tam żadnego innego znaczenia..


Wiesz, popatrz na mój nick na forum i porównaj z saltem ;]

Pozdrawiam, WebNuLL
.radex
Cytat(erix @ 23.12.2008, 21:52:30 ) *
Pora zacząć. tongue.gif


No właśnie smile.gif

Porównaj sobie to:

  1. <?
  2. include('mysql.php');
  3.  
  4. $cfg['mysql']['alias']='log_';
  5. $cfg['salt'] = '_babia_stefa_lubi_ogorki_xD';
  6.  
  7. function new_otoken($nick) {
  8. global $_SESSION; global $cfg;
  9. $ot = otoken($nick);
  10. mysql_query('UPDATE '.$cfg['mysql']['alias'].'users SET u_otoken="'.$ot.'" WHERE u_nick="'.$nick.'"');
  11. $_SESSION['otoken']=$ot;
  12. }
  13. function otoken($nick) {
  14. return sha1(time().$nick);
  15. }
  16. ?>


z tym:
  1. <?php
  2.   include 'mysql.php';
  3.  
  4.   $cfg['mysql']['alias'] = 'log_';
  5.   $cfg['salt'] = '_babia_stefa_lubi_ogorki_xD';
  6.  
  7.   function new_otoken($nick)
  8.   {
  9.      global $_SESSION;
  10.      global $cfg;
  11.      
  12.      $ot = otoken($nick);
  13.      
  14.      mysql_query('UPDATE ' . $cfg['mysql']['alias'] . 'users SET u_otoken="' . $ot . '" WHERE u_nick="' . $nick . '"');
  15.      
  16.      $_SESSION['otoken'] = $ot;
  17.   }
  18.  
  19.   function otoken($nick)
  20.   {
  21.      return sha1(time() . $nick);
  22.   }
  23.   ?>


Nie wiem jak Ty, ale mi się wydaje, że moja wersja jest dużo bardziej czytelna. Ot, bardzo dobry nawyk.

Natomiast mam dwie bardzo ważne uwagi:
1. Zaprzestań stosować globali (po konkrety do wyszukiwarki tongue.gif)
2. Rozpocznij stosować mysql_real_escape_string

globale są złe, a "escapować" ciągi znaków należy przed wysłaniem do bazy danych (dla zabezpieczenia przed sql injection), czyli jeszcze raz:

  1. <?php
  2.   include 'mysql.php';
  3.  
  4.   $cfg['mysql']['alias'] = 'log_';
  5.   $cfg['salt'] = '_babia_stefa_lubi_ogorki_xD';
  6.  
  7.   function new_otoken($nick)
  8.   {
  9.      global $cfg; // to należy wywalić...
  10.      
  11.      $nick = mysql_real_escape_string($nick);
  12.      
  13.      $ot = otoken($nick);
  14.      
  15.      mysql_query('UPDATE ' . $cfg['mysql']['alias'] . 'users SET u_otoken="' . $ot . '" WHERE u_nick="' . $nick . '"');
  16.      
  17.      $_SESSION['otoken'] = $ot;
  18.   }
  19.  
  20.   function otoken($nick)
  21.   {
  22.      return sha1(time() . $nick);
  23.   }
  24.   ?>


PS. Stosuj komentarze w swoim kodzie, bo jak będziesz musiał edytować kod, którego nie ruszałeś od kilku miesięcy to nie będziesz wiedział "o co tu chodzi?" winksmiley.jpg
Kildyt
Skupię się na kodzie.

Jak już napisał @.radex: tabulacja!

  1. <?php
  2. global $_SESSION; global $cfg;
  3. ?>
To jest bez sensu.

  1. <?php
  2. mysql_query('UPDATE '.$cfg['mysql']['alias'].'users SET u_otoken="'.$ot.'" WHERE u_nick="'.$nick.'"');
  3. ?>
Nie wiem dlaczego (pewnie ze względu na przejrzystość), ale przyjęło się, że zapytania do bazy danych podajemy w cudzysłowach.
  1. <?php
  2. mysql_query("UPDATE `$cfg['mysql']['alias']users` SET `u_otoken`='$ot' WHERE `u_nick`='$nick'");
  3. ?>
Czy nie ładniej?

W login.php osobiście najpierw zadeklarowałbym zmienną $login i w niej przefiltrował zmienną globalną $_POST ($_POST['nick']) i przez całość skryptu operowałbym na niej. Strzeżonego Pan Bóg strzeże. winksmiley.jpg

Ogólnie kod jest nieprzejrzysty i to jest jego wielkim minusem.
l0ud
Cytat
przyjęło się, że zapytania do bazy danych podajemy w cudzysłowach.

...chyba w skryptach pokroju jportal. Co jak co, ale to jest bzdurą.
Cytat
Czy nie ładniej?

Chociażby ze względu na kolorowanie - nie tongue.gif
Kildyt
Cytat(l0ud @ 25.12.2008, 22:56:52 ) *
...chyba w skryptach pokroju jportal. Co jak co, ale to jest bzdurą.

Chociażby ze względu na kolorowanie - nie tongue.gif

Wiesz, każdy ma swoje przyzwyczajenia. tongue.gif
Przepraszam bardzo, ale jportal to kiedyś był ho, ho. smile.gif Może teraz się z niego śmiejecie, ale kilka lat temu to był niezły CMS, a o tym świadczy chociażby ilość stron na nim postawiona. Pamiętam, że kiedyś nawet robiłem themy do niego. ^^ Ahh, te czasu. smile.gif
Crozin
  1. <?php
  2. abdef('UPDATE %s SET u_token="%s" WHERE u_nick ="%s";', USERS_TABLE, $token, $nick);
  3. ?>
A tak to chyba jeszcze ładniej?
  • Brak jakiejs obsługi błędów (dla MySQL przykładowo)
  • @ - wywal to coś
  • Dlaczego to co jest liczbą (np. w kolumnie u_access z tabeli użytkowników) przekazujesz jako string?
  • Po co w nazwach kolumn przedrostki (u_nick, u_access)
  • htmlspecialchars() używa się przed wyświetleniem danych, a nie ich wprowadzeniem do bazy danych
  • Wprowadź dwa pola dla hasła (wpisz, powtórz) - nie łatwo się pomylić przy rejestracji z hasłem

Jak napisali wcześniej - nic specialnego. Niechludnie napisane, mało elastyczne.
bim2
Jak już to
  1. <?php
  2. echo "UPDATE {$cfg['mysql']['alias']}users";
  3. ?>

smile.gif
jPortal dla mnie zawsze będzie czymś ważnym, na nim nauczyłem się podstaw i myślenia (no i z pomocą kilku osób). Narzekacie tak na niego, ale to było coś. tongue.gif

Co do skryptu, zawsze wyobrażałem sobie, że będzie to coś na zasadzie:
  1. <?php
  2. require_once 'register.module.php';
  3. if(rgWantLogIn())
  4. {
  5. rgLogIn($_POST[RG_LOGIN], $_POST[RG_PASS]);
  6. }
  7. if(rgIsLogin())
  8. {
  9. echo 'Witaj '.rgGetName(); //wiem wiem, klasa byłaby lepsza, ale mówię już o samych podstawach
  10. } else {
  11. rgShowLoginForm();
  12. }
  13. ?>

Wtedy łatwo zaimplementowac to w róznych innych systemach ;]
Crozin
@bim2: jak już to obiekt, nie klasa

Można by się pokusić o zrobienie jakiejś podstawowej rzeczy w stylu oddzielenia warstwy logiki od widoku.
lukaszkkk
Dodam tylko, ze zgodnie z podrecznikiem php, jesli uzywa sie funkcji

mysql_real_escape_string() to wczesniej trzeba za pomoca funkcji stripslashes() usunac ze zmiennych ukosniki, dodane

automatycznie przez derektywe magic_quotes_gpc.

Ladnie jest to pakazane w linku ponizej. 

php.net
To jest wersja lo-fi głównej zawartości. Aby zobaczyć pełną wersję z większą zawartością, obrazkami i formatowaniem proszę kliknij tutaj.
Invision Power Board © 2001-2025 Invision Power Services, Inc.