Solved

How to check that user session ID is set and that another session info meets certain criteria

Posted on 2016-11-01
20
78 Views
Last Modified: 2016-11-02
Upon successful login I store this info and put it into a session:

$fingerprint = md5($_SERVER['HTTP_USER_AGENT'] . $_SERVER['HTTP_ACCEPT_LANGUAGE']);
$_SESSION['fingerprint'] = $fingerprint;
$_SESSION['randID'] = $row['rand_id'];

Open in new window


Then once logged in I check it again:

$fingerprint = md5($_SERVER['HTTP_USER_AGENT'] . $_SERVER['HTTP_ACCEPT_LANGUAGE']);

Open in new window


On the user dashboard page, I want to check that the randID session exists as well as that $fingerprint matches $_SESSION['fingerprint].

I don't know how to put the together. I need:

if(!isset($_SESSION['safeID'])

Open in new window


&

if(isset($_SESSION['fingerprint']) && $_SESSION['fingerprint'] !== $fingerprint)

Open in new window

0
Comment
Question by:Black Sulfur
  • 8
  • 7
  • 5
20 Comments
 
LVL 108

Expert Comment

by:Ray Paseur
ID: 41868550
Please let me suggest a slightly different design pattern.  You might want to load the entire client record into the session, giving you access to all of the client information with something like this.
$_SESSION['current_client']['id'];
$_SESSION['current_client']['name'];
$_SESSION['current_client']['email'];

Open in new window

The things worth checking from session to session include the HTTP_USER_AGENT and maybe the REMOTE_ADDR.  I say "maybe" because there may still be some users behind proxy servers or on AOL that can get different REMOTE_ADDR assignments between requests.  If your scripts invalidate the session when that changes, you will create an automated annoyance for these poor folks.  I'm not sure whether the HTTP_ACCEPT_LANGUAGE needs to be checked.

Also, the additional step of md5() hashing doesn't add any value here, and will make it difficult to debug.  It's perfectly OK to store clear-text representations in the PHP session.  About the only thing you can't store are certain kinds of objects and resources.
0
 

Author Comment

by:Black Sulfur
ID: 41868596
I left off REMOTE_ADDR because ISP's also change a users IP address at random (it happens to me from time to time) so I didn't think it worthwhile as well as the proxy servers you mentioned.

Noted, regarding creating session variables for other user info. I didn't want too much user info hanging around in sessions and I was just going to get them from the database based on the users ID but it does seem less troublesome just sticking them all into sessions.

I put HTTP_ACCEPT_LANGUAGE in because on a Wordpress site I have, I noticed a lot of the brute force attempts come from countries where English isn't their first language and that probably wouldn't be
en-US,en;q=0.8

but, I might be misunderstanding the point of HTTP_ACCEPT_LANGUAGE.
0
 
LVL 108

Expert Comment

by:Ray Paseur
ID: 41868621
It won't hurt to check HTTP_ACCEPT_LANGUAGE.  Might be interesting to use error_log() to follow how many language changes occur.

The general rules for revalidation are the same for web applications and the ATM PIN number.  Validate when a new ATM card (or user-id) is presented.  Validate when a money-related transaction is requested.  Validate when important information is to be changed on the server.  Validate when too much time has elapsed, etc.
0
 

Author Comment

by:Black Sulfur
ID: 41868663
Thanks for the info, it's much appreciated. I'm still struggling with the answer to get the 2 conditions into one though...
0
 
LVL 108

Expert Comment

by:Ray Paseur
ID: 41868698
Here's how to write the if() statement (technically a "control structure").

if (expr1){ // expression must be true
if ( (expr1) && (expr2) ){ // both expressions must be true
if ( (expr1) && (expr2) && (expr3) ){ // all three expressions must be true

if ( (expr1) || (expr2) ){ // either expression must be true

Open in new window

You can make this really complicated by trying to shoehorn several expressions and logical operators into a small number of statements.  Make a Google search for "cyclomatic complexity" and you'll see why we try to use simpler designs to avoid logical complexity.

Here's the PHP word on "expressions."
http://php.net/manual/en/language.expressions.php

Here's some information on comparisons.
http://php.net/manual/en/language.operators.comparison.php
0
 

Author Comment

by:Black Sulfur
ID: 41868910
Thanks for the info. I have taken into consideration what you've said and have come up with this which seems to work. But before I assume it's okay from my testing, I would like to of course check.

if(!isset($_SESSION['safeID']) || isset($_SESSION['fingerprint']) && $_SESSION['fingerprint'] !== $fingerprint) {

Open in new window


I tried to convert it into plain english. So, to me this reads:

If the safeID session is NOT set OR if the fingerprint session IS set but does not equal the fingerprint variable...

Please let me know if I am correct, wrong, or at least getting warm ;)
0
 
LVL 108

Accepted Solution

by:
Ray Paseur earned 250 total points
ID: 41869008
Maybe something like this to disambiguate the ors & ands.
if(!isset($_SESSION['safeID']) || ( isset($_SESSION['fingerprint']) && $_SESSION['fingerprint'] !== $fingerprint) ) {

Open in new window

I haven't counted the parentheses, but your narrative sounds about right for the statement.  But think about this a little bit... If you're not sure of the code you need to implement the logic, why not change the design to simplify the logic?

Maybe make up a truth table to help visualize the potential conditions.  Anything that reduces cyclomatic complexity will help make testing easier.
0
 
LVL 51

Assisted Solution

by:Julian Hansen
Julian Hansen earned 250 total points
ID: 41869206
Just a suggestion but I find this useful in dealing with unknown array elements

// SET $safeID TO THE SESSION VARIABLE IF IT EXISTS OTHERWISE FALSE
$safeID = isset($_SESSION['safeID']) ? $_SESSION['safeID'] : false;

// SET $fprint = TO COMPARRISON OF $_SESSION['fingerprint'] AND $fingerprint (BUT
// ONLY IF $_SESSION['fingerprint'] EXISTS) OTHERWISE SET IT TO FALSE
$fprint = isset($_SESSION['fingerprint']) ? $_SESSION['fingerprint'] == $fingerprint : false;

// BOTH $safeID AND $fprint ARE SAFE TO USE AND SET TO
// BOOLEAN VALUES SO WE CAN USE THEM AS IS
if ($safeID && $fprint) {
  // good to go
}

Open in new window

0
 
LVL 108

Expert Comment

by:Ray Paseur
ID: 41869274
Just a cautionary note about using isset() values in a ternary operator...  Loose data typing may not be your friend.  Sometimes it's smart to test with empty() instead.  And it almost never hurts to initialize variables and utilize strict comparisons.  Consider this example, wherein the summary comparison if($safeID)  would be evaluated FALSE. The same will happen if the session variable contains any of the falsy  values, including an arithmetic result that came up zero.
<?php // demo/temp_julian.php
/**
 * https://www.experts-exchange.com/questions/28980237/How-to-check-that-user-session-ID-is-set-and-that-another-session-info-meets-certain-criteria.html
 */
error_reporting(E_ALL);

// SET A VARIABLE.  NOW WE HAVE isset() === TRUE & empty() === TRUE
$_SESSION['safeID'] = '';

// SET $safeID TO THE SESSION VARIABLE IF IT EXISTS OTHERWISE FALSE
$safeID = isset($_SESSION['safeID']) ? $_SESSION['safeID'] : false;

// BUT THE LOGIC BREAKS DOWN HERE
if ( $safeID) echo " safeID";
if (!$safeID) echo "!safeID";

Open in new window

Outputs:
!safeID

Open in new window

These references explain what is going on.
http://php.net/manual/en/language.types.php
http://php.net/manual/en/types.comparisons.php
http://php.net/manual/en/language.operators.comparison.php
http://php.net/manual/en/language.types.type-juggling.php
http://php.net/manual/en/language.types.boolean.php#112190
0
 
LVL 51

Expert Comment

by:Julian Hansen
ID: 41869651
If we look at this statement
if(!isset($_SESSION['safeID']) || isset($_SESSION['fingerprint']) && $_SESSION['fingerprint'] !== $fingerprint) {

Open in new window

Changing the isset() to empty will change the outcome of the statement
Consider the following code
<?php
$x = array();
$x['a'] = '';
$x['f'] = 'f';
$f = 'f';
if (!isset($x['a']) || isset($x['f']) && $x['f'] !== $f) {
	echo "True A";
}
else {
	echo "False B";
}

if (empty($x['a']) || !empty($x['f']) && $x['f'] !== $f) {
	echo "True A";
}
else {
	echo "False B";
}

Open in new window

Output
False B
True A

Open in new window


Sidebar
Bracketing the && expression is probably good practice but in this case it makes no difference as the && operator has higher precedence than ||

A || B && C is equivalent to A || (B && C)

Lower precedence operators need to be bracketed
If the desire was A OR B ...... AND C then the following would be required to ensure that A || B is evaluated as a sub-expression before being ANDed with C.
(A || B) && C
0
How to run any project with ease

Manage projects of all sizes how you want. Great for personal to-do lists, project milestones, team priorities and launch plans.
- Combine task lists, docs, spreadsheets, and chat in one
- View and edit from mobile/offline
- Cut down on emails

 
LVL 108

Expert Comment

by:Ray Paseur
ID: 41870341
Setting aside the diversions, I think the central issue here goes to the design pattern.  We're only dealing with a small code fragment, but it seems to be saying that we have potentially got two indicators that must be tested to know whether we have a "valid" session.  And we're trying to shoehorn the tests into a single compound if()  statement.  That's a recipe for confusion; there should always be only one binary indicator of the truth.  So I'm going to suggest that you repackage these tests into something that will give you a single-point-of-truth.

If you're familiar with variable scope this code will look a little odd, because $_SESSION is a superglobal that exists in all scopes and namespaces.  Thus there is no true encapsulation, but at least it gives the single-point functionality.
function is_valid_user($fingerprint=NULL)
{
    // ASSUME FALSE
    $valid_user = FALSE;

    // TEST FOR PRESENCE OF ANY SAFE-ID
    if (!empty($_SESSION['safeID'])) $valid_user = TRUE;

    // TEST FOR FINGERPRINT MATCH
    if (!empty($_SESSION['fingerprint']))
    {
        if ($_SESSION['fingerprint'] === $fingerprint)
        {
            $valid_user = TRUE;
        }
    }

    return $valid_user;
}

Open in new window

This assumes you're using the PHP session in the way it is intended.
1
 

Author Comment

by:Black Sulfur
ID: 41870465
Thanks guys. I don't want to draw this question out but the answers sort of leave me with even more questions, haha.

It has been mentioned that trying to put the multiple if statements into one can lead to confusion which i in my case is true. I would rather split them up but I didn't know how to and if it's possible. Well, I did actually have two separate ones that both ended up having redirects which didn't make sense to do as I was duplicating code.

So, are you saying that I should have two as per your example above and my old way of doing it, except set them equal to true? Then would I say if !== true, redirect the user or I could do it the opposite way and set to false if the statement wasn't true and redirect if ==false.

I haven't implemented any of your code as of yet until I get the methodology that makes sense to me. Then I will worry about empty vs isset etc. I just want the logic right first. I tried to rework what I had but it isn't working properly

if(isset($_SESSION['safeID'])) {
		
		$valid_user = true;
	}
	
	if(isset($_SESSION['fingerprint'])){
		
		if($_SESSION['fingerprint'] === $fingerprint) {
			
			$valid_user = true;
		}
	}
	
	if($valid_user !== true) {
		
		header("location:login.php");
	}

Open in new window

0
 
LVL 51

Expert Comment

by:Julian Hansen
ID: 41870535
Personally I think that is overly verbose. I stand by this answer

There is absolutely nothing wrong with setting up your boolean variables and then testing them in one if statement.

If statement's become problemenatic when it becomes difficult to see what they are doing

if (A &&B)

Is pretty straight forward - expanding that out into several if statements just makes things look untidy.

Your call.
0
 
LVL 108

Expert Comment

by:Ray Paseur
ID: 41870584
@Black-Sulfur: I think we've run this to ground and we're probably wasting your time now, so I'll sign off.  I don't write code to make computers happy; I write code that people can understand, and if that means being a bit more verbose in order to avoid confusion, then I will intentionally be more verbose.  If you're still confused after reading the function code, take a step back and read up on control structures and logic flow.  This is essential knowledge for any programmer, and it pays off when you invest the time to get a strong underpinning of knowledge.  PHP has some good learning resources.
http://php.net/manual/en/language.control-structures.php
http://php.net/manual/en/language.expressions.php
http://php.net/manual/en/control-structures.if.php
http://php.net/manual/en/control-structures.else.php
http://php.net/manual/en/language.types.boolean.php#language.types.boolean.casting

If you want to save yourself some time, E-E has an article about PHP client authentication.  You might start with that as your "platform" and build up your security measures on top of an already working code base.

Best of luck with your project, ~Ray
0
 

Author Comment

by:Black Sulfur
ID: 41870665
@ Julian,

I still haven't learnt about how to do this as I find it confusing and probably won't code this way unless I find in future that I should. Please could you convert this into "normal code" for lack of a better word (please excuse my ignorance)

$safeID = isset($_SESSION['safeID']) ? $_SESSION['safeID'] : false;

Open in new window

0
 

Author Comment

by:Black Sulfur
ID: 41870667
@ Ray,

One last thing before you go, please could you tell me what you would do with this?

 return $valid_user;

Open in new window


How do I use it to redirect the user if the valid_user = false?
0
 
LVL 108

Expert Comment

by:Ray Paseur
ID: 41870846
This is another one of those places where a little learning goes a long, long way!
http://php.net/about.prototypes
http://php.net/language.functions
0
 
LVL 51

Expert Comment

by:Julian Hansen
ID: 41871196
$safeID = isset($_SESSION['safeID']) ? $_SESSION['safeID'] : false;

Open in new window

This is called a ternary operation it has the following form (3 parts hence the name)
(expr) ? true_part : false_part

Open in new window

It is analogous to the following if / then / else
if (expr) {
  true_part;
}
else {
  false_part;
}

Open in new window


It is often used in assignments where the assignment is different depending on some condition.

So
Question: What is 4 / 2?
$response = ($answer==2) ? "Correct" : "Incorrect";

Open in new window

As opposed to
$response = "Incorrect";
if ($answer == 2) {
  $response = "Correct";
}

Open in new window

We can do the assignment on one line instead of 4 without loosing context or increasing complexity of how the code reads.

The reason for using it here is as follows.

$_SESSION['safeID'] may or may not exist. We need to take action based on its state but if it is not set and we try to use it in an expression it will result in an error - so we have to work around the not set condition.

The goal of the ternary operation is to reduce $safeID to an absolute i.e. a defined Boolean value that exists and can be used in an expression without the risk of generating an error.

The expression in question will result in $safeID being set to the value of $_SESSION['safeID'] OR FALSE if the latter does not exist.

The same applies to $fingerprint which in the above example was set to $fprint;

Having populated $safeID and $fprint with values that represent the state of $_SESSION['safeID'] and $fingerprint respectively, it is a trivial matter to test for a valid case using a simple if statement.

Having read your other posts where you state you want to redirect if not a valid user AND if a valid user is defined by both $safeID and $fprint being true. we could do this

if ($safeID && $fprint) {
  // true part
}
else {
   // redirect here
}

Open in new window


If no action is required for a valid user then the true part of the expression is unnecessary and  we could turn this around and only test for the false case - which is where $safeID is FALSE OR $fprint is FALSE
In other words
if (!$safeID || !$fprint) {
   // redirect
}

Open in new window

Alternatively
if (!($safeID && $fprint)) {
  // redirect
}

Open in new window

Which one you use is personal preference both will yield the same result
1
 

Author Comment

by:Black Sulfur
ID: 41871531
Thanks so much guys. Both ways have merit and I will have to just keep coding to find out what works best for me but it's nice to have options. I want to write less code to keep everything neat but sometimes writing verbose code is more understandable to a beginner like me. I am going to do an even split of points, I hope that is okay.
0
 
LVL 51

Expert Comment

by:Julian Hansen
ID: 41871541
Fine by me and you are welcome.
0

Featured Post

Do You Know the 4 Main Threat Actor Types?

Do you know the main threat actor types? Most attackers fall into one of four categories, each with their own favored tactics, techniques, and procedures.

Join & Write a Comment

Both Easy and Powerful How easy is PHP? http://lmgtfy.com?q=how+easy+is+php (http://lmgtfy.com?q=how+easy+is+php)  Very easy.  It has been described as "a programming language even my grandmother can use." How powerful is PHP?  http://en.wikiped…
I imagine that there are some, like me, who require a way of getting currency exchange rates for implementation in web project from time to time, so I thought I would share a solution that I have developed for this purpose. It turns out that Yaho…
The viewer will learn how to count occurrences of each item in an array.
The viewer will learn how to create and use a small PHP class to apply a watermark to an image. This video shows the viewer the setup for the PHP watermark as well as important coding language. Continue to Part 2 to learn the core code used in creat…

760 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

22 Experts available now in Live!

Get 1:1 Help Now