他のユーザーのメール通知設定を変更できないようにした

[Ryooo-k]

Issue

• #8056 他ユーザーのメール通知をOFFにできてしまう

概要

自分以外のユーザーのメール通知をOFFに設定できないようにしました。

変更前

• HTMLでユーザーIDを改変すると、自分以外のユーザーのメール通知をOFFに設定することができる

変更後

• URIのuser_IDとtokenが不整合の場合、アクセスできずメール通知をOFFにできない。

変更確認方法

1. bug/prevent-disabling-other-users-notifications をローカルに取り込む i. git fetch origin pull/8149/head:bug/prevent-disabling-other-users-notifications ii. git checkout bug/prevent-disabling-other-users-notifications
2. foreman start -f Procfile.dev でローカルサーバーを立ち上げる
3. 通知設定変更画面にアクセスする（kimuraさんの通知設定変更の画面）
4. デベロッパーツールを開く（Mac：opt + cmd + i 、Windows：Ctrl + Shift + i ）
5. セレクトモードに切り替える。（下図の赤丸部分を選択する）

6. 「オフにする」のボタンを押す

7. デベロッパーツール上のHTMLで、下図赤枠部が選択されるため、下記手順でtokenを変更する i. "/users/991528156/mail_notification?token=k6Da-oL3cRi8ApNFO9-Gcg"にカーソルを当て、右クリック ii. Edit attributeを選択 iii. "/users/991528156/mail_notification?token=k6Da-oL3cRi8ApNFO9-Gcg" を "/users/991528156/mail_notification?token=037i-ef5n7V4EnPv74mtyQ"に変更する （k6Da-oL3cRi8ApNFO9-Gcgはkimuraさんのtoken、037i-ef5n7V4EnPv74mtyQはkomagataさんのtoken）

8.「オフにする」のボタンを押す

9. 「ユーザーIDもしくはTOKENが違います。」のメッセージが表示されることを確認する。

• ログイン済みの場合の画面

• 未ログインの場合の画面

10. ユーザー名 kimura 、パスワード testtest でログインする
11. kimuraさんの登録情報変更ページにアクセスし、メール通知がONになっていることを確認する

11. ログアウトする
12. ユーザー名 komagata 、パスワード testtest でログインする
13. komagataさんの登録情報変更ページにアクセスし、メール通知がONになっていることを確認する

Screenshot

変更前

全てのユーザーIDを受け入れ、他のユーザーのメール通知をOFFにできる。

変更後

• URIのユーザーIDとtokenの整合が取れている場合、アクセスできメール通知をOFFにできる。

• URIのユーザーIDとtokenが不整合の場合、アクセスできずメール通知をOFFにできない。

[Ryooo-k]

@Shrimprin お疲れ様です！ こちらのレビューをお願いしたいのですが可能でしょうか？ ご確認お願いいたします🙏

[Shrimprin]

@Ryooo-k お疲れ様です！レビュー承知いたしました 👍 今週の土日あたりで対応させていただきます〜

[Shrimprin]

@Ryooo-k お疲れ様です！

動作については問題ないこと確認できました！ 👍

１点、仕様について質問です。 メール通知をOFFにする画面はメール通知をOFFにする対象のユーザー以外からはアクセスされる必要がないという認識です。 なので、画面へのアクセスにログイン必須かつログインユーザーがメール通知をOFFにする対象のユーザーと一致の条件を加えても良いと思ったのですがいかがでしょうか？

[Ryooo-k]

@Shrimprin ご確認ありがとうございます！

画面へのアクセスにログイン必須かつログインユーザーがメール通知をOFFにする対象のユーザーと一致の条件を加えても良いと思ったのですがいかがでしょうか？

確かにメール通知OFF画面に対して直接アクセス制限をかけた方が良いかもしれないですね！ （アクセス制限かけたら現状の条件分岐は一つ減らせそうですし） ログインユーザーがメール通知をOFFにする対象のユーザーと一致の条件は、結局メール通知OFF画面でHTMLを改変すれば今回のバグと同じ現象になってしまうと思うので、まずはアクセス制限をかけるように変更してみますね👍

[Ryooo-k]

@Shrimprin お疲れ様です！ メール通知OFF画面へのアクセスにはログインを必要要件としました。再度、レビューお願いいたします🙏

[Ryooo-k]

@Shrimprin 修正しましたのでご確認お願いします！

[Ryooo-k]

@Shrimprin ご確認ありがとうございます！ レイアウトの部分は、一度machidaさんに相談してみます！

[Ryooo-k]

@machida 先日はお時間いただきありがとうございました！ 内容の議事録と方針をまとめておきます。

相談内容

本issueのバグを修正するにあたり、メール通知画面へのアクセスにはログインを必須にしようとしている。layoutには layout 'not_logged_in' を採用しており、ログイン必須の条件とlayout 'not_logged_in'が相違している状態となった。

解決策１

layoutを変更する。この場合はmachidaさんにデザイン変更依頼をする必要がある。

解決策２

ログインを必須とする場合、ユーザビリティが悪くなる。ログインは必須とせず、解読や予測が難しいURIとした方が良い。フィヨルドトーーク！（GitHub）を参考にすると良い。

方針

ログインは不要で進めます。 フィヨルドトーーク！ではidはuuidで生成されるため、解読や予測が難しいURIとなっていました。 本アプリのidはserialで生成されるため、クエリパラメータにSecureRandom.urlsafe_base64メソッドで生成されるUserモデルのカラムのunsubscribe_email_tokenを追加し、そのtokenを用いて判定しようと思います。

[Ryooo-k]

@Shrimprin 修正しましたのでご確認お願いいたします！ 修正のポイントをまとます。

• ログインは不要としました（→そのためlayoutは変更なし）
• 予測不能なtokenからユーザー情報を取得
• クエリパラメータのuser_idとtokenの整合性を確認し、不整合の場合は処理を実行しない

[Shrimprin]

@Ryooo-k 承知しました！いろいろと仕様をご検討いただきありがとうございます 🙏 土日の間に確認させていただきます～ 👍

[Shrimprin]

@Ryooo-k 確認しました！OKだと思います 👍

他のユーザーは知り得ないtokenを使い、メールを受け取った本人しか変更できなくしたのですね！ こういう方法もあるのだな〜勉強になりました 👏

[Ryooo-k]

@Shrimprin ご確認ありがとうございます！ 自分も勉強になりました！色々ご指摘ありがとうございました✨

@komagata メンバーレビューが完了しましたので、メンターレビューをお願いいたします🙇