diff --git a/app/controllers/lti/ims/authorization_controller.rb b/app/controllers/lti/ims/authorization_controller.rb index 68262e7c486..e01c7423086 100644 --- a/app/controllers/lti/ims/authorization_controller.rb +++ b/app/controllers/lti/ims/authorization_controller.rb @@ -78,9 +78,9 @@ module Lti raise InvalidGrant if params[:grant_type] != JWT_GRANT_TYPE raise InvalidGrant if params[:assertion].blank? jwt_validator = Lti::Oauth2::AuthorizationValidator.new(jwt: params[:assertion], authorization_url: lti_oauth2_authorize_url) - jwt_validator.jwt + jwt_validator.validate! render json: { - access_token: SecureRandom.uuid, + access_token: Lti::Oauth2::AccessToken.create_jwt(aud: request.host, sub: jwt_validator.tool_proxy.guid).to_s, token_type: 'bearer', expires_in: Setting.get('lti.oauth2.access_token.expiration', 1.hour.to_s) } diff --git a/lib/canvas/security.rb b/lib/canvas/security.rb index cf216cf5d0a..fbf6d562899 100644 --- a/lib/canvas/security.rb +++ b/lib/canvas/security.rb @@ -326,7 +326,7 @@ module Canvas::Security if body[:nbf].present? if timestamp_is_future?(body[:nbf]) - raise Canvas::Security::TokenInvalid + raise Canvas::Security::InvalidToken end end end diff --git a/lib/lti/oauth2/access_token.rb b/lib/lti/oauth2/access_token.rb new file mode 100644 index 00000000000..b3d7a0a176f --- /dev/null +++ b/lib/lti/oauth2/access_token.rb @@ -0,0 +1,78 @@ +module Lti + module Oauth2 + class AccessToken + private_class_method :new + + ISS = 'Canvas'.freeze + + attr_reader :aud, :sub + + def self.create_jwt(aud:, sub:) + new(aud: aud, sub: sub) + end + + def self.from_jwt(aud:, jwt:) + decoded_jwt = Canvas::Security.decode_jwt(jwt) + new(aud: aud, sub: decoded_jwt[:sub], jwt: jwt) + rescue Canvas::Security::TokenExpired => e + raise InvalidTokenError, 'token has expired', e.backtrace + rescue StandardError => e + raise InvalidTokenError, e + end + + def initialize(aud:, sub:, jwt: nil) + @_jwt = jwt if jwt + @aud = aud + @sub = sub + end + + def validate! + decoded_jwt = Canvas::Security.decode_jwt(jwt) + check_required_assertions(decoded_jwt.keys) + raise InvalidTokenError, 'invalid iss' if decoded_jwt['iss'] != ISS + raise InvalidTokenError, 'invalid aud' if decoded_jwt[:aud] != aud + raise InvalidTokenError, 'iat must be in the past' unless Time.zone.at(decoded_jwt['iat']) < Time.zone.now + true + rescue InvalidTokenError + raise + rescue Canvas::Security::TokenExpired => e + raise InvalidTokenError, 'token has expired', e.backtrace + rescue StandardError => e + raise InvalidTokenError, e + end + + def to_s + jwt + end + + private + + def decoded_jwt + @_decoded_jwt ||= Canvas::Security.decode_jwt(jwt) + end + + def jwt + @_jwt ||= begin + body = { + iss: ISS, + sub: sub, + exp: Setting.get('lti.oauth2.access_token.exp', 1.hour).to_i.seconds.from_now, + aud: aud, + iat: Time.zone.now.to_i, + nbf: Setting.get('lti.oauth2.access_token.nbf', 30.seconds).to_i.seconds.ago, + jti: SecureRandom.uuid + } + Canvas::Security.create_jwt(body) + end + end + + def check_required_assertions(assertion_keys) + missing_assertions = (%w(iss sub exp aud iat nbf jti) - assertion_keys) + if missing_assertions.present? + raise InvalidTokenError, "the following assertions are missing: #{missing_assertions.join(',')}" + end + end + + end + end +end diff --git a/lib/lti/oauth2/authorization_validator.rb b/lib/lti/oauth2/authorization_validator.rb index 95f3ec61930..012565666de 100644 --- a/lib/lti/oauth2/authorization_validator.rb +++ b/lib/lti/oauth2/authorization_validator.rb @@ -38,6 +38,7 @@ module Lti validated_jwt end end + alias_method :validate!, :jwt def tool_proxy @_tool_proxy ||= begin diff --git a/lib/lti/oauth2/invalid_token_error.rb b/lib/lti/oauth2/invalid_token_error.rb new file mode 100644 index 00000000000..f2356414069 --- /dev/null +++ b/lib/lti/oauth2/invalid_token_error.rb @@ -0,0 +1,4 @@ +module Lti::Oauth2 + class InvalidTokenError < StandardError + end +end diff --git a/spec/apis/lti/ims/authorization_api_spec.rb b/spec/apis/lti/ims/authorization_api_spec.rb index a9d6426fb6e..924b18c1239 100644 --- a/spec/apis/lti/ims/authorization_api_spec.rb +++ b/spec/apis/lti/ims/authorization_api_spec.rb @@ -77,7 +77,8 @@ module Lti it 'returns an access_token' do post auth_endpoint, params - expect(JSON.parse(response.body)['access_token']).not_to be_nil + access_token = Lti::Oauth2::AccessToken.create_jwt(aud: @request.host, sub: tool_proxy.guid) + expect{access_token.validate!}.not_to raise_error end it "allows the use of the 'OAuth.splitSecret'" do diff --git a/spec/lib/lti/oauth2/access_token_spec.rb b/spec/lib/lti/oauth2/access_token_spec.rb new file mode 100644 index 00000000000..6dc3027bb2b --- /dev/null +++ b/spec/lib/lti/oauth2/access_token_spec.rb @@ -0,0 +1,134 @@ +require File.expand_path(File.dirname(__FILE__) + '/../../../spec_helper') +require 'json/jwt' + +module Lti + module Oauth2 + describe AccessToken do + + let(:aud){'http://example.com'} + let(:sub) {'12084434-0c58-4058-b8c0-4af2da9c2ef8'} + let(:body) do + { + iss: 'Canvas', + sub: sub, + exp: 5.minutes.from_now.to_i, + aud: aud, + iat: Time.zone.now.to_i, + nbf: 30.seconds.ago, + jti: '34084434-0c58-405a-b8c0-4af2da9c2efd' + } + end + + describe "#to_s" do + let(:access_token) {Lti::Oauth2::AccessToken.create_jwt(aud: aud, sub: sub)} + + it "is signed by the canvas secret" do + expect{Canvas::Security.decode_jwt(access_token.to_s)}.to_not raise_error + end + + it "has an 'iss' set to 'Canvas'" do + expect(Canvas::Security.decode_jwt(access_token.to_s)['iss']).to eq('Canvas') + end + + it "has an 'aud' set to the current domain" do + expect(Canvas::Security.decode_jwt(access_token.to_s)['aud']).to eq aud + end + + it "has an 'exp' that is derived from the settings" do + Timecop.freeze do + Setting.set('lti.oauth2.access_token.exp', 2.hours) + expect(Canvas::Security.decode_jwt(access_token.to_s)['exp']).to eq 2.hours.from_now.to_i + end + end + + it "has a default 'exp' of 1 hour" do + Timecop.freeze do + expect(Canvas::Security.decode_jwt(access_token.to_s)['exp']).to eq 1.hours.from_now.to_i + end + end + + it "has an 'iat' set to the current time" do + Timecop.freeze do + expect(Canvas::Security.decode_jwt(access_token.to_s)['iat']).to eq Time.zone.now.to_i + end + end + + it "has a 'nbf' derived from the settings" do + Timecop.freeze do + Setting.set('lti.oauth2.access_token.nbf', 2.minutes) + expect(Canvas::Security.decode_jwt(access_token.to_s)['nbf']).to eq 2.minutes.ago.to_i + end + end + + it "has a default 'nbf' 30 seconds ago" do + Timecop.freeze do + expect(Canvas::Security.decode_jwt(access_token.to_s)['nbf']).to eq 30.seconds.ago.to_i + end + end + + it "has a 'jti' that is uniquely generated" do + jti_1 = Canvas::Security.decode_jwt(access_token.to_s)['jti'] + jti_2 = Canvas::Security.decode_jwt(AccessToken.create_jwt(aud: aud, sub: sub).to_s)['jti'] + expect(jti_1).not_to eq jti_2 + end + + it "memoizes the jwt" do + expect(access_token.to_s).to eq access_token.to_s + end + + it "has a 'sub' that is set to the ToolProxy guid" do + expect(Canvas::Security.decode_jwt(access_token.to_s)['sub']).to eq sub + end + + end + + describe ".from_jwt" do + it "raises an InvalidTokenError if not signed by the correct secret" do + invalid_token = Canvas::Security.create_jwt(body, nil, 'invalid') + expect{ Lti::Oauth2::AccessToken.from_jwt(aud: aud, jwt: invalid_token)}.to raise_error InvalidTokenError + end + end + + describe "#validate!" do + let(:token) {Canvas::Security.create_jwt(body)} + let(:access_token) {Lti::Oauth2::AccessToken.from_jwt(aud: aud, jwt: token)} + + it "returns true if there are no errors" do + expect(access_token.validate!).to eq true + end + + it "raises InvalidTokenError if any of the assertions are missing" do + body.delete :jti + expect { access_token.validate! }.to raise_error InvalidTokenError, "the following assertions are missing: jti" + end + + it "raises an InvalidTokenError if 'iss' is not 'Canvas'" do + body[:iss] = 'invalid iss' + expect{ access_token.validate! }.to raise_error InvalidTokenError, 'invalid iss' + end + + it "raises an InvalidTokenError if the 'exp' is in the past" do + body[:exp] = 1.hour.ago + expect{ access_token.validate! }.to raise_error InvalidTokenError, 'token has expired' + end + + it "raises an InvalidTokenError if the 'aud' is different than the passed in 'aud'" do + body[:aud] = 'invalid aud' + expect{ access_token.validate! }.to raise_error InvalidTokenError, 'invalid aud' + end + + it "raises an InvalidTokenError if the 'iat' is in the future" do + body[:iat] = 1.hour.from_now + expect{ access_token.validate! }.to raise_error InvalidTokenError, 'iat must be in the past' + end + + it "raises an InvalidTokenError if the 'nbf' is in the future" do + body[:nbf] = 1.hour.from_now + expect{ access_token.validate! }.to raise_error InvalidTokenError + end + + end + + end + end +end