Project

General

Profile

Bug #4625

Expiring a voucher doesn't disconnect a user who is using that voucher

Added by Gertjan KROEB over 4 years ago. Updated about 4 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Captive Portal
Target version:
Start date:
04/16/2015
Due date:
% Done:

100%

Estimated time:
0.00 h
Affected Version:
All
Affected Architecture:
All

Description

https://forum.pfsense.org/index.php?topic=91435.0

According to https://github.com/pfsense/pfsense/blob/master/etc/inc/voucher.inc#L292 when a voucher is expired manually, a test is executed to see if that voucher is in use.
Or, $cpentry is used the correct way.
$cpentry is an array filled up one (or multiple ) arrays.
In this case, only one "array in array" will be found.

Next issue: if a logged in user/voucher is found, captiveportal_disconnect()is called.
captiveportal_disconnect() presumes that $cpzoneid is set to a valid zone ID.
When calling captiveportal_disconnect() from /etc/inc/voucher.inc $cpzoneid isn't set.

Right now, I use this:

                /* Check if this voucher has any active sessions */
                $cpentry = captiveportal_read_db("WHERE username = '{$voucher}'");
                if (!empty($cpentry) && !empty($cpentry[0]) ) {
                    $cpentry = $cpentry[0];
                    // surface global variable $cpzoneid needed by captiveportal_disconnect()
                    $cpzoneid = $config['captiveportal'][$cpzone]['zoneid'];
                    captiveportal_disconnect($cpentry,null,13);
                    captiveportal_logportalauth($cpentry[4],$cpentry[3],$cpentry[2],"FORCLY TERMINATING VOUCHER {$voucher} SESSION");
                    $unsetindexes[] = $cpentry[5];
                }

Is solves the issue.

The array $cpentry is correctly now.
$cpzoneid is surfaced locally - maybe this should be done more globally.

Note: the only place where $cpzoneid is set, is in /usr/local/captiveportal/index.php : here https://github.com/pfsense/pfsense/blob/master/usr/local/captiveportal/index.php#L58

Associated revisions

Revision abaa7feb (diff)
Added by Ermal Luçi over 4 years ago

Fixes #4625 correct disconnection of users especially when called from xmlrpc code.

Revision 41ee5514 (diff)
Added by Ermal Luçi over 4 years ago

Fixes #4625 correct disconnection of users especially when called from xmlrpc code.

Revision 905e1156 (diff)
Added by Ermal Luçi over 4 years ago

Fixes #4625, manual merge of pull request #1617 for RELENG_2_2 branch on fixing voucher disconnection.

History

#1 Updated by Ermal Luçi over 4 years ago

  • Status changed from New to Feedback
  • % Done changed from 90 to 100

#2 Updated by Ermal Luçi over 4 years ago

#3 Updated by Gertjan KROEB over 4 years ago

The main issue still stands.
The result from
captiveportal_read_db("WHERE username = '{$voucher}'");
h2. is an array in an array.

Example: using afterwards $cpentry2 isn't an "IP", it will NOT be defined.
This will do: $cpentry0[2]

That's why I was doing this:
$cpentry = $cpentry0; /* assigning the first array in $cpentry to $cpentry */
Afterwards, $cpentry4,$cpentry3,$cpentry2 will contain the correct values.

I'm pretty sure (99,99) that the issue isn't solved right now.
Throw in a
echo "

"; print_r($cpentry); echo "
";
after
$cpentry = captiveportal_read_db("WHERE username = '{$voucher}'");
and you'll get the picture ;)

Btw: Ok for assigning $cpzoneid.

#4 Updated by Gertjan KROEB over 4 years ago

Humm. posting html destroys my post.
I meant to write

echo "< pre>"; print_r($cpentry); echo "< /pre>";
/* remove spaces */

#5 Updated by Phillip Davis over 4 years ago

It will be easy if you make the changes in the online GitHub and submit a pull request, then it is clear exactly what code works for you and the devs can easily review it...

#6 Updated by Chris Buechler over 4 years ago

or if it's trivially simple, just throw < pre > tags around it. It picked those up and used them as its indication of pre-formatted text, if the whole block were surrounded by it like:

<pre>
stuff here that won't be formatted
</pre>

then it'd be fine.

#8 Updated by Ermal Luçi over 4 years ago

Pull request merged.

#9 Updated by Ermal Luçi over 4 years ago

#10 Updated by Chris Buechler about 4 years ago

  • Status changed from Feedback to Resolved

fixed

Also available in: Atom PDF