From d7edc45bb64a416d22e06c0112cb77ba47df519b Mon Sep 17 00:00:00 2001 From: David Spencer Date: Wed, 25 Mar 2020 22:23:23 -0500 Subject: [PATCH 1/3] Don't delete Canvas tokens when users and courses are destroyed Deleting Canvas tokens causes some of our downstream apps a lot of heartache. We almost never want to delete them. --- app/models/canvas_course.rb | 2 +- app/models/user.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/canvas_course.rb b/app/models/canvas_course.rb index 0ec155479..15611accf 100644 --- a/app/models/canvas_course.rb +++ b/app/models/canvas_course.rb @@ -1,3 +1,3 @@ class CanvasCourse < ApplicationRecord - has_many :authentications, dependent: :destroy, inverse_of: :canvas_course + has_many :authentications, inverse_of: :canvas_course end diff --git a/app/models/user.rb b/app/models/user.rb index 179fbbdff..bb6c3cabb 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -5,7 +5,7 @@ class User < ApplicationRecord devise :database_authenticatable, :registerable, :confirmable, :recoverable, :rememberable, :trackable, :validatable, :omniauthable - has_many :authentications, dependent: :destroy, inverse_of: :user + has_many :authentications, inverse_of: :user has_many :permissions, dependent: :destroy has_many :roles, through: :permissions From a9bfa1998112551931f80fc1355f2efdc0be3691 Mon Sep 17 00:00:00 2001 From: David Spencer Date: Wed, 25 Mar 2020 22:29:14 -0500 Subject: [PATCH 2/3] Handle Canvas token refresh 500 failures better We don't want to delete tokens when Canvas might succeed later at refreshing. --- Gemfile | 2 +- Gemfile.lock | 4 ++-- app/controllers/application_controller.rb | 9 +++++++++ .../controllers/application_controller_spec.rb | 18 ++++++++++++++++++ 4 files changed, 30 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index c95c47b2b..5d1ab2820 100644 --- a/Gemfile +++ b/Gemfile @@ -29,7 +29,7 @@ gem "strong_password" gem "ims-lti", "~> 2.1.5" # IMS LTI tool consumers and providers gem "json-jwt" gem "jwt" -gem "lms-api", "~>1.9.0" +gem "lms-api", "~>1.15.0" gem "omniauth" gem "omniauth-canvas", "~>1.0.2" gem "rolify" diff --git a/Gemfile.lock b/Gemfile.lock index a07e69981..f4678e776 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -173,7 +173,7 @@ GEM listen (3.2.1) rb-fsevent (~> 0.10, >= 0.10.3) rb-inotify (~> 0.9, >= 0.9.10) - lms-api (1.9.0) + lms-api (1.15.0) activesupport (>= 3.0) httparty loofah (2.4.0) @@ -443,7 +443,7 @@ DEPENDENCIES jwt launchy listen - lms-api (~> 1.9.0) + lms-api (~> 1.15.0) mail_view non-stupid-digest-assets nuclear_secrets diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 67f1f56f8..b47f5cc00 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -68,6 +68,15 @@ def handle_canvas_exception(exception) render_error 500, "Error while accessing Canvas: #{exception.message}.", { exception: exception } end + # Raised when Canvas returned a 500 when trying to refresh the token - this + # usually means Canvas is down or something is wrong with the config. Check + # the oauth key and secret. + rescue_from LMS::Canvas::RefreshToken500Exception, with: :handle_canvas_token_refresh_500 + def handle_canvas_token_refresh_500(exception) + record_exception(exception) + render_error 500, "Error while refreshing Canvas token: #{exception.message}.", { exception: exception } + end + # Raised when a new token cannot be retrieved using the refresh token rescue_from LMS::Canvas::RefreshTokenFailedException, with: :handle_canvas_token_expired diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 0b47e0125..c639250dd 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -119,6 +119,24 @@ def index end end + describe "LMS::Canvas::RefreshToken500Exception" do + controller do + def index + raise LMS::Canvas::RefreshToken500Exception.new("Canvas returned a 500") + end + end + it "renders an error page" do + get :index + expect(response).to have_http_status(500) + end + it "renders error json" do + get :index, format: :json + expect(response).to have_http_status(500) + result = JSON.parse(response.body) + expect(result["message"]).to eq("Error while refreshing Canvas token: Canvas returned a 500.") + end + end + describe "LMS::Canvas::RefreshTokenFailedException" do controller do def index From 4f479dc4dc4bc8caf0946d1095854e4ad0c9574d Mon Sep 17 00:00:00 2001 From: David Spencer Date: Mon, 13 Jul 2020 15:34:46 -0500 Subject: [PATCH 3/3] Use dependent: :nullify --- app/models/canvas_course.rb | 2 +- app/models/user.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/canvas_course.rb b/app/models/canvas_course.rb index 15611accf..638edd5eb 100644 --- a/app/models/canvas_course.rb +++ b/app/models/canvas_course.rb @@ -1,3 +1,3 @@ class CanvasCourse < ApplicationRecord - has_many :authentications, inverse_of: :canvas_course + has_many :authentications, inverse_of: :canvas_course, dependent: :nullify end diff --git a/app/models/user.rb b/app/models/user.rb index bb6c3cabb..5a8f8a4e4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -5,7 +5,7 @@ class User < ApplicationRecord devise :database_authenticatable, :registerable, :confirmable, :recoverable, :rememberable, :trackable, :validatable, :omniauthable - has_many :authentications, inverse_of: :user + has_many :authentications, inverse_of: :user, dependent: :nullify has_many :permissions, dependent: :destroy has_many :roles, through: :permissions