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: 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
2 changes: 1 addition & 1 deletion 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
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
12 changes: 6 additions & 6 deletions features/step_definitions/buyer_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
end

Given(/^a buyer signed up to the provider$/) do
step %(an approved buyer "John" signed up to provider "#{@provider.domain}")
step %(an approved buyer "John" signed up to provider "#{@provider.internal_domain}")
@buyer = Account.find_by_org_name!('John')
step 'buyer "John" has application "TimeMachine"'
@application = @buyer.application_contracts.find_by_name!('TimeMachine')
Expand Down Expand Up @@ -179,7 +179,7 @@


Given('the provider has a buyer') do
step %'the current domain is #{@provider.domain}'
step %'the current domain is #{@provider.external_domain}'

visit signup_path

Expand All @@ -199,7 +199,7 @@
page.should have_content('Thank you')
page.should have_content('We have sent you an email to confirm your email address.')

email = open_email(user.email, with_subject: "#{@provider.domain} API account confirmation")
email = open_email(user.email, with_subject: "#{@provider.external_domain} API account confirmation")
click_first_link_in_email(email)

within login_form do
Expand Down Expand Up @@ -227,8 +227,8 @@ def login_form
end

And(/^has a buyer with (application|service) plan/) do |plan|
step %(provider "#{@provider.domain}" has "service_plans" switch allowed)
step %(a default service of provider "#{@provider.domain}" has name "API")
step %(provider "#{@provider.internal_domain}" has "service_plans" switch allowed)
step %(a default service of provider "#{@provider.internal_domain}" has name "API")
if plan == 'service'
step 'a service plan "Gold" for service "API" exists'
step 'a buyer "Alexander" signed up to service plan "Gold"'
Expand Down Expand Up @@ -261,7 +261,7 @@ def login_form

When(/^the buyer logs in to the provider$/) do
steps %(
When the current domain is #{@provider.domain}
When the current domain is #{@provider.external_domain}
And I go to the login page
And I fill in "Username" with "#{@buyer.admins.first.username}"
And I fill in "Password" with "supersecret"
Expand Down
2 changes: 1 addition & 1 deletion features/step_definitions/invitations_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@
end

Then "an invitation with the admin domain of {account} should be sent to {string}" do |provider, address|
invitation_message_should_be_valid find_latest_email(to: address), provider, provider.self_domain
invitation_message_should_be_valid find_latest_email(to: address), provider, provider.external_admin_domain
end

Then(/^(?:the |)invitation (?:to|from) account "([^\"]*)" should be sent to "([^\"]*)"$/) do |org_name, email|
Expand Down