PacketFence - BTS - PacketFence |
View Issue Details |
|
ID | Project | Category | View Status | Date Submitted | Last Update |
0001505 | PacketFence | guests | public | 2012-08-07 16:12 | 2015-02-13 15:26 |
|
Reporter | fdurand | |
Assigned To | obilodeau | |
Priority | high | Severity | major | Reproducibility | always |
Status | closed | Resolution | open | |
Platform | Linux | OS | Debian | OS Version | 6 |
Product Version | 3.5.0 | |
Target Version | | Fixed in Version | | |
fixed in git revision | |
fixed in mtn revision | |
|
Summary | 0001505: Mac address is null when we try to register a guest with the sponsor method |
Description | When the sponsor click on the link to register a node an error appear in the browser:
discovery of MAC address metadata failed, no meaningful characters in ... in node.pm line 602 (my $tmpMAC = Net::MAC->new( 'mac' => $mac );)
|
Steps To Reproduce | |
Additional Information | |
Tags | No tags attached. |
Relationships | |
Attached Files | captive-portal-sponsor-preregistration-preliminary-fix-v3.patch (4,288) 2012-08-27 13:12 https://www.packetfence.org/bugs/file_download.php?file_id=160&type=bug |
|
Issue History |
Date Modified | Username | Field | Change |
2012-08-07 16:12 | fdurand | New Issue | |
2012-08-07 16:13 | dwuelfrath | Status | new => assigned |
2012-08-07 16:13 | dwuelfrath | Assigned To | => dwuelfrath |
2012-08-07 16:13 | dwuelfrath | Note Added: 0002910 | |
2012-08-07 16:15 | dwuelfrath | Note Added: 0002911 | |
2012-08-07 16:15 | obilodeau | Note Added: 0002912 | |
2012-08-07 16:15 | obilodeau | Priority | normal => high |
2012-08-07 16:15 | obilodeau | Severity | block => major |
2012-08-07 16:15 | obilodeau | Product Version | => 3.5.0 |
2012-08-07 16:18 | dwuelfrath | Note Added: 0002913 | |
2012-08-08 08:57 | obilodeau | Note Added: 0002918 | |
2012-08-08 09:12 | dwuelfrath | Note Added: 0002919 | |
2012-08-14 10:20 | obilodeau | Note Added: 0002926 | |
2012-08-14 10:20 | obilodeau | File Added: captive-portal-sponsor-preregistration-preliminary-fix.patch | |
2012-08-14 10:35 | obilodeau | File Added: captive-portal-sponsor-preregistration-preliminary-fix-v2.patch | |
2012-08-14 10:35 | obilodeau | File Deleted: captive-portal-sponsor-preregistration-preliminary-fix.patch | |
2012-08-14 10:36 | obilodeau | Note Added: 0002927 | |
2012-08-15 16:57 | obilodeau | Note Added: 0002936 | |
2012-08-20 16:47 | obilodeau | Assigned To | dwuelfrath => obilodeau |
2012-08-20 16:48 | obilodeau | Note Added: 0002951 | |
2012-08-27 13:12 | obilodeau | File Added: captive-portal-sponsor-preregistration-preliminary-fix-v3.patch | |
2012-08-27 13:12 | obilodeau | File Deleted: captive-portal-sponsor-preregistration-preliminary-fix-v2.patch | |
2012-08-27 13:12 | obilodeau | Note Added: 0002970 | |
2012-08-29 14:15 | obilodeau | Note Added: 0002987 | |
2012-08-30 11:10 | obilodeau | Note Deleted: 0002987 | |
2012-10-19 13:50 | fgaudreault | Note Added: 0003176 | |
2015-02-13 15:26 | lmunro | Note Added: 0003696 | |
2015-02-13 15:26 | lmunro | Status | assigned => closed |
Notes |
|
|
Introduced in 3.5.0 with the new portal profile feature. |
|
|
|
|
|
|
triaged: a blocker is something so important that none of our core or enabled by default functionality is working. Sponsorship is not as critical as assigning VLANs or deauthenticating a wireless user.
High priority, major severity. |
|
|
|
diff --git a/lib/pf/web.pm b/lib/pf/web.pm
index 2d55915..80df3b4 100644
--- a/lib/pf/web.pm
+++ b/lib/pf/web.pm
@@ -532,7 +532,13 @@ sub web_node_register {
# First blast at consuming portalSession object
my $cgi = $portalSession->getCgi();
my $session = $portalSession->getSession();
- my $mac = $portalSession->getClientMac() if (!defined($portalSession->getGuestNodeMac()));
+ my $mac;
+
+ if (defined($portalSession->getGuestNodeMac)) {
+ $mac = $portalSession->getGuestNodeMac;
+ } else {
+ $mac = $portalSession->getClientMac;
+ }
if ( is_max_reg_nodes_reached($mac, $pid, $info{'category'}) ) {
pf::web::generate_error_page( |
|
|
|
I'm not sure I like that fix. Can't we encapsulate that logic in getClientMac instead?
Are we doing the same test in the CGI's or other pf::web or pf::web::guest methods?
If so, isn't it a sign that this duplication is design that can be improved?
Just thinking out loud. I haven't made my mind yet. |
|
|
|
Good point.
I'll work something out to put that logic in getClientMac since this is the one we call to get the mac...
This is almost the only place where we use a "guestNodeMac" since it is a self-reg (the only other place I can see that use case is in pre self-reg). |
|
|
|
There's another problem with the sponsor workflow. This time it's if the user pre-registers (not on-site yet) and the sponsor clicks on the activation link.
A 0 value was injected in the database instead of the expected undef triggering the 'guest on site' behavior with MAC address 0 (failing miserably) instead of the 'guest is remote' behavior where credentials are created and sent by email. |
|
|
|
A quick fix was attached. However to avoid future regressions, this should be re-worked to make it harder to introduce these subtle mistakes. |
|
|
|
|
|
|
Reminder sent to: dwuelfrath Tomorrow I will design a solution in Portal::Session instead of each and every .cgi and pf::web call that needs to distinguish between pre-reg or not and I will propose the solution in a meeting with you guys. |
|
|
|
|
|
(0003176)
|
fgaudreault
|
2012-10-19 13:50
|
|
|
|
(0003696)
|
lmunro
|
2015-02-13 15:26
|
|
Old issues.
Most are not relevant to PF 4 and up.
Let's reopen the ones that matter when we move to github. |
|