Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow anonymous profile view #1304

Open
wants to merge 10 commits into
base: hometown-dev
Choose a base branch
from
Open

Allow anonymous profile view #1304

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1c4948d
Exempt Emoji API endpoint from auth
ironiridis Apr 10, 2023
9fbcfe7
Move unless criteria into boolean method
ironiridis Apr 10, 2023
0c9c7b8
Add user-oriented helper methods
ironiridis Apr 10, 2023
135b9f5
Allow restricted API lookup if user does not opt-out
ironiridis Apr 10, 2023
86ba692
Allow restricted API account details if user does not opt-out
ironiridis Apr 10, 2023
744f500
Fix front-end to avoid tight request loop on 401 response
ironiridis Apr 10, 2023
7d7e61d
Return empty statuses and tags for users visible through restricted API
ironiridis Apr 10, 2023
e6f8a67
Review fix: Use only directive for all auth skips
ironiridis May 7, 2023
3897eaa
Review fix: Avoid ternary expressions
ironiridis May 7, 2023
641b071
Review fix: Restructure for early return
ironiridis May 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion app/controllers/api/base_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,16 @@ def set_cache_headers
end

def disallow_unauthenticated_api_access?
ENV['DISALLOW_UNAUTHENTICATED_API_ACCESS'] == 'true' || Rails.configuration.x.whitelist_mode
(ENV['DISALLOW_UNAUTHENTICATED_API_ACCESS'] == 'true' || Rails.configuration.x.whitelist_mode) unless current_user
end

def user_would_block_unauthenticated_api_access?(account)
# alternately account.locked? would also be a good candidate for this
disallow_unauthenticated_api_access? && account.user_prefers_noindex?
end

def user_blocks_unauthenticated_api_access
render json: { error: 'This user is only visible to authenticated users' }, status: 401
end

private
Expand Down
6 changes: 5 additions & 1 deletion app/controllers/api/v1/accounts/featured_tags_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,14 @@
class Api::V1::Accounts::FeaturedTagsController < Api::BaseController
before_action :set_account
before_action :set_featured_tags
skip_before_action :require_authenticated_user!, only: [:index]

respond_to :json

def index
if user_would_block_unauthenticated_api_access?(@account)
user_blocks_unauthenticated_api_access and return
end
Comment on lines +11 to +13
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if user_would_block_unauthenticated_api_access?(@account)
user_blocks_unauthenticated_api_access and return
end
if user_would_block_unauthenticated_api_access?(@account)
user_blocks_unauthenticated_api_access and return
end

Since you end up having this statement in a bunch of your controllers, you could move it up to the Api::BaseController and implement it once.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My very basic Ruby grasp is what led me to doing this this way. I needed user_blocks_unauthenticated_api_access to return the JSON object and 401 status, and I wasn't sure how to invoke the render behavior and return from the function. Earlier iterations of this change set had a method that called this, but the calling code would continue and would try to render another result, which resulted in errors in the logs as a response was already being sent. The ... and return pattern was the only way I could think of to solve this, but I'm very open to alternatives (and to learning!)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the and return approach would work still work if you moved this up to a method in the base controller. But maybe not. Controller behavior in Rails is frequently surprising.

render json: @featured_tags, each_serializer: REST::FeaturedTagSerializer
end

Expand All @@ -17,6 +21,6 @@ def set_account
end

def set_featured_tags
@featured_tags = @account.suspended? ? [] : @account.featured_tags
@featured_tags = @account.suspended? || disallow_unauthenticated_api_access? ? [] : @account.featured_tags
end
end
4 changes: 4 additions & 0 deletions app/controllers/api/v1/accounts/lookup_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
# frozen_string_literal: true

class Api::V1::Accounts::LookupController < Api::BaseController
skip_before_action :require_authenticated_user!
before_action -> { authorize_if_got_token! :read, :'read:accounts' }
before_action :set_account

def show
if user_would_block_unauthenticated_api_access?(@account)
user_blocks_unauthenticated_api_access and return
end
render json: @account, serializer: REST::AccountSerializer
end

Expand Down
6 changes: 5 additions & 1 deletion app/controllers/api/v1/accounts/statuses_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,15 @@
class Api::V1::Accounts::StatusesController < Api::BaseController
before_action -> { authorize_if_got_token! :read, :'read:statuses' }
before_action :set_account
skip_before_action :require_authenticated_user!, only: [:index]

after_action :insert_pagination_headers, unless: -> { truthy_param?(:pinned) }

def index
@statuses = load_statuses
if user_would_block_unauthenticated_api_access?(@account)
user_blocks_unauthenticated_api_access and return
end
render json: @statuses, each_serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new(@statuses, current_user&.account_id)
end

Expand All @@ -18,7 +22,7 @@ def set_account
end

def load_statuses
@account.suspended? ? [] : cached_account_statuses
@account.suspended? || disallow_unauthenticated_api_access? ? [] : cached_account_statuses
end

def cached_account_statuses
Expand Down
5 changes: 4 additions & 1 deletion app/controllers/api/v1/accounts_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,14 @@ class Api::V1::AccountsController < Api::BaseController
before_action :check_account_confirmation, except: [:create]
before_action :check_enabled_registrations, only: [:create]

skip_before_action :require_authenticated_user!, only: :create
skip_before_action :require_authenticated_user!, only: [:create, :show]

override_rate_limit_headers :follow, family: :follows

def show
if user_would_block_unauthenticated_api_access?(@account)
user_blocks_unauthenticated_api_access and return
end
render json: @account, serializer: REST::AccountSerializer
end

Expand Down
1 change: 1 addition & 0 deletions app/controllers/api/v1/custom_emojis_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

class Api::V1::CustomEmojisController < Api::BaseController
skip_before_action :set_cache_headers
skip_before_action :require_authenticated_user!

def index
expires_in 3.minutes, public: true
Expand Down
4 changes: 4 additions & 0 deletions app/javascript/mastodon/reducers/timelines.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,10 @@ export default function timelines(state = initialState, action) {
case TIMELINE_EXPAND_REQUEST:
return state.update(action.timeline, initialTimeline, map => map.set('isLoading', true));
case TIMELINE_EXPAND_FAIL:
if (action.error?.response?.status === 401) {
// don't loop continuously on 401 unauthenticated response
return state.update(action.timeline, initialTimeline, map => map.set('hasMore', false));
}
return state.update(action.timeline, initialTimeline, map => map.set('isLoading', false));
case TIMELINE_EXPAND_SUCCESS:
return expandNormalizedTimeline(state, action.timeline, fromJS(action.statuses), action.next, action.partial, action.isLoadingRecent, action.usePendingItems);
Expand Down