ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

provide for better error message on JSON.parse 

why: when client credentials is attempting to decode a jwt and the body
that has been returned is not JSON (ie. html) we were just returning
the same error as if the jwt signature was bad which was not helpful.

fixes INTEROP-8220
flag=none

test plan:
there has been a change pushed since this ticket started that causes a
return of "JWT signature invalid" instead of the html we were getting
but that is still not quite right, so these steps will show the old,
current, and new output.

- Make sure your 1.3 dev key uses a public jwk url instead of a
  public jwk (http://lti13testtool.docker/credential_id/1/public_jwk).
- In the test tool run `dcr web rake jwt:access_token CLIENT_ID=<YOUR_DEV_KEY>`
  Current master branch should error with 400 and "JWS signature invalid".
- If you remove the `JSON::ParserError` from the rescue on
  asymmetric_client_credentials_provider.rb:72, it will return an HTML
  error page (as shown in this ticket).
- When you cherry-pick this current change it will error with 400 and
  "Invalid JSON" which is the true error.

Change-Id: I940e21e9e596f59736c99d972d0a7868715dad11
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/327388
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
Product-Review: Alexis Nast <alexis.nast@instructure.com>
This commit is contained in:
Steve McGee 2023-09-11 17:33:42 -06:00 committed by Steve Mcgee
parent 5dca45178b
commit a4a2ec0beb
2 changed files with 19 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
lib/canvas/oauth/asymmetric_client_credentials_provider.rb
View File

@ -30,12 +30,14 @@ module Canvas::OAuth
end
def valid?
return false if @invalid_json
return false if @invalid_key
validator.valid?
end
def error_message
return "JWK Error: Invalid JSON" if @invalid_json
return "JWS signature invalid." if @invalid_key
return "JWK Error: #{errors.first.message}" if errors.present?
@ -69,7 +71,9 @@ module Canvas::OAuth
def get_jwk_from_url(jwt = nil)
pub_jwk_from_url = CanvasHttp.get(key.public_jwk_url)
JSON::JWT.decode(jwt, JSON::JWK::Set.new(JSON.parse(pub_jwk_from_url.body)))
rescue CanvasHttp::Error, EOFError, JSON::JWT::Exception, JSON::ParserError => e
rescue JSON::ParserError
@invalid_json = true
rescue CanvasHttp::Error, EOFError, JSON::JWT::Exception => e
errors << e
raise JSON::JWS::VerificationFailed
end

14
spec/lib/canvas/oauth/client_credentials_provider_spec.rb
View File

@ -137,6 +137,20 @@ module Canvas::OAuth
end
end
context "when invalid json is returned" do
let(:public_jwk_url_response) { "<html></html>" }
before do
dev_key.update!(public_jwk_url: url)
end
it do
expected_url_called(url, :get, stubbed_response)
expect(subject).to be false
expect(provider.error_message).to be("JWK Error: Invalid JSON")
end
end
context "when the url is not valid giving a 404" do
let(:stubbed_response) { instance_double(Net::HTTPNotFound, body: public_jwk_url_response) }