Skip to content
This repository has been archived by the owner on Feb 28, 2022. It is now read-only.

Qiita login #8

Merged
merged 12 commits into from
May 22, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .env.production.sample
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,6 @@ SMTP_FROM_ADDRESS=notifications@example.com
STREAMING_CLUSTER_NUM=1

SENTRY_DSN=

QIITA_CLIENT_ID=
QIITA_CLIENT_SECRET=
3 changes: 3 additions & 0 deletions .env.sample
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,6 @@ OTP_SECRET=RANDOM_STRING
# Cluster number setting for streaming API server.
# If you comment out following line, cluster number will be `numOfCpuCores - 1`.
STREAMING_CLUSTER_NUM=1

QIITA_CLIENT_ID=
QIITA_CLIENT_SECRET=
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ gem 'link_header'
gem 'local_time'
gem 'nokogiri'
gem 'oj'
gem 'omniauth'
gem 'omniauth_qiita'
gem 'ostatus2', '~> 2.0'
gem 'ox'
gem 'rabl'
Expand Down
23 changes: 22 additions & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ GEM
fabrication (2.16.1)
faker (1.7.3)
i18n (~> 0.5)
faraday (0.12.1)
faraday (0.11.0)
multipart-post (>= 1.2, < 3)
fast_blank (1.0.0)
font-awesome-rails (4.7.0.1)
Expand All @@ -183,6 +183,7 @@ GEM
hamlit (>= 1.2.0)
railties (>= 4.0.1)
hashdiff (0.3.2)
hashie (3.5.5)
highline (1.7.8)
hiredis (0.6.1)
htmlentities (4.3.4)
Expand Down Expand Up @@ -216,6 +217,7 @@ GEM
railties (>= 4.2.0)
thor (>= 0.14, < 2.0)
json (2.1.0)
jwt (1.5.6)
kaminari (1.0.1)
activesupport (>= 4.1.0)
kaminari-actionview (= 1.0.1)
Expand Down Expand Up @@ -258,6 +260,8 @@ GEM
mimemagic (0.3.2)
mini_portile2 (2.1.0)
minitest (5.10.1)
multi_json (1.12.1)
multi_xml (0.6.0)
multipart-post (2.0.0)
net-scp (1.2.1)
net-ssh (>= 2.6.5)
Expand All @@ -267,7 +271,22 @@ GEM
mini_portile2 (~> 2.1.0)
nokogumbo (1.4.10)
nokogiri
oauth2 (1.3.1)
faraday (>= 0.8, < 0.12)
jwt (~> 1.0)
multi_json (~> 1.3)
multi_xml (~> 0.5)
rack (>= 1.2, < 3)
oj (3.0.2)
omniauth (1.6.1)
hashie (>= 3.4.6, < 3.6.0)
rack (>= 1.6.2, < 3)
omniauth-oauth2 (1.4.0)
oauth2 (~> 1.0)
omniauth (~> 1.2)
omniauth_qiita (0.1.0)
multi_json (~> 1.10)
omniauth-oauth2 (~> 1.2)
openssl (2.0.3)
orm_adapter (0.5.0)
ostatus2 (2.0.0)
Expand Down Expand Up @@ -537,6 +556,8 @@ DEPENDENCIES
microformats2
nokogiri
oj
omniauth
omniauth_qiita
ostatus2 (~> 2.0)
ox
paperclip (~> 5.1)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { Link } from 'react-router';
import { FormattedMessage } from 'react-intl';
import PropTypes from 'prop-types';

export default class AlertBar extends React.Component {

render () {
const { isEmailConfirmed } = this.props;

return (
<div className='alert-bar'>
{
(!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.' />
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

@tomoasleep tomoasleep May 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

2017-05-18 23 30 35

</div>
)
}
</div>
);
}

}

AlertBar.propTypes = {
isEmailConfirmed: PropTypes.bool.isRequired,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { connect } from 'react-redux';
import AlertBar from '../components/alert_bar';

const mapStateToProps = state => ({
isEmailConfirmed: state.getIn(['meta', 'is_email_confirmed']),
});

export default connect(mapStateToProps)(AlertBar);
2 changes: 2 additions & 0 deletions app/assets/javascripts/components/features/ui/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import LoadingBarContainer from './containers/loading_bar_container';
import HomeTimeline from '../home_timeline';
import Compose from '../compose';
import TabsBar from './components/tabs_bar';
import AlertBarContainer from './containers/alert_bar_container';
import ModalContainer from './containers/modal_container';
import Notifications from '../notifications';
import { connect } from 'react-redux';
Expand Down Expand Up @@ -144,6 +145,7 @@ class UI extends React.PureComponent {

return (
<div className='ui' ref={this.setRef}>
<AlertBarContainer />
<TabsBar />

{mountedColumns}
Expand Down
1 change: 1 addition & 0 deletions app/assets/javascripts/components/locales/en.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const en = {
"account.unblock": "Unblock @{name}",
"account.unfollow": "Unfollow",
"account.unmute": "Unmute @{name}",
"alert_bar.email_confirm_alert": "Your email address is not confirmed. Please confirm the sent email in 24 hours after registration.",
"boost_modal.combo": "You can press {combo} to skip this next time",
"column.blocks": "Blocked users",
"column.community": "Local timeline",
Expand Down
1 change: 1 addition & 0 deletions app/assets/javascripts/components/locales/ja.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const ja = {
"account.unblock": "ブロック解除",
"account.unfollow": "フォロー解除",
"account.unmute": "ミュート解除",
"alert_bar.email_confirm_alert": "メールアドレスの確認が完了していません。登録後24時間以内に、登録したアドレス宛に送信されたメールを確認してください。",
"boost_modal.combo": "次からは{combo}を押せば、これをスキップできます。",
"column.blocks": "ブロックしたユーザー",
"column.community": "ローカルタイムライン",
Expand Down
11 changes: 11 additions & 0 deletions app/assets/stylesheets/alert.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
.alert-bar {
.alert {
border: 1px solid transparent;
border-radius: 4px;
padding: 12px;

color: DarkGoldenrod;
background-color: Beige;
border-color: Goldenrod;
}
}
2 changes: 2 additions & 0 deletions app/assets/stylesheets/application.scss
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,6 @@
@import 'rtl';

@import 'themes';
@import 'alert';
@import 'code';
@import 'qiita';
56 changes: 56 additions & 0 deletions app/assets/stylesheets/qiita.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
.qiita {
.button {
display: inline-block;
margin-bottom: 0;
font-weight: normal;
text-align: center;
vertical-align: middle;
touch-action: manipulation;
cursor: pointer;
background-image: none;
border: 1px solid transparent;
white-space: nowrap;
padding: 6px 12px;
font-size: 14px;
line-height: 1.42857;
border-radius: 3px;
-webkit-user-select: none;
-moz-user-select: none;
-ms-user-select: none;
user-select: none;
}

.button-primary {
border-color: #4ea30a;
background-color: #59bb0c;
color: #fff;
}

.registration {
width: 100%;
text-transform: none;
}
}

.registrations {
display: flex;
flex-direction: column;
align-items: center;
justify-content: center;

.qiita {
width: 300px;
margin: 5px auto;
text-align: center;
}

.password_registration_closed_caution {
width: 300px;
margin: 10px auto;
}

.info a {
color: white;
text-transform: none;
}
}
2 changes: 1 addition & 1 deletion app/controllers/admin/settings_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

module Admin
class SettingsController < BaseController
BOOLEAN_SETTINGS = %w(open_registrations).freeze
BOOLEAN_SETTINGS = %w(open_registrations prohibit_registrations_except_qiita_oauth).freeze

def index
@settings = Setting.all_as_records
Expand Down
34 changes: 34 additions & 0 deletions app/controllers/auth/omniauth_callbacks_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# frozen_string_literal: true

class Auth::OmniauthCallbacksController < Devise::OmniauthCallbacksController
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

@takashi takashi May 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Copy link

@takashi takashi May 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

if current_user
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

authorization = QiitaAuthorization.find_or_initialize_by(uid: auth_hash[:uid]) do |qiita_authorization|
authorization.user = current_user
end

if authorization.save
flash[:notice] = I18n.t('omniauth_callbacks.success')
else
flash[:alert] = I18n.t('omniauth_callbacks.failure')
end
redirect_to settings_qiita_authorizations_path
else
if authorization = QiitaAuthorization.find_by(uid: auth_hash[:uid])
sign_in(authorization.user)
redirect_to web_path
else
store_omniauth_auth
redirect_to new_user_oauth_registration_path
end
end
end

private

def store_omniauth_auth
session[:devise_omniauth_auth] = request.env['omniauth.auth']
end
end
10 changes: 9 additions & 1 deletion app/controllers/auth/registrations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,15 @@ def after_inactive_sign_up_path_for(_resource)
end

def check_enabled_registrations
redirect_to root_path if single_user_mode? || !Setting.open_registrations
redirect_to root_path if single_user_mode? || !Setting.open_registrations || Setting.prohibit_registrations_except_qiita_oauth
end

def update_resource(resource, params)
if resource.try(:has_dummy_password?)
resource.update_without_current_password(params)
else
super
end
end

private
Expand Down
40 changes: 40 additions & 0 deletions app/controllers/oauth_registrations_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
class OauthRegistrationsController < DeviseController
layout 'auth'

before_action :check_enabled_registrations
before_action :require_omniauth_auth
before_action :require_no_authentication

def new
@oauth_registration = Form::OauthRegistration.from_omniauth_auth(omniauth_auth)
end

def create
@oauth_registration = Form::OauthRegistration.from_omniauth_auth(omniauth_auth)
@oauth_registration.assign_attributes(
params.require(:form_oauth_registration).permit(:email, :username, :password, :password_confirmation).merge(locale: I18n.locale)
)

if @oauth_registration.save
sign_in(@oauth_registration.user)
redirect_to web_path
flash[:notice] = I18n.t('oauth_registration.success')
else
render :new, status: :unprocessable_entity
end
end

private

def omniauth_auth
@omniauth_auth ||= session[:devise_omniauth_auth].try(:deep_symbolize_keys)
end

def check_enabled_registrations
redirect_to root_path if single_user_mode? || !Setting.open_registrations
end

def require_omniauth_auth
redirect_to root_path unless omniauth_auth
end
end
12 changes: 12 additions & 0 deletions app/controllers/settings/qiita_authorizations_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# frozen_string_literal: true

class Settings::QiitaAuthorizationsController < ApplicationController
layout 'admin'

before_action :authenticate_user!

def show
@account = current_account
@qiita_authorization = current_account.user.qiita_authorization
end
end
Loading