Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

パスワードのリセット機能を追加しましょう #648

Open
KATO-Hiro opened this issue Apr 8, 2024 · 13 comments
Open

パスワードのリセット機能を追加しましょう #648

KATO-Hiro opened this issue Apr 8, 2024 · 13 comments

Comments

@KATO-Hiro
Copy link
Collaborator

Description / 説明

  • A clear and concise description of what you want to happen.

Motivation / 動機

  • A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

Other notes / その他

  • Add any other context or screenshots about the feature request here.
  • Will you try to create a pull request?
    • yes / no
@KATO-Hiro
Copy link
Collaborator Author

KATO-Hiro commented Apr 8, 2024

luciaをv2からv3にアップデートしたいところですが、(私にとっては)DBの大幅な修正を伴うため変更コストがかなり大きいと予想しています。

理由

  • Lucia v3 + v3 Prisma adaperで、破壊的な変更が入りました。全てのカラムがキャメルケースであることが要求されていますが、v2でデフォルトだったスネークケースも混在しています。また、APIも全面的に見直されており、ほぼ再実装し直しに近いです。
    • 上記の変更を確認してからは、移行のコストを減らすためキャメルケースでカラムを記述するようにしています

https://lucia-auth.com/upgrade-v3/prisma/postgresql

  • 本番環境のDBの全てのカラムに対して、スネークケースからキャメルケースへ移行するのはかなりリスクが高い状況です
    • ORMとして利用しているPrismaでは、(私が調査・理解している範囲では)カラム名のリネームの機能はなさそうです
    • 新旧のカラムを一時的に用意し、新しいカラムに現在のデータを入れ替える必要があります
      • DBのバックアップを取る
      • CRUD処理なども新旧のカラムを併存させる
      • 上記の変更が本番環境で動作するようになったら、旧カラムを削除
    • CRUDに対して自動テストがないため、手動でチェックが必要です

@prettyhappycatty
Copy link
Collaborator

prettyhappycatty commented Oct 4, 2024

メモ

  • パスワードリセット(現状忘れた人の救済どうする?)

    • 性善説で対応?乗っ取られるのが心配
    • 忘れたつぶやきをした人にだけ声をかけるとか、、、?(大っぴらにリセットできるとはいわない?)
    • 復活はさせずに、新しいアカウントを作ってもらって、登録データの移行だけするとか?(ユーザーデータのコピー、IDは変わっちゃう)
  • メールアドレスの登録を促す

    • メールアドレスの設定欄をユーザー?のテーブルにつくる
    • メアドが正しいことを確認する仕組み
    • リセットリンクまたは、リセット後のPWをメールに送る仕組み、一定時間でexpiredする仕組み
    • 個人情報管理的なドキュメントは必要?(メンテや更新のお知らせするよ、パスワードリセットに使うよ、それ以外には使わないよ)
    • (ログイン後画面などにメールアドレスがない場合に強制的に変更ページに遷移的なものを入れる?)

@KATO-Hiro
Copy link
Collaborator Author

KATO-Hiro commented Oct 4, 2024

復活はさせずに、新しいアカウントを作ってもらって、登録データの移行だけするとか?(ユーザーデータのコピー、IDは変わっちゃう)

該当者・希望者が数名〜20名程度あれば、個人的にはかなり現実的な方法だと思っています。

  • メリット: アプリの改修コストが比較的低め
  • デメリット: ユーザが希望する名前とパスワードを利用できない(この部分をどこまで許容できるかが問題になりそうです)

@KATO-Hiro
Copy link
Collaborator Author

KATO-Hiro commented Oct 4, 2024

タスク(案)

(暫定的な対応)別アカウントを作成して、元のアカウントのデータをコピー

  • Login画面に、パスワードリセットのリンク(Xアカウント?)を用意
  • 影響範囲の調査
  • ユーザおよび回答状況に関するデータをコピーするヘルパーメソッドを用意
    • (可能であれば)自動テストで意図した処理が実行されることを担保
  • 管理者画面で、ボタンなどを配置しアカウントの移行ができるようにする

認証ライブラリLucia(v2)を利用したパスワードリセット機能

  • 影響範囲の調査

  • Luciaのv2からv3へのアップグレード(改修コストが大きすぎるので、できれば回避したい)

  • Lucia(v2)を使用したパスワードリセット機能の実装

    • Userテーブルにメールアドレスと認証済みかどうかのフラグを属性として追加
    • メールによる認証のための属性(トークン用のid、有効期限、ユーザid)のテーブルを用意
    • Luciaの設定を src/lib/server/auth.ts で更新
      • getUserAttributesに、メールアドレスと認証済みかどうかのフラグを追加
    • 新しいトークンを生成
    • トークンのバリデーション
    • アカウント作成ページで、認証されているかどうかの処理などを追加
    • ログインの一部の処理を変更
    • トークンの再送信機能の実装
    • メールによる認証

https://v2.lucia-auth.com/guidebook/email-verification-links/
https://v2.lucia-auth.com/guidebook/password-reset-link/

  • AtCoder IDを利用した本人確認
    • 本人確認用のタブを表示
    • IDの登録時におけるタブの遷移をSvelteのデフォルト機能であるstoreで制御
    • 認証( https://github.com/AtCoder-NoviSteps/AtCoder-NoviStep-Crawler )で使用するサーバをあべみさん個人のものから、VercelのNoviStepsアカウントのものに移行
    • アカウント削除時のモーダルの説明文を更新

@prettyhappycatty
Copy link
Collaborator

ありがとうございます。
v3へのアップグレードはつらそうです。
ユーザー関連テーブルだけではなく、他テーブルもキャメルケースにする必要があると読み取れました、、、

新しいアカウントを作成してデータをコピー、は救済案としてはありそうですね。
IDは変わるけど、パスワードは新しく作ってもらうときに設定してもらえるので、パスワード変更の機能を作る前にも対応できそうに思います。

ちなみに、パスワード変更は今後実装するとして、ID変更は許可しますか?

もし許可するなら、
例えば、今後メールアドレスを入力してもらうとして、メールアドレスの登録のないデータを1年くらいで無効化(退会?は今はできるんでしたっけ)するようにすると、データ容量の削減にもなるし、忘れてしまった人ももともと登録していたIDに戻れるのかなあと。(まとまりのない文章ですみません)

@KATO-Hiro
Copy link
Collaborator Author

ありがとうございます。

他テーブルもキャメルケースにする必要があると読み取れました、、、

おっしゃる通り、v3を利用するには現在あるテーブル全てをキャメルケースにする必要があります。

新しいアカウントを作成してデータをコピー、は救済案としてはありそうですね。

そうですね。
短期間で実現するには、最も現実的な方法だと思います。

ID変更は許可しますか?

・ユーザ側
こちらの意味でしょうか?
ユーザがアカウント名をどうしても変更したくない、という希望はそれなりにありそうです。

例えば、以下のような手順であれば、ユーザはIDの変更をせずに済むかもしれません(この手間を許容できるかは別の問題になりそうです)

・元のユーザのアカウントの回答状況などをダミーアカウントにコピー
・元のユーザのアカウントを一旦削除
・新規アカウントを作成していただく
・ダミーアカウントから新規アカウントにコピー
・上記が正しく実行されたら、ダミーアカウントを削除

・管理者側
同一人物が複数のアカウントを持つことに対しては、特に制限を設けていないです。
例えば、アカウントごとに特定の言語の回答状況を記録するという使い方もあるようです。
(スパムのように膨大なアカウントを作成される場合は、別途対応が必要です)

メールアドレスの登録のないデータを1年くらいで無効化

データ量の削減の観点からとても効果的だと思います。

退会

一般ユーザであれば、ログインができる状態なら「ユーザ」ページからアカウントを削除できます。
管理者に関しては、専用のUIから対応できるようにする予定です。

@prettyhappycatty
Copy link
Collaborator

prettyhappycatty commented Oct 5, 2024

ID変更は許可しますか?

・ユーザ側 こちらの意味でしょうか? ユーザがアカウント名をどうしても変更したくない、という希望はそれなりにありそうです。

例えば、以下のような手順であれば、ユーザはIDの変更をせずに済むかもしれません(この手間を許容できるかは別の問題になりそうです)

今回忘れた人が、新しいアカウントを作成(ユーザーIDが希望通りでない、パスワードは希望通り)

システム側で、旧アカウントの回答状況を新しいアカウントにコピー

ユーザーは新しいアカウントを使って活動する

(システム側で、どこかのタイミングでメールアドレス登録とパスワード初期化を追加)

システム側は、古いアカウントを時間経過(1年程度)で削除

ユーザーは、新しいアカウントを古いアカウントのIDに変更(IDが希望通りになる)

みたいな流れを想定しての質問です。

一番心配してるのは、旧アカウントと主張するものが他のユーザーのものだった場合の乗っ取りなので、、、

・管理者側 同一人物が複数のアカウントを持つことに対しては、特に制限を設けていないです。 例えば、アカウントごとに特定の言語の回答状況を記録するという使い方もあるようです。 (スパムのように膨大なアカウントを作成される場合は、別途対応が必要です)

この使い方は想定していませんでしたが、アリですね!!
データベースに、メールアドレスの一意制約を入れてはいけない形になりますね。

他の回答についても承知しました!

@prettyhappycatty
Copy link
Collaborator

prettyhappycatty commented Oct 5, 2024

順番的には以下でしょうかね?

  • 暫定的に救済する機能

Login画面に、パスワードリセットのリンク(Xアカウント?)を用意
影響範囲の調査
ユーザおよび回答状況に関するデータをコピーするヘルパーメソッドを用意
(可能であれば)自動テストで意図した処理が実行されることを担保
管理者画面で、ボタンなどを配置しアカウントの移行ができるようにする

 - 管理画面(admin権限)で、 [source user id]テキストボックス、 [destination user id]テキストボックス、[実行]ボタン、のようなUIをイメージしました。実行ボタンを押すと、destinationのデータが空の場合に限りコピーが走るもの。
 - フロー
  - パスワードを忘れたユーザが対象のIDをNovistep運営に連絡
  - 運営は、「新しい空のアカウントを作って新しいアカウントのIDを運営に連絡してもらいたい旨」をユーザに連絡する
  - ユーザは、空のアカウントを作り、運営に新しいアカウントのIDを連絡する。
  - 運営は、コピー機能を使ってもとのIDのデータを新しいIDのデータにコピー
 

Userテーブルにメールアドレスと認証済みかどうかのフラグを属性として追加

 - メールアドレスは一意制約はつけない

メールによる認証のための属性(トークン用のid、有効期限、ユーザid)のテーブルを用意
Luciaの設定を src/lib/server/auth.ts で更新
getUserAttributesに、メールアドレスと認証済みかどうかのフラグを追加

新しいトークンを生成
トークンのバリデーション
アカウント作成ページで、認証されているかどうかの処理などを追加
ログインの一部の処理を変更
トークンの再送信機能の実装
メールによる認証

  • (将来的に、画面からパスワード変更は実施できるようにする?リセットリンクからできるだけで十分?)
  • (将来的に、ユーザIDの変更は許可する?)

もし、これで問題なければ、「暫定的に救済する機能」と「メールアドレスを入力する機能」を別Issueで立ててもよろしいでしょうか。

あと、追加で質問すみません。
「AtCoderIdを使った本人確認」と記載されているものは、NoviStepの回答状況とAtCoderの回答状況が矛盾ないことを確認して、本人であることを確認するために利用するイメージであっていますか?
あの機能を作った当初は、回答状況をインポートすることを想定していたもので、本人以外のIDからインポートしないようにするための確認をする機能の位置付けでした。
また、あの機能はAtCoderのユーザーページのUIを読み取ってきているので、リリース前には、UIが変わっていないか自動で確認し、失敗したら通知をするなりIssueを立てるなりの仕組みを入れておきたいですね、、、

@KATO-Hiro
Copy link
Collaborator Author

ありがとうございます。

ID変更の意図や流れについて、教えていただきありがとうございます。
理解できました。

アカウント乗っ取りだけは、なんとしても回避したいですね。

データベースに、メールアドレスの一意制約を入れてはいけない形になりますね。

そうですね。

@KATO-Hiro
Copy link
Collaborator Author

KATO-Hiro commented Oct 6, 2024

ありがとうございます。
示していただいた手順、フローで進めていただければと思います。
大変な部分の設計・実装をお願いしてしまい、すみません🙇。

「暫定的に救済する機能」と「メールアドレスを入力する機能」を別Issueで立ててもよろしいでしょうか。

お手数をおかけしますが、よろしくお願いいたします。

(将来的に、画面からパスワード変更は実施できるようにする?リセットリンクからできるだけで十分?)

個人的には、リセットリンクから実行できれば(極論にはなりますが、「暫定的に救済する機能」のみでも)十分かと思います。
パスワードのリセット希望を明確に出されているのは、ごく一部の方だけですし、システムの改修の労力を考えた現実的な路線ではないかと考えています。

その分、問題一覧のテーブルなどのメインの機能を優先したいと思いますが、いかがでしょうか?

(将来的に、ユーザIDの変更は許可する?)

できれば許可しない方がいいかと思っています。
以前に準備していただいたプロフィールページのURLで、ユーザIDを使っているためです。

「AtCoderIdを使った本人確認」と記載されているものは、NoviStepの回答状況とAtCoderの回答状況が矛盾ないことを確認して、本人であることを確認するために利用するイメージであっていますか?

コメントいただいたように、当初は以下の用途で使用するためのものでした。
パスワードリセット機能の実装が困難な場合の代替案として活用できるかもしれない、という意図でした。
説明不足で、すみません。

本人以外のIDからインポートしないようにするための確認をする機能の位置付けでした。

また、あの機能はAtCoderのユーザーページのUIを読み取ってきているので、リリース前には、UIが変わっていないか自動で確認し、失敗したら通知をするなりIssueを立てるなりの仕組みを入れておきたいですね、、、

そうですね!
運用面まで考慮していただき、ありがとうございます。

@prettyhappycatty
Copy link
Collaborator

その分、問題一覧のテーブルなどのメインの機能を優先したいと思いますが、いかがでしょうか?

はい!メイン機能優先で良いと思います!

できれば許可しない方がいいかと思っています。
以前に準備していただいたプロフィールページのURLで、ユーザIDを使っているためです。

承知しました!

「AtCoderIdを使った本人確認」

回答承知しました!
暫定案には一旦関連しないので、この機能については一旦置いておきます。

「暫定的に救済する機能」と「メールアドレスを入力する機能」を別Issueで立ててもよろしいでしょうか。
お手数をおかけしますが、よろしくお願いいたします。

以下、作成したIssueです
#1346
#1347

@KATO-Hiro
Copy link
Collaborator Author

ご確認いただき、ありがとうございます。
引き続き、よろしくお願いします。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants