refactor: use state-based authentication (#11690)

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Muhsin Keloth <muhsinkeramam@gmail.com>
This commit is contained in:
Shivam Mishra
2025-06-18 17:39:06 +05:30
committed by GitHub
parent 768fa9ab1b
commit f6dbbf0d90
14 changed files with 85 additions and 108 deletions

View File

@@ -1,32 +1,23 @@
class Api::V1::Accounts::Google::AuthorizationsController < Api::V1::Accounts::BaseController
class Api::V1::Accounts::Google::AuthorizationsController < Api::V1::Accounts::OauthAuthorizationController
include GoogleConcern
before_action :check_authorization
def create
email = params[:authorization][:email]
redirect_url = google_client.auth_code.authorize_url(
{
redirect_uri: "#{base_url}/google/callback",
scope: 'email profile https://mail.google.com/',
scope: scope,
response_type: 'code',
prompt: 'consent', # the oauth flow does not return a refresh token, this is supposed to fix it
access_type: 'offline', # the default is 'online'
state: state,
client_id: GlobalConfigService.load('GOOGLE_OAUTH_CLIENT_ID', nil)
}
)
if redirect_url
cache_key = "google::#{email.downcase}"
::Redis::Alfred.setex(cache_key, Current.account.id, 5.minutes)
render json: { success: true, url: redirect_url }
else
render json: { success: false }, status: :unprocessable_entity
end
end
private
def check_authorization
raise Pundit::NotAuthorizedError unless Current.account_user.administrator?
end
end

View File

@@ -1,7 +1,6 @@
class Api::V1::Accounts::Instagram::AuthorizationsController < Api::V1::Accounts::BaseController
class Api::V1::Accounts::Instagram::AuthorizationsController < Api::V1::Accounts::OauthAuthorizationController
include InstagramConcern
include Instagram::IntegrationHelper
before_action :check_authorization
def create
# https://developers.facebook.com/docs/instagram-platform/instagram-api-with-instagram-login/business-login#step-1--get-authorization
@@ -21,10 +20,4 @@ class Api::V1::Accounts::Instagram::AuthorizationsController < Api::V1::Accounts
render json: { success: false }, status: :unprocessable_entity
end
end
private
def check_authorization
raise Pundit::NotAuthorizedError unless Current.account_user.administrator?
end
end

View File

@@ -1,28 +1,19 @@
class Api::V1::Accounts::Microsoft::AuthorizationsController < Api::V1::Accounts::BaseController
class Api::V1::Accounts::Microsoft::AuthorizationsController < Api::V1::Accounts::OauthAuthorizationController
include MicrosoftConcern
before_action :check_authorization
def create
email = params[:authorization][:email]
redirect_url = microsoft_client.auth_code.authorize_url(
{
redirect_uri: "#{base_url}/microsoft/callback",
scope: 'offline_access https://outlook.office.com/IMAP.AccessAsUser.All https://outlook.office.com/SMTP.Send openid profile',
scope: scope,
state: state,
prompt: 'consent'
}
)
if redirect_url
cache_key = "microsoft::#{email.downcase}"
::Redis::Alfred.setex(cache_key, Current.account.id, 5.minutes)
render json: { success: true, url: redirect_url }
else
render json: { success: false }, status: :unprocessable_entity
end
end
private
def check_authorization
raise Pundit::NotAuthorizedError unless Current.account_user.administrator?
end
end

View File

@@ -0,0 +1,23 @@
class Api::V1::Accounts::OauthAuthorizationController < Api::V1::Accounts::BaseController
before_action :check_authorization
protected
def scope
''
end
def state
Current.account.to_sgid(expires_in: 15.minutes).to_s
end
def base_url
ENV.fetch('FRONTEND_URL', 'http://localhost:3000')
end
private
def check_authorization
raise Pundit::NotAuthorizedError unless Current.account_user.administrator?
end
end

View File

@@ -14,7 +14,7 @@ module GoogleConcern
private
def base_url
ENV.fetch('FRONTEND_URL', 'http://localhost:3000')
def scope
'email profile https://mail.google.com/'
end
end

View File

@@ -15,7 +15,7 @@ module MicrosoftConcern
private
def base_url
ENV.fetch('FRONTEND_URL', 'http://localhost:3000')
def scope
'offline_access https://outlook.office.com/IMAP.AccessAsUser.All https://outlook.office.com/SMTP.Send openid profile email'
end
end

View File

@@ -6,7 +6,6 @@ class OauthCallbackController < ApplicationController
)
handle_response
::Redis::Alfred.delete(cache_key)
rescue StandardError => e
ChatwootExceptionTracker.new(e).capture_exception
redirect_to '/'
@@ -64,10 +63,6 @@ class OauthCallbackController < ApplicationController
raise NotImplementedError
end
def cache_key
"#{provider_name}::#{users_data['email'].downcase}"
end
def create_channel_with_inbox
ActiveRecord::Base.transaction do
channel_email = Channel::Email.create!(email: users_data['email'], account: account)
@@ -86,12 +81,17 @@ class OauthCallbackController < ApplicationController
decoded_token[0]
end
def account_id
::Redis::Alfred.get(cache_key)
def account_from_signed_id
raise ActionController::BadRequest, 'Missing state variable' if params[:state].blank?
account = GlobalID::Locator.locate_signed(params[:state])
raise 'Invalid or expired state' if account.nil?
account
end
def account
@account ||= Account.find(account_id)
@account ||= account_from_signed_id
end
# Fallback name, for when name field is missing from users_data

View File

@@ -12,7 +12,6 @@ defineOptions({
provider="google"
:title="$t('INBOX_MGMT.ADD.GOOGLE.TITLE')"
:description="$t('INBOX_MGMT.ADD.GOOGLE.DESCRIPTION')"
:input-placeholder="$t('INBOX_MGMT.ADD.GOOGLE.EMAIL_PLACEHOLDER')"
:submit-button-text="$t('INBOX_MGMT.ADD.GOOGLE.SIGN_IN')"
:error-message="$t('INBOX_MGMT.ADD.GOOGLE.ERROR_MESSAGE')"
/>

View File

@@ -12,7 +12,6 @@ defineOptions({
provider="microsoft"
:title="$t('INBOX_MGMT.ADD.MICROSOFT.TITLE')"
:description="$t('INBOX_MGMT.ADD.MICROSOFT.DESCRIPTION')"
:input-placeholder="$t('INBOX_MGMT.ADD.MICROSOFT.EMAIL_PLACEHOLDER')"
:submit-button-text="$t('INBOX_MGMT.ADD.MICROSOFT.SIGN_IN')"
:error-message="$t('INBOX_MGMT.ADD.MICROSOFT.ERROR_MESSAGE')"
/>

View File

@@ -30,14 +30,9 @@ const props = defineProps({
type: String,
required: true,
},
inputPlaceholder: {
type: String,
required: true,
},
});
const isRequestingAuthorization = ref(false);
const email = ref('');
const client = computed(() => {
if (props.provider === 'microsoft') {
@@ -50,9 +45,7 @@ const client = computed(() => {
async function requestAuthorization() {
try {
isRequestingAuthorization.value = true;
const response = await client.value.generateAuthorization({
email: email.value,
});
const response = await client.value.generateAuthorization();
const {
data: { url },
} = response;
@@ -75,11 +68,6 @@ async function requestAuthorization() {
:header-content="description"
/>
<form class="mt-6" @submit.prevent="requestAuthorization">
<woot-input
v-model="email"
type="email"
:placeholder="inputPlaceholder"
/>
<NextButton
:is-loading="isRequestingAuthorization"
type="submit"

View File

@@ -32,19 +32,20 @@ RSpec.describe 'Google Authorization API', type: :request do
as: :json
expect(response).to have_http_status(:success)
google_service = Class.new { extend GoogleConcern }
response_url = google_service.google_client.auth_code.authorize_url(
{
redirect_uri: "#{ENV.fetch('FRONTEND_URL', 'http://localhost:3000')}/google/callback",
scope: 'email profile https://mail.google.com/',
response_type: 'code',
prompt: 'consent',
access_type: 'offline',
client_id: GlobalConfigService.load('GOOGLE_OAUTH_CLIENT_ID', nil)
}
)
expect(response.parsed_body['url']).to eq response_url
expect(Redis::Alfred.get("google::#{administrator.email}")).to eq(account.id.to_s)
# Validate URL components
url = response.parsed_body['url']
uri = URI.parse(url)
params = CGI.parse(uri.query)
expect(url).to start_with('https://accounts.google.com/o/oauth2/auth')
expect(params['scope']).to eq(['email profile https://mail.google.com/'])
expect(params['redirect_uri']).to eq(["#{ENV.fetch('FRONTEND_URL', 'http://localhost:3000')}/google/callback"])
# Validate state parameter exists and can be decoded back to the account
expect(params['state']).to be_present
decoded_account = GlobalID::Locator.locate_signed(params['state'].first, for: 'default')
expect(decoded_account).to eq(account)
end
end
end

View File

@@ -19,7 +19,6 @@ RSpec.describe 'Microsoft Authorization API', type: :request do
it 'returns unathorized for agent' do
post "/api/v1/accounts/#{account.id}/microsoft/authorization",
headers: agent.create_new_auth_token,
params: { email: administrator.email },
as: :json
expect(response).to have_http_status(:unauthorized)
@@ -28,20 +27,27 @@ RSpec.describe 'Microsoft Authorization API', type: :request do
it 'creates a new authorization and returns the redirect url' do
post "/api/v1/accounts/#{account.id}/microsoft/authorization",
headers: administrator.create_new_auth_token,
params: { email: administrator.email },
as: :json
expect(response).to have_http_status(:success)
microsoft_service = Class.new { extend MicrosoftConcern }
response_url = microsoft_service.microsoft_client.auth_code.authorize_url(
{
redirect_uri: "#{ENV.fetch('FRONTEND_URL', 'http://localhost:3000')}/microsoft/callback",
scope: 'offline_access https://outlook.office.com/IMAP.AccessAsUser.All https://outlook.office.com/SMTP.Send openid profile',
prompt: 'consent'
}
)
expect(response.parsed_body['url']).to eq response_url
expect(Redis::Alfred.get("microsoft::#{administrator.email}")).to eq(account.id.to_s)
# Validate URL components
url = response.parsed_body['url']
uri = URI.parse(url)
params = CGI.parse(uri.query)
expect(url).to start_with('https://login.microsoftonline.com/common/oauth2/v2.0/authorize')
expected_scope = [
'offline_access https://outlook.office.com/IMAP.AccessAsUser.All ' \
'https://outlook.office.com/SMTP.Send openid profile email'
]
expect(params['scope']).to eq(expected_scope)
expect(params['redirect_uri']).to eq(["#{ENV.fetch('FRONTEND_URL', 'http://localhost:3000')}/microsoft/callback"])
# Validate state parameter exists and can be decoded back to the account
expect(params['state']).to be_present
decoded_account = GlobalID::Locator.locate_signed(params['state'].first, for: 'default')
expect(decoded_account).to eq(account)
end
end
end

View File

@@ -4,11 +4,7 @@ RSpec.describe 'Google::CallbacksController', type: :request do
let(:account) { create(:account) }
let(:code) { SecureRandom.hex(10) }
let(:email) { Faker::Internet.email }
let(:cache_key) { "google::#{email.downcase}" }
before do
Redis::Alfred.set(cache_key, account.id)
end
let(:state) { account.to_sgid(expires_in: 15.minutes).to_s }
describe 'GET /google/callback' do
let(:response_body_success) do
@@ -27,7 +23,7 @@ RSpec.describe 'Google::CallbacksController', type: :request do
'redirect_uri' => "#{ENV.fetch('FRONTEND_URL', 'http://localhost:3000')}/google/callback" })
.to_return(status: 200, body: response_body_success.to_json, headers: { 'Content-Type' => 'application/json' })
get google_callback_url, params: { code: code }
get google_callback_url, params: { code: code, state: state }
expect(response).to redirect_to app_email_inbox_agents_url(account_id: account.id, inbox_id: account.inboxes.last.id)
expect(account.inboxes.count).to be 1
@@ -36,7 +32,6 @@ RSpec.describe 'Google::CallbacksController', type: :request do
expect(inbox.channel.reload.provider_config.keys).to include('access_token', 'refresh_token', 'expires_on')
expect(inbox.channel.reload.provider_config['access_token']).to eq response_body_success[:access_token]
expect(inbox.channel.imap_address).to eq 'imap.gmail.com'
expect(Redis::Alfred.get(cache_key)).to be_nil
end
it 'updates inbox channel config if inbox exists with imap_login and authentication is successful' do
@@ -49,14 +44,13 @@ RSpec.describe 'Google::CallbacksController', type: :request do
'redirect_uri' => "#{ENV.fetch('FRONTEND_URL', 'http://localhost:3000')}/google/callback" })
.to_return(status: 200, body: response_body_success.to_json, headers: { 'Content-Type' => 'application/json' })
get google_callback_url, params: { code: code }
get google_callback_url, params: { code: code, state: state }
expect(response).to redirect_to app_email_inbox_settings_url(account_id: account.id, inbox_id: inbox.id)
expect(account.inboxes.count).to be 1
expect(inbox.channel.reload.provider_config.keys).to include('access_token', 'refresh_token', 'expires_on')
expect(inbox.channel.reload.provider_config['access_token']).to eq response_body_success[:access_token]
expect(inbox.channel.imap_address).to eq 'imap.gmail.com'
expect(Redis::Alfred.get(cache_key)).to be_nil
end
it 'creates inboxes with fallback_name when account name is not present in id_token' do
@@ -65,7 +59,7 @@ RSpec.describe 'Google::CallbacksController', type: :request do
'redirect_uri' => "#{ENV.fetch('FRONTEND_URL', 'http://localhost:3000')}/google/callback" })
.to_return(status: 200, body: response_body_success_without_name.to_json, headers: { 'Content-Type' => 'application/json' })
get google_callback_url, params: { code: code }
get google_callback_url, params: { code: code, state: state }
expect(response).to redirect_to app_email_inbox_agents_url(account_id: account.id, inbox_id: account.inboxes.last.id)
expect(account.inboxes.count).to be 1
@@ -79,10 +73,9 @@ RSpec.describe 'Google::CallbacksController', type: :request do
'redirect_uri' => "#{ENV.fetch('FRONTEND_URL', 'http://localhost:3000')}/google/callback" })
.to_return(status: 401)
get google_callback_url, params: { code: code }
get google_callback_url, params: { code: code, state: state }
expect(response).to redirect_to '/'
expect(Redis::Alfred.get(cache_key).to_i).to eq account.id
end
end
end

View File

@@ -4,11 +4,7 @@ RSpec.describe 'Microsoft::CallbacksController', type: :request do
let(:account) { create(:account) }
let(:code) { SecureRandom.hex(10) }
let(:email) { Faker::Internet.email }
let(:cache_key) { "microsoft::#{email.downcase}" }
before do
Redis::Alfred.set(cache_key, account.id)
end
let(:state) { account.to_sgid(expires_in: 15.minutes).to_s }
describe 'GET /microsoft/callback' do
let(:response_body_success) do
@@ -27,7 +23,7 @@ RSpec.describe 'Microsoft::CallbacksController', type: :request do
'redirect_uri' => "#{ENV.fetch('FRONTEND_URL', 'http://localhost:3000')}/microsoft/callback" })
.to_return(status: 200, body: response_body_success.to_json, headers: { 'Content-Type' => 'application/json' })
get microsoft_callback_url, params: { code: code }
get microsoft_callback_url, params: { code: code, state: state }
expect(response).to redirect_to app_email_inbox_agents_url(account_id: account.id, inbox_id: account.inboxes.last.id)
expect(account.inboxes.count).to be 1
@@ -36,7 +32,6 @@ RSpec.describe 'Microsoft::CallbacksController', type: :request do
expect(inbox.channel.reload.provider_config.keys).to include('access_token', 'refresh_token', 'expires_on')
expect(inbox.channel.reload.provider_config['access_token']).to eq response_body_success[:access_token]
expect(inbox.channel.imap_address).to eq 'outlook.office365.com'
expect(Redis::Alfred.get(cache_key)).to be_nil
end
it 'creates updates inbox channel config if inbox exists and authentication is successful' do
@@ -48,14 +43,13 @@ RSpec.describe 'Microsoft::CallbacksController', type: :request do
'redirect_uri' => "#{ENV.fetch('FRONTEND_URL', 'http://localhost:3000')}/microsoft/callback" })
.to_return(status: 200, body: response_body_success.to_json, headers: { 'Content-Type' => 'application/json' })
get microsoft_callback_url, params: { code: code }
get microsoft_callback_url, params: { code: code, state: state }
expect(response).to redirect_to app_email_inbox_settings_url(account_id: account.id, inbox_id: account.inboxes.last.id)
expect(account.inboxes.count).to be 1
expect(inbox.channel.reload.provider_config.keys).to include('access_token', 'refresh_token', 'expires_on')
expect(inbox.channel.reload.provider_config['access_token']).to eq response_body_success[:access_token]
expect(inbox.channel.imap_address).to eq 'outlook.office365.com'
expect(Redis::Alfred.get(cache_key)).to be_nil
end
it 'creates inboxes with fallback_name when account name is not present in id_token' do
@@ -64,7 +58,7 @@ RSpec.describe 'Microsoft::CallbacksController', type: :request do
'redirect_uri' => "#{ENV.fetch('FRONTEND_URL', 'http://localhost:3000')}/microsoft/callback" })
.to_return(status: 200, body: response_body_success_without_name.to_json, headers: { 'Content-Type' => 'application/json' })
get microsoft_callback_url, params: { code: code }
get microsoft_callback_url, params: { code: code, state: state }
expect(response).to redirect_to app_email_inbox_agents_url(account_id: account.id, inbox_id: account.inboxes.last.id)
expect(account.inboxes.count).to be 1
@@ -78,10 +72,9 @@ RSpec.describe 'Microsoft::CallbacksController', type: :request do
'redirect_uri' => "#{ENV.fetch('FRONTEND_URL', 'http://localhost:3000')}/microsoft/callback" })
.to_return(status: 401)
get microsoft_callback_url, params: { code: code }
get microsoft_callback_url, params: { code: code, state: state }
expect(response).to redirect_to '/'
expect(Redis::Alfred.get(cache_key).to_i).to eq account.id
end
end
end