Merge remote-tracking branch 'tootsuite/master'

Conflicts:
 	app/controllers/statuses_controller.rb

.env.test

@@ -1,3 +1,7 @@
 # Federation
 LOCAL_DOMAIN=cb6e6126.ngrok.io
 LOCAL_HTTPS=true
+# test pam authentication
+PAM_ENABLED=true
+PAM_DEFAULT_SERVICE=pam_test
+PAM_CONTROLLED_SERVICE=pam_test_controlled

.travis.yml

@@ -19,6 +19,7 @@ env:
     - LOCAL_HTTPS=true
     - RAILS_ENV=test
     - PARALLEL_TEST_PROCESSORS=2
+    - ALLOW_NOPAM=true
 
 addons:
   postgresql: 9.4
@@ -43,7 +44,7 @@ services:
 
 install:
   - nvm install
-  - bundle install --path=vendor/bundle --without development production --retry=3 --jobs=16
+  - bundle install --path=vendor/bundle --with pam_authentication --without development production --retry=3 --jobs=16
   - yarn install
 
 before_script:

Gemfile

@@ -34,7 +34,7 @@ gem 'devise', '~> 4.4'
 gem 'devise-two-factor', '~> 3.0'
 
 group :pam_authentication, optional: true do
-  gem 'devise_pam_authenticatable2', '~> 9.0'
+  gem 'devise_pam_authenticatable2', '~> 9.1'
 end
 
 gem 'net-ldap', '~> 0.10'

Gemfile.lock

@@ -146,9 +146,9 @@ GEM
       devise (~> 4.0)
       railties (< 5.2)
       rotp (~> 2.0)
-    devise_pam_authenticatable2 (9.0.0)
+    devise_pam_authenticatable2 (9.1.0)
       devise (>= 4.0.0)
-      rpam2 (~> 3.0)
+      rpam2 (~> 4.0)
     diff-lcs (1.3)
     docile (1.1.5)
     domain_name (0.5.20170404)
@@ -467,7 +467,7 @@ GEM
       actionpack (>= 4.2.0, < 5.3)
       railties (>= 4.2.0, < 5.3)
     rotp (2.1.2)
-    rpam2 (3.1.0)
+    rpam2 (4.0.2)
     rqrcode (0.10.1)
       chunky_png (~> 1.0)
     rspec-core (3.7.0)
@@ -642,7 +642,7 @@ DEPENDENCIES
   climate_control (~> 0.2)
   devise (~> 4.4)
   devise-two-factor (~> 3.0)
-  devise_pam_authenticatable2 (~> 9.0)
+  devise_pam_authenticatable2 (~> 9.1)
   doorkeeper (~> 4.2)
   dotenv-rails (~> 2.2)
   fabrication (~> 2.18)

app/controllers/api/v1/statuses_controller.rb

@@ -17,7 +17,7 @@ class Api::V1::StatusesController < Api::BaseController
   end
 
   def context
-    ancestors_results   = @status.in_reply_to_id.nil? ? [] : @status.ancestors(current_account)
+    ancestors_results   = @status.in_reply_to_id.nil? ? [] : @status.ancestors(DEFAULT_STATUSES_LIMIT, current_account)
     descendants_results = @status.descendants(current_account)
     loaded_ancestors    = cache_collection(ancestors_results, Status)
     loaded_descendants  = cache_collection(descendants_results, Status)

app/controllers/statuses_controller.rb

@@ -4,6 +4,8 @@ class StatusesController < ApplicationController
   include SignatureAuthentication
   include Authorization
 
+  ANCESTORS_LIMIT = 20
+
   layout 'public'
 
   before_action :set_account
@@ -17,8 +19,9 @@ class StatusesController < ApplicationController
     respond_to do |format|
       format.html do
         use_pack 'public'
-        @ancestors   = @status.reply? ? cache_collection(@status.ancestors(current_account), Status) : []
+        @ancestors     = @status.reply? ? cache_collection(@status.ancestors(ANCESTORS_LIMIT, current_account), Status) : []
-        @descendants = cache_collection(@status.descendants(current_account), Status)
+        @next_ancestor = @ancestors.size < ANCESTORS_LIMIT ? nil : @ancestors.shift
+        @descendants   = cache_collection(@status.descendants(current_account), Status)
 
         render 'stream_entries/show'
       end

app/javascript/mastodon/features/compose/components/privacy_dropdown.js

@@ -32,6 +32,10 @@ class PrivacyDropdownMenu extends React.PureComponent {
     onChange: PropTypes.func.isRequired,
   };
 
+  state = {
+    mounted: false,
+  };
+
   handleDocumentClick = e => {
     if (this.node && !this.node.contains(e.target)) {
       this.props.onClose();
@@ -54,6 +58,7 @@ class PrivacyDropdownMenu extends React.PureComponent {
   componentDidMount () {
     document.addEventListener('click', this.handleDocumentClick, false);
     document.addEventListener('touchend', this.handleDocumentClick, listenerOptions);
+    this.setState({ mounted: true });
   }
 
   componentWillUnmount () {
@@ -66,12 +71,16 @@ class PrivacyDropdownMenu extends React.PureComponent {
   }
 
   render () {
+    const { mounted } = this.state;
     const { style, items, value } = this.props;
 
     return (
       <Motion defaultStyle={{ opacity: 0, scaleX: 0.85, scaleY: 0.75 }} style={{ opacity: spring(1, { damping: 35, stiffness: 400 }), scaleX: spring(1, { damping: 35, stiffness: 400 }), scaleY: spring(1, { damping: 35, stiffness: 400 }) }}>
         {({ opacity, scaleX, scaleY }) => (
-          <div className='privacy-dropdown__dropdown' style={{ ...style, opacity: opacity, transform: `scale(${scaleX}, ${scaleY})` }} ref={this.setRef}>
+          // It should not be transformed when mounting because the resulting
+          // size will be used to determine the coordinate of the menu by
+          // react-overlays
+          <div className='privacy-dropdown__dropdown' style={{ ...style, opacity: opacity, transform: mounted ? `scale(${scaleX}, ${scaleY})` : null }} ref={this.setRef}>
             {items.map(item => (
               <div role='button' tabIndex='0' key={item.value} data-index={item.value} onKeyDown={this.handleClick} onClick={this.handleClick} className={classNames('privacy-dropdown__option', { active: item.value === value })}>
                 <div className='privacy-dropdown__option__icon'>
@@ -107,9 +116,10 @@ export default class PrivacyDropdown extends React.PureComponent {
 
   state = {
     open: false,
+    placement: null,
   };
 
-  handleToggle = () => {
+  handleToggle = ({ target }) => {
     if (this.props.isUserTouching()) {
       if (this.state.open) {
         this.props.onModalClose();
@@ -120,6 +130,8 @@ export default class PrivacyDropdown extends React.PureComponent {
         });
       }
     } else {
+      const { top } = target.getBoundingClientRect();
+      this.setState({ placement: top * 2 < innerHeight ? 'bottom' : 'top' });
       this.setState({ open: !this.state.open });
     }
   }
@@ -136,7 +148,7 @@ export default class PrivacyDropdown extends React.PureComponent {
   handleKeyDown = e => {
     switch(e.key) {
     case 'Enter':
-      this.handleToggle();
+      this.handleToggle(e);
       break;
     case 'Escape':
       this.handleClose();
@@ -165,7 +177,7 @@ export default class PrivacyDropdown extends React.PureComponent {
 
   render () {
     const { value, intl } = this.props;
-    const { open } = this.state;
+    const { open, placement } = this.state;
 
     const valueOption = this.options.find(item => item.value === value);
 
@@ -185,7 +197,7 @@ export default class PrivacyDropdown extends React.PureComponent {
           />
         </div>
 
-        <Overlay show={open} placement='bottom' target={this}>
+        <Overlay show={open} placement={placement} target={this}>
           <PrivacyDropdownMenu
             items={this.options}
             value={value}

app/javascript/styles/mastodon/stream_entries.scss

@@ -6,7 +6,8 @@
     background: $simple-background-color;
 
     .detailed-status.light,
-    .status.light {
+    .status.light,
+    .more.light {
       border-bottom: 1px solid $ui-secondary-color;
       animation: none;
     }
@@ -290,6 +291,17 @@
       text-decoration: underline;
     }
   }
+
+  .more {
+    color: $classic-primary-color;
+    display: block;
+    padding: 14px;
+    text-align: center;
+
+    &:not(:hover) {
+      text-decoration: none;
+    }
+  }
 }
 
 .embed {

app/models/concerns/status_threading_concern.rb

@@ -3,8 +3,8 @@
 module StatusThreadingConcern
   extend ActiveSupport::Concern
 
-  def ancestors(account = nil)
+  def ancestors(limit, account = nil)
-    find_statuses_from_tree_path(ancestor_ids, account)
+    find_statuses_from_tree_path(ancestor_ids(limit), account)
   end
 
   def descendants(account = nil)
@@ -13,14 +13,21 @@ module StatusThreadingConcern
 
   private
 
-  def ancestor_ids
+  def ancestor_ids(limit)
-    Rails.cache.fetch("ancestors:#{id}") do
+    key = "ancestors:#{id}"
-      ancestor_statuses.pluck(:id)
+    ancestors = Rails.cache.fetch(key)
+
+    if ancestors.nil? || ancestors[:limit] < limit
+      ids = ancestor_statuses(limit).pluck(:id).reverse!
+      Rails.cache.write key, limit: limit, ids: ids
+      ids
+    else
+      ancestors[:ids].last(limit)
     end
   end
 
-  def ancestor_statuses
+  def ancestor_statuses(limit)
-    Status.find_by_sql([<<-SQL.squish, id: in_reply_to_id])
+    Status.find_by_sql([<<-SQL.squish, id: in_reply_to_id, limit: limit])
       WITH RECURSIVE search_tree(id, in_reply_to_id, path)
       AS (
         SELECT id, in_reply_to_id, ARRAY[id]
@@ -34,7 +41,8 @@ module StatusThreadingConcern
       )
       SELECT id
       FROM search_tree
-      ORDER BY path DESC
+      ORDER BY path
+      LIMIT :limit
     SQL
   end
 

app/views/stream_entries/_status.html.haml

@@ -14,6 +14,10 @@
   entry_classes = h_class + ' ' + mf_classes + ' ' + style_classes
 
 - if status.reply? && include_threads
+  - if @next_ancestor
+    .entry{ class: entry_classes }
+      = link_to short_account_status_url(@next_ancestor.account.username, @next_ancestor), class: 'more light'  do
+        = t('statuses.show_more')
   = render partial: 'stream_entries/status', collection: @ancestors, as: :status, locals: { is_predecessor: true, direct_reply_id: status.in_reply_to_id }
 
 .entry{ class: entry_classes }

config/environments/test.rb

@@ -59,3 +59,14 @@ Rails.application.configure do
 end
 
 Paperclip::Attachment.default_options[:path] = "#{Rails.root}/spec/test_files/:class/:id_partition/:style.:extension"
+
+# set fake_data for pam, don't do real calls, just use fake data
+if ENV['PAM_ENABLED'] == 'true'
+  Rpam2.fake_data =
+    {
+      usernames: Set['pam_user1', 'pam_user2'],
+      servicenames: Set['pam_test', 'pam_test_controlled'],
+      password: '123456',
+      env: { email: 'pam@example.com' }
+    }
+end

spec/controllers/auth/sessions_controller_spec.rb

@@ -48,6 +48,57 @@ RSpec.describe Auth::SessionsController, type: :controller do
       request.env['devise.mapping'] = Devise.mappings[:user]
     end
 
+    context 'using PAM authentication' do
+      context 'using a valid password' do
+        before do
+          post :create, params: { user: { email: "pam_user1", password: '123456' } }
+        end
+
+        it 'redirects to home' do
+          expect(response).to redirect_to(root_path)
+        end
+
+        it 'logs the user in' do
+          expect(controller.current_user).to be_instance_of(User)
+        end
+      end
+
+      context 'using an invalid password' do
+        before do
+          post :create, params: { user: { email: "pam_user1", password: 'WRONGPW' } }
+        end
+
+        it 'shows a login error' do
+          expect(flash[:alert]).to match I18n.t('devise.failure.invalid', authentication_keys: 'Email')
+        end
+
+        it "doesn't log the user in" do
+          expect(controller.current_user).to be_nil
+        end
+      end
+
+      context 'using a valid email and existing user' do
+        let(:user) do
+          account = Fabricate.build(:account, username: 'pam_user1')
+          account.save!(validate: false)
+          user = Fabricate(:user, email: 'pam@example.com', password: nil, account: account)
+          user
+        end
+
+        before do
+          post :create, params: { user: { email: user.email, password: '123456' } }
+        end
+
+        it 'redirects to home' do
+          expect(response).to redirect_to(root_path)
+        end
+
+        it 'logs the user in' do
+          expect(controller.current_user).to eq user
+        end
+      end
+    end
+
     context 'using password authentication' do
       let(:user) { Fabricate(:user, email: 'foo@bar.com', password: 'abcdefgh') }
 

spec/models/concerns/status_threading_concern_spec.rb

@@ -14,34 +14,34 @@ describe StatusThreadingConcern do
     let!(:viewer) { Fabricate(:account, username: 'viewer') }
 
     it 'returns conversation history' do
-      expect(reply3.ancestors).to include(status, reply1, reply2)
+      expect(reply3.ancestors(4)).to include(status, reply1, reply2)
     end
 
     it 'does not return conversation history user is not allowed to see' do
       reply1.update(visibility: :private)
       status.update(visibility: :direct)
 
-      expect(reply3.ancestors(viewer)).to_not include(reply1, status)
+      expect(reply3.ancestors(4, viewer)).to_not include(reply1, status)
     end
 
     it 'does not return conversation history from blocked users' do
       viewer.block!(jeff)
-      expect(reply3.ancestors(viewer)).to_not include(reply1)
+      expect(reply3.ancestors(4, viewer)).to_not include(reply1)
     end
 
     it 'does not return conversation history from muted users' do
       viewer.mute!(jeff)
-      expect(reply3.ancestors(viewer)).to_not include(reply1)
+      expect(reply3.ancestors(4, viewer)).to_not include(reply1)
     end
 
     it 'does not return conversation history from silenced and not followed users' do
       jeff.update(silenced: true)
-      expect(reply3.ancestors(viewer)).to_not include(reply1)
+      expect(reply3.ancestors(4, viewer)).to_not include(reply1)
     end
 
     it 'does not return conversation history from blocked domains' do
       viewer.block_domain!('example.com')
-      expect(reply3.ancestors(viewer)).to_not include(reply2)
+      expect(reply3.ancestors(4, viewer)).to_not include(reply2)
     end
 
     it 'ignores deleted records' do
@@ -49,10 +49,32 @@ describe StatusThreadingConcern do
       second_status = Fabricate(:status, thread: first_status, account: alice)
 
       # Create cache and delete cached record
-      second_status.ancestors
+      second_status.ancestors(4)
       first_status.destroy
 
-      expect(second_status.ancestors).to eq([])
+      expect(second_status.ancestors(4)).to eq([])
+    end
+
+    it 'can return more records than previously requested' do
+      first_status  = Fabricate(:status, account: bob)
+      second_status = Fabricate(:status, thread: first_status, account: alice)
+      third_status = Fabricate(:status, thread: second_status, account: alice)
+
+      # Create cache
+      second_status.ancestors(1)
+
+      expect(third_status.ancestors(2)).to eq([first_status, second_status])
+    end
+
+    it 'can return fewer records than previously requested' do
+      first_status  = Fabricate(:status, account: bob)
+      second_status = Fabricate(:status, thread: first_status, account: alice)
+      third_status = Fabricate(:status, thread: second_status, account: alice)
+
+      # Create cache
+      second_status.ancestors(2)
+
+      expect(third_status.ancestors(1)).to eq([second_status])
     end
   end
 

spec/views/stream_entries/show.html.haml_spec.rb

@@ -48,7 +48,7 @@ describe 'stream_entries/show.html.haml', without_verify_partial_doubles: true d
     assign(:stream_entry, reply.stream_entry)
     assign(:account, alice)
     assign(:type, reply.stream_entry.activity_type.downcase)
-    assign(:ancestors, reply.stream_entry.activity.ancestors(bob) )
+    assign(:ancestors, reply.stream_entry.activity.ancestors(1, bob) )
     assign(:descendants, reply.stream_entry.activity.descendants(bob))
 
     render