toukanno
@toukanno (kurisu erisu)

Are you sure you want to delete the question?

Leaving a resolved question undeleted may help others!

PHP DBなし ログインについて

Q&A

Closed

すでに新規登録ずみで ログインするだけのファイルを2つ作りました。
ログインができるように2つのファイルをつくりましたcsvファイルを読み込んでいきます。
ユーザには1,附番ID,2,ユーザーID、3,ユーザの名前、4,パスワードが割り振られています。
もっといい書き方があれば教えてほしいです。
戻って修正ができずにそのまま登録されてしまうのも問題です。

login.php
<?php
session_start();
if (isset($_SESSION['id'])) {
  header('Location: top.php'); //ログインしていればtop.phpへリダイレクト
  exit;
}
?>
<!DOCTYPE html>
<html lang="ja">

<head>
  <meta charset="UTF-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <title>ログイン</title>

</head>

<body>
  <header>

    <div class="login"><a href="signup.php">新規登録はこちらから</a></div>
  </header>
  <main>
    <form action="login_done.php" method="post">
      <div class="form_contents">
        <h2>ログイン</h2>

        <div class="userid">
          <div class="hissu">必須</div>
          <lavel>ID</lavel><br>
          <input type="text" class="formbox" size="40" name="userid" required>
        </div>
        <div class="password">
          <div class="hissu">必須</div><label>パスワード</label><br>
          <input type="password" class="formbox" size="40" name="password" id="password" required>
        </div>
        <div class="login">
          <input type="submit" name='login' class="submit_button" size="35" value="ログイン">
        </div>
      </div>
    </form>
  </main>


</body>

</html>

login_done.php
<?php
//ログインするための画面
$e = "";
session_start();
//ログイン済みかを確認
if (isset($_SESSION['id'])) {
  header('Location: top.php'); //ログインしていればtexttable.phpへリダイレクト
  exit;
}
//ログイン機能



if (!empty($_POST['userid']) && !empty($_POST['password'])) {
  // print_r($_POST);
  // print_r($_SESSION);
  $userid = $_POST['userid'];
  $password = $_POST['password'];
  $user = fopen("csv/user.csv", "r");

  while ($line = fgets($user)) {
    $users = explode(",", $line);
    $ids[] = trim($users[1]);
    $passwords[] = $users[3];
    // print_r($users[3]);
    // print_r($users[1]);
  }
  fclose($user);

  if (in_array($userid, $ids)) {

    $user = fopen("csv/user.csv", "r");
    while ($line = fgets($user)) {

      $users = explode(",", $line);
      if ($users[1] == $userid && trim($users[3]) == $password) {
        $_SESSION['id'] = $users[0];
        $_SESSION['name'] = trim($users[2]);
        $e = "<a href='top.php' style = 'color:blue'>トップページへ</a>";
        // print_r($users[0]);
        print_r($_SESSION['id']);
        print_r($_SESSION['name']);
        break;
      } else {
        $e = "<a href='login.php' style = 'color:red'>パスワードが違います</a>";
        // break;
      }
    }
    fclose($user);
  } else {
    $e = "<a href='login.php' style = 'color:red'>IDが違います</a>";
  }
}


?>
<!DOCTYPE html>
<html lang="en">

<head>
  <meta charset="UTF-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <title>Document</title>
</head>

<body>
  <?php echo $e; ?>
  <form action="top.php" method="post">
    <input type="hidden" value="<?php echo $_POST['userid']; ?>" name="name">
    <input type="hidden" value="<?php echo $_POST['name']; ?>" name="mail">
    <input type="hidden" value="<?php echo $_POST['password']; ?>" name="password">
  </form>
</body>

</html>
1

2Answer

まず大前提ですが、最初に回答されている方の仰っていることがとても丁寧と感じると共に、内容に関しても同意です。
ですが、敢えてこのままcsvでいくのであれば…という発想で気になった点を。

パスワード認証

どうしてもcsvということであれば、せめてpassword_hashとpassword_verify等、ハッシュ化を必ずしましょう。
password_hash
password_verify

パスワードをハッシュ化せず平文で保存・管理する方法は「やめたほうが良い」ではなく、絶対に避けるべきです。

なお、パスワードは「本人しか知らない」が原則なので、ハッシュ化と似ている暗号化も、システム管理者がパスワードを知り得てしまうので避けましょう(平文よりは遥かにマシですが)。

「IDが違います」「パスワードが違います」エラー文をやめる

親切心だと思いますが、セキュリティの観点からこれもやめるべきです。
なぜなら「パスワードが違います」が出たら「IDは合っている」ということが分かってしまうので、不正ログインがしやすくなるためです。
IDかパスどちらか違う場合は、同一メッセージで「IDもしくはパスワードが違います」を出すべきです。

一致するユーザーの判定

csvファイルの中身を全部変数に入れて、さらに認証時にも再度csvファイルを読み込むのはオーバーヘッドが非常に大きいです。
csvファイル読みつつ、IDパスワードが一致するデータがあればその場でループをbreakして結果をreturnした方が良いでしょう。

配列の初期化

$ids$passwordsといった配列の初期化をしましょう。

$ids = []; // PHP7未満は $ids = array();じゃないと動かない
$ids[] = 'hoge';

PHPでは初期化しなくても動きますが(NoticeかWarning出るかもしれません)、言語によっては処理が通らない書き方です。

スコープが長い

処理を関数に分割して、一つ一つの処理を短くした方が良いでしょう。
例えばcsvを取り込む一連の処理を

function import_csv() {
// csv読み込み処理をまとめる
}

にする等。
そうすることでコードのメンテナンス性、保守性、可読性が向上します。

命名規則

変数名にローマ字は避けましょう。
必須フィールドならhissuではなくrequired

コーディング規約

シングルクォートとダブルクォートが混在しているので統一しましょう。

スタイル指定

htmlタグのstyle属性は一般的に非推奨とすることが多いので、cssファイルを作ってclass名でスタイルを当てると良いでしょう。

lang="ja"にする

lang="en"は英語ページとして指定しているので、lang="ja"にしましょう。

最後に

と、ここまでいろいろ書いておきながら、振り出しに戻りますが。
元も子もない話になってしまいますが、やはりデータベースを使った方が良いかと思います。
そして、フレームワーク(PHPであればLaravelがデファクト)で構築が良いかと。

質問者さんがこのcsv認証システムを作られている目的を把握していないのですが、

もし勉強のためであれば、セキュリティやパフォーマンスの面から実際のプロダクトでcsv認証システムが使われることは無い(あってはならない)ので、フレームワークを使った勉強の方が遥かに意味を持ちます。

逆に何かしらのプロダクトで実際に使うのが目的であれば、より一層フレームワークの使用を推奨します。
各種セキュリティやエラー処理等を漏れなくPurePHPで実装するのは慣れたプログラマであっても難易度が高く実装漏れのリスクもあり、時間も長くかかる等、百害あって一利なしです。

以上です。長文失礼いたしました。

3Like

課題だったらセキュリティ云々をここで言っても仕方がないので、その辺は無視して進めます。
構造や効率も特に考えません。
作るのが楽な感じでいきます。

まず先に共通で使うファイルを作っておきます。

common.php
<?php

class User
{
    public $no;
    public $id;
    public $name;
    public $password;
    public function __construct($no, $id, $name, $password)
    {
        $this->no = $no;
        $this->id = $id;
        $this->name = $name;
        $this->password = $password;
    }
}


function h($a)
{
    return htmlspecialchars($a);
}

/**
 * @param mixed $a 
 * @return ?string
 */
function s(&$a)
{
    if ($a === null || !is_string($a) || $a === '')
        return null;

    return $a;
}

それからログインの処理はlogin.phpに書いて、login_done.phpは表示処理だけにします。
PHPでfgetsを使うことはまず無いです。csv読み込みをライブラリを使わずにやるならfgetcsvを使います。
また、ループが二回あるのでまとめてidをキーの配列にします。
各処理も関数に分けます。

login.php
<?php
require_once 'common.php';

session_start();
if (isset($_SESSION['user'])) {
  header('Location: top.php'); //ログインしていればtop.phpへリダイレクト
  exit;
}

/** @return array<string,User> */
function getUsers()
{
    $fp = fopen("csv/user.csv", "r");
    $users = [];
    while ($line = fgetcsv($fp)) {
        if (!isset($line[3]) || $line[3] === '')
            continue;
        $user = new User(...$line);
        $users[$user->id] = $user;
    }
    fclose($fp);
    return $users;
}

function login($id, $password)
{
    $users = getUsers();
    if (!isset($users[$id]))
        return false;
    if ($users[$id]->password !== $password)
        return false;

    $_SESSION['user'] = $users[$id];
    return true;
}

$error = null;
$id = s($_POST['userid']);
$password = s($_POST['password']);

if (isset($id, $password)) {
    if (login($id, $password)){
        header('Location: login_done.php');
        exit;
    }
    $error = 'IDまたはパスワードが違います。';
}



?>
<!DOCTYPE html>
<html lang="ja">

<head>
  <meta charset="UTF-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <title>ログイン</title>

</head>

<body>
  <header>

    <div class="login"><a href="signup.php">新規登録はこちらから</a></div>
  </header>
  <main>
    <form action="" method="post">
      <div class="form_contents">
        <h2>ログイン</h2>

        <?php if($error): ?>
          <div style="color:red"><?= h($error) ?></div>
        <?php endif; ?>

        <div class="userid">
          <div class="hissu">必須</div>
          <lavel>ID</lavel><br>
          <input type="text" class="formbox" size="40" name="userid"
                 value="<?= h($id) ?>" required>
        </div>
        <div class="password">
          <div class="hissu">必須</div><label>パスワード</label><br>
          <input type="password" class="formbox" size="40" name="password" id="password"
                 value="<?= h($password) ?>" required>
        </div>
        <div class="login">
          <input type="submit" name='login' class="submit_button" size="35" value="ログイン">
        </div>
      </div>
    </form>
  </main>


</body>

</html>

csvは逐次調べていって見つけたものだけ返す方が効率は良いですが、まあその場合はfunction getUser($id)のようにしてください。

フォーム送信先をlogin.phpにするとフォームでエラーになった場合の表示が楽です。
成功した場合にリダイレクトに変えました。

login_done.php
<?php
require_once 'common.php';

session_start();

if (!isset($_SESSION['user'])) {
  header('Location: login.php');
  exit;
}

$user = $_SESSION['user'];

?>
<!DOCTYPE html>
<html>

<head>
  <meta charset="UTF-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <title>Document</title>
</head>

<body>
  <div>
    <?= h($user->id) ?>: <?= h($user->name) ?>
  </div>
  <div>
    <a href="top.php">top</a>
  </div>

</body>

</html>

Userクラスを作った方が楽なのでクラスにしていますが、別に配列でもいいです。

1Like

Comments

  1. @toukanno

    Questioner

     すごく丁寧に教えて下さり有難うございます!!
    require_onceでファイルを読み込んだり 関数を使ってまとめるということはまだしていませんでした。そしてまだ使ったことのない記法もたくさん使っているます。なのでとても分かりやすく教えて下さり感謝していますがおおまかに理解はできるものの完璧な理解にはなっていませんので後々一つずつ確認していきます。

Your answer might help someone💌