Skip to content

Commit

Permalink
5815 allowed email domains (#5919)
Browse files Browse the repository at this point in the history
* Front end changes

- added a row for the specific email domain sign up
- added corresponding locales

* Created specific email domain sign up site setting

- added specific email domain sign up to site settings in `tenant.rb`

* Rspec tests and data migration

* external controller tests + rubocop fixes

* rubocop fixes

* Rspec test fix for tenants controller spec

- added the setting SpecificEmailDomainSignUp

* rubocop fix

* test fixes

* Changed Specific Email Domain Sign Up to Allowed Domains

- changed all instances of previous name to AllowedDomains

* Delete db/data/20240806205559_add_domain_specific_email_signup_to_site_settings.rb

* gemfile change

* schema change

* locale changes

---------

Co-authored-by: Ahmad Farhat <[email protected]>
  • Loading branch information
alihadimazeh and farhatahmad authored Aug 13, 2024
1 parent 7d248f2 commit 8ea1848
Show file tree
Hide file tree
Showing 11 changed files with 254 additions and 31 deletions.
8 changes: 6 additions & 2 deletions app/assets/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,10 @@
"open": "Open Registration",
"invite": "Join by Invitation",
"approval": "Approve/Decline"
}
},
"allowed_domains": "Allowed Email Domains",
"allowed_domains_signup_description": "Allow specific email domains to sign up. Format must be: @test.com,domain.com",
"enter_allowed_domains_rule" : "Enter the allowed domains"
}
},
"room_configuration": {
Expand Down Expand Up @@ -420,7 +423,8 @@
"privacy_policy_updated": "The privacy notice has been updated.",
"helpcenter_updated": "The help center link has been updated.",
"terms_of_service_updated": "The terms of service have been updated.",
"maintenance_updated": "The maintenance banner has been updated."
"maintenance_updated": "The maintenance banner has been updated.",
"allowed_domains_signup_updated": "The allowed email domains have been updated."
},
"recording": {
"recording_visibility_updated": "The recording visibility has been updated.",
Expand Down
14 changes: 14 additions & 0 deletions app/controllers/api/v1/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ def create
# Users created by a user will have the creator language by default with a fallback to the server configured default_locale.
create_user_params[:language] = current_user&.language || I18n.default_locale if create_user_params[:language].blank?

# renders an error if the user is signing up with an invalid domain based off site settings
return render_error errors: Rails.configuration.custom_error_msgs[:unauthorized], status: :forbidden unless valid_domain?

user = UserCreator.new(user_params: create_user_params.except(:invite_token), provider: current_provider, role: default_role).call

smtp_enabled = ENV['SMTP_SERVER'].present?
Expand Down Expand Up @@ -184,6 +187,17 @@ def valid_invite_token
Invitation.destroy_by(email: create_user_params[:email].downcase, provider: current_provider,
token: create_user_params[:invite_token]).present?
end

def valid_domain?
allowed_domains_emails = SettingGetter.new(setting_name: 'AllowedDomains', provider: current_provider).call
return true if allowed_domains_emails.blank?

domains = allowed_domains_emails.split(',')
domains.each do |domain|
return true if create_user_params[:email].end_with?(domain)
end
false
end
end
end
end
13 changes: 13 additions & 0 deletions app/controllers/external_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ def create_user
return redirect_to root_path(error: Rails.configuration.custom_error_msgs[:invite_token_invalid])
end

return render_error status: :forbidden unless valid_domain?(user_info[:email])

# Create the user if they dont exist
if new_user
user = UserCreator.new(user_params: user_info, provider: current_provider, role: default_role).call
Expand Down Expand Up @@ -164,4 +166,15 @@ def build_user_info(credentials)
verified: true
}
end

def valid_domain?(email)
allowed_domain_emails = SettingGetter.new(setting_name: 'AllowedDomains', provider: current_provider).call
return true if allowed_domain_emails.blank?

domains = allowed_domain_emails.split(',')
domains.each do |domain|
return true if email.end_with?(domain)
end
false
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,12 @@ import useRoles from '../../../../hooks/queries/admin/roles/useRoles';
export default function Registration() {
const { t } = useTranslation();
const { data: env } = useEnv();
const { data: siteSettings } = useSiteSettings(['RoleMapping', 'DefaultRole', 'ResyncOnLogin', 'RegistrationMethod']);
const { data: siteSettings } = useSiteSettings(['RoleMapping', 'DefaultRole', 'ResyncOnLogin', 'RegistrationMethod', 'AllowedDomains']);
const { data: roles } = useRoles();
const updateRegistrationMethod = useUpdateSiteSetting('RegistrationMethod');
const updateDefaultRole = useUpdateSiteSetting('DefaultRole');
const updateRoleMapping = useUpdateSiteSetting('RoleMapping');
const updateDomainSignUp = useUpdateSiteSetting('AllowedDomains');

return (
<>
Expand Down Expand Up @@ -99,6 +100,24 @@ export default function Registration() {
</Button>
</Stack>
</Row>

<Row className="mb-3">
<strong> {t('admin.site_settings.registration.allowed_domains')} </strong>
<p className="text-muted">{t('admin.site_settings.registration.allowed_domains_signup_description')}</p>
<Stack direction="horizontal">
<input
className="form-control"
placeholder={t('admin.site_settings.registration.enter_allowed_domains_rule')}
/>
<Button
variant="brand"
className="ms-2"
onClick={(e) => updateDomainSignUp.mutate({ value: e.target.previousSibling.value })}
>
{t('update')}
</Button>
</Stack>
</Row>
</>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ export default function useUpdateSiteSetting(name) {
case 'Maintenance':
toast.success(t('toast.success.site_settings.maintenance_updated'));
break;
case 'AllowedDomains':
toast.success(t('toast.success.site_settings.allowed_domains_signup_updated'));
break;
default:
toast.success(t('toast.success.site_settings.site_setting_updated'));
}
Expand Down
3 changes: 2 additions & 1 deletion app/services/tenant_setup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ def create_site_settings
{ setting: Setting.find_by(name: 'DefaultRole'), provider: @provider, value: 'User' },
{ setting: Setting.find_by(name: 'DefaultRecordingVisibility'), provider: @provider, value: 'Published' },
{ setting: Setting.find_by(name: 'Maintenance'), provider: @provider, value: '' },
{ setting: Setting.find_by(name: 'SessionTimeout'), provider: @provider, value: '1' }
{ setting: Setting.find_by(name: 'SessionTimeout'), provider: @provider, value: '1' },
{ setting: Setting.find_by(name: 'AllowedDomains'), value: '', provider: @provider }
]
end

Expand Down
23 changes: 23 additions & 0 deletions db/data/20240812210436_add_allowed_domains_to_site_settings.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# frozen_string_literal: true

class AddAllowedDomainsToSiteSettings < ActiveRecord::Migration[7.1]
def up
setting = Setting.find_or_create_by(name: 'AllowedDomains')

SiteSetting.create!(setting:, value: '', provider: 'greenlight') unless SiteSetting.exists?(setting:, provider: 'greenlight')

Tenant.find_each do |tenant|
SiteSetting.create!(setting:, value: '', provider: tenant.name) unless SiteSetting.exists?(setting:, provider: tenant.name)
end
end

def down
Tenant.find_each do |tenant|
SiteSetting.find_by(setting: Setting.find_by(name: 'Maintenance'), provider: tenant.name)&.destroy
end

SiteSetting.find_by(setting: Setting.find_by(name: 'Maintenance'), provider: 'greenlight')&.destroy

Setting.find_by(name: 'AllowedDomains')&.destroy
end
end
2 changes: 1 addition & 1 deletion db/data_schema.rb
Original file line number Diff line number Diff line change
@@ -1 +1 @@
DataMigrate::Data.define(version: 20240423162700)
DataMigrate::Data.define(version: 20240812210436)
3 changes: 2 additions & 1 deletion spec/controllers/admin/tenants_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

require 'rails_helper'

RSpec.describe Api::V1::Admin::TenantsController, type: :controller do
RSpec.describe Api::V1::Admin::TenantsController do
let(:user) { create(:user, :with_super_admin) }
let(:valid_tenant_params) do
{
Expand Down Expand Up @@ -146,6 +146,7 @@ def create_settings_permissions_meetingoptions
Setting.find_or_create_by(name: 'HelpCenter')
Setting.find_or_create_by(name: 'Maintenance')
Setting.find_or_create_by(name: 'SessionTimeout')
Setting.find_or_create_by(name: 'AllowedDomains')

Permission.find_or_create_by(name: 'CreateRoom')
Permission.find_or_create_by(name: 'ManageUsers')
Expand Down
135 changes: 110 additions & 25 deletions spec/controllers/external_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

require 'rails_helper'

RSpec.describe ExternalController, type: :controller do
RSpec.describe ExternalController do
let(:fake_setting_getter) { instance_double(SettingGetter) }

describe '#create_user' do
Expand Down Expand Up @@ -80,7 +80,7 @@

expect do
get :create_user, params: { provider: 'openid_connect' }
end.to change(User, :count).by(0)
end.not_to change(User, :count)
end

it 'looks the user up based on email' do
Expand All @@ -90,7 +90,7 @@

expect do
get :create_user, params: { provider: 'openid_connect' }
end.to change(User, :count).by(0)
end.not_to change(User, :count)
end

context 'redirect' do
Expand Down Expand Up @@ -212,40 +212,52 @@
email: '[email protected]')
end

it 'overwrites the saved values with the values from the authentication provider if true' do
allow_any_instance_of(SettingGetter).to receive(:call).and_return(true)
context 'value is true' do
before do
reg_method = instance_double(SettingGetter)
allow(SettingGetter).to receive(:new).with(setting_name: 'ResyncOnLogin', provider: 'greenlight').and_return(reg_method)
allow(reg_method).to receive(:call).and_return(true)
end

request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
it 'overwrites the saved values with the values from the authentication provider if true' do
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]

get :create_user, params: { provider: 'openid_connect' }
get :create_user, params: { provider: 'openid_connect' }

user.reload
expect(user.name).to eq(OmniAuth.config.mock_auth[:openid_connect]['info']['name'])
expect(user.email).to eq(OmniAuth.config.mock_auth[:openid_connect]['info']['email'])
end
user.reload
expect(user.name).to eq(OmniAuth.config.mock_auth[:openid_connect]['info']['name'])
expect(user.email).to eq(OmniAuth.config.mock_auth[:openid_connect]['info']['email'])
end

it 'does not overwrite the saved values with the values from the authentication provider if false' do
allow_any_instance_of(SettingGetter).to receive(:call).and_return(false)
it 'does not overwrite the role even if true' do
allow_any_instance_of(SettingGetter).to receive(:call).and_return(true)
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]

request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
new_role = create(:role)
user.update(role: new_role)

get :create_user, params: { provider: 'openid_connect' }
get :create_user, params: { provider: 'openid_connect' }

user.reload
expect(user.name).to eq('Example Name')
expect(user.email).to eq('[email protected]')
expect(user.reload.role).to eq(new_role)
end
end

it 'does not overwrite the role even if true' do
allow_any_instance_of(SettingGetter).to receive(:call).and_return(true)
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
context 'value is false' do
before do
reg_method = instance_double(SettingGetter)
allow(SettingGetter).to receive(:new).with(setting_name: 'ResyncOnLogin', provider: 'greenlight').and_return(reg_method)
allow(reg_method).to receive(:call).and_return(false)
end

new_role = create(:role)
user.update(role: new_role)
it 'does not overwrite the saved values with the values from the authentication provider if false' do
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]

get :create_user, params: { provider: 'openid_connect' }
get :create_user, params: { provider: 'openid_connect' }

expect(user.reload.role).to eq(new_role)
user.reload
expect(user.name).to eq('Example Name')
expect(user.email).to eq('[email protected]')
end
end
end

Expand Down Expand Up @@ -325,6 +337,79 @@
end
end

context 'Allowed Domains' do
context 'restricted domain not set' do
before do
site_settings = instance_double(SettingGetter)
allow(SettingGetter).to receive(:new).with(setting_name: 'AllowedDomains', provider: 'greenlight').and_return(site_settings)
allow(site_settings).to receive(:call).and_return('')
end

it 'creates the user' do
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]

expect { get :create_user, params: { provider: 'openid_connect' } }.to change(User, :count).from(0).to(1)
end
end

context 'restricted domain set to 1 domain' do
before do
site_settings = instance_double(SettingGetter)
allow(SettingGetter).to receive(:new).with(setting_name: 'AllowedDomains', provider: 'greenlight').and_return(site_settings)
allow(site_settings).to receive(:call).and_return('@domain.com')
end

it 'creates the user if the domain is allowed' do
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
request.env['omniauth.auth'][:info][:email] = '[email protected]'

expect { get :create_user, params: { provider: 'openid_connect' } }.to change(User, :count).from(0).to(1)
end

it 'does not create if the domain is not allowed' do
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]

expect { get :create_user, params: { provider: 'openid_connect' } }.not_to change(User, :count)
end
end

context 'restricted domain set to multiple domain' do
before do
site_settings = instance_double(SettingGetter)
allow(SettingGetter).to receive(:new).with(setting_name: 'AllowedDomains', provider: 'greenlight').and_return(site_settings)
allow(site_settings).to receive(:call).and_return('@example.com,@test.com,@domain.com')
end

it 'creates the user if the domain is allowed 1' do
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
request.env['omniauth.auth'][:info][:email] = '[email protected]'

expect { get :create_user, params: { provider: 'openid_connect' } }.to change(User, :count).from(0).to(1)
end

it 'creates the user if the domain is allowed 2' do
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
request.env['omniauth.auth'][:info][:email] = '[email protected]'

expect { get :create_user, params: { provider: 'openid_connect' } }.to change(User, :count).from(0).to(1)
end

it 'creates the user if the domain is allowed 3' do
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
request.env['omniauth.auth'][:info][:email] = '[email protected]'

expect { get :create_user, params: { provider: 'openid_connect' } }.to change(User, :count).from(0).to(1)
end

it 'does not create if the domain is not allowed' do
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
request.env['omniauth.auth'][:info][:email] = '[email protected]'

expect { get :create_user, params: { provider: 'openid_connect' } }.not_to change(User, :count)
end
end
end

context 'Role mapping' do
let!(:role1) { create(:role, name: 'role1') }

Expand Down
Loading

0 comments on commit 8ea1848

Please sign in to comment.