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

Make #admin_domain private and fix DomainSubstitution deprecation warnings #3001

Merged
merged 14 commits into from
Jul 19, 2022
Merged
2 changes: 1 addition & 1 deletion app/controllers/master/devportal/auth_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def show
def show_self
account = Account.find_by!(self_domain: params.require(:self_domain))

redirect_to callback_url(account, domain: account.external_self_domain)
redirect_to callback_url(account, domain: account.external_admin_domain)
end

protected
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/partners/providers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def create
tracking = ThreeScale::Analytics.user_tracking(@user)
tracking.identify({})
tracking.track('Signup', {})
render json: {id: @account.id, provider_key: @account.api_key, end_point: @account.self_domain, success: true}
render json: {id: @account.id, provider_key: @account.api_key, end_point: @account.external_admin_domain, success: true}
else
render json: {errors: {user: @user.errors, account: @account.errors}, success: false}, status: :unprocessable_entity
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/provider/domains_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class Provider::DomainsController < Provider::BaseController

def recover
email_param = params.require(:email)
domains = site_account.managed_users.where(email: email_param).map{|p| p.account.self_domain}.uniq
domains = site_account.managed_users.where(email: email_param).map{|p| p.account.external_admin_domain}.uniq

ProviderUserMailer.lost_domain(email_param, domains).deliver_later unless domains.empty?
end
Expand Down
4 changes: 2 additions & 2 deletions app/events/domains/provider_domains_changed_event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ def self.create(provider, parent_event = nil)
parent_event_type: parent_event&.class&.name,

provider: provider,
admin_domains: [ provider.self_domain ],
developer_domains: [ provider.domain ],
admin_domains: [provider.internal_admin_domain],
developer_domains: [provider.internal_domain],

metadata: {
provider_id: provider.id,
Expand Down
2 changes: 1 addition & 1 deletion app/helpers/cms/url_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def cms_published_url(page)

def cms_uri(page)
uri = URI.parse(access_code_url)
uri.host = page.provider.domain
uri.host = page.provider.external_domain
uri.query = { return_to: page.path ? page.path : HARD_WIRED_PATHS[page.system_name],
access_code: current_account.site_access_code,
cms_token: page.provider.settings.cms_token! }.to_query
Expand Down
2 changes: 2 additions & 0 deletions app/lib/fields/extra_fields.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ def extra_fields_valid?
def read_attribute_for_validation(name)
if fields_definitions_source_root && extra_field?(name)
extra_fields.try!(:[], name.to_s)
elsif %i[domain self_domain].include?(name)
josemigallas marked this conversation as resolved.
Show resolved Hide resolved
ThreeScale::Deprecation.silence { super }
else
super
end
Expand Down
2 changes: 1 addition & 1 deletion app/lib/signup/impersonation_admin_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def self.build(account:)
impersonation_admin = account.users.new(
{
username: username,
email: "#{username}+#{account.self_domain}@#{config['domain']}",
email: "#{username}+#{account.internal_domain}@#{config['domain']}",
first_name: '3scale',
last_name: 'Admin',
state: 'active'
Expand Down
2 changes: 1 addition & 1 deletion app/lib/three_scale/analytics/salesforce.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def update_invoice_status(invoice)

traits = {
account_id: @provider.id,
self_domain: @provider.internal_self_domain,
self_domain: @provider.internal_admin_domain,

plan: bought_plan.name,
plan_id: bought_plan.id,
Expand Down
2 changes: 1 addition & 1 deletion app/lib/three_scale/analytics/user_tracking.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def basic_traits

account_id: @account.id,
domain: @account.internal_domain,
self_domain: @account.internal_self_domain
self_domain: @account.provider? ? @account.internal_admin_domain : nil
}
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ def callback_account
end

def host
Account.master.external_self_domain
Account.master.external_admin_domain
end

attr_reader :client, :request
Expand Down
2 changes: 1 addition & 1 deletion app/lib/three_scale/oauth2/service_discovery_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def callback_account
end

def host
Account.master.external_self_domain
Account.master.external_admin_domain
end

attr_reader :client, :request
Expand Down
2 changes: 1 addition & 1 deletion app/mailers/notification_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ def limit_mail(event, receiver, mail_name)
end

def default_url_options
super.merge(host: provider_account.try!(:admin_domain))
super.merge(host: provider_account&.external_admin_domain)
end

def t_subject(key, options = {})
Expand Down
2 changes: 1 addition & 1 deletion app/mailers/post_office.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def message_notification(message, recipient)
msg_url = if receiver.buyer?
developer_portal.admin_messages_inbox_url(recipient, host: receiver.provider_account.external_domain)
else # provider or master
provider_admin_messages_inbox_url(recipient, host: receiver.external_self_domain)
provider_admin_messages_inbox_url(recipient, host: receiver.external_admin_domain)
end

@sender = message.sender.org_name
Expand Down
2 changes: 1 addition & 1 deletion app/messengers/alert_messenger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def domain
end

def self_domain
@alert.account.external_self_domain
@alert.account.external_admin_domain
end

def send_alert(alert, sender)
Expand Down
2 changes: 1 addition & 1 deletion app/messengers/cinstance_messenger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def setup(cinstance, *)
def new_contract(cinstance)
@_template_name = 'new_application'

@url = app_routes.provider_admin_application_url(cinstance, host: cinstance.account.provider_account.admin_domain)
@url = app_routes.provider_admin_application_url(cinstance, host: cinstance.account.provider_account.external_admin_domain)

assign_drops :url => @url

Expand Down
4 changes: 2 additions & 2 deletions app/messengers/invoice_messenger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def setup(invoice)
@cost = format_cost(@invoice.cost)

@invoice_url = if @buyer_account.provider?
app_routes.provider_admin_account_invoice_url(@invoice, :host => @buyer_account.external_self_domain)
app_routes.provider_admin_account_invoice_url(@invoice, :host => @buyer_account.external_admin_domain)
else
developer_portal_routes.admin_account_invoice_url(@invoice, :host => @invoice.provider_account.external_domain)
end
Expand Down Expand Up @@ -84,7 +84,7 @@ def payment_url(invoice)
return '' if type.nil? || type == :bogus

if invoice.provider_account.master?
app_routes.polymorphic_url([:provider, :admin, :account, type.to_sym],host: invoice.buyer_account.external_self_domain)
app_routes.polymorphic_url([:provider, :admin, :account, type.to_sym],host: invoice.buyer_account.external_admin_domain)
else
developer_portal_routes.polymorphic_url([:admin, :account, type.to_sym], host: invoice.provider_account.external_domain)
end
Expand Down
4 changes: 2 additions & 2 deletions app/messengers/plans_messenger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ def setup(application, new_plan)
@buyer = application.user_account
@user = @buyer.admins.first
@plan = new_plan
@credit_card_url = developer_portal_routes.admin_account_payment_details_url(:host => @provider.domain)
@credit_card_url = developer_portal_routes.admin_account_payment_details_url(:host => @provider.external_domain)

assign_drops :application => @application,
:provider => Liquid::Drops::Provider.new(@provider),
Expand All @@ -20,7 +20,7 @@ def plan_change_request(application, new_plan)
@buyer = application.user_account
@plan = new_plan

url = app_routes.provider_admin_application_url(application, host: application.account.provider_account.admin_domain)
url = app_routes.provider_admin_application_url(application, host: application.account.provider_account.external_admin_domain)
# Pending: Create a view for the body.
body = %|#{@buyer.org_name} are requesting to have their plan changed to #{@plan.name} for application #{application.name}. You can do this from the application page: #{url}|

Expand Down
10 changes: 5 additions & 5 deletions app/models/account/provider_domains.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def is_admin_domain?(domain)

def is_master_domain?(domain)
return true if ThreeScale.master_on_premises?
master.domain == domain or master.self_domain == domain
master.match_internal_domain?(domain) or master.match_internal_admin_domain?(domain)
end

def same_domain(domain)
Expand Down Expand Up @@ -100,7 +100,7 @@ def generate_domains!
end

def domains_present?
domain.present? && self_domain.present?
self[:domain].present? && self[:self_domain].present?
end

def subdomain=(name)
Expand All @@ -117,7 +117,7 @@ def assign_domains(domains)
end

def subdomain
subdomain_from(domain)
subdomain_from(self[:domain])
end

def superdomain
Expand Down Expand Up @@ -145,7 +145,7 @@ def self_subdomain=(name)
end

def self_subdomain
subdomain_from(self_domain)
subdomain_from(self[:self_domain])
end

def self.unique?(attr:, val:, scope: all)
Expand Down Expand Up @@ -203,7 +203,7 @@ def self_domain_uniqueness
end

def domain_not_self_domain
if domain && self_domain && domain == self_domain
if read_attribute(:domain) && read_attribute(:self_domain) && (read_attribute(:domain) == read_attribute(:self_domain))
if subdomain
errors.add(:subdomain, :same)
errors.add(:self_subdomain, :same)
Expand Down
24 changes: 12 additions & 12 deletions app/models/account/provider_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -210,18 +210,6 @@ def published_application_plans
application_plans.published
end

def admin_domain
if provider?
if self_domain.present?
self_domain
else # connect case
Account.master.domain
end
else
raise ProviderOnlyMethodCalledError
end
end

def admin_base_url
build_base_url(admin_domain)
end
Expand Down Expand Up @@ -336,6 +324,18 @@ def build_base_url(host)

private

def admin_domain
if provider?
if self_domain.present?
self_domain
else # connect case
Account.master.domain
end
else
raise ProviderOnlyMethodCalledError
end
end

def ensure_provider
raise ProviderOnlyMethodCalledError if self.buyer?
end
Expand Down
2 changes: 1 addition & 1 deletion app/observers/post_observer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def after_commit_on_create(post)
TopicMailer.new_post(subscriber, post).deliver_later unless subscriber.email_unverified?
end

url = admin_forum_topic_url(post.topic, host: post.forum.account.self_domain)
url = admin_forum_topic_url(post.topic, host: post.forum.account.external_admin_domain)

name = user ? user.account.org_name : 'Anonymous User'
message.body = <<-MSG
Expand Down
2 changes: 1 addition & 1 deletion app/presenters/provider_oauth_flow_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def test_flow_callback_url
attr_reader :self_domain

def domain_parameters
callback_account.self_domain == self_domain ? {} : { self_domain: self_domain }
callback_account.external_admin_domain == self_domain ? {} : { self_domain: self_domain }
end

def account_domain
Expand Down
4 changes: 2 additions & 2 deletions app/representers/account_representer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ module AccountRepresenter
end

with_options(if: ->(*) { provider? }) do
property :admin_domain
property :domain
property :admin_domain # TODO
property :domain # TODO
property :admin_base_url
property :base_url
property :from_email
Expand Down
2 changes: 1 addition & 1 deletion app/views/api_docs/mailer/new_path_notification.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Hello support team!

New Path <%= @base_path %> has been added to ActiveDocs (<%= @service.id %>)
for provider <%= @account.domain %> (<%= @account.id %>).
for provider <%= @account.external_domain %> (<%= @account.id %>).
If you want to contact the provider, please do <%= @reply_to %>.

No action is required, if the basePath looks very fishy
Expand Down
4 changes: 2 additions & 2 deletions app/views/buyers/accounts/show.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ h1 data-hook="account-show"
td = link_to @account.external_domain, public_domain(@account), target: "_blank"
tr
th Admin domain
td = link_to @account.external_self_domain,
provider_admin_dashboard_url(host: @account.external_self_domain),
td = link_to @account.external_admin_domain,
provider_admin_dashboard_url(host: @account.external_admin_domain),
target: "_blank"
- if @account.admins.present?
tr
Expand Down
2 changes: 1 addition & 1 deletion app/views/emails/account_approved.text.liquid
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Dear {{ user.display_name }},

{{ provider.name }} has approved your signup for the {{ provider.name }} API.

You may now view and manage your app/API key at https://{{ provider.domain }}/admin/
You may now view and manage your app/API key at https://{{ provider.external_domain }}/admin/

If you have problems logging into the account please contact {{ provider.support_email }}.

Expand Down
2 changes: 1 addition & 1 deletion app/views/shared/_buyer_menu.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<% unless request.host == site_account.admin_domain %>
<% unless request.host == site_account.external_admin_domain%>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt in this case, as I am not sure if request.host is supposed to be before or after substitution...
But on the other hand, I couldn't even find where this partial is used, I guess it might not be used anywhere.

<%= main_menu_item(:root, 'Home', root_path) %>
<% end %>

Expand Down
2 changes: 1 addition & 1 deletion app/workers/delete_account_hierarchy_worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def batch_description
id = account.id
org_name = account.org_name
if account.provider?
"Deleting provider [##{id}] #{org_name} - #{account.admin_domain}"
"Deleting provider [##{id}] #{org_name} - #{account.internal_admin_domain}"
else
"Deleting buyer [##{id}] of provider #{org_name}"
end
Expand Down
10 changes: 5 additions & 5 deletions db/seeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@
###

user_login = ENV.fetch('USER_LOGIN', 'admin')
user_email = ENV['USER_EMAIL'].presence || "#{user_login}@#{provider.domain}"
user_email = ENV['USER_EMAIL'].presence || "#{user_login}@#{provider.internal_domain}"
user_password = ENV.fetch('USER_PASSWORD') { SecureRandom.base64(32) }

user = User.create!(username: user_login, password: user_password, password_confirmation: user_password) do |user|
Expand Down Expand Up @@ -176,7 +176,7 @@
impersonation_admin_username = impersonation_admin_config['username']
impersonation_admin.attributes = {
username: impersonation_admin_username,
email: "#{impersonation_admin_username}+#{provider.self_domain}@#{impersonation_admin_config['domain']}",
email: "#{impersonation_admin_username}+#{provider.external_admin_domain}@#{impersonation_admin_config['domain']}",
first_name: '3scale',
last_name: 'Admin'
}
Expand Down Expand Up @@ -226,16 +226,16 @@

if master_login && master_password
puts <<~INFO
Master Domain: #{master.admin_domain}
Master Domain: #{master.external_admin_domain}
Master User Login: #{master_login}
Master User Password: #{master_password}
Master RW access token: #{master_access_token}\n
INFO
end

puts <<~INFO
Provider Admin Domain: #{provider.admin_domain}
Provider Portal Domain: #{provider.domain}
Provider Admin Domain: #{provider.external_admin_domain}
Provider Portal Domain: #{provider.external_domain}
Provider User Login: #{user_login}
Provider User Password: #{user_password}
APIcast Access Token: #{apicast_access_token}
Expand Down
2 changes: 1 addition & 1 deletion doc/liquid/drops.html
Original file line number Diff line number Diff line change
Expand Up @@ -2782,4 +2782,4 @@ <h3>
<h3>
<a id="builtin_fields-3" class="anchor" href="#builtin_fields-3" aria-hidden="true"><span aria-hidden="true" class="octicon octicon-link"></span></a>builtin_fields</h3>
<p>Returns all built-in fields with values for this user.</p>
<hr>
<hr>
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@
buyer_name = SecureRandom.uuid # Use Faker ? use FactoryBot.create to generate just he values?
plan_name = SecureRandom.uuid

step %{an application plan "#{plan_name}" of provider "#{@provider.domain}"}
step %{an application plan "#{plan_name}" of provider "#{@provider.internal_domain}"}
step %{a buyer "#{buyer_name}" signed up to application plan "#{plan_name}"}

@application = Account.find_by_org_name!(buyer_name).bought_cinstance
Expand Down