Merge remote-tracking branch 'tootsuite/master'
Conflicts: app/controllers/statuses_controller.rb
This commit is contained in:
		
						commit
						ac1ac20ee9
					
				
					 14 changed files with 160 additions and 32 deletions
				
			
		|  | @ -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 | ||||
|  |  | |||
|  | @ -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: | ||||
|  |  | |||
							
								
								
									
										2
									
								
								Gemfile
									
									
									
									
									
								
							
							
						
						
									
										2
									
								
								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' | ||||
|  |  | |||
|  | @ -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) | ||||
|  |  | |||
|  | @ -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) | ||||
|  |  | |||
|  | @ -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) : [] | ||||
|         @descendants = cache_collection(@status.descendants(current_account), Status) | ||||
|         @ancestors     = @status.reply? ? cache_collection(@status.ancestors(ANCESTORS_LIMIT, 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 | ||||
|  |  | |||
|  | @ -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} | ||||
|  |  | |||
|  | @ -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 { | ||||
|  |  | |||
|  | @ -3,8 +3,8 @@ | |||
| module StatusThreadingConcern | ||||
|   extend ActiveSupport::Concern | ||||
| 
 | ||||
|   def ancestors(account = nil) | ||||
|     find_statuses_from_tree_path(ancestor_ids, account) | ||||
|   def ancestors(limit, account = nil) | ||||
|     find_statuses_from_tree_path(ancestor_ids(limit), account) | ||||
|   end | ||||
| 
 | ||||
|   def descendants(account = nil) | ||||
|  | @ -13,14 +13,21 @@ module StatusThreadingConcern | |||
| 
 | ||||
|   private | ||||
| 
 | ||||
|   def ancestor_ids | ||||
|     Rails.cache.fetch("ancestors:#{id}") do | ||||
|       ancestor_statuses.pluck(:id) | ||||
|   def ancestor_ids(limit) | ||||
|     key = "ancestors:#{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 | ||||
|     Status.find_by_sql([<<-SQL.squish, id: in_reply_to_id]) | ||||
|   def ancestor_statuses(limit) | ||||
|     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 | ||||
| 
 | ||||
|  |  | |||
|  | @ -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 } | ||||
|  |  | |||
|  | @ -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 | ||||
|  |  | |||
|  | @ -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') } | ||||
| 
 | ||||
|  |  | |||
|  | @ -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 | ||||
| 
 | ||||
|  |  | |||
|  | @ -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 | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue