Skip to content

Commit

Permalink
logout of instfs during explicit canvas logout
Browse files Browse the repository at this point in the history
fixes RECNVS-248

test-plan:
  [happy path]
  - have instfs configured and enabled
  - login to canvas
  - logout of canvas (do not expect instfs cookie to be removed)
  - attempt to access <instfs>/session/ensure; should get oauth
    redirected to canvas login despite existing cookie
  - login to canvas anew
  - attempt to access <instfs>/session/ensure; should get 204 response

  [no instfs path]
  - have instfs disabled
  - logout of canvas; should not get a page error or other unexpected
    behavior

  [broken instfs path]
  - have instfs configured and enabled in canvas, but service down
  - logout of canvas; should not get a page error or other unexpected
    behavior, but should still see an error report for the failure to
    logout of instfs

Change-Id: Ibb4c846cfe48151df3dab04bf8fde39c2738ee4a
Reviewed-on: https://gerrit.instructure.com/141372
Tested-by: Jenkins
Reviewed-by: Andrew Huff <[email protected]>
QA-Review: Collin Parrish <[email protected]>
Product-Review: Jacob Fugal <[email protected]>
  • Loading branch information
lukfugl committed Feb 21, 2018
1 parent 839c8ae commit edcef8d
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 0 deletions.
1 change: 1 addition & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1908,6 +1908,7 @@ def destroy_session

def logout_current_user
@current_user.try(:stamp_logout_time!)
InstFS.logout(@current_user)
destroy_session
end

Expand Down
21 changes: 21 additions & 0 deletions lib/inst_fs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,24 @@ def login_pixel(user, session)
end
end

def logout(user)
return unless user && enabled?
CanvasHttp.delete(logout_url(user))
rescue CanvasHttp::Error => e
Canvas::Errors.capture_exception(:page_view, e)
end

def authenticated_url(attachment, options={})
query_params = { token: access_jwt(attachment, options) }
query_params[:download] = 1 if options[:download]
access_url(attachment, query_params)
end

def logout_url(user)
query_params = { token: logout_jwt(user) }
service_url("/session", query_params)
end

def authenticated_thumbnail_url(attachment, options={})
query_params = { token: access_jwt(attachment, options) }
query_params[:geometry] = options[:geometry] if options[:geometry]
Expand Down Expand Up @@ -129,5 +141,14 @@ def session_jwt(user)
resource: '/session/ensure'
}, expires_in.from_now, self.jwt_secret)
end

def logout_jwt(user)
expires_in = Setting.get('instfs.logout_jwt.expiration_minutes', '5').to_i.minutes
Canvas::Security.create_jwt({
iat: Time.now.utc.to_i,
user_id: user.global_id&.to_s,
resource: '/session'
}, expires_in.from_now, self.jwt_secret)
end
end
end
30 changes: 30 additions & 0 deletions spec/lib/inst_fs_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
before do
@app_host = 'http://test.host'
@secret = "supersecretyup"
allow(InstFS).to receive(:enabled?).and_return(true)
allow(Canvas::DynamicSettings).to receive(:find).with(any_args).and_call_original
allow(Canvas::DynamicSettings).to receive(:find).
with(service: "inst-fs", default_ttl: 5.minutes).
Expand Down Expand Up @@ -226,4 +227,33 @@
end
end
end

context "logout" do
it "makes a DELETE request against the logout url" do
expect(CanvasHttp).to receive(:delete).with(match(%r{/session[^/\w]}))
InstFS.logout(user_model)
end

it "includes jwt in DELETE request" do
expect(CanvasHttp).to receive(:delete).with(match(%r{\?token=}))
InstFS.logout(user_model)
end

it "skips if user absent" do
expect(CanvasHttp).not_to receive(:delete)
InstFS.logout(nil)
end

it "skips if not enabled" do
allow(InstFS).to receive(:enabled?).and_return(false)
expect(CanvasHttp).not_to receive(:delete)
InstFS.logout(user_model)
end

it "logs then ignores error if DELETE request fails" do
allow(CanvasHttp).to receive(:delete).and_raise(CanvasHttp::Error, "broken request")
expect(Canvas::Errors).to receive(:capture_exception).once
InstFS.logout(user_model)
end
end
end

0 comments on commit edcef8d

Please sign in to comment.