Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions app/controllers/api/v8/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,19 @@ class UsersController < Api::V8::BaseController

swagger_path '/api/v8/users/{user_id}/set_password_managed_by_courses_mooc_fi' do
operation :post do
key :description, 'Sets the boolean password_managed_by_courses_mooc_fi for the user with the given id to true.'
key :description, 'Sets the boolean password_managed_by_courses_mooc_fi for the user with the given id to true and records the courses.mooc.fi user id.'
key :operationId, 'setPasswordManagedByCoursesMoocFi'
key :produces, ['application/json']
key :tags, ['user']
parameter '$ref': '#/parameters/user_id'
parameter do
key :name, :courses_mooc_fi_user_id
key :in, :formData
key :description, "The user's id on courses.mooc.fi"
key :required, true
key :type, :string
end
response 400, '$ref': '#/responses/error'
response 403, '$ref': '#/responses/error'
response 404, '$ref': '#/responses/error'
response 200 do
Expand Down Expand Up @@ -228,8 +236,14 @@ def destroy
def set_password_managed_by_courses_mooc_fi
only_admins!

if params[:courses_mooc_fi_user_id].blank?
return render json: {
errors: { courses_mooc_fi_user_id: ['must be present'] }
}, status: :bad_request
end

user = User.find_by!(id: params[:id])
User.transaction do
user = User.find_by!(id: params[:id])
user.password_managed_by_courses_mooc_fi = true
user.password_hash = nil
user.salt = nil
Expand All @@ -241,7 +255,7 @@ def set_password_managed_by_courses_mooc_fi
}
end
render json: {
errors: @user.errors
errors: user.errors
}, status: :bad_request
end

Expand Down
39 changes: 39 additions & 0 deletions app/controllers/participants_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ def show
add_breadcrumb 'Participants', :participants_path
add_breadcrumb @user.username, participant_path(@user)
@app_data = JSON.pretty_generate(JSON.parse(@user.user_app_data.to_json))
@courses_mooc_fi_status = @user.courses_mooc_fi_migration_status
@courses_mooc_fi_status_label = courses_mooc_fi_status_label(@user, @courses_mooc_fi_status)
@courses_mooc_fi_force_migrate_available = !@user.managed_externally? || courses_mooc_fi_account_missing?(@courses_mooc_fi_status)
else
add_breadcrumb 'My stats', participant_path(@user)
end
Expand Down Expand Up @@ -131,7 +134,43 @@ def password_reset_link
@password_reset_link = @user.generate_password_reset_link
end

def force_migrate_to_courses_mooc_fi
@user = User.find(params[:id])
authorize! :view_participant_information, @user
return respond_forbidden('This feature is only available to admins') unless current_user.administrator?
return respond_forbidden('This feature is disabled for admin accounts') if @user.administrator?

if @user.managed_externally? && !courses_mooc_fi_account_missing?(@user.courses_mooc_fi_migration_status)
return redirect_to participant_path(@user), alert: 'User is already managed by courses.mooc.fi.'
end

result = @user.force_migrate_to_courses_mooc_fi
if result[:success]
redirect_to participant_path(@user), notice:
"User force-migrated to courses.mooc.fi (id: #{result[:courses_mooc_fi_user_id]}). " \
"They have no password yet — use 'Generate password reset link' below to give them one."
else
redirect_to participant_path(@user), alert: "Force migration to courses.mooc.fi failed: #{result[:error]}"
end
end

private
# nil means the live status is unknown (unreachable/unconfigured) -- trust the local flag
# instead of treating the account as missing.
def courses_mooc_fi_account_missing?(status)
status.present? && (!status[:shadow_user_exists] || status[:deleted_at].present?)
end

def courses_mooc_fi_status_label(user, status)
return 'Broken: flagged as migrated locally but missing the target id' if user.externally_managed_without_target?
if user.managed_externally?
return "Broken: flagged as migrated locally, but courses.mooc.fi doesn't have a live account for this user" if courses_mooc_fi_account_missing?(status)
return 'Fully migrated'
end
return 'Inconsistent: courses.mooc.fi already has a password, but it isn\'t linked locally' if status&.dig(:password_set)

'Not migrated'
end
def index_json_data
result = []
@participants.each do |user|
Expand Down
115 changes: 100 additions & 15 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,9 @@ def externally_managed_without_target?


def authenticate_via_courses_mooc_fi(submitted_password)
auth_url = SiteSetting.value('courses_mooc_fi_auth_url')
auth_url = courses_mooc_fi_url('/api/v0/tmc-server/users/authenticate')

conn = Faraday.new(request: { open_timeout: 2, timeout: 10 }) do |f|
f.request :json
f.response :json
end
conn = courses_mooc_fi_connection

response = conn.post(auth_url) do |req|
req.headers['Content-Type'] = 'application/json'
Expand Down Expand Up @@ -245,12 +242,9 @@ def authenticate_via_courses_mooc_fi(submitted_password)


def update_password_via_courses_mooc_fi(old_password, new_password)
update_url = SiteSetting.value('courses_mooc_fi_update_password_url')
update_url = courses_mooc_fi_url('/api/v0/tmc-server/users/change-password')

conn = Faraday.new(request: { open_timeout: 2, timeout: 10 }) do |f|
f.request :json
f.response :json
end
conn = courses_mooc_fi_connection

begin
response = conn.post(update_url) do |req|
Expand Down Expand Up @@ -301,14 +295,11 @@ def update_password_via_courses_mooc_fi(old_password, new_password)

def post_new_user_to_courses_mooc_fi(password)
Rails.logger.info("Posting new user #{self.email} to courses.mooc.fi")
create_url = SiteSetting.value('courses_mooc_fi_create_user_url')
create_url = courses_mooc_fi_url('/api/v0/tmc-server/users/create')

# Best-effort call made inline during logins/password changes: tight timeouts so a hung
# courses.mooc.fi can't stall authentication (migration retries on the next attempt).
conn = Faraday.new(request: { open_timeout: 2, timeout: 10 }) do |f|
f.request :json
f.response :json
end
conn = courses_mooc_fi_connection

begin
response = conn.post(create_url) do |req|
Expand Down Expand Up @@ -363,6 +354,89 @@ def post_new_user_to_courses_mooc_fi(password)
end
end

# Ensures the courses.mooc.fi shadow account exists (reusing the get-or-create lookup, a no-op
# if it's already there), then hands off password ownership locally -- never sends a password,
# so the user is passwordless until generate_password_reset_link runs. Local state is only
# updated after a confirmed remote success, so a failure (e.g. re-creating a soft-deleted linked
# id collides on the primary key) never leaves the account half-migrated.
def force_migrate_to_courses_mooc_fi
conn = courses_mooc_fi_connection

response = conn.get(courses_mooc_fi_url("/api/v0/tmc-server/users-by-upstream-id/#{id}")) do |req|
req.headers['Accept'] = 'application/json'
req.headers['Authorization'] = Rails.application.secrets.tmc_server_secret_for_communicating_to_secret_project
end

data = response.body
if response.status == 200 && data.is_a?(Hash) && data['id'].present?
update!(
password_managed_by_courses_mooc_fi: true,
courses_mooc_fi_user_id: data['id'],
argon_hash: nil,
salt: nil,
password_hash: nil
)
Rails.logger.info("User #{self.email} force-migrated to courses.mooc.fi by an admin (id=#{data['id']})")
{ success: true, courses_mooc_fi_user_id: data['id'] }
else
Rails.logger.error("Force migration to courses.mooc.fi failed for user #{self.email}: status=#{response.status}, body=#{data.inspect}")
{ success: false, error: "status=#{response.status}, body=#{data.inspect}" }
end

rescue Faraday::ClientError => e
status = e.response&.dig(:status)
body = e.response&.dig(:body)
Rails.logger.error("Force migration to courses.mooc.fi errored for user #{self.email}: status=#{status}, body=#{body.inspect}")
{ success: false, error: "status=#{status}, body=#{body.inspect}" }

rescue => e
Rails.logger.error("Force migration to courses.mooc.fi unexpectedly failed for user #{self.email}: #{e.message}")
{ success: false, error: e.message }
end

# Live, display-only read of courses.mooc.fi's view of this user -- never gates any action.
# nil means genuinely unknown (unconfigured, network error, unexpected response), not "not migrated".
def courses_mooc_fi_migration_status
return nil if SiteSetting.value('courses_mooc_fi_base_url').blank?

conn = courses_mooc_fi_connection

response = conn.get(courses_mooc_fi_url("/api/v0/tmc-server/users-by-upstream-id/#{id}/status")) do |req|
req.headers['Accept'] = 'application/json'
req.headers['Authorization'] = Rails.application.secrets.tmc_server_secret_for_communicating_to_secret_project
end

data = response.body
unless response.status == 200 && data.is_a?(Hash)
Rails.logger.error("Fetching courses.mooc.fi migration status failed for user #{self.email}: status=#{response.status}, body=#{data.inspect}")
return nil
end

{
shadow_user_exists: data['shadow_user_exists'],
courses_mooc_fi_user_id: data['courses_mooc_fi_user_id'],
password_set: data['password_set'],
deleted_at: data['deleted_at']
}

rescue Faraday::ClientError => e
status = e.response&.dig(:status)
body = e.response&.dig(:body)
Rails.logger.error("Fetching courses.mooc.fi migration status errored for user #{self.email}: status=#{status}, body=#{body.inspect}")
nil

rescue => e
Rails.logger.error("Fetching courses.mooc.fi migration status unexpectedly failed for user #{self.email}: #{e.message}")
nil
end

def courses_mooc_fi_profile_url
return nil if courses_mooc_fi_user_id.blank?
return nil if SiteSetting.value('courses_mooc_fi_base_url').blank?

courses_mooc_fi_url("/manage/users/#{courses_mooc_fi_user_id}")
end

def password_reset_key
action_tokens.find { |t| t.action == 'reset_password' }
end
Expand Down Expand Up @@ -500,6 +574,17 @@ def processing_submissions_count_for_exercise(exercise_name, course_id)
end

private
def courses_mooc_fi_connection
Faraday.new(request: { open_timeout: 2, timeout: 10 }) do |f|
f.request :json
f.response :json
end
end

def courses_mooc_fi_url(path)
"#{SiteSetting.value('courses_mooc_fi_base_url')}#{path}"
end

def course_ids_arel
courses = Course.arel_table
submissions = Submission.arel_table
Expand Down
28 changes: 28 additions & 0 deletions app/views/participants/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,34 @@
<% if current_user.administrator? && !@user.administrator? %>
<li><%= link_to 'Generate password reset link', password_reset_link_participant_path, class: 'btn btn-primary' %> (shown to you because you're an admin)</li>
<% end %>
<% if current_user.administrator? %>
<li>Courses.mooc.fi managed: <%= @user.password_managed_by_courses_mooc_fi ? 'yes' : 'no' %> (shown to you because you're an admin)</li>
<li>Courses.mooc.fi user id: <%= @user.courses_mooc_fi_user_id.presence || 'not set' %> (shown to you because you're an admin)</li>
<li>Courses.mooc.fi status: <%= @courses_mooc_fi_status_label %> (shown to you because you're an admin)</li>
<% if @courses_mooc_fi_status %>
<li>
Courses.mooc.fi says: account exists=<%= @courses_mooc_fi_status[:shadow_user_exists] %>,
id=<%= @courses_mooc_fi_status[:courses_mooc_fi_user_id] || 'none' %>,
password set=<%= @courses_mooc_fi_status[:password_set] %><% if @courses_mooc_fi_status[:deleted_at] %>, deleted at <%= @courses_mooc_fi_status[:deleted_at] %><% end %>
(shown to you because you're an admin)
</li>
<% else %>
<li>Could not reach courses.mooc.fi to confirm live status. (shown to you because you're an admin)</li>
<% end %>
<% end %>
<% if current_user.administrator? && !@user.administrator? && @courses_mooc_fi_force_migrate_available %>
<% confirm_text = if @user.managed_externally?
"#{@user.email} is flagged as migrated locally, but courses.mooc.fi doesn't have a live account for them. Force migrating will (re)create the account on courses.mooc.fi and relink them to it. Continue?"
elsif @courses_mooc_fi_status&.dig(:password_set)
"#{@user.email} already has a working password on courses.mooc.fi. Force migrating will still wipe their local tmc-server password and relink them to that existing account. Continue?"
else
"Force migrate #{@user.email} to courses.mooc.fi now?"
end %>
<li><%= button_to 'Force migrate to courses.mooc.fi', force_migrate_to_courses_mooc_fi_participant_path, method: :post, class: 'btn btn-primary', data: { confirm: confirm_text } %> (shown to you because you're an admin)</li>
<% end %>
<% if current_user.administrator? && @user.courses_mooc_fi_profile_url %>
<li><%= link_to 'View on courses.mooc.fi', @user.courses_mooc_fi_profile_url, target: '_blank', rel: 'noopener', class: 'btn btn-primary' %> (shown to you because you're an admin)</li>
<% end %>
</ul>
</section>
</div>
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@
end
member do
get 'password_reset_link', to: 'participants#password_reset_link'
post 'force_migrate_to_courses_mooc_fi', to: 'participants#force_migrate_to_courses_mooc_fi'
end
end

Expand Down
6 changes: 2 additions & 4 deletions config/site.defaults.yml
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,5 @@ course_instruction_page: http://mooc.fi/courses/general/ohjelmointi/
# Teacher manual link
teacher_manual_url: http://testmycode.github.io/tmc-server/usermanual/

# URLs for password management via courses.mooc.fi
courses_mooc_fi_auth_url:
courses_mooc_fi_update_password_url:
courses_mooc_fi_create_user_url:
# Base URL for password management and admin lookups via courses.mooc.fi, e.g. https://courses.mooc.fi
courses_mooc_fi_base_url:
29 changes: 29 additions & 0 deletions spec/controllers/api/v8/users_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -163,4 +163,33 @@ def do_update(old_password)
expect(response).to have_http_status(200)
end
end

describe 'POST set_password_managed_by_courses_mooc_fi' do
before :each do
controller.current_user = admin
end

it 'rejects a blank courses_mooc_fi_user_id with a clean 400 instead of erroring' do
post :set_password_managed_by_courses_mooc_fi, params: { id: user.id }

expect(response).to have_http_status(400)
expect(user.reload.password_managed_by_courses_mooc_fi).to eq(false)
end

it 'marks the user as managed and clears the local password on success' do
user.password = 'oldpassword'
user.save!
moocfi_id = SecureRandom.uuid

post :set_password_managed_by_courses_mooc_fi, params: { id: user.id, courses_mooc_fi_user_id: moocfi_id }

expect(response).to have_http_status(200)
user.reload
expect(user.password_managed_by_courses_mooc_fi).to eq(true)
expect(user.courses_mooc_fi_user_id).to eq(moocfi_id)
expect(user.argon_hash).to be_nil
expect(user.salt).to be_nil
expect(user.password_hash).to be_nil
end
end
end
Loading
Loading