diff options
-rw-r--r-- | app/controllers/api/base_controller.rb | 2 | ||||
-rw-r--r-- | app/controllers/api/v1/accounts_controller.rb | 26 | ||||
-rw-r--r-- | app/controllers/auth/confirmations_controller.rb | 10 | ||||
-rw-r--r-- | app/controllers/auth/registrations_controller.rb | 1 | ||||
-rw-r--r-- | app/models/user.rb | 7 | ||||
-rw-r--r-- | app/services/app_sign_up_service.rb | 23 | ||||
-rw-r--r-- | app/views/user_mailer/confirmation_instructions.html.haml | 8 | ||||
-rw-r--r-- | app/views/user_mailer/confirmation_instructions.text.erb | 2 | ||||
-rw-r--r-- | config/initializers/rack_attack.rb | 4 | ||||
-rw-r--r-- | config/locales/devise.en.yml | 1 | ||||
-rw-r--r-- | config/routes.rb | 2 | ||||
-rw-r--r-- | db/migrate/20181219235220_add_created_by_application_id_to_users.rb | 8 | ||||
-rw-r--r-- | db/schema.rb | 5 | ||||
-rw-r--r-- | lib/mastodon/accounts_cli.rb | 2 | ||||
-rw-r--r-- | spec/controllers/api/v1/accounts_controller_spec.rb | 34 | ||||
-rw-r--r-- | spec/fabricators/user_fabricator.rb | 1 | ||||
-rw-r--r-- | spec/models/user_spec.rb | 14 | ||||
-rw-r--r-- | spec/services/app_sign_up_service_spec.rb | 41 |
18 files changed, 171 insertions, 20 deletions
diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb index ac8de5fc0..2bf8e82db 100644 --- a/app/controllers/api/base_controller.rb +++ b/app/controllers/api/base_controller.rb @@ -68,7 +68,7 @@ class Api::BaseController < ApplicationController end def require_user! - if current_user && !current_user.disabled? + if current_user && !current_user.disabled? && current_user.confirmed? set_user_activity elsif current_user render json: { error: 'Your login is currently disabled' }, status: 403 diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index f711c4676..6e4084c4e 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -1,14 +1,16 @@ # frozen_string_literal: true class Api::V1::AccountsController < Api::BaseController - before_action -> { authorize_if_got_token! :read, :'read:accounts' }, except: [:follow, :unfollow, :block, :unblock, :mute, :unmute] + before_action -> { authorize_if_got_token! :read, :'read:accounts' }, except: [:create, :follow, :unfollow, :block, :unblock, :mute, :unmute] before_action -> { doorkeeper_authorize! :follow, :'write:follows' }, only: [:follow, :unfollow] before_action -> { doorkeeper_authorize! :follow, :'write:mutes' }, only: [:mute, :unmute] before_action -> { doorkeeper_authorize! :follow, :'write:blocks' }, only: [:block, :unblock] + before_action -> { doorkeeper_authorize! :write, :'write:accounts' }, only: [:create] - before_action :require_user!, except: [:show] - before_action :set_account + before_action :require_user!, except: [:show, :create] + before_action :set_account, except: [:create] before_action :check_account_suspension, only: [:show] + before_action :check_enabled_registrations, only: [:create] respond_to :json @@ -16,6 +18,16 @@ class Api::V1::AccountsController < Api::BaseController render json: @account, serializer: REST::AccountSerializer end + def create + token = AppSignUpService.new.call(doorkeeper_token.application, account_params) + response = Doorkeeper::OAuth::TokenResponse.new(token) + + headers.merge!(response.headers) + + self.response_body = Oj.dump(response.body) + self.status = response.status + end + def follow FollowService.new.call(current_user.account, @account, reblogs: truthy_param?(:reblogs)) @@ -62,4 +74,12 @@ class Api::V1::AccountsController < Api::BaseController def check_account_suspension gone if @account.suspended? end + + def account_params + params.permit(:username, :email, :password, :agreement) + end + + def check_enabled_registrations + forbidden if single_user_mode? || !Setting.open_registrations + end end diff --git a/app/controllers/auth/confirmations_controller.rb b/app/controllers/auth/confirmations_controller.rb index 7af9cbe81..c28c7471c 100644 --- a/app/controllers/auth/confirmations_controller.rb +++ b/app/controllers/auth/confirmations_controller.rb @@ -6,9 +6,9 @@ class Auth::ConfirmationsController < Devise::ConfirmationsController before_action :set_body_classes before_action :set_user, only: [:finish_signup] - # GET/PATCH /users/:id/finish_signup def finish_signup return unless request.patch? && params[:user] + if @user.update(user_params) @user.skip_reconfirmation! bypass_sign_in(@user) @@ -31,4 +31,12 @@ class Auth::ConfirmationsController < Devise::ConfirmationsController def user_params params.require(:user).permit(:email) end + + def after_confirmation_path_for(_resource_name, user) + if user.created_by_application && truthy_param?(:redirect_to_app) + user.created_by_application.redirect_uri + else + super + end + end end diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb index 088832be3..f2a832542 100644 --- a/app/controllers/auth/registrations_controller.rb +++ b/app/controllers/auth/registrations_controller.rb @@ -26,6 +26,7 @@ class Auth::RegistrationsController < Devise::RegistrationsController resource.locale = I18n.locale resource.invite_code = params[:invite_code] if resource.invite_code.blank? + resource.agreement = true resource.build_account if resource.account.nil? end diff --git a/app/models/user.rb b/app/models/user.rb index 44e0d1113..77e48ed4b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -36,6 +36,7 @@ # invite_id :bigint(8) # remember_token :string # chosen_languages :string is an Array +# created_by_application_id :bigint(8) # class User < ApplicationRecord @@ -66,6 +67,7 @@ class User < ApplicationRecord belongs_to :account, inverse_of: :user belongs_to :invite, counter_cache: :uses, optional: true + belongs_to :created_by_application, class_name: 'Doorkeeper::Application', optional: true accepts_nested_attributes_for :account has_many :applications, class_name: 'Doorkeeper::Application', as: :owner @@ -74,6 +76,7 @@ class User < ApplicationRecord validates :locale, inclusion: I18n.available_locales.map(&:to_s), if: :locale? validates_with BlacklistedEmailValidator, if: :email_changed? validates_with EmailMxValidator, if: :validate_email_dns? + validates :agreement, acceptance: { allow_nil: false, accept: [true, 'true', '1'] }, on: :create scope :recent, -> { order(id: :desc) } scope :admins, -> { where(admin: true) } @@ -294,7 +297,7 @@ class User < ApplicationRecord end if resource.blank? - resource = new(email: attributes[:email]) + resource = new(email: attributes[:email], agreement: true) if Devise.check_at_sign && !resource[:email].index('@') resource[:email] = Rpam2.getenv(resource.find_pam_service, attributes[:email], attributes[:password], 'email', false) resource[:email] = "#{attributes[:email]}@#{resource.find_pam_suffix}" unless resource[:email] @@ -307,7 +310,7 @@ class User < ApplicationRecord resource = joins(:account).find_by(accounts: { username: attributes[Devise.ldap_uid.to_sym].first }) if resource.blank? - resource = new(email: attributes[:mail].first, account_attributes: { username: attributes[Devise.ldap_uid.to_sym].first }) + resource = new(email: attributes[:mail].first, agreement: true, account_attributes: { username: attributes[Devise.ldap_uid.to_sym].first }) resource.ldap_setup(attributes) end diff --git a/app/services/app_sign_up_service.rb b/app/services/app_sign_up_service.rb new file mode 100644 index 000000000..1878587e8 --- /dev/null +++ b/app/services/app_sign_up_service.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class AppSignUpService < BaseService + def call(app, params) + return unless allowed_registrations? + + user_params = params.slice(:email, :password, :agreement) + account_params = params.slice(:username) + user = User.create!(user_params.merge(created_by_application: app, password_confirmation: user_params[:password], account_attributes: account_params)) + + Doorkeeper::AccessToken.create!(application: app, + resource_owner_id: user.id, + scopes: app.scopes, + expires_in: Doorkeeper.configuration.access_token_expires_in, + use_refresh_token: Doorkeeper.configuration.refresh_token_enabled?) + end + + private + + def allowed_registrations? + Setting.open_registrations && !Rails.configuration.x.single_user_mode + end +end diff --git a/app/views/user_mailer/confirmation_instructions.html.haml b/app/views/user_mailer/confirmation_instructions.html.haml index 1f088a16f..f75f7529a 100644 --- a/app/views/user_mailer/confirmation_instructions.html.haml +++ b/app/views/user_mailer/confirmation_instructions.html.haml @@ -55,8 +55,12 @@ %tbody %tr %td.button-primary - = link_to confirmation_url(@resource, confirmation_token: @token) do - %span= t 'devise.mailer.confirmation_instructions.action' + - if @resource.created_by_application + = link_to confirmation_url(@resource, confirmation_token: @token, redirect_to_app: 'true') do + %span= t 'devise.mailer.confirmation_instructions.action_with_app', app: @resource.created_by_application.name + - else + = link_to confirmation_url(@resource, confirmation_token: @token) do + %span= t 'devise.mailer.confirmation_instructions.action' %table.email-table{ cellspacing: 0, cellpadding: 0 } %tbody diff --git a/app/views/user_mailer/confirmation_instructions.text.erb b/app/views/user_mailer/confirmation_instructions.text.erb index e01eecb27..65b4626c6 100644 --- a/app/views/user_mailer/confirmation_instructions.text.erb +++ b/app/views/user_mailer/confirmation_instructions.text.erb @@ -4,7 +4,7 @@ <%= t 'devise.mailer.confirmation_instructions.explanation', host: site_hostname %> -=> <%= confirmation_url(@resource, confirmation_token: @token) %> +=> <%= confirmation_url(@resource, confirmation_token: @token, redirect_to_app: @resource.created_by_application ? 'true' : nil) %> <%= strip_tags(t('devise.mailer.confirmation_instructions.extra_html', terms_path: about_more_url, policy_path: terms_url)) %> diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index 8756b8fbf..35302e37b 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -57,6 +57,10 @@ class Rack::Attack req.authenticated_user_id if req.post? && req.path.start_with?('/api/v1/media') end + throttle('throttle_api_sign_up', limit: 5, period: 30.minutes) do |req| + req.ip if req.post? && req.path == '/api/v1/accounts' + end + throttle('protected_paths', limit: 25, period: 5.minutes) do |req| req.ip if req.post? && req.path =~ PROTECTED_PATHS_REGEX end diff --git a/config/locales/devise.en.yml b/config/locales/devise.en.yml index 20938e47b..bd0642b25 100644 --- a/config/locales/devise.en.yml +++ b/config/locales/devise.en.yml @@ -18,6 +18,7 @@ en: mailer: confirmation_instructions: action: Verify email address + action_with_app: Confirm and return to %{app} explanation: You have created an account on %{host} with this email address. You are one click away from activating it. If this wasn't you, please ignore this email. extra_html: Please also check out <a href="%{terms_path}">the rules of the instance</a> and <a href="%{policy_path}">our terms of service</a>. subject: 'Mastodon: Confirmation instructions for %{instance}' diff --git a/config/routes.rb b/config/routes.rb index 7723a08af..808bb5acd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -336,7 +336,7 @@ Rails.application.routes.draw do resources :relationships, only: :index end - resources :accounts, only: [:show] do + resources :accounts, only: [:create, :show] do resources :statuses, only: :index, controller: 'accounts/statuses' resources :followers, only: :index, controller: 'accounts/follower_accounts' resources :following, only: :index, controller: 'accounts/following_accounts' diff --git a/db/migrate/20181219235220_add_created_by_application_id_to_users.rb b/db/migrate/20181219235220_add_created_by_application_id_to_users.rb new file mode 100644 index 000000000..17ce900af --- /dev/null +++ b/db/migrate/20181219235220_add_created_by_application_id_to_users.rb @@ -0,0 +1,8 @@ +class AddCreatedByApplicationIdToUsers < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def change + add_reference :users, :created_by_application, foreign_key: { to_table: 'oauth_applications', on_delete: :nullify }, index: false + add_index :users, :created_by_application_id, algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index 51a7b5e74..e47960b16 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2018_12_13_185533) do +ActiveRecord::Schema.define(version: 2018_12_19_235220) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -637,8 +637,10 @@ ActiveRecord::Schema.define(version: 2018_12_13_185533) do t.bigint "invite_id" t.string "remember_token" t.string "chosen_languages", array: true + t.bigint "created_by_application_id" t.index ["account_id"], name: "index_users_on_account_id" t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true + t.index ["created_by_application_id"], name: "index_users_on_created_by_application_id" t.index ["email"], name: "index_users_on_email", unique: true t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true end @@ -730,6 +732,7 @@ ActiveRecord::Schema.define(version: 2018_12_13_185533) do add_foreign_key "subscriptions", "accounts", name: "fk_9847d1cbb5", on_delete: :cascade add_foreign_key "users", "accounts", name: "fk_50500f500d", on_delete: :cascade add_foreign_key "users", "invites", on_delete: :nullify + add_foreign_key "users", "oauth_applications", column: "created_by_application_id", on_delete: :nullify add_foreign_key "web_push_subscriptions", "oauth_access_tokens", column: "access_token_id", on_delete: :cascade add_foreign_key "web_push_subscriptions", "users", on_delete: :cascade add_foreign_key "web_settings", "users", name: "fk_11910667b2", on_delete: :cascade diff --git a/lib/mastodon/accounts_cli.rb b/lib/mastodon/accounts_cli.rb index b21968223..bbda244ea 100644 --- a/lib/mastodon/accounts_cli.rb +++ b/lib/mastodon/accounts_cli.rb @@ -73,7 +73,7 @@ module Mastodon def create(username) account = Account.new(username: username) password = SecureRandom.hex - user = User.new(email: options[:email], password: password, admin: options[:role] == 'admin', moderator: options[:role] == 'moderator', confirmed_at: Time.now.utc) + user = User.new(email: options[:email], password: password, agreement: true, admin: options[:role] == 'admin', moderator: options[:role] == 'moderator', confirmed_at: options[:confirmed] ? Time.now.utc : nil) if options[:reattach] account = Account.find_local(username) || Account.new(username: username) diff --git a/spec/controllers/api/v1/accounts_controller_spec.rb b/spec/controllers/api/v1/accounts_controller_spec.rb index c506fb5f0..f5f65c000 100644 --- a/spec/controllers/api/v1/accounts_controller_spec.rb +++ b/spec/controllers/api/v1/accounts_controller_spec.rb @@ -19,6 +19,40 @@ RSpec.describe Api::V1::AccountsController, type: :controller do end end + describe 'POST #create' do + let(:app) { Fabricate(:application) } + let(:token) { Doorkeeper::AccessToken.find_or_create_for(app, nil, 'read write', nil, false) } + let(:agreement) { nil } + + before do + post :create, params: { username: 'test', password: '12345678', email: 'hello@world.tld', agreement: agreement } + end + + context 'given truthy agreement' do + let(:agreement) { 'true' } + + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'returns a new access token as JSON' do + expect(body_as_json[:access_token]).to_not be_blank + end + + it 'creates a user' do + user = User.find_by(email: 'hello@world.tld') + expect(user).to_not be_nil + expect(user.created_by_application_id).to eq app.id + end + end + + context 'given no agreement' do + it 'returns http unprocessable entity' do + expect(response).to have_http_status(422) + end + end + end + describe 'GET #show' do let(:scopes) { 'read:accounts' } diff --git a/spec/fabricators/user_fabricator.rb b/spec/fabricators/user_fabricator.rb index 7dfbdb52d..8f5956501 100644 --- a/spec/fabricators/user_fabricator.rb +++ b/spec/fabricators/user_fabricator.rb @@ -3,4 +3,5 @@ Fabricator(:user) do email { sequence(:email) { |i| "#{i}#{Faker::Internet.email}" } } password "123456789" confirmed_at { Time.zone.now } + agreement true end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index c82919597..856254ce4 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -106,19 +106,19 @@ RSpec.describe User, type: :model do end it 'should allow a non-blacklisted user to be created' do - user = User.new(email: 'foo@example.com', account: account, password: password) + user = User.new(email: 'foo@example.com', account: account, password: password, agreement: true) expect(user.valid?).to be_truthy end it 'should not allow a blacklisted user to be created' do - user = User.new(email: 'foo@mvrht.com', account: account, password: password) + user = User.new(email: 'foo@mvrht.com', account: account, password: password, agreement: true) expect(user.valid?).to be_falsey end it 'should not allow a subdomain blacklisted user to be created' do - user = User.new(email: 'foo@mvrht.com.topdomain.tld', account: account, password: password) + user = User.new(email: 'foo@mvrht.com.topdomain.tld', account: account, password: password, agreement: true) expect(user.valid?).to be_falsey end @@ -210,17 +210,17 @@ RSpec.describe User, type: :model do end it 'should not allow a user to be created unless they are whitelisted' do - user = User.new(email: 'foo@example.com', account: account, password: password) + user = User.new(email: 'foo@example.com', account: account, password: password, agreement: true) expect(user.valid?).to be_falsey end it 'should allow a user to be created if they are whitelisted' do - user = User.new(email: 'foo@mastodon.space', account: account, password: password) + user = User.new(email: 'foo@mastodon.space', account: account, password: password, agreement: true) expect(user.valid?).to be_truthy end it 'should not allow a user with a whitelisted top domain as subdomain in their email address to be created' do - user = User.new(email: 'foo@mastodon.space.userdomain.com', account: account, password: password) + user = User.new(email: 'foo@mastodon.space.userdomain.com', account: account, password: password, agreement: true) expect(user.valid?).to be_falsey end @@ -242,7 +242,7 @@ RSpec.describe User, type: :model do it_behaves_like 'Settings-extended' do def create! - User.create!(account: Fabricate(:account), email: 'foo@mastodon.space', password: 'abcd1234') + User.create!(account: Fabricate(:account), email: 'foo@mastodon.space', password: 'abcd1234', agreement: true) end def fabricate diff --git a/spec/services/app_sign_up_service_spec.rb b/spec/services/app_sign_up_service_spec.rb new file mode 100644 index 000000000..d480df348 --- /dev/null +++ b/spec/services/app_sign_up_service_spec.rb @@ -0,0 +1,41 @@ +require 'rails_helper' + +RSpec.describe AppSignUpService, type: :service do + let(:app) { Fabricate(:application, scopes: 'read write') } + let(:good_params) { { username: 'alice', password: '12345678', email: 'good@email.com', agreement: true } } + + subject { described_class.new } + + describe '#call' do + it 'returns nil when registrations are closed' do + Setting.open_registrations = false + expect(subject.call(app, good_params)).to be_nil + end + + it 'raises an error when params are missing' do + expect { subject.call(app, {}) }.to raise_error ActiveRecord::RecordInvalid + end + + it 'creates an unconfirmed user with access token' do + access_token = subject.call(app, good_params) + expect(access_token).to_not be_nil + user = User.find_by(id: access_token.resource_owner_id) + expect(user).to_not be_nil + expect(user.confirmed?).to be false + end + + it 'creates access token with the app\'s scopes' do + access_token = subject.call(app, good_params) + expect(access_token).to_not be_nil + expect(access_token.scopes.to_s).to eq 'read write' + end + + it 'creates an account' do + access_token = subject.call(app, good_params) + expect(access_token).to_not be_nil + user = User.find_by(id: access_token.resource_owner_id) + expect(user).to_not be_nil + expect(user.account).to_not be_nil + end + end +end |