Qiita login #8

tomoasleep · 2017-05-07T14:39:07Z

QiitaのOAuthを利用してアカウント作成とログインが行えるようにする。

仕様

ホーム画面 ( /about )とログイン画面 ( /session/new ) に「Qiitaアカウントで参加ボタン」が追加される
ボタンを押すと、紐付いている場合はログインが行える。
- 紐付いていない場合は紐付け作業を行う
  - ログインしていない場合は専用の登録フォーム ( `` ) でアカウントの登録作業を行う
    - 専用フォームではメールアドレスとアカウント名を入力してMastodonアカウントの新規作成を行う
    - パスワードはランダムな文字列がダミーのパスワードとして入る
      - ユーザーには知らされないので再設定するまでパスワードログインできない
      - 設定画面からパスワードを設定することが出来る。最初の1回だけ現在のパスワードを入れずにパスワードを変更することが出来る
  - ログインしている場合では、MastodonアカウントとQiitaアカウントを紐付ける
設定画面から紐付けが行える
- 現在は紐付け解除はできない

動作を見たい場合の注意

QIitaで自分のアカウントでアプリケーションを作成し、.env に適宜以下の内容を追加しておいてください。

# XXXXを自分のアプリケーションのtokenの値に変える。
QIITA_CLIENT_ID=XXXX
QIITA_CLIENT_SECRET=XXXX

tomoasleep · 2017-05-11T10:18:25Z

@takashi @yujinakayama レビューお願いします 🙏

tomoasleep · 2017-05-12T03:09:40Z

@takashi の助言でCookieStoreにAuthHash保存するようにした。

tomoasleep · 2017-05-15T12:41:43Z

そういえば、メール認証をしないとログインできないので新規登録までに、

Qiitaで参加ボタンを押す
新規登録フォームでユーザーネームとメールアドレスを送信する
メール認証をする
Qiitaで参加ボタンを押してログイン

…と使えるようになるまでに2回Qiitaで参加ボタンを押す必要があるのが面倒だ…

takashi · 2017-05-16T05:44:07Z

app/models/oauth_authorization.rb

@@ -0,0 +1,3 @@
+class OauthAuthorization < ApplicationRecord


OauthAuthorization という名前、 Authorization と2回行っているようで不自然なのと、 Oauth(という仕様)とのAuthorization のようにとれ、違和感があります。

この場合 Qiita との Authorization なわけなので、QiitaAuthorization とするのはどうでしょうか

takashi · 2017-05-16T05:51:02Z

app/models/form/oauth_registration.rb

+    User.new(
+      email: email,
+      locale: locale,
+      password: password,


ここで password をユーザの代わりに追加しているけど、このパスワードはユーザーに知らされる？

知らされないので、パスワードを再発行するまでパスワード認証は実質利用できない状態です。

dummy_password_flag を足して初回だけ現在のパスワードを入力しなくてもパスワードを変更できるようにした

初回のみ現在のパスワードを入力するinputを非表示にしている。

最初にパスワードを変更する場合だけ 新しい じゃなくした

takashi · 2017-05-16T06:07:52Z

app/controllers/auth/omniauth_callbacks_controller.rb

+  private
+
+  def store_omniauth_auth
+    session[:devise_omniauth_auth] = request.env['omniauth.auth'].to_json,


cookie store を使用しているならば、serializer を marshal にしていても json にしていても hash はそのまま格納できるはずです。 to_json 必要なさそう

tomoasleep · 2017-05-18T11:01:57Z

新規登録の認証の仕様の変更

現在の仕様だと、Qiitaアカウントで新規登録してログインした状態になるまで都度2回QiitaのOAuthの認可画面を表示する必要があるので、新規登録してから1日の間はメール認証しなくてもログイン出来るように仕様を変更する。その場合、画面上部にアラートが表示される。

tomoasleep · 2017-05-18T11:04:51Z

負荷対策用の、新規登録の制限

(負荷対策のために、)管理画面からQiita連携以外での新規登録を停止することも出来るようにする。

tomoasleep · 2017-05-18T11:56:13Z

@takashi 再度レビューお願いします

takashi · 2017-05-18T12:16:55Z

app/controllers/auth/omniauth_callbacks_controller.rb

@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+class Auth::OmniauthCallbacksController < Devise::OmniauthCallbacksController


今更なのですが、ここで行っているのは結局 QiitaAuthorization モデルの作成ですし、Setting の方も QiitaAuthorizationsController なのでAuth::QiitaAuthorizationsController が適当なのではないかと思います 🙇

QiitaAuthorizationsを確かに作成してはいるのですが、どちらかと言うとここはDeviseの提供する仕組みの上でOmniauthのcallbackを処理する性格のほうが強いので、Deviseの命名に寄せていた方が良いと考えているのですが、どう思いますか？

Omniauthのcallbackを処理するが、実際には Oauth Server 側 (今回だとQiita) で明示的にResource Owner のリソースを OauthClient 受け渡すための callback url を記述しているので、omniauth という gem が提供している仕組みではあるが QiitaAuthorization を作成する、という考え方がしっくりくるのかな、と思った。
けど特に強い意見があるわけではないのでこのままでも大丈夫です 🙆‍♂️

補足すると、 qiita というアクションに違和感があって、アクションは原則 crud のみで構成されるべき、という rails の controller の実装的にこうすればいいんじゃないか、とおもった、というのもあります (Qiita:Authorization#create でもいいのではという意味)

qiita というアクションに違和感があって、アクションは原則 crud のみで構成されるべき、という rails の controller の実装的にこうすればいいんじゃないか

これはたしかにそうだと思いますが、今回は素直に提供する仕組みに乗っかって様子見ようと思います。forkなので。

なるほど、本家がそういう実装なのか。 okです 👌

takashi · 2017-05-18T12:22:45Z

app/assets/javascripts/components/features/ui/components/alert_bar.jsx

+}
+
+AlertBar.propTypes = {
+  isEmailConfirmed: PropTypes.bool,


これは require 属性をつけなくても大丈夫ですか？

付けといたほうが良さそうですね、thx 🙇

takashi · 2017-05-18T12:27:51Z

app/assets/javascripts/components/features/ui/components/alert_bar.jsx

+        {
+          (!isEmailConfirmed &&
+            <div className='alert'>
+              <i className='fa fa-fw fa-exclamation-triangle' /><FormattedMessage id='alert_bar.email_confirm_alert' defaultMessage='Your email address is not confirmed. Please confirm the sent email.' />


defaultMessage が使われるのはどのようなケースですか？

Mastodonはja, en以外のlocaleを用意していて、それらの他のlocaleを使用している場合に使われます。

takashi · 2017-05-18T12:38:12Z

app/controllers/auth/omniauth_callbacks_controller.rb

+      else
+        flash[:alert] = I18n.t('omniauth_callbacks.failure')
+      end
+        redirect_to settings_qiita_authorizations_path


インデントが1つ深いかも

takashi · 2017-05-18T12:51:44Z

app/controllers/auth/omniauth_callbacks_controller.rb

+  def qiita
+    auth_hash = request.env['omniauth.auth']
+
+    if current_user


この「current_user が存在した場合」というのは既存の password 認証のアカウントに Qiita アカウントに紐付けるというプロセスなので、今すぐには必要ない部分ですか？(消す必要はないと思っているが確認です)

はい、現状使われることはない部分です 🙆

tomoasleep · 2017-05-19T02:54:23Z

@takashi 修正 or コメントしたので再度レビューお願いします 🙏

takashi · 2017-05-19T12:01:35Z

app/assets/javascripts/components/locales/ja.jsx

@@ -14,6 +14,7 @@ const ja = {
  "account.unblock": "ブロック解除",
  "account.unfollow": "フォロー解除",
  "account.unmute": "ミュート解除",
+  "alert_bar.email_confirm_alert": "メールアドレスの確認が完了していません。登録したアドレス宛に送信されたメールを確認してください。",


1日の期限をもってログインできなくなるよ、という旨をここで書いたほうが親切かなと思ったけどどうでしょう？

確かにそうですね👍

直した ✌️

takashi

LGTM

tomoasleep · 2017-05-22T05:19:39Z

@takashi レビューthx 🙏

Conflicts: Gemfile Gemfile.lock app/javascript/mastodon/features/compose/index.js app/serializers/initial_state_serializer.rb app/views/about/_registration.html.haml app/views/admin/settings/edit.html.haml app/views/auth/registrations/edit.html.haml app/views/shared/_og.html.haml config/initializers/omniauth.rb package.json Look at f6df9d0 . The diff is too difficult to read however, in file app/views/auth/registrations/edit.html.haml, `simple_form.labels.defaults.password` was suddenly replaced to `simple_form.labels.defaults.new_password`. This can be proved by this file on ef27a0b which is a parent commit of that. However, this replacement has been continued for about 3 years. And there is no significant difference. So, I decided not to recover this replacement. ref: - 1cd0497 - increments#8 - f4d549d - mastodon#8703

tomoasleep force-pushed the qiita-login branch 2 times, most recently from e3b5ad2 to 4cb7f0e Compare May 10, 2017 07:52

tomoasleep changed the title ~~[WIP] Qiita login~~ Qiita login May 11, 2017

takashi suggested changes May 16, 2017

View reviewed changes

tomoasleep force-pushed the qiita-login branch 4 times, most recently from 0057a13 to b7aee85 Compare May 18, 2017 10:46

tomoasleep force-pushed the qiita-login branch from b7aee85 to 1cb0d61 Compare May 18, 2017 11:14

takashi suggested changes May 18, 2017

View reviewed changes

tomoasleep force-pushed the qiita-login branch from fd75b67 to 229e63c Compare May 19, 2017 05:43

takashi reviewed May 19, 2017

View reviewed changes

tomoasleep added 11 commits May 22, 2017 13:50

Add OauthAuthorization model

df7153d

Add registration page with qiita account

bdbcc38

Add a button to login with qiita account

6951186

Add setting page for qiita integration

7f47227

Rename OauthAuthorization to QiitaAuthorization

7ba531e

Don't serialize auth hash in controller

f96d0e9

Enable users to update password without inputing current password once

1cd0497

Allow unconfirmed access for a day after signup

252719d

Add site setting to disable password registration

af8d68b

fixup! Allow unconfirmed access for a day after signup

ad2400b

fixup! Add registration page with qiita account

1c88c77

fixup! Allow unconfirmed access for a day after signup

28462ed

tomoasleep force-pushed the qiita-login branch from da674e2 to 28462ed Compare May 22, 2017 04:53

takashi approved these changes May 22, 2017

View reviewed changes

tomoasleep merged commit 75748eb into increments:increments May 22, 2017

tomoasleep deleted the qiita-login branch May 22, 2017 05:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Qiita login #8

Qiita login #8

tomoasleep commented May 7, 2017 •

edited

Loading

tomoasleep commented May 11, 2017

tomoasleep commented May 12, 2017

tomoasleep commented May 15, 2017

takashi May 16, 2017

takashi May 16, 2017

tomoasleep May 17, 2017

tomoasleep May 18, 2017

tomoasleep May 18, 2017

takashi May 16, 2017

tomoasleep commented May 18, 2017

tomoasleep commented May 18, 2017

tomoasleep commented May 18, 2017

takashi May 18, 2017

tomoasleep May 18, 2017

takashi May 19, 2017

takashi May 19, 2017 •

edited

Loading

tomoasleep May 22, 2017

takashi May 22, 2017 •

edited

Loading

takashi May 18, 2017

tomoasleep May 18, 2017

takashi May 18, 2017

tomoasleep May 18, 2017 •

edited

Loading

takashi May 18, 2017

tomoasleep May 18, 2017

takashi May 18, 2017

tomoasleep May 18, 2017

tomoasleep commented May 19, 2017

takashi May 19, 2017

tomoasleep May 19, 2017

tomoasleep May 22, 2017

takashi left a comment

tomoasleep commented May 22, 2017

		@@ -0,0 +1,34 @@
		# frozen_string_literal: true

		class Auth::OmniauthCallbacksController < Devise::OmniauthCallbacksController

Qiita login #8

Qiita login #8

Conversation

tomoasleep commented May 7, 2017 • edited Loading

仕様

動作を見たい場合の注意

tomoasleep commented May 11, 2017

tomoasleep commented May 12, 2017

tomoasleep commented May 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomoasleep commented May 18, 2017

新規登録の認証の仕様の変更

tomoasleep commented May 18, 2017

負荷対策用の、新規登録の制限

tomoasleep commented May 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takashi May 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takashi May 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomoasleep May 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomoasleep commented May 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takashi left a comment

Choose a reason for hiding this comment

tomoasleep commented May 22, 2017

tomoasleep commented May 7, 2017 •

edited

Loading

takashi May 19, 2017 •

edited

Loading

takashi May 22, 2017 •

edited

Loading

tomoasleep May 18, 2017 •

edited

Loading