Solved

opinion on how my script is programmed (codeigniter)

Posted on 2010-11-08
4
306 Views
Last Modified: 2012-06-27
Hi,

I need your opinion on:
if my script is well programmed. It is made in codeigniter.

Before I continue, sorry for this long post.

As I previously stated in another post I'm just a rookie.
So I wanted to ask you guys what you think of my application so far.

Is it well written? How about the placement of the files/code?
What would you do?

The following code has the basics ofhttp://www.phpandstuff.com/articles/codeigniter-doctrine-from-scratch-day-1-install-and-setup

Oke so lets get started:
My application is a start of a cms system: Simple admin login and forgot password. When you forgot the password, a random new password will be emailed.

Login screen:
I created a table user for both the admins and users... I make the difference using a "user level" - 1 being the admin, 2 or above the normal users.
- When the form is being submit, the "submit" function is being called.
- In the submit function we have a "_submit_validate" function with form validations
- In the "_submit_validate" function we have a callback to authenticate

authenticate function in the login controller:
    public function authenticate() {

        $isAuthenticated = Current_User::login($this->input->post('username'),
                                    $this->input->post('password'));
        $isAdmin = Current_User::user()->ulevel;
        if ($isAuthenticated == TRUE && $isAdmin == 1) {
            return TRUE;
            }
        return FALSE;
    }

Open in new window


Instead of duplicating the Current_User::login() model function, I kept the same code and added a user level check.

fyi in the user model I added
$this->hasColumn('ulevel', 'integer', 5);

Open in new window

in the setTableDefinition function.

And now for the diffcult part...

Forgot Password:
I have a simple form with one email imputbox and a submit button.
- When the form is being submit, the "submit" function is being called.
- In the submit function we have a "_submit_validate" function with form validations
- In the "_submit_validate" function we have a callback to forgot_password

forgot_password and _send_password from the forgot_password controller:
public function forgot_password() {

        $pass = Current_User::forgot_password($this->input->post('email'));
        $this->_send_password($pass, $this->input->post('email'));
    }
    private function _send_password($pass, $email) {

        $this->load->library('email');
        $this->email->from('admin@aptest.com', 'apTest');
        $this->email->to($email); 
        
        $this->email->subject('Reset Password - apTest');
        $this->email->message('Here is your new password: ' & $pass);    
        
        $this->email->send();        
    }

Open in new window


current_user model (partial):
public static function forgot_password($email) {
        if ($u = Doctrine::getTable('User')->findOneByEmail($email)) {
            $newPassword = Current_User::user()->reset_password();            
            $u->password = $newPassword;
            $u->save();
            return $newPassword;
        }
        return FALSE;
    }

Open in new window


user model (partial):
    public function Reset_Password() {
        return $newPassword = random_string('numeric', 16);
    }

Open in new window


Now I'm kind of lost, if this is well programmed or not?. What do you think? Any opinion is appreciated :)

Kind regards
0
Comment
Question by:Mutsop
  • 2
4 Comments
 
LVL 17

Accepted Solution

by:
sweetfa2 earned 250 total points
ID: 34091553
Ok, lets go:

Authenticate function - you are not actually authenticating (ie. you are really authenticating admin - so call it adminAuthentication if that is what you want it to do).  Good programming practice is to create a method to authenticate.  A separate function is to check if in admin role.  So authentication v. authorisation are really two different functions.

Apart from that - it doesn't seem to bad.
0
 
LVL 13

Assisted Solution

by:darren-w-
darren-w- earned 250 total points
ID: 34092392
Just a little code refactoring:

with:
 public function forgot_password() {

        $pass = Current_User::forgot_password($this->input->post('email'));
        $this->_send_password($pass, $this->input->post('email'));
    }

break it out into two methods

public function forgot_password() {
        $this->_send_password($this->getPassword(), $this->input->post('email'));
    }

private function getPassword(){
      return Current_User::forgot_password($this->input->post('email'));
}

Also:

public static function forgot_password($email) {
        if ($u = Doctrine::getTable('User')->findOneByEmail($email)) {
            $newPassword = Current_User::user()->reset_password();            
            $u->password = $newPassword;
            $u->save();
            return $newPassword;
        }
        return FALSE;
    }

to

public static function forgot_password($email) {
        if ($u = Doctrine::getTable('User')->findOneByEmail($email)) {          
            $u->password =$this->getNewPassword();
            $u->save();
            return $u->password;
        }
        return FALSE;
    }


private function getNewPassword() {
return Current_User::user()->reset_password();
}


Sure more can be done...


0
 
LVL 3

Author Comment

by:Mutsop
ID: 34092770
@sweetfa2:
So you mean splitting the authentication part and checking for if that authenticated member has the rights to view a certain page (like an admin level)?

@darren-w-:
I do indeed need to refactor most of the script, problem is I don't quite know how and what should be script apart.
0
 
LVL 17

Expert Comment

by:sweetfa2
ID: 34095208
Yes that is what I mean.
0

Featured Post

New My Cloud Pro Series - organize everything!

With space to keep virtually everything, the My Cloud Pro Series offers your team the network storage to edit, save and share production files from anywhere with an internet connection. Compatible with both Mac and PC, you're able to protect your content regardless of OS.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Suggested Solutions

Title # Comments Views Activity
Echo vs ?><?php  html code 4 47
Php pie charts 3 26
How to convert my query to the proper format? 5 19
Checking if varaible is empty 6 33
I imagine that there are some, like me, who require a way of getting currency exchange rates for implementation in web project from time to time, so I thought I would share a solution that I have developed for this purpose. It turns out that Yaho…
Password hashing is better than message digests or encryption, and you should be using it instead of message digests or encryption.  Find out why and how in this article, which supplements the original article on PHP Client Registration, Login, Logo…
Explain concepts important to validation of email addresses with regular expressions. Applies to most languages/tools that uses regular expressions. Consider email address RFCs: Look at HTML5 form input element (with type=email) regex pattern: T…
The viewer will learn how to create a basic form using some HTML5 and PHP for later processing. Set up your basic HTML file. Open your form tag and set the method and action attributes.: (CODE) Set up your first few inputs one for the name and …

863 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

21 Experts available now in Live!

Get 1:1 Help Now