Skip to content

Commit

Permalink
Require agreement param to be true in the API when creating an account
Browse files Browse the repository at this point in the history
  • Loading branch information
Gargron committed Dec 24, 2018
1 parent 232e3dc commit 2bf7bb5
Show file tree
Hide file tree
Showing 9 changed files with 37 additions and 23 deletions.
2 changes: 1 addition & 1 deletion app/controllers/api/v1/accounts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def check_account_suspension
end

def account_params
params.permit(:username, :email, :password)
params.permit(:username, :email, :password, :agreement)
end

def check_enabled_registrations
Expand Down
1 change: 1 addition & 0 deletions app/controllers/auth/registrations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def build_resource(hash = nil)

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
Expand Down
5 changes: 3 additions & 2 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,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) }
Expand Down Expand Up @@ -296,7 +297,7 @@ def self.pam_get_user(attributes = {})
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]
Expand All @@ -309,7 +310,7 @@ def self.ldap_get_user(attributes = {})
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

Expand Down
2 changes: 1 addition & 1 deletion app/services/app_sign_up_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class AppSignUpService < BaseService
def call(app, params)
return unless allowed_registrations?

user_params = params.slice(:email, :password)
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))

Expand Down
2 changes: 1 addition & 1 deletion lib/mastodon/accounts_cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def rotate(username = nil)
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)
Expand Down
31 changes: 21 additions & 10 deletions spec/controllers/api/v1/accounts_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,23 +22,34 @@
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' }
post :create, params: { username: 'test', password: '12345678', email: 'hello@world.tld', agreement: agreement }
end

it 'returns http success' do
expect(response).to have_http_status(200)
end
context 'given truthy agreement' do
let(:agreement) { 'true' }

it 'returns a new access token as JSON' do
expect(body_as_json[:access_token]).to_not be_blank
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

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
context 'given no agreement' do
it 'returns http unprocessable entity' do
expect(response).to have_http_status(422)
end
end
end

Expand Down
1 change: 1 addition & 0 deletions spec/fabricators/user_fabricator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@
email { sequence(:email) { |i| "#{i}#{Faker::Internet.email}" } }
password "123456789"
confirmed_at { Time.zone.now }
agreement true
end
14 changes: 7 additions & 7 deletions spec/models/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,19 +106,19 @@
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
Expand Down Expand Up @@ -210,17 +210,17 @@
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

Expand All @@ -242,7 +242,7 @@

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
Expand Down
2 changes: 1 addition & 1 deletion spec/services/app_sign_up_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

RSpec.describe AppSignUpService, type: :service do
let(:app) { Fabricate(:application, scopes: 'read write') }
let(:good_params) { { username: 'alice', password: '12345678', email: 'good@email.com' } }
let(:good_params) { { username: 'alice', password: '12345678', email: 'good@email.com', agreement: true } }

subject { described_class.new }

Expand Down

0 comments on commit 2bf7bb5

Please sign in to comment.