Security suggestion

[mjpilot]

Very Nice software; Powerful and yet simple to use! Great job!

I do have a request/suggestion: Security for us is a big issue! I would recommend 2 things:

1. Removing the dropdown menu from admin login and replace with text field.

2. Moving all the admin files to an admin folder! This way we can .htaccess the folder and increase security.

[Klemen]

1. You can remove this by changing lines 123-133 in admin.php (open in Notepad):

Code: Select all

<td><select name="user">
<?php
$sql = "SELECT * FROM `hesk_users`";
$result = hesk_dbQuery($sql) or hesk_error("$hesklang[cant_sql]: $sql</p><p>$hesklang[mysql_said]:<br>".mysql_error()."</p><p>$hesklang[contact_webmsater] $hesk_settings[webmaster_mail]");
while ($row=hesk_dbFetchAssoc($result))
{
    echo '<option value="'.$row['id'].'">'.$row['user'].'</option>';
}

?>
</select></td>

Chage to:

Code: Select all

<td><input type="text" name="user"></td>

2. This is much more work, you can try yourself if you want. But rest assured no one can access any admin pages without providing a valid username and password. You could also additionally password protect individual files for example with htaccess:
http://www.google.com/search?hl=en&q=ht ... dual+files

[Poooh]

I think the first request is a must in the script and I would like the second one to happen also

[mjpilot]

Thank Klemen,
Both suggestions I can implant myself but if this is not implemented in the next version and we decide to upgrade, we lose all the work!

But... #1 is very easy and I will do this and #2... I did not know I can protect one file. Thanks!

[Klemen]

No problem. Next version will definitely have an option added to remove or show the drop-down list of usernames. Not sure about moving files to "admin" folder, but it might be added as well.

[3cwired_com]

Ok, I tried this "fix" you posted, and noticied that this doesnt work atleast for version .94, as the values sql is looking for from the form is the actual id number, and the actual username of the user.

i had tested this by entering the corresponding user id, and password, and i was able to log in that way, but again not with the username.

Do you know of a quick and reliable fix for this?

Thanks.

Great script, I plan making a decent donation within a month or two when I finish up development on some other things.

[tsjaar]

You should change the function do_login in admin.php

change this:

Code: Select all

function do_login() {
global $hesklang;
$user=hesk_isNumber($_POST['user'],$hesklang['select_username']);
$pass=hesk_input($_POST['pass'],$hesklang['enter_pass']);

$sql = "SELECT * FROM `hesk_users` WHERE `id`=$user LIMIT 1";

into something like this (not tested):

Code: Select all

function do_login() {
global $hesklang;
$user=hesk_input($_POST['user'],$hesklang['select_username']);
$pass=hesk_input($_POST['pass'],$hesklang['enter_pass']);

$sql = "SELECT * FROM `hesk_users` WHERE `user`=$user LIMIT 1";

I didn't test this, but I think it should work. (might need to adjust the error message)

[3cwired_com]

that looks like it should work too, I am not even testing but agreeing, I just havent had time to play with it myself, i will do it later on this evening and report back.

Thanks.

[bzbatl]

Actually, this fix didn't work for me. It gives me an error that "Administrator column does not exist".

Any other ideas on how to successfully change the drop down to a standard login box?

[3cwired_com]

Ok you are very right, thanks for pointing that out, its the untrained eye, as I have why I didn't realize the very minor mistake.

Here is the code you should copy to make it work:

Code: Select all

function do_login() {
global $hesklang;
$user=hesk_input($_POST['user'],$hesklang['select_username']);
$pass=hesk_input($_POST['pass'],$hesklang['enter_pass']);

$sql = "SELECT * FROM `hesk_users` WHERE `user`='$user' LIMIT 1"; 

I guess as you can see, I never applied it to my usage, I was quite busy myself. But a quick run down as to why it didnt work, was because since the username is now a string and not a numeric variable it needs to be enclosed in 'single quotes'

Cheers!

[bzbatl]

You are the BEES KNEES!!!

Thanks!

[tsjaar]

... since the username is now a string and not a numeric variable it needs to be enclosed in 'single quotes' ...

You're right.

[3cwired_com]

No problem it happens to the best of us! Look at all of these MS programs!

[Klemen]

Who said MS programs are written by the best

[3cwired_com]

yea that was the joke... they are not the best indeed....