Changing Captive Portal default values? [Captive Portal bug found!]

Shudnawz

I've set up my Captive Portal, and it works as expected. However, the bandwidth limiter on the main setup page does no seem to do much of anything. The limiter on every authorized host does, however, work like a charm. Only problem is, nothing is set there as default, and modding every host as they get access it boring at best. So, can I somehow mod the function "add host to pass-throug MAC-list" to set a default value of my choice (or even better, copy the value from the inert limiter on the main page)?

Shudnawz

I think I've narrowed it down to this function;

WRONG CODE

But I'm uncertain what the ": 0" does on the $dwfaultbw_up line.

This seems to be it:

function portal_allow($clientip,$clientmac,$username,$password = null, $attributes = null, $pipeno = null, $radiusctx = null)  {
	global $redirurl, $g, $config, $type, $passthrumac, $_POST, $cpzone;

...

			$dwfaultbw_up = isset($config['captiveportal'][$cpzone]['bwdefaultup']) ? $config['captiveportal'][$cpzone]['bwdefaultup'] : 0;
			$dwfaultbw_down = isset($config['captiveportal'][$cpzone]['bwdefaultdn']) ? $config['captiveportal'][$cpzone]['bwdefaultdn'] : 0;
			$bw_up = isset($attributes['bw_up']) ? round(intval($attributes['bw_up'])/1000, 2) : $dwfaultbw_up;
			$bw_down = isset($attributes['bw_down']) ? round(intval($attributes['bw_down'])/1000, 2) : $dwfaultbw_down;

I'm rather certain I could change the zeroes to any number I want to let that be the default bandwidth limit for this CP….I think.

Shudnawz

But, but, but….WHAT?!

As far as I can tell from the source code of the php-page services_captiveportal.php, $bwdefaultdn and $bwdefaultup are set there, as the values in the "Default download/upload" boxes. And those values are in turn used in the function portal_allow() which is called when a client is allowed access through the portal, as per my last update in the post above. Why, then, is no value set even though I have a default BW-limit? Is something broken?

EDIT:
Yes, captiveportal.inc is broken within the portal_allow() function. It sets the bandwidth variables AFTER it tries to write them to the $mac item. Adjusting the position of the variable-init fixes it;

	if (!isset($sessionid)) {
		/* generate unique session ID */
		$tod = gettimeofday();
		$sessionid = substr(md5(mt_rand() . $tod['sec'] . $tod['usec'] . $clientip . $clientmac), 0, 16);

		$dwfaultbw_up = isset($config['captiveportal'][$cpzone]['bwdefaultup']) ? $config['captiveportal'][$cpzone]['bwdefaultup'] : 0;
		$dwfaultbw_down = isset($config['captiveportal'][$cpzone]['bwdefaultdn']) ? $config['captiveportal'][$cpzone]['bwdefaultdn'] : 0;
		$bw_up = isset($attributes['bw_up']) ? round(intval($attributes['bw_up'])/1000, 2) : $dwfaultbw_up;
		$bw_down = isset($attributes['bw_down']) ? round(intval($attributes['bw_down'])/1000, 2) : $dwfaultbw_down;

		if ($passthrumac) {
			$mac = array();
			$mac['mac'] = $clientmac;
			$mac['ip'] = $clientip; /* Used only for logging */
			if (isset($config['captiveportal'][$cpzone]['passthrumacaddusername'])) {
				$mac['username'] = $username;
				if ($attributes['voucher'])
					$mac['logintype'] = "voucher";
			}
			$mac['descr'] =  "Auto added {$username} on {$clientip}, given {$bw_down}/{$bw_up} Kbit/s";
			if (!empty($bw_up))
				$mac['bw_up'] = $bw_up;
			if (!empty($bw_down))
				$mac['bw_down'] = $bw_down;
			if (!is_array($config['captiveportal'][$cpzone]['passthrumac']))
				$config['captiveportal'][$cpzone]['passthrumac'] = array();
			$config['captiveportal'][$cpzone]['passthrumac'][] = $mac;
			unlock($cpdblck);
			$macrules = captiveportal_passthrumac_configure_entry($mac);
			file_put_contents("{$g['tmp_path']}/macentry_{$cpzone}.rules.tmp", $macrules);
			mwexec("/sbin/ipfw -x {$cpzone} -q {$g['tmp_path']}/macentry_{$cpzone}.rules.tmp");
			$writecfg = true;
		} else {
			/* See if a pipeno is passed, if not start sessions because this means there isn't one atm */
			if (is_null($pipeno))
				$pipeno = captiveportal_get_next_dn_ruleno();

			/* if the pool is empty, return appropriate message and exit */
			if (is_null($pipeno)) {
				portal_reply_page($redirurl, "error", "System reached maximum login capacity");
				log_error("WARNING!  Captive portal has reached maximum login capacity");
				unlock($cpdblck);
				return;
			}

			// $dwfaultbw_up = isset($config['captiveportal'][$cpzone]['bwdefaultup']) ? $config['captiveportal'][$cpzone]['bwdefaultup'] : 0;
			// $dwfaultbw_down = isset($config['captiveportal'][$cpzone]['bwdefaultdn']) ? $config['captiveportal'][$cpzone]['bwdefaultdn'] : 0;
			// $bw_up = isset($attributes['bw_up']) ? round(intval($attributes['bw_up'])/1000, 2) : $dwfaultbw_up;
			// $bw_down = isset($attributes['bw_down']) ? round(intval($attributes['bw_down'])/1000, 2) : $dwfaultbw_down;

The last four commented lines are the original position, which breaks the function. They are instead moved to right before "if ($passthrumac) {"

eri--

Fixed in 2.1.1 with better fix than yours.

Shudnawz

Fair enough, will wait for that then. Never claimed to be a php-wizkid. =)

eri--

It was not on the side of the php but having a solution to suit the whole module :).

Shudnawz

All well and good, I'm sure. =)
Well my fugly fix works for now…can I expect the entire .inc-file to be replaced by the update, or should I revert my change once 2.1.1 is out?

eri--

It will be replaced.