PacketFence - BTS - PacketFence
View Issue Details
0001505PacketFenceguestspublic2012-08-07 16:122015-02-13 15:26
fdurand 
obilodeau 
highmajoralways
closedopen 
LinuxDebian6
3.5.0 
 
0001505: Mac address is null when we try to register a guest with the sponsor method
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 );)
No tags attached.
patch 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
2012-08-07 16:12fdurandNew Issue
2012-08-07 16:13dwuelfrathStatusnew => assigned
2012-08-07 16:13dwuelfrathAssigned To => dwuelfrath
2012-08-07 16:13dwuelfrathNote Added: 0002910
2012-08-07 16:15dwuelfrathNote Added: 0002911
2012-08-07 16:15obilodeauNote Added: 0002912
2012-08-07 16:15obilodeauPrioritynormal => high
2012-08-07 16:15obilodeauSeverityblock => major
2012-08-07 16:15obilodeauProduct Version => 3.5.0
2012-08-07 16:18dwuelfrathNote Added: 0002913
2012-08-08 08:57obilodeauNote Added: 0002918
2012-08-08 09:12dwuelfrathNote Added: 0002919
2012-08-14 10:20obilodeauNote Added: 0002926
2012-08-14 10:20obilodeauFile Added: captive-portal-sponsor-preregistration-preliminary-fix.patch
2012-08-14 10:35obilodeauFile Added: captive-portal-sponsor-preregistration-preliminary-fix-v2.patch
2012-08-14 10:35obilodeauFile Deleted: captive-portal-sponsor-preregistration-preliminary-fix.patch
2012-08-14 10:36obilodeauNote Added: 0002927
2012-08-15 16:57obilodeauNote Added: 0002936
2012-08-20 16:47obilodeauAssigned Todwuelfrath => obilodeau
2012-08-20 16:48obilodeauNote Added: 0002951
2012-08-27 13:12obilodeauFile Added: captive-portal-sponsor-preregistration-preliminary-fix-v3.patch
2012-08-27 13:12obilodeauFile Deleted: captive-portal-sponsor-preregistration-preliminary-fix-v2.patch
2012-08-27 13:12obilodeauNote Added: 0002970
2012-08-29 14:15obilodeauNote Added: 0002987
2012-08-30 11:10obilodeauNote Deleted: 0002987
2012-10-19 13:50fgaudreaultNote Added: 0003176
2015-02-13 15:26lmunroNote Added: 0003696
2015-02-13 15:26lmunroStatusassigned => closed

Notes
(0002910)
dwuelfrath   
2012-08-07 16:13   
Introduced in 3.5.0 with the new portal profile feature.
(0002911)
dwuelfrath   
2012-08-07 16:15   
Check: https://github.com/inverse-inc/packetfence/commit/8d0b401b51bf91caba75fef82640d44d2af7c627#L5R45 [^]

and: https://github.com/inverse-inc/packetfence/commit/8d0b401b51bf91caba75fef82640d44d2af7c627#L10R684 [^]
(0002912)
obilodeau   
2012-08-07 16:15   
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.
(0002913)
dwuelfrath   
2012-08-07 16:18   
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(
(0002918)
obilodeau   
2012-08-08 08:57   
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.
(0002919)
dwuelfrath   
2012-08-08 09:12   
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).
(0002926)
obilodeau   
2012-08-14 10:20   
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.
(0002927)
obilodeau   
2012-08-14 10:36   
A quick fix was attached. However to avoid future regressions, this should be re-worked to make it harder to introduce these subtle mistakes.
(0002936)
obilodeau   
2012-08-15 16:57   
Quick fix applied to stable at ff5ee42. https://github.com/inverse-inc/packetfence/commit/ff5ee429de126560b610449168da3a28a03009e2 [^]

A better fix is coming for the devel branch.
(0002951)
obilodeau   
2012-08-20 16:48   
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.
(0002970)
obilodeau   
2012-08-27 13:12   
updated patch
(0003176)
fgaudreault   
2012-10-19 13:50   
Derek, is this fixed?
(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.