Please login or register.

Login with username, password and session length
Advanced search  
Pages: [1] 2

Author Topic: Login/404 error bug  (Read 971 times)

Fred K

  • Still trying to learn stuff
  • ULTIMATE member
  • ******
  • Karma: 130
  • Posts: 2728
    • Personal
Login/404 error bug
« on: August 28, 2012, 05:46:30 PM »

I've noticed this bug before but never bothered to report it since it's not very big or aggressive, but now it's beinning to annoy me, so ... it should be fixed somehow, imho.

When we login the notification function actually sends us to Error 404 before refreshing to administration. This bug is normally not apparent since the refresh is set so low that you don't notice it, but I'm kind of weird in the way that I like for my users to know what's going on when a page refreshes, so I added a $note to the login process ("You are being logged in") and slowed the refresh down so I could see that the message is actually you know, readable. And that's when it happens.

The $title of the notification reads: Error
The $note of the notification reads: "You are being logged in".
See the discrepancy between the two messages there? And the page title says "404 Error". Which of course should not happen unless there's actually an error.

My guess is the root of the error is in the login code, which is this (mods marked blue):
Quote
if(isset($_POST['Loginform']) && !_ADMIN) {
   $user = checkUserPass($_POST['uname']);
   $pass = checkUserPass($_POST['pass']);
   unset($_POST['uname'],$_POST['pass']);
   if (md5($user) === s('username') && md5($pass) === s('password') && empty($_POST['******'])) {
      $_SESSION[_SITE.'Logged_In'] = token();
      notification(2,'You are being logged in','administration');
   } else { die( notification(2,l('err_Login'),'login')); }
}

I've run a test where I've changed the first var in notification() to 0 (according to my understanding of the notification function's code, that would indicate that the action is accepted and should not throw a warning or error message (?)), but then I get denied access with the message that I'm not allowed to do that until I'm logged in... So it seems that 2 is the only viable option for the first var but that ... throws the login process to the 404 page, which is just wrong.

I think it's a bug. Bugs should be squashed.
Logged

philmoz

  • High flyer
  • ULTIMATE member
  • ******
  • Karma: 161
  • Posts: 1988
    • fiddle 'n fly
Re: Login/404 error bug
« Reply #1 on: August 29, 2012, 01:49:47 PM »

first line of function notification doesn't make sense...
Code: [Select]
    $title = $error == 0 ? l('operation_completed') : ($error !== 0 ? l('admin_error') : l('warning'));
talking it through...
if $error == 0 -- true -->  $title = l('operation_completed') ;
if $error == 0 -- false -->
           if $error !== 0  -- true -->  $title =  l('admin_error');
           if $error !== 0 -- false -->  $title = l('warning');
This last line will never be activated... as it will only set $title if $error == 0, but only initially accessed if $error !== 0.

Back on topic,
 notification(2,'You are being logged in','administration');
 notification with the error var = 2 was only supposed to be for failed critical actions (which make part of that section slightly redundant with the refresh time set at '0')


So, Fred, the issue is in function notification, not login.
I reckon, since the $error is then !== 0, and from above,
 if $error !== 0  -- true -->  $title =  l('admin_error');
so you get an error displayed when all is ok....

Thoughts from others...?
« Last Edit: August 29, 2012, 01:54:33 PM by philmoz »
Logged
Of all the things I have lost, it is my mind that I miss the most.

Fred K

  • Still trying to learn stuff
  • ULTIMATE member
  • ******
  • Karma: 130
  • Posts: 2728
    • Personal
Re: Login/404 error bug
« Reply #2 on: August 29, 2012, 06:13:43 PM »

Good to hear from you Phil!

Quote
$title = $error == 0 ? l('operation_completed') : ($error !== 0 ? l('admin_error') : l('warning'));

As far as I can tell, this line only comes into play once you're logged in (hence the "not allowed to do that" message when I tried tapping into it for login). The login process itself doesn't seem to have any acceptance message, only a denial message for when things go wrong - which makes sense as long as the url refresh is at 0.

The weird part is that when login is ok the redirect goes to 404 before hiking off to administration. The fact that I see this at all comes from originally slowing down the refresh rate so that people would see the notification when commenting or using the contact form, and the side effect of that is this login error thingy.

Wouldn't it be simpler to solve this "bug" with some processing bit, similar to the logout thing (see center(), case 'logout')? I.e in login block of user-pass check, "if(login == ok) {say "something positive"; do URL refresh to admin;} else {say "bad robot";}"? Rather than using the notification() function which obviously isn't performing correctly in this instance.

Rewriting the notification() function seems something better left for a next version, no?
Logged

nukpana

  • Hero Member
  • *****
  • Karma: 71
  • Posts: 663
Re: Login/404 error bug
« Reply #3 on: August 30, 2012, 02:43:14 AM »

Well, notification is a culprit, but not the main one in this. 

There are a few reasons, one is a DUH moment....
1. DUH!! The login form action points to _SITE' . administration/', so if you error on the login you get a 404 since administration isn't a valid categorySEF yet.

Now, when you do get a valid login, it still points to a 404 because the token doesn't get set until after the refresh - I am clueless as to why, but it was just a VERY VERY quick check.

A quick way to solve this is have the login form point to login, then let the notification redirect.

2. I am unsure on the die part below, so I removed it in my test...
Code: [Select]
} else { die( notification(2,l('err_Login'),'login')); }
3. Notification is busted with the title... I don't get it so I add another parameter:
Code: [Select]
//NOTIFICATION
function notification($error = 0, $note = '', $link = '', array $on = null) {
// adds a "Warning" option
$title = $error == 0 ? l('operation_completed') : ($error !== 0? l('admin_error') : l('warning'));

// Reset Title...
if( isset($on['error']) ) {
$title = $on['error'];
}
if( isset($on['success']) ) {
$title = $on['success'];
}
.......

In use:
Code: [Select]
if (checkMathCaptcha() && md5($user) === s('username') && md5($pass) === s('password')) {
//if (md5($user) === s('username') && md5($pass) === s('password') && checkMathCaptcha()) {
$_SESSION[_SITE.'Logged_In'] = token();
notification(2,'You are being logged in','administration', array('success' => 'Yes!!!'));
} else {
notification(2, l('err_Login'), 'login');
}

Firebug shows a 200 OK status using the login


Firebug shows 404 using administration:


More testing is needed, but I am off to watch Walking Dead Season 2!!
Logged

sibas

  • Sr. Member
  • ****
  • Karma: 23
  • Posts: 451
    • www.simply4all.net
Re: Login/404 error bug
« Reply #4 on: August 30, 2012, 12:26:04 PM »

I have report this bug in topic
http://snewscms.com/forum/index.php/topic,10364.0.html
and still not have solution although Keyroks marked as SOLVED
Logged

nukpana

  • Hero Member
  • *****
  • Karma: 71
  • Posts: 663
Re: Login/404 error bug
« Reply #5 on: August 30, 2012, 01:02:49 PM »

OK, so I think I fixed it.

1. Startup area should look like the below code.  The else statement is gone...
Code: [Select]
// Patch #18 - 1.7.1 - revised string by KikkoMax
if (checkMathCaptcha() && md5($user) === s('username') && md5($pass) === s('password')) {
//if (md5($user) === s('username') && md5($pass) === s('password') && checkMathCaptcha()) {
$_SESSION[_SITE.'Logged_In'] = token();
notification(2,'','administration');
# } else {
# die( notification(2,l('err_Login'),'login'));
}

2. The first function login htm_input should read login instead of administration - it will be redirected to administration() in center anyway:
Code: [Select]
# echo html_input('form', '', 'post', '', '', '', '', '', '', '', '', '', 'post', _SITE.'administration/', '');
echo html_input('form', '', 'post', '', '', '', '', '', '', '', '', '', 'post', _SITE . 'login/', '');

3. Since center points to administration() on the submit of the Loginform, then a change must be done from 1.6 - may be a cleaner way... no time atm...
Code: [Select]
# In center...
case isset($_POST['Loginform']):
if( !_ADMIN ) {
echo notification(1, l('err_Login'),'login');
} else {
administration();
}
return;
break;

4. For Fred's modification, since by default, it doesn't show a message and if it does, it will show Error, then the message:

See below for updated code:

http://snewscms.com/forum/index.php/topic,10499.msg69461.html#msg69461
« Last Edit: September 02, 2012, 02:18:32 AM by nukpana »
Logged

Fred K

  • Still trying to learn stuff
  • ULTIMATE member
  • ******
  • Karma: 130
  • Posts: 2728
    • Personal
Re: Login/404 error bug
« Reply #6 on: August 30, 2012, 03:29:05 PM »

@Sibas: Sorry for double-posting. I couldn't find any answers when I searched for stuff like "login error" and "404 error" so I missed your post. One question is if the fixes offered in that topic have all been verified or not? I'm thinking of the e_notices in particular.

Another eye-opener was Rui's comment:
Quote from: Rui Mendes on March 5, 2012
Hello everybody.
I fix all problems/errors one week ago. Some dudes are are testing.
Very soon we publish as official (snews1.7.2).

(My emphasis, just for fun...)
What happened to 1.7.2?

------------* --------------
@nukpana
Tested the "ok, so I think I fixed it" fix ... here's what appears when going to domain/login/ to log in:

Quote
Error
Wrong username and/or password and/or sum entered.

Back


Note that I just went to login, didn't even attempt to actually login, which btw isn't possible because there, erm, is no login form...
« Last Edit: August 30, 2012, 03:56:56 PM by Fred K »
Logged

Keyrocks

  • Doug
  • ULTIMATE member
  • ******
  • Karma: 449
  • Posts: 6019
  • Semantically Challenged
    • snews.ca
Re: Login/404 error bug
« Reply #7 on: August 30, 2012, 04:31:24 PM »

OK, so I think I fixed it.

Steps 1, 2 and 3 definitely eliminated the error message visible immediately after logging in; at least it tests OK for me on a default 1.7.1 localhost install. (did not apply step 4).

I have report this bug in topic - http://snewscms.com/forum/index.php/topic,10364.0.html - and still not have solution although Keyroks marked as SOLVED

Thanks for the reminder Sibas. I'd forgotten about the work we were doing in that topic string. At the time (Feb. 16.12) it appeared that the approach I worked out made sense... it did get rid of the "Cannot modify header information" error message which is why I did mark it as [SOLVED].

@Sibas: Sorry for double-posting. I couldn't find any answers when I searched for stuff like "login error" and "404 error" so I missed your post. One question is if the fixes offered in that topic have all been verified or not? I'm thinking of the e_notices in particular.

Not that I am aware of. As far as all the E_Notices (Undefined variables and indexes) go, I eventually reached a road-block - detailed here when I started adding - isset() - to strings in function processing... that caused query values to be saved (processed) as "1" instead of the correct values. It was beyond me (and I had other projects to finish) so I stopped working on it. Sibas never mentioned if he had more success after that point. I just ran a quick visual test using error_reporting(-1); (fresh default install) and the E-notices are all still there.

Another eye-opener was Rui's comment:
Quote from: Rui Mendes on March 5, 2012
Hello everybody.
I fix all problems/errors one week ago. Some dudes are are testing.
Very soon we publish as official (snews1.7.2).
(My emphasis, just for fun...)
What happened to 1.7.2?

I can't remember ever receiving or testing a copy of Rui's noted fixes and I don't know if any other "Dudes" did so. I'm not sure what his reference to snews 1.7.2 was about either.
« Last Edit: August 30, 2012, 05:09:59 PM by Keyrocks »
Logged
Do it now... later may not come.
-------------------------------------------------------------------------------------------------
sNews 1.6 MESU | sNews 1.6 MEMU

Fred K

  • Still trying to learn stuff
  • ULTIMATE member
  • ******
  • Karma: 130
  • Posts: 2728
    • Personal
Re: Login/404 error bug
« Reply #8 on: August 30, 2012, 04:45:36 PM »

Ok, I'll revisit nuk's fix without step 4 and see what's going on. I did manage to get rid of the 404 appearance on login in a really simple way. But that doesn't mean it's a good way... ;)

Fix? Add 'administration' to the public section of $l['cat_listSEF'], like, after 'login'...
No more 404 or Error message. Of course no more notification of any kind, but that should be possible to fix.
Logged

nukpana

  • Hero Member
  • *****
  • Karma: 71
  • Posts: 663
Re: Login/404 error bug
« Reply #9 on: August 30, 2012, 05:34:11 PM »

@fred, having the administration slug public isn't correct. It should be invalid until you log in, so having the 404 on administration is correct. The change should happen in the form itself as its a simpler solution - have the form process to a public url aka login.

Now for step #4.  If you choose to have a message like so:
Code: [Select]
notification(2,'You are now being redirected','administration');
Without the modification it will show like so:


Even the back button shouldn't be there either... leading back to notifications needing a rewrite...
« Last Edit: August 30, 2012, 06:43:23 PM by nukpana »
Logged

Fred K

  • Still trying to learn stuff
  • ULTIMATE member
  • ******
  • Karma: 130
  • Posts: 2728
    • Personal
Re: Login/404 error bug
« Reply #10 on: August 30, 2012, 06:43:10 PM »

Quote
having the admin sef public isnt correct. It s/b invalid until you log in, so the 404 on admin is correct.

Yes, having administration in public sefCat list is bad, I know that
Quote
doesn't mean it's a good way
but 404 on admin when login is successful isn't correct either. Well, it's correct from a process point of view (admin isn't an existing SEF until login process is complete)m but not from a user point of view. The obvious solution is to set the URL refresh rate on error to 0, that way no-one sees the error message, but that isn't right either. And it screws up notifications on comments and contact forms.

So I guess we're (I'm?) back to square one.

I redid the test with your fix from earlier and, yes, step 1-3 works but if I do the thing I want to (slow down refresh and add a notification message on login), with your suggested addition (resetting $title), the Error! title still appears. I thought adding a thing to this line:
Code: [Select]
notification('2','You are being signed in','administration'); would help so I tested with adding ,'' after the $goto link, but that of course doesn't work (page blanks and no refresh happens), so - square 1. On successful login we get 404 and any message gets headline Error even though it's not an error.

I guess to really fix this a number of functions would need fundamental rethinking. Oh, well.
« Last Edit: August 30, 2012, 07:50:58 PM by Fred K »
Logged

sibas

  • Sr. Member
  • ****
  • Karma: 23
  • Posts: 451
    • www.simply4all.net
Re: Login/404 error bug
« Reply #11 on: August 30, 2012, 06:56:06 PM »

I think I found the problem, occur in function login() when we post in  _SITE.'administration/
but we post it before we have login..

So find in function login() the first echo and change it to
Code: [Select]
//echo html_input('form', '', 'post', '', '', '', '', '', '', '', '', '', 'post', _SITE.'administration/', '');
echo html_input('form', '', 'post', '', '', '', '', '', '', '', '', '', 'post', _SITE.'', '');

seems like this is the problem but still have problem with notification!
Logged

nukpana

  • Hero Member
  • *****
  • Karma: 71
  • Posts: 663
Re: Login/404 error bug
« Reply #12 on: August 30, 2012, 07:35:41 PM »

@fred, call the function like so: (not exact, the phone is bad for code)

Notification(2, 'your in', direct, array('success' => 'new title'));


I am rethinking this func..... I am adding an array to the end of the func params for options... Ala js plugins..... But!!!! I think not addding the link param should take away the link (need to test....)

see below for an update:
http://snewscms.com/forum/index.php/topic,10499.msg69461.html#msg69461
« Last Edit: August 31, 2012, 01:06:01 AM by nukpana »
Logged

Fred K

  • Still trying to learn stuff
  • ULTIMATE member
  • ******
  • Karma: 130
  • Posts: 2728
    • Personal
Re: Login/404 error bug
« Reply #13 on: August 30, 2012, 07:45:01 PM »

I made a temporary fix that works for me for now (imagine the word temporary being big, bold and underlined if you will), which is simply to remove $title from the error == 2 {} statement in notification and adding a "wait" case in center() which just says "case 'wait': notification(); break;". "wait" is added to cat_listSEF and as the target url for login form and contact form. Presto, no 404 and no weird headline, and we can have a slightly longer refresh rate than 0 for the url redirect if we like.

Of course, there's that back link... :P

A more solid fix would be welcome I think.

/* Edit
While I remember it, although on a different topic, there are at least three separate forms which uses id="post" - off the top of my head it's comments, contact and login. Two of those should get their own unique id, in a possible future revision. (For the record, iirc I think we already did that change initially in 1.7... but I could be wrong. Goodness knows I am, a lot.)
jm2cfwiw
« Last Edit: August 31, 2012, 02:25:48 AM by Fred K »
Logged

nukpana

  • Hero Member
  • *****
  • Karma: 71
  • Posts: 663
Re: Login/404 error bug
« Reply #14 on: August 31, 2012, 01:05:25 AM »

OK, for notifications, I think I have a solution that can work with the current function:

UPDATED
Code: [Select]
<?php

// Eq added new parameter - $option array
function notification($error 0$note ''$link '', array $options null) {
// adds a "Warning" option
$title $error == l('operation_completed') : ($error !== l('admin_error') : l('warning') );
$note = (!$note || empty($note)) ? ''  '<p>'.$note.'</p>';
switch(true){
case (!$link):
$goto '';
break;
case ($link == 'home'):
$goto '<p><a href="'._SITE.'">'.l('backhome').'</a></p>';
break;
case ($link != 'home'):
$goto '<p><a href="'._SITE.$link.'/" title="'.$link.'">'.l('back').'</a></p>';
break;
}
/* EQ 
$options array
title  - Override the title variable
goto  - Set to false to remove the goto link
time  - Set the refresh time
noNote - Set to true to only show the title
*/
if( isset($options['title']) ) $title  $options['title'];
if( isset($options['time']) ) $time  $options['time'];
else $time 0;
if( isset($options['noNote']) && $options['noNote'] === true )
$note ' ';
if( isset($options['goto']) && $options['goto'] === false )
$goto  '';

# Updated!!! If a note is not set in param 2, but a title is set, then show the title
if( isset($options['title']) && $note == '' )
$note  ' ';


if ($error == 2) {
$_SESSION[_SITE.'fatal'] = $note == '' '' '<h3>'.$title.'</h3>'.$note.$goto;
// EQ - set the content time
echo '<meta http-equiv="refresh" content="' .$time .'; url='._SITE.$link.'/">';
return;
} else {
$output '<h3>'.$title.'</h3>'.$note.$goto;
return $output;
}
}

?>

Example usage - Set the refresh timer, no back link, & custom title:
Code: [Select]
notification(
2,
'',
'administration',
array(
'title' => 'Welcome!',
'time' => 2,
'goto' => false
)
);

Note - this has not been tested with other functions, but if none of the options are set, then it should be normal. Testing is needed.
« Last Edit: September 02, 2012, 02:19:43 AM by nukpana »
Logged
Pages: [1] 2