diff --git a/app/controllers/api/v1/accounts/conversations/direct_uploads_controller.rb b/app/controllers/api/v1/accounts/conversations/direct_uploads_controller.rb index f4ac05d6e..915ade8a3 100644 --- a/app/controllers/api/v1/accounts/conversations/direct_uploads_controller.rb +++ b/app/controllers/api/v1/accounts/conversations/direct_uploads_controller.rb @@ -1,6 +1,17 @@ class Api::V1::Accounts::Conversations::DirectUploadsController < ActiveStorage::DirectUploadsController + include DeviseTokenAuth::Concerns::SetUserByToken + include RequestExceptionHandler + include AccessTokenAuthHelper include EnsureCurrentAccountHelper + + skip_before_action :verify_authenticity_token, if: :authenticate_by_access_token? + + around_action :handle_with_exception + before_action :authenticate_access_token!, if: :authenticate_by_access_token? + before_action :validate_bot_access_token!, if: :authenticate_by_access_token? + before_action :authenticate_user!, unless: :authenticate_by_access_token? before_action :current_account + before_action :validate_token_api_access, if: :authenticate_by_access_token? before_action :conversation def create @@ -11,6 +22,16 @@ class Api::V1::Accounts::Conversations::DirectUploadsController < ActiveStorage: private + def authenticate_by_access_token? + request.headers[:api_access_token].present? || request.headers[:HTTP_API_ACCESS_TOKEN].present? + end + + def validate_token_api_access + return if Current.account.api_and_webhooks_enabled? + + render json: { error: 'API access is not enabled for this account' }, status: :forbidden + end + def conversation @conversation ||= Current.account.conversations.find_by(display_id: params[:conversation_id]) end diff --git a/app/controllers/concerns/ensure_current_account_helper.rb b/app/controllers/concerns/ensure_current_account_helper.rb index ea36a48f2..7ed39fa98 100644 --- a/app/controllers/concerns/ensure_current_account_helper.rb +++ b/app/controllers/concerns/ensure_current_account_helper.rb @@ -14,6 +14,8 @@ module EnsureCurrentAccountHelper account_accessible_for_user?(account) elsif @resource.is_a?(AgentBot) account_accessible_for_bot?(account) + else + render_unauthorized(I18n.t('errors.account.not_authorized')) end account end @@ -21,7 +23,7 @@ module EnsureCurrentAccountHelper def account_accessible_for_user?(account) @current_account_user = account.account_users.find_by(user_id: current_user.id) Current.account_user = @current_account_user - render_unauthorized('You are not authorized to access this account') unless @current_account_user + render_unauthorized(I18n.t('errors.account.not_authorized')) unless @current_account_user end def account_accessible_for_bot?(account) diff --git a/app/javascript/dashboard/composables/useFileUpload.js b/app/javascript/dashboard/composables/useFileUpload.js index a0c7e3297..49809f606 100644 --- a/app/javascript/dashboard/composables/useFileUpload.js +++ b/app/javascript/dashboard/composables/useFileUpload.js @@ -2,6 +2,7 @@ import { useMapGetter } from 'dashboard/composables/store'; import { useAlert } from 'dashboard/composables'; import { useI18n } from 'vue-i18n'; import { DirectUpload } from 'activestorage'; +import { setDirectUploadAuthHeaders } from 'dashboard/helper/directUploadsHelper'; import { checkFileSizeLimit } from 'shared/helpers/FileHelper'; import { getMaxUploadSizeByChannel } from '@chatwoot/utils'; import { @@ -21,7 +22,6 @@ export const useFileUpload = ({ inbox, attachFile, isPrivateNote = false }) => { const { t } = useI18n(); const accountId = useMapGetter('getCurrentAccountId'); - const currentUser = useMapGetter('getCurrentUser'); const currentChat = useMapGetter('getSelectedChat'); const globalConfig = useMapGetter('globalConfig/get'); @@ -78,10 +78,7 @@ export const useFileUpload = ({ inbox, attachFile, isPrivateNote = false }) => { `/api/v1/accounts/${accountId.value}/conversations/${currentChat.value.id}/direct_uploads`, { directUploadWillCreateBlobWithXHR: xhr => { - xhr.setRequestHeader( - 'api_access_token', - currentUser.value.access_token - ); + setDirectUploadAuthHeaders(xhr); }, } ); diff --git a/app/javascript/dashboard/helper/directUploadsHelper.js b/app/javascript/dashboard/helper/directUploadsHelper.js new file mode 100644 index 000000000..6fafcee20 --- /dev/null +++ b/app/javascript/dashboard/helper/directUploadsHelper.js @@ -0,0 +1,19 @@ +import Auth from 'dashboard/api/auth'; + +export const setDirectUploadAuthHeaders = xhr => { + const { + 'access-token': accessToken, + 'token-type': tokenType, + client, + expiry, + uid, + } = Auth.getAuthData() || {}; + + if (!accessToken) return; + + xhr.setRequestHeader('access-token', accessToken); + xhr.setRequestHeader('token-type', tokenType); + xhr.setRequestHeader('client', client); + xhr.setRequestHeader('expiry', expiry); + xhr.setRequestHeader('uid', uid); +}; diff --git a/app/javascript/dashboard/helper/specs/directUploadsHelper.spec.js b/app/javascript/dashboard/helper/specs/directUploadsHelper.spec.js new file mode 100644 index 000000000..69c90a485 --- /dev/null +++ b/app/javascript/dashboard/helper/specs/directUploadsHelper.spec.js @@ -0,0 +1,61 @@ +import { setDirectUploadAuthHeaders } from '../directUploadsHelper'; +import Auth from 'dashboard/api/auth'; + +vi.mock('dashboard/api/auth', () => ({ + default: { getAuthData: vi.fn() }, +})); + +describe('setDirectUploadAuthHeaders', () => { + const buildXhr = () => ({ setRequestHeader: vi.fn() }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('sets the five session auth headers from the auth cookie', () => { + Auth.getAuthData.mockReturnValue({ + 'access-token': 'token-123', + 'token-type': 'Bearer', + client: 'client-123', + expiry: '9999', + uid: 'agent@example.com', + }); + const xhr = buildXhr(); + + setDirectUploadAuthHeaders(xhr); + + expect(xhr.setRequestHeader).toHaveBeenCalledTimes(5); + expect(xhr.setRequestHeader).toHaveBeenCalledWith( + 'access-token', + 'token-123' + ); + expect(xhr.setRequestHeader).toHaveBeenCalledWith('token-type', 'Bearer'); + expect(xhr.setRequestHeader).toHaveBeenCalledWith('client', 'client-123'); + expect(xhr.setRequestHeader).toHaveBeenCalledWith('expiry', '9999'); + expect(xhr.setRequestHeader).toHaveBeenCalledWith( + 'uid', + 'agent@example.com' + ); + }); + + it('does not set any header when there is no auth data', () => { + Auth.getAuthData.mockReturnValue(false); + const xhr = buildXhr(); + + setDirectUploadAuthHeaders(xhr); + + expect(xhr.setRequestHeader).not.toHaveBeenCalled(); + }); + + it('does not set any header when the access token is missing', () => { + Auth.getAuthData.mockReturnValue({ + client: 'client-123', + uid: 'agent@example.com', + }); + const xhr = buildXhr(); + + setDirectUploadAuthHeaders(xhr); + + expect(xhr.setRequestHeader).not.toHaveBeenCalled(); + }); +}); diff --git a/app/javascript/dashboard/mixins/fileUploadMixin.js b/app/javascript/dashboard/mixins/fileUploadMixin.js index 965846401..e4a2c9dce 100644 --- a/app/javascript/dashboard/mixins/fileUploadMixin.js +++ b/app/javascript/dashboard/mixins/fileUploadMixin.js @@ -3,6 +3,7 @@ import { useAlert } from 'dashboard/composables'; import { checkFileSizeLimit } from 'shared/helpers/FileHelper'; import { getMaxUploadSizeByChannel } from '@chatwoot/utils'; import { DirectUpload } from 'activestorage'; +import { setDirectUploadAuthHeaders } from 'dashboard/helper/directUploadsHelper'; import { DEFAULT_MAXIMUM_FILE_UPLOAD_SIZE, resolveMaximumFileUploadSize, @@ -77,10 +78,7 @@ export default { `/api/v1/accounts/${this.accountId}/conversations/${this.currentChat.id}/direct_uploads`, { directUploadWillCreateBlobWithXHR: xhr => { - xhr.setRequestHeader( - 'api_access_token', - this.currentUser.access_token - ); + setDirectUploadAuthHeaders(xhr); }, } ); diff --git a/config/locales/en.yml b/config/locales/en.yml index 011957fb4..1b66699f2 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -75,6 +75,7 @@ en: errors: account: + not_authorized: You are not authorized to access this account reporting_timezone: invalid: is not a valid timezone support_email: diff --git a/spec/controllers/api/v1/accounts/conversations/direct_uploads_controller_spec.rb b/spec/controllers/api/v1/accounts/conversations/direct_uploads_controller_spec.rb index 089b16b59..406f2e07c 100644 --- a/spec/controllers/api/v1/accounts/conversations/direct_uploads_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/conversations/direct_uploads_controller_spec.rb @@ -7,27 +7,120 @@ RSpec.describe '/api/v1/accounts/:account_id/conversations/:conversation_id/dire let(:contact) { create(:contact, account: account, email: nil) } let(:contact_inbox) { create(:contact_inbox, contact: contact, inbox: web_widget.inbox) } let(:conversation) { create(:conversation, contact: contact, account: account, inbox: web_widget.inbox, contact_inbox: contact_inbox) } + let(:blob_params) do + { + blob: { + filename: 'avatar.png', + byte_size: '1234', + checksum: 'dsjbsdhbfif3874823mnsdbf', + content_type: 'image/png' + } + } + end + + def create_direct_upload(headers) + post api_v1_account_conversation_direct_uploads_path(account_id: account.id, conversation_id: conversation.display_id), + params: blob_params, + headers: headers, + as: :json + end describe 'POST /api/v1/accounts/:account_id/conversations/:conversation_id/direct_uploads' do - context 'when post request is made' do - it 'creates attachment message in conversation' do - contact + context 'when it is an unauthenticated request' do + it 'returns unauthorized without any credentials' do + create_direct_upload({}) - post api_v1_account_conversation_direct_uploads_path(account_id: account.id, conversation_id: conversation.display_id), - params: { - blob: { - filename: 'avatar.png', - byte_size: '1234', - checksum: 'dsjbsdhbfif3874823mnsdbf', - content_type: 'image/png' - } - }, - headers: { api_access_token: agent.access_token.token }, - as: :json + expect(response).to have_http_status(:unauthorized) + end + + it 'returns unauthorized with an empty api_access_token header' do + create_direct_upload({ api_access_token: '' }) + + expect(response).to have_http_status(:unauthorized) + end + + it 'returns unauthorized with an invalid api_access_token header' do + create_direct_upload({ api_access_token: 'invalid-token' }) + + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when it is an authenticated request with an api access token' do + it 'creates the blob for the direct upload' do + create_direct_upload({ api_access_token: agent.access_token.token }) expect(response).to have_http_status(:success) - json_response = response.parsed_body - expect(json_response['content_type']).to eq('image/png') + expect(response.parsed_body['content_type']).to eq('image/png') + end + + it 'returns unauthorized for an agent of another account' do + other_agent = create(:user, account: create(:account), role: :agent) + + create_direct_upload({ api_access_token: other_agent.access_token.token }) + + expect(response).to have_http_status(:unauthorized) + end + + it 'returns unauthorized for an agent bot token' do + agent_bot = create(:agent_bot, account: account) + + create_direct_upload({ api_access_token: agent_bot.access_token.token }) + + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when the account api_and_webhooks feature is disabled' do + before do + allow(Account).to receive(:find).and_call_original + allow(Account).to receive(:find).with(account.id.to_s).and_return(account) + allow(account).to receive(:api_and_webhooks_enabled?).and_return(false) + end + + it 'returns forbidden for a token-authenticated request' do + create_direct_upload({ api_access_token: agent.access_token.token }) + + expect(response).to have_http_status(:forbidden) + end + + it 'still creates the blob for a session-authenticated request' do + create_direct_upload(agent.create_new_auth_token) + + expect(response).to have_http_status(:success) + expect(response.parsed_body['content_type']).to eq('image/png') + end + end + + context 'when it is an authenticated session request' do + it 'creates the blob for the direct upload' do + create_direct_upload(agent.create_new_auth_token) + + expect(response).to have_http_status(:success) + expect(response.parsed_body['content_type']).to eq('image/png') + end + + it 'creates the blob when the serialized api access token is empty' do + create_direct_upload(agent.create_new_auth_token.merge('api_access_token' => '')) + + expect(response).to have_http_status(:success) + expect(response.parsed_body['content_type']).to eq('image/png') + end + end + + context 'when forgery protection is enabled' do + around do |example| + original = ActionController::Base.allow_forgery_protection + ActionController::Base.allow_forgery_protection = true + example.run + ActionController::Base.allow_forgery_protection = original + end + + it 'creates the blob for a token-authenticated request without a CSRF token' do + create_direct_upload({ api_access_token: agent.access_token.token }) + + expect(response).to have_http_status(:success) + expect(response.parsed_body['content_type']).to eq('image/png') end end end