refactor: align conversation direct uploads with standard account auth (#15039)
Conversation attachment uploads now go through the same authentication that every other account-scoped API endpoint uses. Agents continue to attach files exactly as before, and the upload request is now tied to the agent's dashboard session instead of a separately serialized access token. Because the upload request is now authenticated, the dashboard proves the agent's session directly instead of passing `currentUser.access_token`. This keeps uploads working alongside the profile access-token changes in #14973, including on accounts where that token is serialized as empty. ## What changed - `Api::V1::Accounts::Conversations::DirectUploadsController` now runs the standard account auth stack: API access token when the `api_access_token` header is present, dashboard session (devise-token-auth) otherwise, with agent-bot tokens rejected. Previously it inherited `ActiveStorage::DirectUploadsController` directly and did not run any authentication. - `EnsureCurrentAccountHelper#ensure_current_account` now returns `401` when a request has neither an authenticated user nor a bot resource, instead of continuing. This closes the same gap for any controller that relies on the helper. - The dashboard direct-upload paths (`useFileUpload.js` and the legacy `fileUploadMixin.js`) now attach the agent's session headers to the upload request via a new `directUploadsHelper.js`, instead of sending `currentUser.access_token`. ## How to test 1. As a logged-in agent, open a conversation and attach a file. Upload should succeed as before, on installs with direct uploads enabled. 2. Confirm attachments still work for an agent on an account whose profile access token is not serialized (e.g. a Cloud plan without `api_and_webhooks`). 3. Send a `POST` to `/api/v1/accounts/:account_id/conversations/:conversation_id/direct_uploads` with no credentials, an empty `api_access_token`, or an invalid token, and confirm it returns `401`. 4. Confirm a valid agent of the account (via API token or session) gets `200`, while an agent of a different account gets `401`.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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);
|
||||
},
|
||||
}
|
||||
);
|
||||
|
||||
@@ -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);
|
||||
};
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -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);
|
||||
},
|
||||
}
|
||||
);
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user