Skip to content

Commit

Permalink
fix confirmation_url in user creation API
Browse files Browse the repository at this point in the history
This fix will make confirmation_ur context-aware and return
the proper host domain in the response.

closes FOO-4343
flag=none

test plan notes:
- cannot test locally because distinction between environments/instances
  does not exist in local development
- can test on cd, beta, or test
- this fix is patterned after a similar fix found here:
  https://gerrit.instructure.com/c/canvas-lms/+/340437

test plan:
- create auth token for chosen environment/instance
- make POST with these key/values:

user[name]:Sour Patch Ghosty
pseudonym[unique_id]:spghosty
pseudonym[send_confirmation]:true
communication_channel[confirmation_url]:true
communication_channel[type]:email
communication_channel[address]:spghosty@instructure.com

- in the response, confirmation_url should point to the chosen
  environment/instance
- also, existing/modified tests should pass

[skip-crystalball]
[fsc-max-nodes=18]
[fsc-timeout=40]

Change-Id: I0e35841228c277c3d410acbcd55c65360948e748
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/344917
QA-Review: Michael Hulse <michael.hulse@instructure.com>
Product-Review: Michael Hulse <michael.hulse@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: August Thornton <august@instructure.com>
  • Loading branch information
mhulse committed Apr 18, 2024
1 parent b9ff018 commit f754ec0
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 22 deletions.
18 changes: 12 additions & 6 deletions app/models/communication_channel.rb
Expand Up @@ -290,14 +290,20 @@ def not_otp_communication_channel
errors.add(:workflow_state, "Can't remove a user's SMS that is used for one time passwords") if id == user.otp_communication_channel_id
end

# Public: Build the url where this record can be confirmed.
# Public: Provides base components for an email confirmation URL.
#
# Constructs and returns a hash of components necessary to build a
# confirmation URL for email type communication channels.
#
# Returns a string.
def confirmation_url
return "" unless path_type == TYPE_EMAIL

"#{HostUrl.protocol}://#{HostUrl.context_host(context)}/register/#{confirmation_code}"
# Returns a hash with :context, and :confirmation_code if the path type is
# email; returns nil otherwise.
def confirmation_url_data
return nil unless path_type == TYPE_EMAIL

{
context:,
confirmation_code:
}
end

def context
Expand Down
46 changes: 46 additions & 0 deletions app/presenters/communication_channel_presenter.rb
@@ -0,0 +1,46 @@
# frozen_string_literal: true

# Copyright (C) 2024 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.

class CommunicationChannelPresenter
def initialize(communication_channel, request)
@communication_channel = communication_channel
@request = request
end

def confirmation_url
data = @communication_channel.confirmation_url_data
return "" if incomplete_data?(data)

build_url(data)
end

private

def incomplete_data?(data)
data.nil? || data[:confirmation_code].nil?
end

def host_url(data)
# NOTE: multiple_root_accounts plugin will override HostUrl
HostUrl.context_host(data[:context], @request.try(:host_with_port))
end

def build_url(data)
"#{HostUrl.protocol}://#{host_url(data)}/register/#{data[:confirmation_code]}"
end
end
10 changes: 9 additions & 1 deletion lib/api/v1/user.rb
Expand Up @@ -131,7 +131,15 @@ def user_json(user, current_user, session, includes = [], context = @context, en

json[:locale] = user.locale if includes.include?("locale")
json[:effective_locale] = I18n.locale if includes.include?("effective_locale") && user == current_user
json[:confirmation_url] = user.communication_channels.email.first.try(:confirmation_url) if includes.include?("confirmation_url")

if includes.include?("confirmation_url")
email_channel = user.communication_channels.email.first
if email_channel.present?
presenter = CommunicationChannelPresenter.new(email_channel, request)
confirmation_url = presenter.confirmation_url
json[:confirmation_url] = confirmation_url if confirmation_url.present?
end
end

if includes.include?("last_login")
last_login = user.last_login || user.read_attribute(:last_login)
Expand Down
37 changes: 23 additions & 14 deletions spec/apis/v1/users_api_spec.rb
Expand Up @@ -1548,6 +1548,14 @@ def create_user_skip_cc_confirm(admin_user)
users = User.where(name: "Test User").to_a
expect(users.length).to be 1
user = users.first

# setup communication channel
communication_channel = user.communication_channels.email.first
expect(communication_channel).not_to be_nil
# create presenter with a mock request
request = instance_double("ActionDispatch::Request", host_with_port: "example.com")
presenter = CommunicationChannelPresenter.new(communication_channel, request)

expect(user.name).to eql "Test User"
expect(user.short_name).to eql "Test"
expect(user.sortable_name).to eql "User, T."
Expand All @@ -1559,20 +1567,21 @@ def create_user_skip_cc_confirm(admin_user)
expect(pseudonym.unique_id).to eql "test@example.com"
expect(pseudonym.sis_user_id).to eql "12345"

expect(JSON.parse(response.body)).to eq({
"name" => "Test User",
"short_name" => "Test",
"sortable_name" => "User, T.",
"id" => user.id,
"created_at" => user.created_at.iso8601,
"sis_user_id" => "12345",
"sis_import_id" => user.pseudonym.sis_batch_id,
"login_id" => "test@example.com",
"integration_id" => nil,
"locale" => "en",
"confirmation_url" => user.communication_channels.email.first.confirmation_url,
"uuid" => user.uuid
})
expected_response = {
"name" => "Test User",
"short_name" => "Test",
"sortable_name" => "User, T.",
"id" => user.id,
"created_at" => user.created_at.iso8601,
"sis_user_id" => "12345",
"sis_import_id" => user.pseudonym.sis_batch_id,
"login_id" => "test@example.com",
"integration_id" => nil,
"locale" => "en",
"confirmation_url" => presenter.confirmation_url,
"uuid" => user.uuid
}
expect(JSON.parse(response.body)).to eq(expected_response)
end

it "accepts a valid destination param" do
Expand Down
4 changes: 3 additions & 1 deletion spec/models/communication_channel_spec.rb
Expand Up @@ -230,7 +230,9 @@
expect(HostUrl).to receive(:context_host).and_return("test.canvas.com")
expect(CanvasSlug).to receive(:generate).and_return("abc123")
communication_channel_model
expect(@cc.confirmation_url).to eql("https://test.canvas.com/register/abc123")
mock_request = instance_double("ActionDispatch::Request", host_with_port: "test.canvas.com")
presenter = CommunicationChannelPresenter.new(@cc, mock_request)
expect(presenter.confirmation_url).to eql("https://test.canvas.com/register/abc123")
end

it "only allows email, or sms as path types" do
Expand Down

0 comments on commit f754ec0

Please sign in to comment.