From bd87e6667975bc3b5bfaf3e1cdff97e041ed4c98 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 21 Jul 2019 04:08:00 +0200 Subject: Remove WebSub subscriptions (#11303) --- spec/fabricators/subscription_fabricator.rb | 7 --- spec/models/subscription_spec.rb | 67 ---------------------- spec/policies/subscription_policy_spec.rb | 24 -------- .../services/batched_remove_status_service_spec.rb | 3 - spec/services/remove_status_service_spec.rb | 3 - spec/services/suspend_account_service_spec.rb | 8 +-- 6 files changed, 2 insertions(+), 110 deletions(-) delete mode 100644 spec/fabricators/subscription_fabricator.rb delete mode 100644 spec/models/subscription_spec.rb delete mode 100644 spec/policies/subscription_policy_spec.rb (limited to 'spec') diff --git a/spec/fabricators/subscription_fabricator.rb b/spec/fabricators/subscription_fabricator.rb deleted file mode 100644 index 347dab5df..000000000 --- a/spec/fabricators/subscription_fabricator.rb +++ /dev/null @@ -1,7 +0,0 @@ -Fabricator(:subscription) do - account - callback_url "http://example.com/callback" - secret "foobar" - expires_at "2016-11-28 11:30:07" - confirmed false -end diff --git a/spec/models/subscription_spec.rb b/spec/models/subscription_spec.rb deleted file mode 100644 index b83979d13..000000000 --- a/spec/models/subscription_spec.rb +++ /dev/null @@ -1,67 +0,0 @@ -require 'rails_helper' - -RSpec.describe Subscription, type: :model do - let(:alice) { Fabricate(:account, username: 'alice') } - - subject { Fabricate(:subscription, account: alice) } - - describe '#expired?' do - it 'return true when expires_at is past' do - subject.expires_at = 2.days.ago - expect(subject.expired?).to be true - end - - it 'return false when expires_at is future' do - subject.expires_at = 2.days.from_now - expect(subject.expired?).to be false - end - end - - describe 'lease_seconds' do - it 'returns the time remaining until expiration' do - datetime = 1.day.from_now - subscription = Subscription.new(expires_at: datetime) - travel_to(datetime - 12.hours) do - expect(subscription.lease_seconds).to eq(12.hours) - end - end - end - - describe 'lease_seconds=' do - it 'sets expires_at to min expiration when small value is provided' do - subscription = Subscription.new - datetime = 1.day.from_now - too_low = Subscription::MIN_EXPIRATION - 1000 - travel_to(datetime) do - subscription.lease_seconds = too_low - end - - expected = datetime + Subscription::MIN_EXPIRATION.seconds - expect(subscription.expires_at).to be_within(1.0).of(expected) - end - - it 'sets expires_at to value when valid value is provided' do - subscription = Subscription.new - datetime = 1.day.from_now - valid = Subscription::MIN_EXPIRATION + 1000 - travel_to(datetime) do - subscription.lease_seconds = valid - end - - expected = datetime + valid.seconds - expect(subscription.expires_at).to be_within(1.0).of(expected) - end - - it 'sets expires_at to max expiration when large value is provided' do - subscription = Subscription.new - datetime = 1.day.from_now - too_high = Subscription::MAX_EXPIRATION + 1000 - travel_to(datetime) do - subscription.lease_seconds = too_high - end - - expected = datetime + Subscription::MAX_EXPIRATION.seconds - expect(subscription.expires_at).to be_within(1.0).of(expected) - end - end -end diff --git a/spec/policies/subscription_policy_spec.rb b/spec/policies/subscription_policy_spec.rb deleted file mode 100644 index 21d60c15f..000000000 --- a/spec/policies/subscription_policy_spec.rb +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' -require 'pundit/rspec' - -RSpec.describe SubscriptionPolicy do - let(:subject) { described_class } - let(:admin) { Fabricate(:user, admin: true).account } - let(:john) { Fabricate(:user).account } - - permissions :index? do - context 'admin?' do - it 'permits' do - expect(subject).to permit(admin, Subscription) - end - end - - context '!admin?' do - it 'denies' do - expect(subject).to_not permit(john, Subscription) - end - end - end -end diff --git a/spec/services/batched_remove_status_service_spec.rb b/spec/services/batched_remove_status_service_spec.rb index d52e7f484..f84256f18 100644 --- a/spec/services/batched_remove_status_service_spec.rb +++ b/spec/services/batched_remove_status_service_spec.rb @@ -14,11 +14,8 @@ RSpec.describe BatchedRemoveStatusService, type: :service do before do allow(Redis.current).to receive_messages(publish: nil) - stub_request(:post, 'http://example.com/push').to_return(status: 200, body: '', headers: {}) - stub_request(:post, 'http://example.com/salmon').to_return(status: 200, body: '', headers: {}) stub_request(:post, 'http://example.com/inbox').to_return(status: 200) - Fabricate(:subscription, account: alice, callback_url: 'http://example.com/push', confirmed: true, expires_at: 30.days.from_now) jeff.user.update(current_sign_in_at: Time.zone.now) jeff.follow!(alice) hank.follow!(alice) diff --git a/spec/services/remove_status_service_spec.rb b/spec/services/remove_status_service_spec.rb index 48191d47c..06676ec45 100644 --- a/spec/services/remove_status_service_spec.rb +++ b/spec/services/remove_status_service_spec.rb @@ -10,12 +10,9 @@ RSpec.describe RemoveStatusService, type: :service do let!(:bill) { Fabricate(:account, username: 'bill', protocol: :activitypub, domain: 'example2.com', inbox_url: 'http://example2.com/inbox') } before do - stub_request(:post, 'http://example.com/push').to_return(status: 200, body: '', headers: {}) - stub_request(:post, 'http://example.com/salmon').to_return(status: 200, body: '', headers: {}) stub_request(:post, 'http://example.com/inbox').to_return(status: 200) stub_request(:post, 'http://example2.com/inbox').to_return(status: 200) - Fabricate(:subscription, account: alice, callback_url: 'http://example.com/push', confirmed: true, expires_at: 30.days.from_now) jeff.follow!(alice) hank.follow!(alice) diff --git a/spec/services/suspend_account_service_spec.rb b/spec/services/suspend_account_service_spec.rb index 896ac17a3..eebbbc12a 100644 --- a/spec/services/suspend_account_service_spec.rb +++ b/spec/services/suspend_account_service_spec.rb @@ -18,7 +18,6 @@ RSpec.describe SuspendAccountService, type: :service do let!(:favourite) { Fabricate(:favourite, account: account) } let!(:active_relationship) { Fabricate(:follow, account: account) } let!(:passive_relationship) { Fabricate(:follow, target_account: account) } - let!(:subscription) { Fabricate(:subscription, account: account) } let!(:remote_alice) { Fabricate(:account, inbox_url: 'https://alice.com/inbox', protocol: :activitypub) } let!(:remote_bob) { Fabricate(:account, inbox_url: 'https://bob.com/inbox', protocol: :activitypub) } @@ -31,9 +30,8 @@ RSpec.describe SuspendAccountService, type: :service do account.favourites, account.active_relationships, account.passive_relationships, - account.subscriptions ].map(&:count) - }.from([1, 1, 1, 1, 1, 1, 1]).to([0, 0, 0, 0, 0, 0, 0]) + }.from([1, 1, 1, 1, 1, 1]).to([0, 0, 0, 0, 0, 0]) end it 'sends a delete actor activity to all known inboxes' do @@ -62,7 +60,6 @@ RSpec.describe SuspendAccountService, type: :service do let!(:favourite) { Fabricate(:favourite, account: remote_bob) } let!(:active_relationship) { Fabricate(:follow, account: remote_bob, target_account: account) } let!(:passive_relationship) { Fabricate(:follow, target_account: remote_bob) } - let!(:subscription) { Fabricate(:subscription, account: remote_bob) } it 'deletes associated records' do is_expected.to change { @@ -73,9 +70,8 @@ RSpec.describe SuspendAccountService, type: :service do remote_bob.favourites, remote_bob.active_relationships, remote_bob.passive_relationships, - remote_bob.subscriptions ].map(&:count) - }.from([1, 1, 1, 1, 1, 1, 1]).to([0, 0, 0, 0, 0, 0, 0]) + }.from([1, 1, 1, 1, 1, 1]).to([0, 0, 0, 0, 0, 0]) end it 'sends a reject follow to follwer inboxes' do -- cgit From bd1545de5ee9655130e5357bb9cb6449520a6292 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 21 Jul 2019 18:08:02 +0200 Subject: Change locale detection to run once per session (#8657) Fix #6462 --- app/controllers/application_controller.rb | 6 +----- app/controllers/concerns/localized.rb | 13 ++++++++----- config/application.rb | 3 +++ spec/controllers/concerns/localized_spec.rb | 16 +++++----------- 4 files changed, 17 insertions(+), 21 deletions(-) (limited to 'spec') diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 51e9764d4..5863fe168 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -138,11 +138,7 @@ class ApplicationController < ActionController::Base def respond_with_error(code) respond_to do |format| format.any { head code } - - format.html do - set_locale - render "errors/#{code}", layout: 'error', status: code - end + format.html { render "errors/#{code}", layout: 'error', status: code } end end diff --git a/app/controllers/concerns/localized.rb b/app/controllers/concerns/localized.rb index 145549bcd..b43859d9d 100644 --- a/app/controllers/concerns/localized.rb +++ b/app/controllers/concerns/localized.rb @@ -4,16 +4,19 @@ module Localized extend ActiveSupport::Concern included do - before_action :set_locale + around_action :set_locale end private def set_locale - I18n.locale = default_locale - I18n.locale = current_user.locale if user_signed_in? - rescue I18n::InvalidLocale - I18n.locale = default_locale + locale = current_user.locale if respond_to?(:user_signed_in?) && user_signed_in? + locale ||= session[:locale] ||= default_locale + locale = default_locale unless I18n.available_locales.include?(locale.to_sym) + + I18n.with_locale(locale) do + yield + end end def default_locale diff --git a/config/application.rb b/config/application.rb index 4534ede49..f49deffbb 100644 --- a/config/application.rb +++ b/config/application.rb @@ -114,6 +114,9 @@ module Mastodon Doorkeeper::AuthorizationsController.layout 'modal' Doorkeeper::AuthorizedApplicationsController.layout 'admin' Doorkeeper::Application.send :include, ApplicationExtension + Devise::FailureApp.send :include, AbstractController::Callbacks + Devise::FailureApp.send :include, HttpAcceptLanguage::EasyAccess + Devise::FailureApp.send :include, Localized end end end diff --git a/spec/controllers/concerns/localized_spec.rb b/spec/controllers/concerns/localized_spec.rb index 76c3de118..7635d10e1 100644 --- a/spec/controllers/concerns/localized_spec.rb +++ b/spec/controllers/concerns/localized_spec.rb @@ -7,16 +7,10 @@ describe ApplicationController, type: :controller do include Localized def success - head 200 + render plain: I18n.locale, status: 200 end end - around do |example| - current_locale = I18n.locale - example.run - I18n.locale = current_locale - end - before do routes.draw { get 'success' => 'anonymous#success' } end @@ -25,19 +19,19 @@ describe ApplicationController, type: :controller do it 'sets available and preferred language' do request.headers['Accept-Language'] = 'ca-ES, fa' get 'success' - expect(I18n.locale).to eq :fa + expect(response.body).to eq 'fa' end it 'sets available and compatible language if none of available languages are preferred' do request.headers['Accept-Language'] = 'fa-IR' get 'success' - expect(I18n.locale).to eq :fa + expect(response.body).to eq 'fa' end it 'sets default locale if none of available languages are compatible' do request.headers['Accept-Language'] = '' get 'success' - expect(I18n.locale).to eq :en + expect(response.body).to eq 'en' end end @@ -48,7 +42,7 @@ describe ApplicationController, type: :controller do sign_in(user) get 'success' - expect(I18n.locale).to eq :ca + expect(response.body).to eq 'ca' end end -- cgit From 7de8c51873b51d8450f7a6597a43d454964d0407 Mon Sep 17 00:00:00 2001 From: ThibG Date: Sun, 21 Jul 2019 18:10:40 +0200 Subject: Play animated custom emoji on hover (#11348) * Play animated custom emoji on hover in status * Play animated custom emoji on hover in display names * Play animated custom emoji on hover in bios/bio fields * Add support for animation on hover on public pages emojis too * Fix tests * Code style cleanup --- app/javascript/mastodon/components/display_name.js | 44 +++++++++++++++++++++- .../mastodon/components/status_content.js | 32 ++++++++++++++++ .../mastodon/features/account/components/header.js | 43 ++++++++++++++++++++- app/javascript/mastodon/features/emoji/emoji.js | 2 +- app/javascript/packs/public.js | 9 +++++ app/lib/formatter.rb | 15 +++++--- spec/lib/formatter_spec.rb | 26 ++++++------- 7 files changed, 149 insertions(+), 22 deletions(-) (limited to 'spec') diff --git a/app/javascript/mastodon/components/display_name.js b/app/javascript/mastodon/components/display_name.js index 6b9dd6f81..70ef82789 100644 --- a/app/javascript/mastodon/components/display_name.js +++ b/app/javascript/mastodon/components/display_name.js @@ -1,6 +1,7 @@ import React from 'react'; import ImmutablePropTypes from 'react-immutable-proptypes'; import PropTypes from 'prop-types'; +import { autoPlayGif } from 'mastodon/initial_state'; export default class DisplayName extends React.PureComponent { @@ -10,6 +11,47 @@ export default class DisplayName extends React.PureComponent { localDomain: PropTypes.string, }; + _updateEmojis () { + const node = this.node; + + if (!node || autoPlayGif) { + return; + } + + const emojis = node.querySelectorAll('.custom-emoji'); + + for (var i = 0; i < emojis.length; i++) { + let emoji = emojis[i]; + if (emoji.classList.contains('status-emoji')) { + continue; + } + emoji.classList.add('status-emoji'); + + emoji.addEventListener('mouseenter', this.handleEmojiMouseEnter, false); + emoji.addEventListener('mouseleave', this.handleEmojiMouseLeave, false); + } + } + + componentDidMount () { + this._updateEmojis(); + } + + componentDidUpdate () { + this._updateEmojis(); + } + + handleEmojiMouseEnter = ({ target }) => { + target.src = target.getAttribute('data-original'); + } + + handleEmojiMouseLeave = ({ target }) => { + target.src = target.getAttribute('data-static'); + } + + setRef = (c) => { + this.node = c; + } + render () { const { others, localDomain } = this.props; @@ -39,7 +81,7 @@ export default class DisplayName extends React.PureComponent { } return ( - + {displayName} {suffix} ); diff --git a/app/javascript/mastodon/components/status_content.js b/app/javascript/mastodon/components/status_content.js index 06f5b4aad..8a05415af 100644 --- a/app/javascript/mastodon/components/status_content.js +++ b/app/javascript/mastodon/components/status_content.js @@ -7,6 +7,7 @@ import Permalink from './permalink'; import classnames from 'classnames'; import PollContainer from 'mastodon/containers/poll_container'; import Icon from 'mastodon/components/icon'; +import { autoPlayGif } from 'mastodon/initial_state'; const MAX_HEIGHT = 642; // 20px * 32 (+ 2px padding at the top) @@ -71,12 +72,35 @@ export default class StatusContent extends React.PureComponent { } } + _updateStatusEmojis () { + const node = this.node; + + if (!node || autoPlayGif) { + return; + } + + const emojis = node.querySelectorAll('.custom-emoji'); + + for (var i = 0; i < emojis.length; i++) { + let emoji = emojis[i]; + if (emoji.classList.contains('status-emoji')) { + continue; + } + emoji.classList.add('status-emoji'); + + emoji.addEventListener('mouseenter', this.handleEmojiMouseEnter, false); + emoji.addEventListener('mouseleave', this.handleEmojiMouseLeave, false); + } + } + componentDidMount () { this._updateStatusLinks(); + this._updateStatusEmojis(); } componentDidUpdate () { this._updateStatusLinks(); + this._updateStatusEmojis(); } onMentionClick = (mention, e) => { @@ -95,6 +119,14 @@ export default class StatusContent extends React.PureComponent { } } + handleEmojiMouseEnter = ({ target }) => { + target.src = target.getAttribute('data-original'); + } + + handleEmojiMouseLeave = ({ target }) => { + target.src = target.getAttribute('data-static'); + } + handleMouseDown = (e) => { this.startXY = [e.clientX, e.clientY]; } diff --git a/app/javascript/mastodon/features/account/components/header.js b/app/javascript/mastodon/features/account/components/header.js index e5b60e33e..cab67c607 100644 --- a/app/javascript/mastodon/features/account/components/header.js +++ b/app/javascript/mastodon/features/account/components/header.js @@ -79,6 +79,47 @@ class Header extends ImmutablePureComponent { return !location.pathname.match(/\/(followers|following)\/?$/); } + _updateEmojis () { + const node = this.node; + + if (!node || autoPlayGif) { + return; + } + + const emojis = node.querySelectorAll('.custom-emoji'); + + for (var i = 0; i < emojis.length; i++) { + let emoji = emojis[i]; + if (emoji.classList.contains('status-emoji')) { + continue; + } + emoji.classList.add('status-emoji'); + + emoji.addEventListener('mouseenter', this.handleEmojiMouseEnter, false); + emoji.addEventListener('mouseleave', this.handleEmojiMouseLeave, false); + } + } + + componentDidMount () { + this._updateEmojis(); + } + + componentDidUpdate () { + this._updateEmojis(); + } + + handleEmojiMouseEnter = ({ target }) => { + target.src = target.getAttribute('data-original'); + } + + handleEmojiMouseLeave = ({ target }) => { + target.src = target.getAttribute('data-static'); + } + + setRef = (c) => { + this.node = c; + } + render () { const { account, intl, domain, identity_proofs } = this.props; @@ -200,7 +241,7 @@ class Header extends ImmutablePureComponent { const acct = account.get('acct').indexOf('@') === -1 && domain ? `${account.get('acct')}@${domain}` : account.get('acct'); return ( -
+
{info} diff --git a/app/javascript/mastodon/features/emoji/emoji.js b/app/javascript/mastodon/features/emoji/emoji.js index 01b5a6664..359bb7ffd 100644 --- a/app/javascript/mastodon/features/emoji/emoji.js +++ b/app/javascript/mastodon/features/emoji/emoji.js @@ -29,7 +29,7 @@ const emojify = (str, customEmojis = {}) => { // if you want additional emoji handler, add statements below which set replacement and return true. if (shortname in customEmojis) { const filename = autoPlayGif ? customEmojis[shortname].url : customEmojis[shortname].static_url; - replacement = `${shortname}`; + replacement = `${shortname}`; return true; } return false; diff --git a/app/javascript/packs/public.js b/app/javascript/packs/public.js index 0c60d828e..b58622a8d 100644 --- a/app/javascript/packs/public.js +++ b/app/javascript/packs/public.js @@ -44,6 +44,12 @@ function main() { } }; + const getEmojiAnimationHandler = (swapTo) => { + return ({ target }) => { + target.src = target.getAttribute(swapTo); + }; + }; + ready(() => { const locale = document.documentElement.lang; @@ -108,6 +114,9 @@ function main() { if (parallaxComponents.length > 0 ) { new Rellax('.parallax', { speed: -1 }); } + + delegate(document, '.custom-emoji', 'mouseover', getEmojiAnimationHandler('data-original')); + delegate(document, '.custom-emoji', 'mouseout', getEmojiAnimationHandler('data-static')); }); delegate(document, '.webapp-btn', 'click', ({ target, button }) => { diff --git a/app/lib/formatter.rb b/app/lib/formatter.rb index 6c1239963..65059efa0 100644 --- a/app/lib/formatter.rb +++ b/app/lib/formatter.rb @@ -137,11 +137,7 @@ class Formatter def encode_custom_emojis(html, emojis, animate = false) return html if emojis.empty? - emoji_map = if animate - emojis.each_with_object({}) { |e, h| h[e.shortcode] = full_asset_url(e.image.url) } - else - emojis.each_with_object({}) { |e, h| h[e.shortcode] = full_asset_url(e.image.url(:static)) } - end + emoji_map = emojis.each_with_object({}) { |e, h| h[e.shortcode] = [full_asset_url(e.image.url), full_asset_url(e.image.url(:static))] } i = -1 tag_open_index = nil @@ -157,7 +153,14 @@ class Formatter emoji = emoji_map[shortcode] if emoji - replacement = "\":#{encode(shortcode)}:\"" + original_url, static_url = emoji + replacement = begin + if animate + "\":#{encode(shortcode)}:\"" + else + "\":#{encode(shortcode)}:\"" + end + end before_html = shortname_start_index.positive? ? html[0..shortname_start_index - 1] : '' html = before_html + replacement + html[i + 1..-1] i += replacement.size - (shortcode.size + 2) - 1 diff --git a/spec/lib/formatter_spec.rb b/spec/lib/formatter_spec.rb index 96d2fc7e0..b8108a247 100644 --- a/spec/lib/formatter_spec.rb +++ b/spec/lib/formatter_spec.rb @@ -261,7 +261,7 @@ RSpec.describe Formatter do let(:text) { ':coolcat: Beep boop' } it 'converts the shortcode to an image tag' do - is_expected.to match(/:coolcat::coolcat::coolcat::coolcat: Beep boop
' } it 'converts the shortcode to an image tag' do - is_expected.to match(/

:coolcat::coolcat:Beep :coolcat: boop

' } it 'converts the shortcode to an image tag' do - is_expected.to match(/Beep :coolcat:Beep boop
:coolcat:

' } it 'converts the shortcode to an image tag' do - is_expected.to match(/
:coolcat::coolcat::coolcat::coolcat::coolcat: Beep boop
' } it 'converts shortcode to image tag' do - is_expected.to match(/

:coolcat::coolcat:Beep :coolcat: boop

' } it 'converts shortcode to image tag' do - is_expected.to match(/Beep :coolcat:Beep boop
:coolcat:

' } it 'converts shortcode to image tag' do - is_expected.to match(/
:coolcat::coolcat: Date: Mon, 22 Jul 2019 10:48:50 +0200 Subject: Change unconfirmed user login behaviour (#11375) Allow access to account settings, 2FA, authorized applications, and account deletions to unconfirmed and pending users, as well as users who had their accounts disabled. Suspended users cannot update their e-mail or password or delete their account. Display account status on account settings page, for example, when an account is frozen, limited, unconfirmed or pending review. After sign up, login users straight away and show a simple page that tells them the status of their account with links to account settings and logout, to reduce onboarding friction and allow users to correct wrongly typed e-mail addresses. Move the final sign-up step of SSO integrations to be the same as above to reduce code duplication. --- app/controllers/about_controller.rb | 2 +- app/controllers/api/base_controller.rb | 2 +- app/controllers/application_controller.rb | 6 +-- app/controllers/auth/confirmations_controller.rb | 21 +------- .../auth/omniauth_callbacks_controller.rb | 2 +- app/controllers/auth/registrations_controller.rb | 9 +++- app/controllers/auth/sessions_controller.rb | 4 +- app/controllers/auth/setup_controller.rb | 58 ++++++++++++++++++++++ .../oauth/authorized_applications_controller.rb | 2 + app/controllers/settings/deletes_controller.rb | 7 +++ app/controllers/settings/sessions_controller.rb | 2 + .../confirmations_controller.rb | 2 + .../recovery_codes_controller.rb | 2 + .../two_factor_authentications_controller.rb | 2 + app/javascript/styles/mastodon/admin.scss | 58 +++++++++++++--------- app/javascript/styles/mastodon/forms.scss | 7 +++ app/models/concerns/omniauthable.rb | 2 +- app/models/user.rb | 6 ++- .../auth/confirmations/finish_signup.html.haml | 15 ------ app/views/auth/registrations/_sessions.html.haml | 4 +- app/views/auth/registrations/_status.html.haml | 16 ++++++ app/views/auth/registrations/edit.html.haml | 35 +++++++------ app/views/auth/setup/show.html.haml | 23 +++++++++ .../oauth/authorized_applications/index.html.haml | 2 +- config/locales/en.yml | 9 +++- config/routes.rb | 5 +- db/seeds.rb | 2 +- spec/controllers/api/base_controller_spec.rb | 42 +++++++++++++++- spec/controllers/application_controller_spec.rb | 4 +- .../auth/confirmations_controller_spec.rb | 41 --------------- .../auth/registrations_controller_spec.rb | 25 +++++++--- spec/controllers/auth/sessions_controller_spec.rb | 4 +- .../settings/deletes_controller_spec.rb | 17 +++++++ spec/features/log_in_spec.rb | 4 +- spec/models/user_spec.rb | 4 +- 35 files changed, 298 insertions(+), 148 deletions(-) create mode 100644 app/controllers/auth/setup_controller.rb delete mode 100644 app/views/auth/confirmations/finish_signup.html.haml create mode 100644 app/views/auth/registrations/_status.html.haml create mode 100644 app/views/auth/setup/show.html.haml (limited to 'spec') diff --git a/app/controllers/about_controller.rb b/app/controllers/about_controller.rb index 33bac9bbc..31cf17710 100644 --- a/app/controllers/about_controller.rb +++ b/app/controllers/about_controller.rb @@ -7,7 +7,7 @@ class AboutController < ApplicationController before_action :set_instance_presenter before_action :set_expires_in - skip_before_action :check_user_permissions, only: [:more, :terms] + skip_before_action :require_functional!, only: [:more, :terms] def show; end diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb index eca558f42..6f33a1ea9 100644 --- a/app/controllers/api/base_controller.rb +++ b/app/controllers/api/base_controller.rb @@ -7,7 +7,7 @@ class Api::BaseController < ApplicationController include RateLimitHeaders skip_before_action :store_current_location - skip_before_action :check_user_permissions + skip_before_action :require_functional! before_action :set_cache_headers diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index b8a1faf77..41ce1a0ca 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -25,7 +25,7 @@ class ApplicationController < ActionController::Base rescue_from Mastodon::NotPermittedError, with: :forbidden before_action :store_current_location, except: :raise_not_found, unless: :devise_controller? - before_action :check_user_permissions, if: :user_signed_in? + before_action :require_functional!, if: :user_signed_in? def raise_not_found raise ActionController::RoutingError, "No route matches #{params[:unmatched_route]}" @@ -57,8 +57,8 @@ class ApplicationController < ActionController::Base forbidden unless current_user&.staff? end - def check_user_permissions - forbidden if current_user.disabled? || current_user.account.suspended? + def require_functional! + redirect_to edit_user_registration_path unless current_user.functional? end def after_sign_out_path_for(_resource_or_scope) diff --git a/app/controllers/auth/confirmations_controller.rb b/app/controllers/auth/confirmations_controller.rb index c28c7471c..0d7c6e7c2 100644 --- a/app/controllers/auth/confirmations_controller.rb +++ b/app/controllers/auth/confirmations_controller.rb @@ -4,34 +4,15 @@ class Auth::ConfirmationsController < Devise::ConfirmationsController layout 'auth' before_action :set_body_classes - before_action :set_user, only: [:finish_signup] - def finish_signup - return unless request.patch? && params[:user] - - if @user.update(user_params) - @user.skip_reconfirmation! - bypass_sign_in(@user) - redirect_to root_path, notice: I18n.t('devise.confirmations.send_instructions') - else - @show_errors = true - end - end + skip_before_action :require_functional! private - def set_user - @user = current_user - end - def set_body_classes @body_classes = 'lighter' end - 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 diff --git a/app/controllers/auth/omniauth_callbacks_controller.rb b/app/controllers/auth/omniauth_callbacks_controller.rb index bbf63bed3..682c77016 100644 --- a/app/controllers/auth/omniauth_callbacks_controller.rb +++ b/app/controllers/auth/omniauth_callbacks_controller.rb @@ -27,7 +27,7 @@ class Auth::OmniauthCallbacksController < Devise::OmniauthCallbacksController if resource.email_verified? root_path else - finish_signup_path + auth_setup_path(missing_email: '1') end end end diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb index 83797cf1f..019caf9c1 100644 --- a/app/controllers/auth/registrations_controller.rb +++ b/app/controllers/auth/registrations_controller.rb @@ -9,6 +9,9 @@ class Auth::RegistrationsController < Devise::RegistrationsController before_action :set_sessions, only: [:edit, :update] before_action :set_instance_presenter, only: [:new, :create, :update] before_action :set_body_classes, only: [:new, :create, :edit, :update] + before_action :require_not_suspended!, only: [:update] + + skip_before_action :require_functional!, only: [:edit, :update] def new super(&:build_invite_request) @@ -43,7 +46,7 @@ class Auth::RegistrationsController < Devise::RegistrationsController end def after_sign_up_path_for(_resource) - new_user_session_path + auth_setup_path end def after_sign_in_path_for(_resource) @@ -102,4 +105,8 @@ class Auth::RegistrationsController < Devise::RegistrationsController def set_sessions @sessions = current_user.session_activations end + + def require_not_suspended! + forbidden if current_account.suspended? + end end diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index fb8615c31..7e6dbf19e 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -6,8 +6,10 @@ class Auth::SessionsController < Devise::SessionsController layout 'auth' skip_before_action :require_no_authentication, only: [:create] - skip_before_action :check_user_permissions, only: [:destroy] + skip_before_action :require_functional! + prepend_before_action :authenticate_with_two_factor, if: :two_factor_enabled?, only: [:create] + before_action :set_instance_presenter, only: [:new] before_action :set_body_classes diff --git a/app/controllers/auth/setup_controller.rb b/app/controllers/auth/setup_controller.rb new file mode 100644 index 000000000..46c5f2958 --- /dev/null +++ b/app/controllers/auth/setup_controller.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +class Auth::SetupController < ApplicationController + layout 'auth' + + before_action :authenticate_user! + before_action :require_unconfirmed_or_pending! + before_action :set_body_classes + before_action :set_user + + skip_before_action :require_functional! + + def show + flash.now[:notice] = begin + if @user.pending? + I18n.t('devise.registrations.signed_up_but_pending') + else + I18n.t('devise.registrations.signed_up_but_unconfirmed') + end + end + end + + def update + # This allows updating the e-mail without entering a password as is required + # on the account settings page; however, we only allow this for accounts + # that were not confirmed yet + + if @user.update(user_params) + redirect_to auth_setup_path, notice: I18n.t('devise.confirmations.send_instructions') + else + render :show + end + end + + helper_method :missing_email? + + private + + def require_unconfirmed_or_pending! + redirect_to root_path if current_user.confirmed? && current_user.approved? + end + + def set_user + @user = current_user + end + + def set_body_classes + @body_classes = 'lighter' + end + + def user_params + params.require(:user).permit(:email) + end + + def missing_email? + truthy_param?(:missing_email) + end +end diff --git a/app/controllers/oauth/authorized_applications_controller.rb b/app/controllers/oauth/authorized_applications_controller.rb index f3d235366..fb8389034 100644 --- a/app/controllers/oauth/authorized_applications_controller.rb +++ b/app/controllers/oauth/authorized_applications_controller.rb @@ -7,6 +7,8 @@ class Oauth::AuthorizedApplicationsController < Doorkeeper::AuthorizedApplicatio before_action :authenticate_resource_owner! before_action :set_body_classes + skip_before_action :require_functional! + include Localized def destroy diff --git a/app/controllers/settings/deletes_controller.rb b/app/controllers/settings/deletes_controller.rb index dd19aadf6..97fe4d328 100644 --- a/app/controllers/settings/deletes_controller.rb +++ b/app/controllers/settings/deletes_controller.rb @@ -5,6 +5,9 @@ class Settings::DeletesController < Settings::BaseController before_action :check_enabled_deletion before_action :authenticate_user! + before_action :require_not_suspended! + + skip_before_action :require_functional! def show @confirmation = Form::DeleteConfirmation.new @@ -29,4 +32,8 @@ class Settings::DeletesController < Settings::BaseController def delete_params params.require(:form_delete_confirmation).permit(:password) end + + def require_not_suspended! + forbidden if current_account.suspended? + end end diff --git a/app/controllers/settings/sessions_controller.rb b/app/controllers/settings/sessions_controller.rb index 84ebb21f2..df5ace803 100644 --- a/app/controllers/settings/sessions_controller.rb +++ b/app/controllers/settings/sessions_controller.rb @@ -4,6 +4,8 @@ class Settings::SessionsController < Settings::BaseController before_action :authenticate_user! before_action :set_session, only: :destroy + skip_before_action :require_functional! + def destroy @session.destroy! flash[:notice] = I18n.t('sessions.revoke_success') diff --git a/app/controllers/settings/two_factor_authentication/confirmations_controller.rb b/app/controllers/settings/two_factor_authentication/confirmations_controller.rb index 02652a36c..3145e092d 100644 --- a/app/controllers/settings/two_factor_authentication/confirmations_controller.rb +++ b/app/controllers/settings/two_factor_authentication/confirmations_controller.rb @@ -8,6 +8,8 @@ module Settings before_action :authenticate_user! before_action :ensure_otp_secret + skip_before_action :require_functional! + def new prepare_two_factor_form end diff --git a/app/controllers/settings/two_factor_authentication/recovery_codes_controller.rb b/app/controllers/settings/two_factor_authentication/recovery_codes_controller.rb index 874bf532b..09a759860 100644 --- a/app/controllers/settings/two_factor_authentication/recovery_codes_controller.rb +++ b/app/controllers/settings/two_factor_authentication/recovery_codes_controller.rb @@ -7,6 +7,8 @@ module Settings before_action :authenticate_user! + skip_before_action :require_functional! + def create @recovery_codes = current_user.generate_otp_backup_codes! current_user.save! diff --git a/app/controllers/settings/two_factor_authentications_controller.rb b/app/controllers/settings/two_factor_authentications_controller.rb index e12c43074..6904076e4 100644 --- a/app/controllers/settings/two_factor_authentications_controller.rb +++ b/app/controllers/settings/two_factor_authentications_controller.rb @@ -7,6 +7,8 @@ module Settings before_action :authenticate_user! before_action :verify_otp_required, only: [:create] + skip_before_action :require_functional! + def show @confirmation = Form::TwoFactorConfirmation.new end diff --git a/app/javascript/styles/mastodon/admin.scss b/app/javascript/styles/mastodon/admin.scss index 373a10260..f625bc139 100644 --- a/app/javascript/styles/mastodon/admin.scss +++ b/app/javascript/styles/mastodon/admin.scss @@ -204,29 +204,6 @@ $content-width: 840px; border: 0; } } - - .muted-hint { - color: $darker-text-color; - - a { - color: $highlight-text-color; - } - } - - .positive-hint { - color: $valid-value-color; - font-weight: 500; - } - - .negative-hint { - color: $error-value-color; - font-weight: 500; - } - - .neutral-hint { - color: $dark-text-color; - font-weight: 500; - } } @media screen and (max-width: $no-columns-breakpoint) { @@ -249,6 +226,41 @@ $content-width: 840px; } } +hr.spacer { + width: 100%; + border: 0; + margin: 20px 0; + height: 1px; +} + +.muted-hint { + color: $darker-text-color; + + a { + color: $highlight-text-color; + } +} + +.positive-hint { + color: $valid-value-color; + font-weight: 500; +} + +.negative-hint { + color: $error-value-color; + font-weight: 500; +} + +.neutral-hint { + color: $dark-text-color; + font-weight: 500; +} + +.warning-hint { + color: $gold-star; + font-weight: 500; +} + .filters { display: flex; flex-wrap: wrap; diff --git a/app/javascript/styles/mastodon/forms.scss b/app/javascript/styles/mastodon/forms.scss index 456ee4e0d..ac99124ea 100644 --- a/app/javascript/styles/mastodon/forms.scss +++ b/app/javascript/styles/mastodon/forms.scss @@ -300,6 +300,13 @@ code { } } + .input.static .label_input__wrapper { + font-size: 16px; + padding: 10px; + border: 1px solid $dark-text-color; + border-radius: 4px; + } + input[type=text], input[type=number], input[type=email], diff --git a/app/models/concerns/omniauthable.rb b/app/models/concerns/omniauthable.rb index 283033083..b9c124841 100644 --- a/app/models/concerns/omniauthable.rb +++ b/app/models/concerns/omniauthable.rb @@ -43,7 +43,7 @@ module Omniauthable # Check if the user exists with provided email if the provider gives us a # verified email. If no verified email was provided or the user already # exists, we assign a temporary email and ask the user to verify it on - # the next step via Auth::ConfirmationsController.finish_signup + # the next step via Auth::SetupController.show user = User.new(user_params_from_auth(auth)) user.account.avatar_remote_url = auth.info.image if auth.info.image =~ /\A#{URI.regexp(%w(http https))}\z/ diff --git a/app/models/user.rb b/app/models/user.rb index 31c99630c..474c77293 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -161,7 +161,11 @@ class User < ApplicationRecord end def active_for_authentication? - super && approved? + true + end + + def functional? + confirmed? && approved? && !disabled? && !account.suspended? end def inactive_message diff --git a/app/views/auth/confirmations/finish_signup.html.haml b/app/views/auth/confirmations/finish_signup.html.haml deleted file mode 100644 index 9d09b74e1..000000000 --- a/app/views/auth/confirmations/finish_signup.html.haml +++ /dev/null @@ -1,15 +0,0 @@ -- content_for :page_title do - = t('auth.confirm_email') - -= simple_form_for(current_user, as: 'user', url: finish_signup_path, html: { role: 'form'}) do |f| - - if @show_errors && current_user.errors.any? - #error_explanation - - current_user.errors.full_messages.each do |msg| - = msg - %br - - .fields-group - = f.input :email, wrapper: :with_label, required: true, hint: false - - .actions - = f.submit t('auth.confirm_email'), class: 'button' diff --git a/app/views/auth/registrations/_sessions.html.haml b/app/views/auth/registrations/_sessions.html.haml index d7d96a1bb..395e36a9f 100644 --- a/app/views/auth/registrations/_sessions.html.haml +++ b/app/views/auth/registrations/_sessions.html.haml @@ -1,6 +1,8 @@ -%h4= t 'sessions.title' +%h3= t 'sessions.title' %p.muted-hint= t 'sessions.explanation' +%hr.spacer/ + .table-wrapper %table.table.inline-table %thead diff --git a/app/views/auth/registrations/_status.html.haml b/app/views/auth/registrations/_status.html.haml new file mode 100644 index 000000000..b38a83d67 --- /dev/null +++ b/app/views/auth/registrations/_status.html.haml @@ -0,0 +1,16 @@ +%h3= t('auth.status.account_status') + +- if @user.account.suspended? + %span.negative-hint= t('user_mailer.warning.explanation.suspend') +- elsif @user.disabled? + %span.negative-hint= t('user_mailer.warning.explanation.disable') +- elsif @user.account.silenced? + %span.warning-hint= t('user_mailer.warning.explanation.silence') +- elsif !@user.confirmed? + %span.warning-hint= t('auth.status.confirming') +- elsif !@user.approved? + %span.warning-hint= t('auth.status.pending') +- else + %span.positive-hint= t('auth.status.functional') + +%hr.spacer/ diff --git a/app/views/auth/registrations/edit.html.haml b/app/views/auth/registrations/edit.html.haml index 694461fdf..710ee5c68 100644 --- a/app/views/auth/registrations/edit.html.haml +++ b/app/views/auth/registrations/edit.html.haml @@ -1,25 +1,28 @@ - content_for :page_title do - = t('auth.security') + = t('settings.account_settings') + += render 'status' + +%h3= t('auth.security') = simple_form_for(resource, as: resource_name, url: registration_path(resource_name), html: { method: :put, class: 'auth_edit' }) do |f| = render 'shared/error_messages', object: resource - if !use_seamless_external_login? || resource.encrypted_password.present? - .fields-group - = f.input :email, wrapper: :with_label, input_html: { 'aria-label' => t('simple_form.labels.defaults.email') }, required: true, hint: false - - .fields-group - = f.input :current_password, wrapper: :with_label, input_html: { 'aria-label' => t('simple_form.labels.defaults.current_password'), :autocomplete => 'off' }, required: true - - .fields-group - = f.input :password, wrapper: :with_label, label: t('simple_form.labels.defaults.new_password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.new_password'), :autocomplete => 'off' }, hint: false - - .fields-group - = f.input :password_confirmation, wrapper: :with_label, label: t('simple_form.labels.defaults.confirm_new_password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.confirm_new_password'), :autocomplete => 'off' } - + .fields-row + .fields-row__column.fields-group.fields-row__column-6 + = f.input :email, wrapper: :with_label, input_html: { 'aria-label' => t('simple_form.labels.defaults.email') }, required: true, disabled: current_account.suspended? + .fields-row__column.fields-group.fields-row__column-6 + = f.input :current_password, wrapper: :with_label, input_html: { 'aria-label' => t('simple_form.labels.defaults.current_password'), :autocomplete => 'off' }, required: true, disabled: current_account.suspended? + + .fields-row + .fields-row__column.fields-group.fields-row__column-6 + = f.input :password, wrapper: :with_label, label: t('simple_form.labels.defaults.new_password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.new_password'), :autocomplete => 'off' }, hint: t('simple_form.hints.defaults.password'), disabled: current_account.suspended? + .fields-row__column.fields-group.fields-row__column-6 + = f.input :password_confirmation, wrapper: :with_label, label: t('simple_form.labels.defaults.confirm_new_password'), input_html: { 'aria-label' => t('simple_form.labels.defaults.confirm_new_password'), :autocomplete => 'off' }, disabled: current_account.suspended? .actions - = f.button :button, t('generic.save_changes'), type: :submit + = f.button :button, t('generic.save_changes'), type: :submit, class: 'button', disabled: current_account.suspended? - else %p.hint= t('users.seamless_external_login') @@ -27,7 +30,7 @@ = render 'sessions' -- if open_deletion? +- if open_deletion? && !current_account.suspended? %hr.spacer/ - %h4= t('auth.delete_account') + %h3= t('auth.delete_account') %p.muted-hint= t('auth.delete_account_html', path: settings_delete_path) diff --git a/app/views/auth/setup/show.html.haml b/app/views/auth/setup/show.html.haml new file mode 100644 index 000000000..8bb44ca7f --- /dev/null +++ b/app/views/auth/setup/show.html.haml @@ -0,0 +1,23 @@ +- content_for :page_title do + = t('auth.setup.title') + +- if missing_email? + = simple_form_for(@user, url: auth_setup_path) do |f| + = render 'shared/error_messages', object: @user + + .fields-group + %p.hint= t('auth.setup.email_below_hint_html') + + .fields-group + = f.input :email, required: true, hint: false, input_html: { 'aria-label' => t('simple_form.labels.defaults.email'), :autocomplete => 'off' } + + .actions + = f.submit t('admin.accounts.change_email.label'), class: 'button' +- else + .simple_form + %p.hint= t('auth.setup.email_settings_hint_html', email: content_tag(:strong, @user.email)) + +.form-footer + %ul.no-list + %li= link_to t('settings.account_settings'), edit_user_registration_path + %li= link_to t('auth.logout'), destroy_user_session_path, data: { method: :delete } diff --git a/app/views/oauth/authorized_applications/index.html.haml b/app/views/oauth/authorized_applications/index.html.haml index 19af5f55d..7203d758d 100644 --- a/app/views/oauth/authorized_applications/index.html.haml +++ b/app/views/oauth/authorized_applications/index.html.haml @@ -17,7 +17,7 @@ = application.name - else = link_to application.name, application.website, target: '_blank', rel: 'noopener' - %th!= application.scopes.map { |scope| t(scope, scope: [:doorkeeper, :scopes]) }.join('
') + %th!= application.scopes.map { |scope| t(scope, scope: [:doorkeeper, :scopes]) }.join(', ') %td= l application.created_at %td - unless application.superapp? diff --git a/config/locales/en.yml b/config/locales/en.yml index 89c52b84a..9e1be87be 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -524,7 +524,6 @@ en: apply_for_account: Request an invite change_password: Password checkbox_agreement_html: I agree to the server rules and terms of service - confirm_email: Confirm email delete_account: Delete account delete_account_html: If you wish to delete your account, you can proceed here. You will be asked for confirmation. didnt_get_confirmation: Didn't receive confirmation instructions? @@ -544,6 +543,14 @@ en: reset_password: Reset password security: Security set_new_password: Set new password + setup: + email_below_hint_html: If the below e-mail address is incorrect, you can change it here and receive a new confirmation e-mail. + email_settings_hint_html: The confirmation e-mail was sent to %{email}. If that e-mail address is not correct, you can change it in account settings. + title: Setup + status: + account_status: Account status + confirming: Waiting for e-mail confirmation to be completed. + pending: Your application is pending review by our staff. This may take some time. You will receive an e-mail if your application is approved. trouble_logging_in: Trouble logging in? authorize_follow: already_following: You are already following this account diff --git a/config/routes.rb b/config/routes.rb index 27b536641..b6c215888 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -34,7 +34,10 @@ Rails.application.routes.draw do devise_scope :user do get '/invite/:invite_code', to: 'auth/registrations#new', as: :public_invite - match '/auth/finish_signup' => 'auth/confirmations#finish_signup', via: [:get, :patch], as: :finish_signup + + namespace :auth do + resource :setup, only: [:show, :update], controller: :setup + end end devise_for :users, path: 'auth', controllers: { diff --git a/db/seeds.rb b/db/seeds.rb index b112cf073..0bfb5d0db 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -1,4 +1,4 @@ -Doorkeeper::Application.create!(name: 'Web', superapp: true, redirect_uri: Doorkeeper.configuration.native_redirect_uri, scopes: 'read write follow') +Doorkeeper::Application.create!(name: 'Web', superapp: true, redirect_uri: Doorkeeper.configuration.native_redirect_uri, scopes: 'read write follow push') domain = ENV['LOCAL_DOMAIN'] || Rails.configuration.x.local_domain account = Account.find_or_initialize_by(id: -99, actor_type: 'Application', locked: true, username: domain) diff --git a/spec/controllers/api/base_controller_spec.rb b/spec/controllers/api/base_controller_spec.rb index 750ccc8cf..05a42d1c1 100644 --- a/spec/controllers/api/base_controller_spec.rb +++ b/spec/controllers/api/base_controller_spec.rb @@ -15,7 +15,7 @@ describe Api::BaseController do end end - describe 'Forgery protection' do + describe 'forgery protection' do before do routes.draw { post 'success' => 'api/base#success' } end @@ -27,7 +27,45 @@ describe Api::BaseController do end end - describe 'Error handling' do + describe 'non-functional accounts handling' do + let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read') } + + controller do + before_action :require_user! + end + + before do + routes.draw { post 'success' => 'api/base#success' } + allow(controller).to receive(:doorkeeper_token) { token } + end + + it 'returns http forbidden for unconfirmed accounts' do + user.update(confirmed_at: nil) + post 'success' + expect(response).to have_http_status(403) + end + + it 'returns http forbidden for pending accounts' do + user.update(approved: false) + post 'success' + expect(response).to have_http_status(403) + end + + it 'returns http forbidden for disabled accounts' do + user.update(disabled: true) + post 'success' + expect(response).to have_http_status(403) + end + + it 'returns http forbidden for suspended accounts' do + user.account.suspend! + post 'success' + expect(response).to have_http_status(403) + end + end + + describe 'error handling' do ERRORS_WITH_CODES = { ActiveRecord::RecordInvalid => 422, Mastodon::ValidationError => 422, diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 27946b60f..1811500df 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -187,10 +187,10 @@ describe ApplicationController, type: :controller do expect(response).to have_http_status(200) end - it 'returns http 403 if user who signed in is suspended' do + it 'redirects to account status page' do sign_in(Fabricate(:user, account: Fabricate(:account, suspended: true))) get 'success' - expect(response).to have_http_status(403) + expect(response).to redirect_to(edit_user_registration_path) end end diff --git a/spec/controllers/auth/confirmations_controller_spec.rb b/spec/controllers/auth/confirmations_controller_spec.rb index e9a471fc5..0b6b74ff9 100644 --- a/spec/controllers/auth/confirmations_controller_spec.rb +++ b/spec/controllers/auth/confirmations_controller_spec.rb @@ -50,45 +50,4 @@ describe Auth::ConfirmationsController, type: :controller do end end end - - describe 'GET #finish_signup' do - subject { get :finish_signup } - - let(:user) { Fabricate(:user) } - before do - sign_in user, scope: :user - @request.env['devise.mapping'] = Devise.mappings[:user] - end - - it 'renders finish_signup' do - is_expected.to render_template :finish_signup - expect(assigns(:user)).to have_attributes id: user.id - end - end - - describe 'PATCH #finish_signup' do - subject { patch :finish_signup, params: { user: { email: email } } } - - let(:user) { Fabricate(:user) } - before do - sign_in user, scope: :user - @request.env['devise.mapping'] = Devise.mappings[:user] - end - - context 'when email is valid' do - let(:email) { 'new_' + user.email } - - it 'redirects to root_path' do - is_expected.to redirect_to root_path - end - end - - context 'when email is invalid' do - let(:email) { '' } - - it 'renders finish_signup' do - is_expected.to render_template :finish_signup - end - end - end end diff --git a/spec/controllers/auth/registrations_controller_spec.rb b/spec/controllers/auth/registrations_controller_spec.rb index a4337039e..3e11b34b5 100644 --- a/spec/controllers/auth/registrations_controller_spec.rb +++ b/spec/controllers/auth/registrations_controller_spec.rb @@ -46,6 +46,15 @@ RSpec.describe Auth::RegistrationsController, type: :controller do post :update expect(response).to have_http_status(200) end + + context 'when suspended' do + it 'returns http forbidden' do + request.env["devise.mapping"] = Devise.mappings[:user] + sign_in(Fabricate(:user, account_attributes: { username: 'test', suspended_at: Time.now.utc }), scope: :user) + post :update + expect(response).to have_http_status(403) + end + end end describe 'GET #new' do @@ -94,9 +103,9 @@ RSpec.describe Auth::RegistrationsController, type: :controller do post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678' } } end - it 'redirects to login page' do + it 'redirects to setup' do subject - expect(response).to redirect_to new_user_session_path + expect(response).to redirect_to auth_setup_path end it 'creates user' do @@ -120,9 +129,9 @@ RSpec.describe Auth::RegistrationsController, type: :controller do post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678' } } end - it 'redirects to login page' do + it 'redirects to setup' do subject - expect(response).to redirect_to new_user_session_path + expect(response).to redirect_to auth_setup_path end it 'creates user' do @@ -148,9 +157,9 @@ RSpec.describe Auth::RegistrationsController, type: :controller do post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678', 'invite_code': invite.code } } end - it 'redirects to login page' do + it 'redirects to setup' do subject - expect(response).to redirect_to new_user_session_path + expect(response).to redirect_to auth_setup_path end it 'creates user' do @@ -176,9 +185,9 @@ RSpec.describe Auth::RegistrationsController, type: :controller do post :create, params: { user: { account_attributes: { username: 'test' }, email: 'test@example.com', password: '12345678', password_confirmation: '12345678', 'invite_code': invite.code } } end - it 'redirects to login page' do + it 'redirects to setup' do subject - expect(response).to redirect_to new_user_session_path + expect(response).to redirect_to auth_setup_path end it 'creates user' do diff --git a/spec/controllers/auth/sessions_controller_spec.rb b/spec/controllers/auth/sessions_controller_spec.rb index 71fcc1a6e..87ef4f2bb 100644 --- a/spec/controllers/auth/sessions_controller_spec.rb +++ b/spec/controllers/auth/sessions_controller_spec.rb @@ -160,8 +160,8 @@ RSpec.describe Auth::SessionsController, type: :controller do let(:unconfirmed_user) { user.tap { |u| u.update!(confirmed_at: nil) } } let(:accept_language) { 'fr' } - it 'shows a translated login error' do - expect(flash[:alert]).to eq(I18n.t('devise.failure.unconfirmed', locale: accept_language)) + it 'redirects to home' do + expect(response).to redirect_to(root_path) end end diff --git a/spec/controllers/settings/deletes_controller_spec.rb b/spec/controllers/settings/deletes_controller_spec.rb index 35fd64e9b..996872efd 100644 --- a/spec/controllers/settings/deletes_controller_spec.rb +++ b/spec/controllers/settings/deletes_controller_spec.rb @@ -15,6 +15,15 @@ describe Settings::DeletesController do get :show expect(response).to have_http_status(200) end + + context 'when suspended' do + let(:user) { Fabricate(:user, account_attributes: { username: 'alice', suspended_at: Time.now.utc }) } + + it 'returns http forbidden' do + get :show + expect(response).to have_http_status(403) + end + end end context 'when not signed in' do @@ -49,6 +58,14 @@ describe Settings::DeletesController do it 'marks account as suspended' do expect(user.account.reload).to be_suspended end + + context 'when suspended' do + let(:user) { Fabricate(:user, account_attributes: { username: 'alice', suspended_at: Time.now.utc }) } + + it 'returns http forbidden' do + expect(response).to have_http_status(403) + end + end end context 'with incorrect password' do diff --git a/spec/features/log_in_spec.rb b/spec/features/log_in_spec.rb index 53a1f9b12..f6c26cd0f 100644 --- a/spec/features/log_in_spec.rb +++ b/spec/features/log_in_spec.rb @@ -31,12 +31,12 @@ feature "Log in" do context do given(:confirmed_at) { nil } - scenario "A unconfirmed user is not able to log in" do + scenario "A unconfirmed user is able to log in" do fill_in "user_email", with: email fill_in "user_password", with: password click_on I18n.t('auth.login') - is_expected.to have_css(".flash-message", text: failure_message("unconfirmed")) + is_expected.to have_css("div.admin-wrapper") end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 856254ce4..d7c0b5359 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -506,7 +506,7 @@ RSpec.describe User, type: :model do context 'when user is not confirmed' do let(:confirmed_at) { nil } - it { is_expected.to be false } + it { is_expected.to be true } end end @@ -522,7 +522,7 @@ RSpec.describe User, type: :model do context 'when user is not confirmed' do let(:confirmed_at) { nil } - it { is_expected.to be false } + it { is_expected.to be true } end end end -- cgit