Skip to content

Commit

Permalink
Add setting to change ttl on AccessVerifier
Browse files Browse the repository at this point in the history
5 minutes sometimes just isn't enough. This makes it so it is configurable and not hard coded.

Test Plan:

* Add a video to a course
* insert a video tag into the html of a page. ie:
```
<p><video style="border: 2px solid black;" width="450" height="225" controls="controls">
<source src="/courses/<course_id>/files/<file_id>/download" /></video></p>
```
* Inspect the network traffic while watching the video.
* Obtain the jwt used to download the video.
* Inspect that jwt and the exp should be the minutes out of what ever is set in the Setting, default 5 minutes.
  • Loading branch information
bfcoder committed Sep 28, 2021
1 parent beac08e commit 5915d1d
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 5 deletions.
4 changes: 1 addition & 3 deletions app/models/users/access_verifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@

module Users
module AccessVerifier
TTL_MINUTES = 5

class InvalidVerifier < RuntimeError
end

Expand All @@ -40,7 +38,7 @@ def self.generate(claims)
jwt_claims[:root_account_id] = root_account.global_id.to_s if root_account
jwt_claims.merge!(claims.slice(:oauth_host, :return_url, :fallback_url))

expires = TTL_MINUTES.minutes.from_now
expires = Setting.get('access_verifier.ttl_minutes', '5').to_i.minutes.from_now
key = nil # use default key
{ sf_verifier: Canvas::Security.create_jwt(jwt_claims, expires, key, :HS512) }
end
Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/files_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ def file_with_path(path)

# second use after verifier expiration but before session expiration.
# expired verifier should be ignored but session should still be extended
Timecop.freeze((Users::AccessVerifier::TTL_MINUTES + 1).minutes.from_now) do
Timecop.freeze((Setting.get('access_verifier.ttl_minutes', '5').to_i + 1).minutes.from_now) do
get 'show', params: verifier.merge(id: file.id)
end
expect(response).to be_successful
Expand Down
2 changes: 1 addition & 1 deletion spec/models/users/access_verifier_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ module Users

it "raises InvalidVerifier if too old" do
verifier = Users::AccessVerifier.generate(user: user)
Timecop.freeze(10.minutes.from_now) do
Timecop.freeze((Setting.get('access_verifier.ttl_minutes', '5').to_i + 1).minutes.from_now) do
expect { Users::AccessVerifier.validate(verifier) }.to raise_exception(Canvas::Security::TokenExpired)
end
end
Expand Down

0 comments on commit 5915d1d

Please sign in to comment.