Hot answers tagged

5

isset() accepts multiple parameters, you can simplify: if (isset($parameters["inserted_id"]) && isset($parameters["inserted_password"])) { To if (isset($parameters["inserted_id"], $parameters["inserted_password"])) { When ever I read anyone's code or database schema and I see any mention of id, I immediately assume that I am dealing with a ...


4

Yes. Definitely. You can use a lookup array. $langs = [ 'en' => 'gb', 'tr' => 'tr', ]; $defaultLang = 'it'; if (isset($_GET['lang'])) { $lang = strtolower($_GET['lang']); $_SESSION['lang'] = $langs[$lang] ?? $defaultLang; } elseif (!isset($_SESSION['lang'])) { // meaning, no $_GET['lang'] and no $_SESSION['lang'] $_SESSION['...


4

I'll take a more holistic approach than mickmackusa, he already talked about a lot of details. I don't see any problem with the fact that you use an AJAX call to verify user credentials, and login. There's no real difference, security wise, when I compare this with a normal form submission. Any hacker will simply bypass your Javascript and call login.inc....


3

here are some comments on the class code which I hope are of some use to you. Dependency Injection Creating the PDO object inside the constructor introduces a POD dependency on User class. Think about SOLID principles when writing classes. The "S" in "SOLID" stands for Single Responsibility. Classes should have a Single responsibility - one specific purpose....


3

First of all, you are heavily overthinking it. And, as a result, over-engineer the code, making it mostly overkill. A theory is a good thing, in reality it is always a trade-off between security and user experience. For example, on most sites, including Stack Overflow, a user is "remembered", which is essentially like an endless session which doesn't care ...


3

Your question I want to know if the way i did is correct, and if there's a safer way to do this You need to ask how secure $_SESSION['email'] is. Could that value be updated, perhaps by a user profile update page? Would it be simpler to use an attribute that would perhaps be stored in a relational table, to allow multiple users to be denoted as an admin?...


2

The trim() function usage is OK. The biggest problem here is a hash security. A permanent md5( rand(0,1000) ); hash is anything but security. It's so easily guessable that you can count it doesn't exist at all. Password reminder hashes are generated per request, each time anew. And it should be something less predictable, http://php.net/manual/en/function....


2

For session management, you might want to take a look at SessionHandlerInterface. Generally, your class can handle it all, but further on - you can set custom session data storage, like Memcache or Redis. It seems like you're using php7(param typehints), but no return types. Consider doing that too. For instance, UserSessionHelper::sessionHash can be ...


2

As an overarching rule, I never write scripts with the "stfu operator" (@). It looks like you plan to refine the session starting line, so I won't dwell. You are calling time() 5 separate times in your function. Because there is no benefit in recording their differences in terms of microseconds, I recommend that you call time() once, and cache the value in ...


2

Abstraction Your class represents a user, user registration, authentication, and data access all at the same time. This is not ideal because with so many responsibilities, it is difficult to reason about and to maintain if your requirements change. To correct this, first break your class into: User Registration Authentication UserDb Once you have done this,...


1

The answer by suspectus covers DI, type declarations and documentation well. There are a few other aspects I noticed, covered in the sections below. Avoiding else when possible Most of the methods aren't too long, though user_register() is a bit on the long side. There isn't really a need to have the else keyword, since the if block before it has a return ...


1

I haven't written JavaScript or dealt with Flask for a couple years, but here are some suggestions for the Python code: black can automatically format your code to be more idiomatic. isort can group and sort your imports automatically. flake8 with a strict complexity limit will give you more hints to write idiomatic Python: [flake8] max-complexity = 4 ...


1

I don't use $_SESSION super global array and don't know much about it, so please correct me if I'm wrong, isn't this code vulnerable to session fixation attacks ? AFAIK the magic behind $_SESSION is a session id cookie, I think it's default name is PHPSESSID, and in your code when the user logs in you don't change this cookie! I did little test before I ...


1

Another thing I noticed is this: } catch (PDOerrorInfo $e){} What is PDOerrorInfo? Did you declare that as a subclass of Exception? If so, when do you expect that to be thrown? Otherwise are you thinking of the PDO method errorInfo()? If that is the case, this catch is basically useless, since a catch can basically only handle instances of Exception. ...


1

Securing PHP sessions is like art and the defualt session handler is not safe far from it and alot can go wrong with it.. For example on shared webhosting when the sessions are configured to run in one directory. A attacker could run the PHP code on his hosting account <?php session_start(); $session_id = session_id(); $path = session_save_path() ...


1

Quickfire opinions: Don't use MD5 for anything. MD5 is cryptographically broken. Especially do not put passwords through it. Don't use passwords to generate an authentication token. Use a SecureRandom in conjunction with the claimed ID. Keep a server-side copy of the salt and check the authentication token on every request. Only require the password once ...


1

A few things I noticed, First of all this is plain wrong (if it's pure PDO): $link->query("SELECT useragent_hash, user_ip FROM users WHERE username = :username"); $link->bind(':username', $_SESSION['username']); $link->execute(); PDO::query — Executes an SQL statement, returning a result set as a PDOStatement object http://php.net/manual/en/...


1

I know little to nothing about Elm, but speaking of PHP your code is awfully duplicated. Sometimes it duplicates itself, either across different files or in the same file and also it duplicates the functionality already exists in PHP. For example, PHP can log errors for you, and not a single line of code have to be written for that. Processing input $...


1

At the beginnig: im providing this answer for PHP 7.2. You should switch to the latest version (7.3 was released days ago. Also 7.0 will reach EOL soon). Keep in mind to stick to the latest or LTS version. Also please use english only in your code. Namings in Mexican/French/Spanish/German and s.o. will decrease your maintainablility heavily. Okay let's ...


1

In most of the cases it is ok. But in a multi-webserver environment (where you have 1 database and multiple web servers with same dns name, for load balancing purpose, as it could be using Azure) each web server will keep its own list of visitors and will never show all the visitors. To avoid this you should store the visitor list in the database.


Only top voted, non community-wiki answers of a minimum length are eligible