質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう!

ただいまの
回答率

88.93%

ログイン日時をセッションに格納したい

解決済

回答 2

投稿 編集

  • 評価
  • クリップ 1
  • VIEW 602

Roo

score 55

前提・実現したいこと

最終ログイン日時を取得したい。

発生している問題・エラーメッセージ

私の現在のコードだと日時アップデートqueryを実行する前に
if ($row = $stmt->fetch(PDO::FETCH_ASSOC)) {
ここのfetchの日時を取得しています。

query実行後に、
ログイン日時をセッションに格納したいのですが、どのように修正すれば宜しいでしょうか?

該当のソースコード

<?php
session_start();  

if (isset($_SESSION["NAME"])) {
header("Location: my.php");
exit;
}

$db['host'] = "hoge";  
$db['user'] = "hoge";  
$db['pass'] = "hoge";
$db['dbname'] = "hoge"; 
$date = date('Y-m-d H:i:s');

$errorMessage = "";
$errorMessage_2 = "";

require_once 'hasu_test.php';

if (isset($_POST["login"])) {
if (empty($_POST["mail"])) { 
$errorMessage = '※空やでやで';
} else if (empty($_POST["password"])) {
$errorMessage_2 = '※空やで。';
} else if (!preg_match('/^[!-~]+@[!-~]+$/',  $_POST["mail"])){   
$errorMessage = '※正規表現外mail';
} else if (!preg_match("/\A(?=.*?[a-z])(?=.*?\d)[a-z\d]{8,100}+\z/i", $_POST["password"])){ 
$errorMessage_2 = '※正規表現外pass';
} else if (!preg_match("/^.{4,32}$/",$_POST["mail"])){    
$errorMessage = '※桁数上mail';
} else if (!preg_match("/^.{4,32}$/",$_POST["password"])){
$errorMessage_2 = '※桁数上pass';

}else{

if (!empty($_POST["mail"]) && !empty($_POST["password"])) {
$mail = $_POST["mail"];

$dsn = sprintf('mysql:host=%s; dbname=%s; charset=utf8;unix_socket=/tmp/mysql.sock', $db['host'], $db['dbname']);

try {
$pdo = new PDO($dsn, $db['user'], $db['pass'], array(PDO::ATTR_ERRMODE=>PDO::ERRMODE_EXCEPTION));
$stmt = $pdo->prepare('SELECT * FROM member WHERE mail = ?');
$stmt->execute(array($mail));
$password = $_POST["password"];

if ($row = $stmt->fetch(PDO::FETCH_ASSOC)) {
if (password_verify($password, $row['password'])) {
session_regenerate_id(true);

$id = $row['id'];
$sql = "SELECT * FROM member WHERE id = $id";  //入力したIDからユーザー名を取得
$stmt = $pdo->query($sql);
foreach ($stmt as $row) {
$row['mail'];  
}

$stmt = $pdo->prepare('SELECT * FROM member WHERE sdate = ?');
$stmt->execute(array($date));
$sql = "UPDATE member SET sdate='$date' WHERE id='$id'";
$result = $pdo->query($sql);



$_SESSION["sdate"] = $row['sdate'];
$_SESSION["mail"] = $row['mail'];
header("location: my.php");
exit();  


} else {
$errorMessage = '※mailまたはPasswordが間違っています。';
}
} else {
$errorMessage = '※該当データなしmailまたはPasswordが間違っています。';
}
} catch (PDOException $e) {
$errorMessage =   header("Location: 500.php");         
}
}
}
}

?>
  • 気になる質問をクリップする

    クリップした質問は、後からいつでもマイページで確認できます。

    またクリップした質問に回答があった際、通知やメールを受け取ることができます。

    クリップを取り消します

  • 良い質問の評価を上げる

    以下のような質問は評価を上げましょう

    • 質問内容が明確
    • 自分も答えを知りたい
    • 質問者以外のユーザにも役立つ

    評価が高い質問は、TOPページの「注目」タブのフィードに表示されやすくなります。

    質問の評価を上げたことを取り消します

  • 評価を下げられる数の上限に達しました

    評価を下げることができません

    • 1日5回まで評価を下げられます
    • 1日に1ユーザに対して2回まで評価を下げられます

    質問の評価を下げる

    teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。

    • プログラミングに関係のない質問
    • やってほしいことだけを記載した丸投げの質問
    • 問題・課題が含まれていない質問
    • 意図的に内容が抹消された質問
    • 過去に投稿した質問と同じ内容の質問
    • 広告と受け取られるような投稿

    評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。

    質問の評価を下げたことを取り消します

    この機能は開放されていません

    評価を下げる条件を満たしてません

    評価を下げる理由を選択してください

    詳細な説明はこちら

    上記に当てはまらず、質問内容が明確になっていない質問には「情報の追加・修正依頼」機能からコメントをしてください。

    質問の評価を下げる機能の利用条件

    この機能を利用するためには、以下の事項を行う必要があります。

質問への追記・修正、ベストアンサー選択の依頼

  • m.ts10806

    2019/03/28 15:45

    どんなに簡単な問題でもコードが滅茶苦茶だと回答者の読む気力を奪ってしまい、回答が得られなくなります。(要件以外のことに気が散ります)

    キャンセル

  • Roo

    2019/03/28 15:50

    失礼いたしました。
    修正いたします

    キャンセル

  • m.ts10806

    2019/03/28 16:15

    全角スペースだいぶ残ってますけども。

    キャンセル

回答 2

checkベストアンサー

+1

コードの細かい指摘についてはyambejpさんの指摘通りです。
プリペアドステートメント使ってたり使っていなかったりコーディングスタイルに一貫性がないのもまずいと思います。

今回はすごく簡単です。
「ログイン日時」は自動で更新がかかるCURRENT_TIMESTAMPではないんですよね?
sdateが「ログイン日時」としたらシステム側で払い出した日時でupdateをかけているように見えます。

それでしたら「システム側で払い出した日時」をそのままセッションに入れれば良いのではないでしょうか。(今回の答え)


既に指摘があるように無駄なコードが多いです。
少なくとも下記2点のSQLは不要
SELECT * FROM member WHERE id = $id
SELECT * FROM member WHERE sdate = ?

最初にSELECT * FROM member WHERE mail = ?を実行していることから「mailはプライマリーキーで一意」というのが推察されます。

ということはこの最初のmailによるSELECTでユーザーデータは確保できているわけです。
そこで「パスワードも照合してログイン認証できた」のであれば、updateもいっそのことmailをWHERE条件にしても差し支えないのでは?

同じメールアドレスで別ユーザをとれるような(悪さし放題の)仕組みであれば仕方ないかもしれませんが、それならパスワードも最初の検索条件に入れる必要がありますね。

無駄なコードを書かないためにも流れをきちんと整理しましょう。
YES/NOだけで良いのでフローを書いてみると良いです。(手書きでも良い)
イメージ説明

ちなみにコードフォーマットはEclipseなどのフォーマット機能がついたエディタを使用してください。手でやるなら最初から組みなおすくらいじゃないと無理です。

下記、Eclipseのフォーマット機能を使った例(全角スペースは手動で取り除きました。)

<?php
session_start();

if (isset($_SESSION["NAME"])) {
    header("Location: my.php");
    exit();
}

$db['host'] = "hoge";
$db['user'] = "hoge";
$db['pass'] = "hoge";
$db['dbname'] = "hoge";
$date = date('Y-m-d H:i:s');

$errorMessage = "";
$errorMessage_2 = "";

require_once 'hasu_test.php';

if (isset($_POST["login"])) {
    if (empty($_POST["mail"])) {
        $errorMessage = '※空やでやで';
    } else if (empty($_POST["password"])) {
        $errorMessage_2 = '※空やで。';
    } else if (! preg_match('/^[!-~]+@[!-~]+$/', $_POST["mail"])) {
        $errorMessage = '※正規表現外mail';
    } else if (! preg_match("/\A(?=.*?[a-z])(?=.*?\d)[a-z\d]{8,100}+\z/i", $_POST["password"])) {
        $errorMessage_2 = '※正規表現外pass';
    } else if (! preg_match("/^.{4,32}$/", $_POST["mail"])) {
        $errorMessage = '※桁数上mail';
    } else if (! preg_match("/^.{4,32}$/", $_POST["password"])) {
        $errorMessage_2 = '※桁数上pass';
    } else {

        if (! empty($_POST["mail"]) && ! empty($_POST["password"])) {
            $mail = $_POST["mail"];

            $dsn = sprintf('mysql:host=%s; dbname=%s; charset=utf8;unix_socket=/tmp/mysql.sock', $db['host'], $db['dbname']);

            try {
                $pdo = new PDO($dsn, $db['user'], $db['pass'], array(
                    PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
                ));
                $stmt = $pdo->prepare('SELECT * FROM member WHERE mail = ?');
                $stmt->execute(array(
                    $mail
                ));
                $password = $_POST["password"];

                if ($row = $stmt->fetch(PDO::FETCH_ASSOC)) {
                    if (password_verify($password, $row['password'])) {
                        session_regenerate_id(true);

                        $id = $row['id'];
                        $sql = "SELECT * FROM member WHERE id = $id"; // 入力したIDからユーザー名を取得
                        $stmt = $pdo->query($sql);
                        foreach ($stmt as $row) {
                            $row['mail'];
                        }

                        $stmt = $pdo->prepare('SELECT * FROM member WHERE sdate = ?');
                        $stmt->execute(array(
                            $date
                        ));
                        $sql = "UPDATE member SET sdate='$date' WHERE id='$id'";
                        $result = $pdo->query($sql);

                        $_SESSION["sdate"] = $row['sdate'];
                        $_SESSION["mail"] = $row['mail'];
                        header("location: my.php");
                        exit();
                    } else {
                        $errorMessage = '※mailまたはPasswordが間違っています。';
                    }
                } else {
                    $errorMessage = '※該当データなしmailまたはPasswordが間違っています。';
                }
            } catch (PDOException $e) {
                $errorMessage = header("Location: 500.php");
            }
        }
    }
}

?>

全体的な懸念事項(既存指摘以外メイン):

1.今回のコード上、どこにもNAMEって保存されてないのに下記がある。

if (isset($_SESSION["NAME"])) {
    header("Location: my.php");
    exit();
}


フロー上はログイン日時に入れた現在日時をセッションに格納のところで入れるものだと思う。mailは入れてるからそれでいいのでは?
またはtrue/falseだけ保管しておくキーを別途設けるとか。

2.header()は送信するだけで返り値は持たない、のにこれは何?

$errorMessage = header("Location: 500.php");

3.$errorMessageとか$errorMessage_2とかつけてるけど、変数名で何のエラーかが不明。
それなら下記みたいに配列で持っておいて、適宜取り出したほうが良い。
マジックナンバーは使ってはいけません。

$errorMessage['mail'][] ='必須です';
$errorMessage['password][] ='必須です';

4.というかこれはログイン画面ですよね。
パスワード何桁とかわざわざチェックするのは悪意あるユーザーのヒントになります。
どっちも必須チェックくらいにとどめるべき。

5.ifのスコープが長すぎる
フォーマットかけたらよく分かると思います。特に短い方が後に来るコードは読みづらいです。
可読性が低く、それはメンテナンス性の低下も招きます。
elseを使うにしても以下のどちらが読みやすいか考えてみてください。
1:

if($test){
  /*

   長い処理




  */
}else{
  echo 'error!';
}


2:

if(!$test){
  echo 'error!';
}else{
  /*

   長い処理




  */
}

3:

if(!$test){
  die('error!'); //関数ならここでreturn
}
/*

   長い処理




*/


正常な場合のみにフォーカスを当てがちでですが、異常な方がプログラムとしては大事です。
先に異常なほうを書きましょう。正常なほうは後回しでも問題ないです。

入力チェックの処理を外だしして関数呼び出してメインの処理はエラーメッセージの有無だけにしたほうがスッキリします。
「本筋の処理と直接関係がないのなら外出し」「同じようなコードは変数で分岐かけて共通化」が基本です。

6:PDOExceptionほぼ握りつぶしてる

            } catch (PDOException $e) {
                $errorMessage = header("Location: 500.php");
            }


何のために例外捕捉するようにしたのか。これでは何か異常を検知したときに原因を調べられない。
セキュリティ観点から画面に出力する必要はないので、せめてログに出力してください。オブジェクトなので$eをjson_encode()かけたものでもいいから。

※Javaの記事ばかりですが考え方は同じ。握りつぶしちゃダメです。何のための例外をキャッチさせてるのか考えましょう。

投稿

編集

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2019/03/28 17:50

    いやおかしい。ログイン日時がCURRENT_TIMESTAMPっておかしい。
    ログイン以外のときに更新されちゃダメじゃないですか?
    ユーザー情報をユーザーが更新したら更新されるわけですよね。
    「最終ログイン日時」ならCURRENT_TIMESTAMPじゃなくてnull okのtimestamp型にしてプログラム側から更新するようにすべきです。なんの用途に使うのか知らないですが…
    「自動ログアウトのための無操作時間の計測のため」ならそれこそセッションに「last_action_time」とか用意しといてそこだけ上書きしていけば良いですし。
    ユーザーが更新を意識しない操作のたびにテーブルにupdateかけては重くなる一方かと。

    キャンセル

  • 2019/03/28 18:05

    ご指摘感謝いたします。
    そうですね、現状ユーザーからのアクションは、ログインのみですが、
    今後I/Passの変更等あった際に、きっと同じところでつまずていました。
    ありがとうございます。


    >CURRENT_TIMESTAMPであっても同じ。プログラムとDBは同じサーバーであればdate()でとってくれば同じ値だと思います。
    >どうしても心配ならupdateしたあとにmailでSELECTかければ良いです

    ここのコードがわかりません。
    もしよろしければご助言いただけませんか?

    キャンセル

  • 2019/03/28 18:47

    フロー書いたら良いかと。
    というか、私が提示したフローが一部変わるだけです。プログラムから払いだすか、DBから改めてとってくるか。

    キャンセル

+1

 $password = $_POST["password"];
で受けた生パスワードを
SQLから取り出した$row['password']と比較している

if (password_verify($password, $row['password'])) {

ということはDBに生パスワードを保持しているということですか?
まずはそれを直すところから始めてください

$sql = "SELECT * FROM member WHERE id = $id";

についても一応DBから取り出した$idを利用しているので
省略しても問題ないですが、値を渡すときはprepareでやる
くせをつけたほうがよいです。

      foreach ($stmt as $row) {
$row['mail'];  // メール
}

意味不明

      $stmt = $pdo->prepare('SELECT * FROM member WHERE sdate = ?');
$stmt->execute(array($date));

検索後に参照しないで更新処理をするのも意味がわからない。
ここもprepareで処理してみてください

      $_SESSION["sdate"] = $row['sdate'];
$_SESSION["mail"] = $row['mail'];

結局この$rowは最初の「SELECT * FROM member WHERE mail = ?」の
値ですよね?間の処理がすべて無駄?

そもそもがsession_start()してませんよね?

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2019/03/28 16:25

    ご指摘感謝します
    ・ハッシュ化しています
    ・prepareくせ努力します
    ・session_startしています
    ・等々コード修正していますので再度ご指摘いただけると幸いです。

    キャンセル

15分調べてもわからないことは、teratailで質問しよう!

  • ただいまの回答率 88.93%
  • 質問をまとめることで、思考を整理して素早く解決
  • テンプレート機能で、簡単に質問をまとめられる

関連した質問

同じタグがついた質問を見る