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.

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

5815 signup specific domains #5916

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"
}
},
"specific_email_domain_signup": "Specific Email Domain Signup",
"specific_email_domain_signup_description": "Allow specific email domains to sign up. Format must be: @test.com,domain.com",
alihadimazeh marked this conversation as resolved.
Show resolved Hide resolved
"enter_domain_signup_rule" : "Enter a specific domain signup rule"
}
},
"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.",
"specific_email_domain_signup_updated": "The specific email domain sign up has 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?
specific_domain_emails = SettingGetter.new(setting_name: 'SpecificEmailDomainSignUp', provider: current_provider).call
return true if specific_domain_emails.blank?

domains = specific_domain_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)
specific_domain_emails = SettingGetter.new(setting_name: 'SpecificEmailDomainSignUp', provider: current_provider).call
return true if specific_domain_emails.blank?

domains = specific_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', 'SpecificEmailDomainSignUp']);
const { data: roles } = useRoles();
const updateRegistrationMethod = useUpdateSiteSetting('RegistrationMethod');
const updateDefaultRole = useUpdateSiteSetting('DefaultRole');
const updateRoleMapping = useUpdateSiteSetting('RoleMapping');
const updateDomainSignUp = useUpdateSiteSetting('SpecificEmailDomainSignUp');

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

<Row className="mb-3">
<strong> {t('admin.site_settings.registration.specific_email_domain_signup')} </strong>
<p className="text-muted">{t('admin.site_settings.registration.specific_email_domain_signup_description')}</p>
<Stack direction="horizontal">
<input
className="form-control"
// TODO add proper placeholder and defultValue
placeholder={t('admin.site_settings.registration.enter_domain_signup_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 'SpecificEmailDomainSignUp':
toast.success(t('toast.success.site_settings.specific_email_domain_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: 'SpecificEmailDomainSignUp'), value: '', provider: @provider }
]
end

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# frozen_string_literal: true

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

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: 'SpecificEmailDomainSignUp')&.destroy
end
end
alihadimazeh marked this conversation as resolved.
Show resolved Hide resolved
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: 20240806205559)
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: 'SpecificEmailDomainSignUp')

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 'Specific Email Domain Signup' do
context 'restricted domain not set' do
before do
site_settings = instance_double(SettingGetter)
allow(SettingGetter).to receive(:new).with(setting_name: 'SpecificEmailDomainSignUp', 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: 'SpecificEmailDomainSignUp', 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: 'SpecificEmailDomainSignUp', 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
Loading