Phishing Project Refactoring to Use a User Object

/*
I know you mentioned that you need to add validation here,
which I agree with.

I don't really like the approach of having a DB query outside of this class
to read this information and then sending to this class to instantiate.
Why not have that query here in this class so that, given DB connection
as dependency, this class has full ability to set up the user, rather
than having some other logic in the application to do this? Why should
some other part of the code need to know things like what table the user
data is stored in, what the schema for that table looks like, etc.

Should DBManager be passed to constructor as dependency since it is used
in other methods?

Just try to be consistent as to whether this class is supposed to interact
with database or not. It seems odd to mix approaches here. If the intent
is to handle ALL database access for users through this class (i.e. this
class IS the model), then it should truly handle ALL access including
getting the user info which is currently being passed to constructor.

This model seems really unclear in terms of how projects are handled.
Should projects (and/or a collection of projects) have their own object
definitions?  At a minimum, should this be an array of projects rather
than previous, most recent, last (which seems unclear as to what the
difference between these is anyway). An ordered array would let you 
get to whatever project you were interested in. Also, should there not
be 1 to 1 relationship between projects and unique URL's since they
are based on project ID's?  Here it seems like the user object can
have multiple projects, but only one unique URL, which doesn't make
sense to me.
*/
public function __construct($user)
{
    $this->id = $user['USR_UserId']; //required
    $this->username = $user['USR_Username']; //required
    $this->email = $user['USR_Email']; //required
    $this->firstName = $user['USR_FirstName']; //required
    $this->lastName = $user['USR_LastName']; //required
    $this->uniqueURLId = $user['USR_UniqueURLId'];
    $this->password = $user['USR_Password']; //required
    $this->mostRecentProject = $user['USR_ProjectMostRecent'];
    $this->previousProject = $user['USR_ProjectPrevious'];
    $this->lastProject = $user['USR_ProjectLast'];
}


/*
Is this method named properly?  It does more than check, so perhaps it should
be called setURLIdForProjectId().
It seems unclear to me in this model if a user could have multiple URL's in
case where they have multiple project ID's. If this is not the case,
what value does projectID even have with regards to this URL?
This method only considers happy path.  What if query fails (for example if
all URL's in database must be unique and generated string matches existing
URL in database)?
*/
private function checkURLId($projectId) {
    if(is_null($this->uniqueURLId)) {
        $db = new DBManager();
        $this->uniqueURLId = RandomObjectGeneration::random_str(15) . $projectId;
        $sql = "UPDATE gaig_users.users SET 

/*
What class is this a method on? TemplateConfiguration class?
Why would this functionality exist in that class?
You might consider a class whose specific purpose is to build the list
of recipients given all necessary dependencies.

Pass DBManager dependency to class or method.
*/
public function getValidUsers($returnUsers, $periodInWeeks) {
    $db = new DBManager();
/*
Don't use SELECT *. It is a terrible habit.
*/
    $sql = "SELECT * FROM gaig_users.users;";
/*
Don't leak PDO implementation outside of DBManager.  Why does template
class need to know about how to instantiate user objects?
*/
    $users = $db->query($sql,array(),array('\PDO::ATTR_CURSOR'),array('\PDO::CURSOR_SCROLL'));
    $usersIterator = new PDOIterator($users);
    foreach($usersIterator as $user) {
        $tempUser = new User_Test($user);
/*
As noted in other code comments, this pushUser method is not working as you expect
*/
        $tempUser->pushUser($returnUsers,$periodInWeeks,$this);
    }
    return $returnUsers;
}

/*
It seems like PhishingContrller is most logical place to do dependency
set up, as here you are reading request, instantiating templates, etc.
Perhaps this is place where you need to instantiate your additional
dependencies such as user collection.

I would suggest that perhaps you break this sendEmail method up. I don't
see a constructor for this class, but you might want to think about a
structure such as
public function __construct(Request $request) {
    // Handle the request.
    // This method could set dependencies on object such as
    // template and email config
    this.handleRequest($request);

    // instantiate DB connection
    // this controller can then pass this dependency to classes
    // functions it calls that need it
    this.db = new DBManager();

    // set time period. Right now you hard code this but perhaps it
    // should be passed to class or derived fromn request
     this.periodInWeeks = 4;

    // get user list
    this.users = ...; // some class/method that builds your user list
}

public function sendEmail() {
    // pass all dependencies here to static executeEmail method
    // of email class
    Email::executeEmail(
        this.emailConfig,
        this.templateConfig,
        this.users
    );
}
*/
public function sendEmail(Request $request) {
    try {
        $templateConfig = new TemplateConfiguration(
            array(
                'templateName'=>$request->input('emailTemplate'),
                'companyName'=>$request->input('companyName'),
                'projectName'=>$request->input('projectData')['projectName'],
                'projectId'=>intval($request->input('projectData')['projectId'])
            )
        );

        $periodInWeeks = 4;
        $users = array();
        $emailConfig = new EmailConfiguration(
            array(
                'host'=>$request->input('hostName'),
                'port'=>$request->input('port'),
                'authUsername'=>$request->input('username'),
                'authPassword'=>$request->input('password'),
                'fromEmail'=>$request->input('fromEmail'),
                'subject'=>$request->input('subject'),
                'users'=>$templateConfig->getValidUsers($users,$periodInWeeks)
            )
        );

        Email::executeEmail($emailConfig,$templateConfig);
    } catch(ConfigurationException $ce) {
        //will be doing something here - what still has yet to be defined (likely just log the exception)
    } catch(EmailException $ee) {
        //will be doing something here - what still has yet to be defined (likely just log the exception)
    }
}

/*
I would try to pass the user list as argument to this method as noted above.

Maybe name should be executeEmailToUserList() or something more specific
*/
public static function executeEmail(
    EmailConfiguration $emailConfig,
    TemplateConfiguration $templateConfig)
{
    self::setTemplateConfig($templateConfig);
    self::setEmailConfig($emailConfig);


    try {
        foreach($emailConfig->getUsers() as $user) {
            self::sendEmail($user);
/*
I would suggest that updateUserProjects should be method on user class,
such that you would change this to:
$user->updateUserProjects()
*/
            self::updateUserProjects($user);
        }
    } catch(Exception $e) {
        throw new EmailException(__CLASS__ . ' Exception',0,$e);
    }
}

/*
Move this method to user class, as it has nothing to do with email sending.
Parameter passed to this method probably need to change so as to 
provide to this method the information in needs about the project. 
Perhaps pass Project object?
*/
private function updateUserProjects($user) {
    $db = new DBManager();
/*
Line is too long.  A suggestion on writing readable SQL might be to do:
$sql = "
    UPDATE gaig_users.users
    SET USR_ProjectMostRecent=?,
        USR_ProjectPrevious=?, 
        USR_ProjectLast=?
    WHERE USR_Username=?
";
I also find heredoc/nowdoc syntax helpful for more complex queries.
*/
    $sql = "UPDATE gaig_users.users SET USR_ProjectMostRecent=?, USR_ProjectPrevious=?, 
            USR_ProjectLast=? WHERE USR_Username=?;";
    $bindings = array(self::$templateConfig->getProjectName(),
        $user['USR_ProjectMostRecent'],
        $user['USR_ProjectPrevious'],
        $user['USR_Username']
    );
    $db->query($sql,$bindings);
}


private static function sendEmail($user) {    
    $templateData = array(
        'companyName'=>self::$templateConfig->getCompanyName(),
        'projectName'=>self::$templateConfig->getProjectName(),
        'projectId'=>self::$templateConfig->getProjectId(),
        'lastName'=>$user->getLastName(),
        'username'=>$user->getUsername(),
        'urlId'=>$user->getUniqueURLId()
    );
    $subject = self::$emailConfig->getSubject();
    $from = self::$emailConfig->getFromEmail();
/*
You getter name on user class might lead to some confusion here.
getEmailAddress?
*/
    $to = $user->getEmail();
    $mailResult = Mail::send(
        ['html' => self::$templateConfig->getTemplate()],
        $templateData,
        function($m) use ($from, $to, $subject) {
/*
Don't mix method chaining here. I would either chain them all (if possible)
or chain none of them.
*/
            $m->from($from);
            $m->to($to)
                ->subject($subject);
        }
    );
    if(!$mailResult) {
        throw new FailureException('Email failed to send to ' . $to . ', from ' . $from);
    }
}

/*
Probably need similar function here for setting user collection
*/
private static function setTemplateConfig(TemplateConfiguration $templateConfig) {
    se

class RandomObjectGeneration
{

/*
Consider placing default "keyspace" value as constant on class.
Method signature might then look like:
public static function random_str($length,
    $keyspace = RandomObjectGeneration::KEYSPACE)
*/
public static function random_str($length, $keyspace = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ')
{
/*
You should validate that $length is positive integer, not just numeric.
You don't want a values like 3.1415926, 0, -50, etc. passed in here.
These would all pass your current validation.
*/
    if(is_null($length) || !is_numeric($length)) {
/*
Give meaningful exception message.
*/
        throw new Exception();
    }
/*
Validate $keyspace as well. Non-zero length string as minimum validation.
*/
    $str = '';
    $max = mb_strlen($keyspace) - 1;
    for ($i = 0; $i < $length; ++$i) {
        $str .= $keyspace[random_int(0, $max)];
    }
    return $str;
}
}