From d45dc04a3ea8d2c36092c9342bae473406c9352d Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Tue, 15 Jan 2019 17:36:32 +0100 Subject: [PATCH 1/9] Limit redirect to host --- app/controllers/application_controller.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 99ce4289..fd6ceac4 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -20,7 +20,14 @@ class ApplicationController < ActionController::Base def render_not_authorized respond_to do |format| - format.html { redirect_to(request.referrer || :root, alert: t('application.not_authorized')) } + format.html do + if request.referrer.present? && request.referrer.include?(request.base_url) + destination = request.referrer + else + destination = :root + end + redirect_to(destination, alert: t('application.not_authorized')) + end format.json { render json: {error: t('application.not_authorized')}, status: :unauthorized } end end From 7eab61dc8f92cba0bc97e9f9198e4e8be6e669a1 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Tue, 15 Jan 2019 17:36:57 +0100 Subject: [PATCH 2/9] Show nice name for StudyGroups --- app/models/study_group.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/models/study_group.rb b/app/models/study_group.rb index b86f4969..13167047 100644 --- a/app/models/study_group.rb +++ b/app/models/study_group.rb @@ -8,4 +8,12 @@ class StudyGroup < ApplicationRecord has_many :users, through: :study_group_memberships, source_type: 'ExternalUser' has_many :submissions belongs_to :consumer + + def to_s + if name.blank? + "StudyGroup " + id.to_s + else + name + end + end end From 0b5ec1820dcfa9b1722ef579c8a9deed66acc23b Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Tue, 15 Jan 2019 17:37:34 +0100 Subject: [PATCH 3/9] Fix link for external users --- app/views/external_users/index.html.slim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/external_users/index.html.slim b/app/views/external_users/index.html.slim index ea6d7a48..b9450323 100644 --- a/app/views/external_users/index.html.slim +++ b/app/views/external_users/index.html.slim @@ -10,7 +10,7 @@ h1 = ExternalUser.model_name.human(count: 2) tbody - @users.each do |user| tr - td = link_to_if(policy(user).show?, user.displayname) + td = link_to_if(policy(user).show?, user.displayname, user) td = link_to_if(policy(user.consumer).show?, user.consumer, user.consumer) td = link_to(t('shared.show'), user) if policy(user).show? From ff8c1f6838e3e182657c2010ef527c4a85a589f8 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Thu, 17 Jan 2019 18:34:04 +0100 Subject: [PATCH 4/9] Use index? to validate permissions of classes instead of show? for obj. --- app/views/application/_breadcrumbs.html.slim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/application/_breadcrumbs.html.slim b/app/views/application/_breadcrumbs.html.slim index 4240834f..9af36b87 100644 --- a/app/views/application/_breadcrumbs.html.slim +++ b/app/views/application/_breadcrumbs.html.slim @@ -8,7 +8,7 @@ - if object li.breadcrumb-item = object - else - li.breadcrumb-item = link_to_if(policy(model).show?, model.model_name.human(count: 2), send(:"#{model.model_name.collection}_path")) + li.breadcrumb-item = link_to_if(policy(model).index?, model.model_name.human(count: 2), send(:"#{model.model_name.collection}_path")) - if object li.breadcrumb-item = link_to_if(policy(object).show?, object, send(:"#{model.model_name.singular}_path", object)) li.breadcrumb-item.active From c498d2b08b9263c773b9b5a5a896844c8fcd1963 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Thu, 17 Jan 2019 18:44:11 +0100 Subject: [PATCH 5/9] Allow sign in via case insensitive email address --- app/models/internal_user.rb | 2 +- config/initializers/sorcery.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/internal_user.rb b/app/models/internal_user.rb index 9cef2cb4..4e13e908 100644 --- a/app/models/internal_user.rb +++ b/app/models/internal_user.rb @@ -2,7 +2,7 @@ class InternalUser < User authenticates_with_sorcery! - validates :email, presence: true, uniqueness: true + validates :email, presence: true, uniqueness: true, case_sensitive: false validates :password, confirmation: true, if: :password_void?, on: :update, presence: true validates :role, inclusion: {in: ROLES} diff --git a/config/initializers/sorcery.rb b/config/initializers/sorcery.rb index 1735265f..46d97ff6 100644 --- a/config/initializers/sorcery.rb +++ b/config/initializers/sorcery.rb @@ -158,7 +158,7 @@ Rails.application.config.sorcery.configure do |config| # downcase the username before trying to authenticate, default is false # Default: `false` # - # user.downcase_username_before_authenticating = + user.downcase_username_before_authenticating = true # change default email attribute. From 4308f0f3cda1458db5a0205d492fab889d67f5f2 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Thu, 17 Jan 2019 19:05:54 +0100 Subject: [PATCH 6/9] Hide other occurrences of the same user from appearing in wrong StudyGroup --- app/controllers/study_groups_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/study_groups_controller.rb b/app/controllers/study_groups_controller.rb index 9e92949e..54b4aa92 100644 --- a/app/controllers/study_groups_controller.rb +++ b/app/controllers/study_groups_controller.rb @@ -15,7 +15,7 @@ class StudyGroupsController < ApplicationController def edit @search = @study_group.users.search(params[:q]) - @members = StudyGroupMembership.where(user: @search.result) + @members = StudyGroupMembership.where(user: @search.result, study_group: @study_group) end def update From f7030e3506940bccc53af699647b05dfeffb9754 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Thu, 31 Jan 2019 13:06:28 +0100 Subject: [PATCH 7/9] Refactor redirect if user is not authorized --- app/controllers/application_controller.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index fd6ceac4..4916847c 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -21,12 +21,12 @@ class ApplicationController < ActionController::Base def render_not_authorized respond_to do |format| format.html do - if request.referrer.present? && request.referrer.include?(request.base_url) - destination = request.referrer + # Prevent redirect loop + if request.url == request.referrer + redirect_to :root, alert: t('application.not_authorized') else - destination = :root + redirect_back fallback_location: :root, allow_other_host: false, alert: t('application.not_authorized') end - redirect_to(destination, alert: t('application.not_authorized')) end format.json { render json: {error: t('application.not_authorized')}, status: :unauthorized } end From c74f2bd4bab2a0fad6e076585cde829924e6d6b2 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Tue, 5 Feb 2019 12:11:32 +0100 Subject: [PATCH 8/9] Fix response format handling when creating a comment --- app/controllers/comments_controller.rb | 30 +++++++++++--------------- config/routes.rb | 2 +- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index ef2ee4db..fcddf876 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -37,30 +37,26 @@ class CommentsController < ApplicationController def create @comment = Comment.new(comment_params_without_request_id) - respond_to do |format| - if @comment.save - if comment_params[:request_id] - request_for_comment = RequestForComment.find(comment_params[:request_id]) - send_mail_to_author @comment, request_for_comment - send_mail_to_subscribers @comment, request_for_comment - end - - render :show, status: :created, location: @comment - else - render json: @comment.errors, status: :unprocessable_entity + if @comment.save + if comment_params[:request_id] + request_for_comment = RequestForComment.find(comment_params[:request_id]) + send_mail_to_author @comment, request_for_comment + send_mail_to_subscribers @comment, request_for_comment end + + render :show, status: :created, location: @comment + else + render json: @comment.errors, status: :unprocessable_entity end authorize! end # PATCH/PUT /comments/1.json def update - respond_to do |format| - if @comment.update(comment_params_without_request_id) - render :show, status: :ok, location: @comment - else - render json: @comment.errors, status: :unprocessable_entity - end + if @comment.update(comment_params_without_request_id) + render :show, status: :ok, location: @comment + else + render json: @comment.errors, status: :unprocessable_entity end authorize! end diff --git a/config/routes.rb b/config/routes.rb index 3e6dca68..2fcf7d6b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -26,7 +26,7 @@ Rails.application.routes.draw do get '/my_rfc_activity', as: 'my_rfc_activity', to: 'request_for_comments#get_rfcs_with_my_comments' delete '/comment_by_id', to: 'comments#destroy_by_id' - put '/comments', to: 'comments#update' + put '/comments', to: 'comments#update', defaults: { format: :json } resources :subscriptions, only: [:create, :destroy] do member do From 2df992102fa3c627ad687d964b88aed05aad7631 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Tue, 5 Feb 2019 12:13:46 +0100 Subject: [PATCH 9/9] Allow LTI clients to specify a redirect target --- app/controllers/sessions_controller.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 8d7760fc..7e536c6c 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -20,9 +20,13 @@ class SessionsController < ApplicationController def create_through_lti store_lti_session_data(consumer: @consumer, parameters: params) store_nonce(params[:oauth_nonce]) - redirect_to(implement_exercise_path(@exercise), - notice: t("sessions.create_through_lti.session_#{lti_outcome_service?(@exercise.id, @current_user.id , @consumer.id) ? 'with' : 'without'}_outcome", - consumer: @consumer)) + if params[:redirect_target] + redirect_to(params[:redirect_target]) + else + redirect_to(implement_exercise_path(@exercise), + notice: t("sessions.create_through_lti.session_#{lti_outcome_service?(@exercise.id, @current_user.id , @consumer.id) ? 'with' : 'without'}_outcome", + consumer: @consumer)) + end end def destroy