diff --git a/app/controllers/api/v8/users_controller.rb b/app/controllers/api/v8/users_controller.rb index 097b7a4ff..d1adbd208 100644 --- a/app/controllers/api/v8/users_controller.rb +++ b/app/controllers/api/v8/users_controller.rb @@ -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 @@ -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 @@ -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 diff --git a/app/controllers/participants_controller.rb b/app/controllers/participants_controller.rb index 65d44bf8c..c4a738ebc 100644 --- a/app/controllers/participants_controller.rb +++ b/app/controllers/participants_controller.rb @@ -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 @@ -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| diff --git a/app/models/user.rb b/app/models/user.rb index 85e18e3d3..a152b6ae3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -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' @@ -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| @@ -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| @@ -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 @@ -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 diff --git a/app/views/participants/show.html.erb b/app/views/participants/show.html.erb index 2bc75c8d5..f88e9938a 100644 --- a/app/views/participants/show.html.erb +++ b/app/views/participants/show.html.erb @@ -20,6 +20,34 @@ <% if current_user.administrator? && !@user.administrator? %>
  • <%= link_to 'Generate password reset link', password_reset_link_participant_path, class: 'btn btn-primary' %> (shown to you because you're an admin)
    • <% end %> + <% if current_user.administrator? %> +
  • Courses.mooc.fi managed: <%= @user.password_managed_by_courses_mooc_fi ? 'yes' : 'no' %> (shown to you because you're an admin)
    • +
  • Courses.mooc.fi user id: <%= @user.courses_mooc_fi_user_id.presence || 'not set' %> (shown to you because you're an admin)
    • +
  • Courses.mooc.fi status: <%= @courses_mooc_fi_status_label %> (shown to you because you're an admin)
    • + <% if @courses_mooc_fi_status %> +
  • + 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) +
    • + <% else %> +
  • Could not reach courses.mooc.fi to confirm live status. (shown to you because you're an admin)
    • + <% 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 %> +
  • <%= 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)
    • + <% end %> + <% if current_user.administrator? && @user.courses_mooc_fi_profile_url %> +
  • <%= 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)
    • + <% end %> diff --git a/config/routes.rb b/config/routes.rb index 77da25dff..3dc83e7a7 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -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 diff --git a/config/site.defaults.yml b/config/site.defaults.yml index 36ea5a787..d4c737d59 100644 --- a/config/site.defaults.yml +++ b/config/site.defaults.yml @@ -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: diff --git a/spec/controllers/api/v8/users_controller_spec.rb b/spec/controllers/api/v8/users_controller_spec.rb index 8ebe0b271..fee8b6c12 100644 --- a/spec/controllers/api/v8/users_controller_spec.rb +++ b/spec/controllers/api/v8/users_controller_spec.rb @@ -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 diff --git a/spec/controllers/participants_controller_spec.rb b/spec/controllers/participants_controller_spec.rb index 518007ba7..e64c613c0 100644 --- a/spec/controllers/participants_controller_spec.rb +++ b/spec/controllers/participants_controller_spec.rb @@ -27,4 +27,171 @@ end end end + + describe 'POST /force_migrate_to_courses_mooc_fi' do + describe 'when logged in as a non-admin' do + before :each do + controller.current_user = @user + end + + it 'is forbidden' do + post :force_migrate_to_courses_mooc_fi, params: { id: @user.id } + expect(response.code.to_i).to eq(403) + end + end + + describe 'when logged in as an admin' do + before :each do + controller.current_user = FactoryBot.create(:admin) + end + + it 'is forbidden when the target user is an admin' do + admin_target = FactoryBot.create(:admin) + expect_any_instance_of(User).not_to receive(:force_migrate_to_courses_mooc_fi) + post :force_migrate_to_courses_mooc_fi, params: { id: admin_target.id } + expect(response.code.to_i).to eq(403) + end + + it 'refuses when the user is already managed externally and courses.mooc.fi confirms the account is live' do + @user.update!(password_managed_by_courses_mooc_fi: true, courses_mooc_fi_user_id: SecureRandom.uuid) + expect_any_instance_of(User).to receive(:courses_mooc_fi_migration_status).and_return( + { shadow_user_exists: true, courses_mooc_fi_user_id: @user.courses_mooc_fi_user_id, password_set: true, deleted_at: nil } + ) + expect_any_instance_of(User).not_to receive(:force_migrate_to_courses_mooc_fi) + post :force_migrate_to_courses_mooc_fi, params: { id: @user.id } + expect(response).to redirect_to(participant_path(@user)) + expect(flash[:alert]).to match(/already managed/) + end + + it 'refuses when the user is already managed externally and the live status is unknown' do + @user.update!(password_managed_by_courses_mooc_fi: true, courses_mooc_fi_user_id: SecureRandom.uuid) + expect_any_instance_of(User).to receive(:courses_mooc_fi_migration_status).and_return(nil) + expect_any_instance_of(User).not_to receive(:force_migrate_to_courses_mooc_fi) + post :force_migrate_to_courses_mooc_fi, params: { id: @user.id } + expect(response).to redirect_to(participant_path(@user)) + expect(flash[:alert]).to match(/already managed/) + end + + it 'allows re-migrating when managed externally locally but courses.mooc.fi confirms the account is gone' do + @user.update!(password_managed_by_courses_mooc_fi: true, courses_mooc_fi_user_id: SecureRandom.uuid) + expect_any_instance_of(User).to receive(:courses_mooc_fi_migration_status).and_return( + { shadow_user_exists: false, courses_mooc_fi_user_id: nil, password_set: false, deleted_at: nil } + ) + expect_any_instance_of(User).to receive(:force_migrate_to_courses_mooc_fi).and_return({ success: true, courses_mooc_fi_user_id: 'new-id' }) + post :force_migrate_to_courses_mooc_fi, params: { id: @user.id } + expect(response).to redirect_to(participant_path(@user)) + expect(flash[:notice]).to match(/force-migrated/) + end + + it 'allows re-migrating when managed externally locally but the linked courses.mooc.fi account was deleted' do + @user.update!(password_managed_by_courses_mooc_fi: true, courses_mooc_fi_user_id: SecureRandom.uuid) + expect_any_instance_of(User).to receive(:courses_mooc_fi_migration_status).and_return( + { shadow_user_exists: true, courses_mooc_fi_user_id: @user.courses_mooc_fi_user_id, password_set: false, deleted_at: Time.current } + ) + expect_any_instance_of(User).to receive(:force_migrate_to_courses_mooc_fi).and_return({ success: true, courses_mooc_fi_user_id: 'new-id' }) + post :force_migrate_to_courses_mooc_fi, params: { id: @user.id } + expect(response).to redirect_to(participant_path(@user)) + expect(flash[:notice]).to match(/force-migrated/) + end + + it 'migrates the user when they are not yet managed externally and the migration succeeds' do + expect_any_instance_of(User).to receive(:force_migrate_to_courses_mooc_fi).and_return({ success: true, courses_mooc_fi_user_id: 'abc-123' }) + post :force_migrate_to_courses_mooc_fi, params: { id: @user.id } + expect(response).to redirect_to(participant_path(@user)) + expect(flash[:notice]).to match(/force-migrated/) + expect(flash[:notice]).to match(/abc-123/) + end + + it 'shows the exact error when the migration fails' do + expect_any_instance_of(User).to receive(:force_migrate_to_courses_mooc_fi).and_return({ success: false, error: 'status=422, body={"error"=>"boom"}' }) + post :force_migrate_to_courses_mooc_fi, params: { id: @user.id } + expect(response).to redirect_to(participant_path(@user)) + expect(flash[:alert]).to match(/status=422/) + end + end + end + + describe 'GET /show' do + describe 'when logged in as an admin' do + before :each do + controller.current_user = FactoryBot.create(:admin) + end + + it 'shows the migration status when courses.mooc.fi confirms the user is not migrated' do + expect_any_instance_of(User).to receive(:courses_mooc_fi_migration_status).and_return( + { shadow_user_exists: false, courses_mooc_fi_user_id: nil, password_set: false, deleted_at: nil } + ) + get :show, params: { id: @user.id } + expect(response).to be_successful + expect(assigns(:courses_mooc_fi_status_label)).to eq('Not migrated') + end + + it 'flags an inconsistency when courses.mooc.fi already has a password but the user is not linked locally' do + expect_any_instance_of(User).to receive(:courses_mooc_fi_migration_status).and_return( + { shadow_user_exists: true, courses_mooc_fi_user_id: SecureRandom.uuid, password_set: true, deleted_at: nil } + ) + get :show, params: { id: @user.id } + expect(response).to be_successful + expect(assigns(:courses_mooc_fi_status_label)).to match(/Inconsistent/) + end + + it 'shows fully migrated when the user is already managed externally and courses.mooc.fi confirms the account is live' do + @user.update!(password_managed_by_courses_mooc_fi: true, courses_mooc_fi_user_id: SecureRandom.uuid) + expect_any_instance_of(User).to receive(:courses_mooc_fi_migration_status).and_return( + { shadow_user_exists: true, courses_mooc_fi_user_id: @user.courses_mooc_fi_user_id, password_set: true, deleted_at: nil } + ) + get :show, params: { id: @user.id } + expect(response).to be_successful + expect(assigns(:courses_mooc_fi_status_label)).to eq('Fully migrated') + expect(assigns(:courses_mooc_fi_force_migrate_available)).to eq(false) + end + + it 'shows fully migrated when the user is already managed externally and the live status is unknown' do + @user.update!(password_managed_by_courses_mooc_fi: true, courses_mooc_fi_user_id: SecureRandom.uuid) + expect_any_instance_of(User).to receive(:courses_mooc_fi_migration_status).and_return(nil) + get :show, params: { id: @user.id } + expect(response).to be_successful + expect(assigns(:courses_mooc_fi_status_label)).to eq('Fully migrated') + expect(assigns(:courses_mooc_fi_force_migrate_available)).to eq(false) + end + + it 'flags drift and re-enables force migrate when locally managed but courses.mooc.fi has no live account' do + @user.update!(password_managed_by_courses_mooc_fi: true, courses_mooc_fi_user_id: SecureRandom.uuid) + expect_any_instance_of(User).to receive(:courses_mooc_fi_migration_status).and_return( + { shadow_user_exists: false, courses_mooc_fi_user_id: nil, password_set: false, deleted_at: nil } + ) + get :show, params: { id: @user.id } + expect(response).to be_successful + expect(assigns(:courses_mooc_fi_status_label)).to match(/Broken/) + expect(assigns(:courses_mooc_fi_force_migrate_available)).to eq(true) + end + + it 'flags drift and re-enables force migrate when the linked courses.mooc.fi account was deleted' do + @user.update!(password_managed_by_courses_mooc_fi: true, courses_mooc_fi_user_id: SecureRandom.uuid) + expect_any_instance_of(User).to receive(:courses_mooc_fi_migration_status).and_return( + { shadow_user_exists: true, courses_mooc_fi_user_id: @user.courses_mooc_fi_user_id, password_set: false, deleted_at: Time.current } + ) + get :show, params: { id: @user.id } + expect(response).to be_successful + expect(assigns(:courses_mooc_fi_status_label)).to match(/Broken/) + expect(assigns(:courses_mooc_fi_force_migrate_available)).to eq(true) + end + + it 'flags the broken state when managed locally but missing the target id' do + @user.update!(password_managed_by_courses_mooc_fi: true, courses_mooc_fi_user_id: nil) + expect_any_instance_of(User).to receive(:courses_mooc_fi_migration_status).and_return(nil) + get :show, params: { id: @user.id } + expect(response).to be_successful + expect(assigns(:courses_mooc_fi_status_label)).to match(/Broken/) + end + + it 'degrades gracefully when courses.mooc.fi cannot be reached' do + expect_any_instance_of(User).to receive(:courses_mooc_fi_migration_status).and_return(nil) + get :show, params: { id: @user.id } + expect(response).to be_successful + expect(assigns(:courses_mooc_fi_status)).to be_nil + expect(assigns(:courses_mooc_fi_status_label)).to eq('Not migrated') + end + end + end end diff --git a/spec/controllers/setup/course_assistans_controller_spec.rb b/spec/controllers/setup/course_assistans_controller_spec.rb index 2f5f9023b..7ea9f7d74 100644 --- a/spec/controllers/setup/course_assistans_controller_spec.rb +++ b/spec/controllers/setup/course_assistans_controller_spec.rb @@ -24,7 +24,7 @@ user3 = FactoryBot.create(:user) @course.assistants << [user1, user2, user3] get :index, params: { organization_id: @organization.slug, course_id: @course.id } - expect(assigns(:assistants)).to eq([user1, user2, user3]) + expect(assigns(:assistants).sort_by(&:id)).to eq([user1, user2, user3].sort_by(&:id)) end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e6e476d8d..571f2ae03 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -270,6 +270,27 @@ expect(User.authenticate('root', 'ilikecookies')).to be_nil end + describe 'courses_mooc_fi_profile_url' do + it 'is nil when the user has no courses.mooc.fi id' do + user = User.create!(login: 'localuser', password: 'secret123', email: 'localuser@example.com') + SiteSetting.all_settings['courses_mooc_fi_base_url'] = 'https://courses.mooc.fi' + expect(user.courses_mooc_fi_profile_url).to be_nil + end + + it 'is nil when courses_mooc_fi_base_url is not configured' do + user = User.create!(login: 'manageduser', password: 'secret123', email: 'managed@example.com', courses_mooc_fi_user_id: SecureRandom.uuid) + SiteSetting.all_settings['courses_mooc_fi_base_url'] = nil + expect(user.courses_mooc_fi_profile_url).to be_nil + end + + it 'builds the profile url from the configured base url and the courses.mooc.fi id' do + id = SecureRandom.uuid + user = User.create!(login: 'manageduser', password: 'secret123', email: 'managed@example.com', courses_mooc_fi_user_id: id) + SiteSetting.all_settings['courses_mooc_fi_base_url'] = 'https://courses.mooc.fi' + expect(user.courses_mooc_fi_profile_url).to eq("https://courses.mooc.fi/manage/users/#{id}") + end + end + describe 'migrating to courses.mooc.fi on login' do it 'posts a locally-managed user on successful authentication' do user = User.create!(login: 'localuser', password: 'secret123', email: 'localuser@example.com')