From b34116ebf0b1e503cb82cb07b2a6d99e71f70609 Mon Sep 17 00:00:00 2001 From: Dmitry Trager Date: Thu, 6 Apr 2017 18:03:33 +0300 Subject: [PATCH] # This is a combination of 3 commits. # The first commit's message is: Compare redirect_uri and grant uri without query when doing checks from authorization code request # This is the 2nd commit message: Fix xss by escaping tags content_tag body is correctly escaped when rendered even if called is wrapped by raw # This is the 3rd commit message: Fix CI review --- NEWS.md | 1 + lib/doorkeeper/oauth/authorization_code_request.rb | 5 ++++- spec/lib/oauth/authorization_code_request_spec.rb | 13 ++++++++++++- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index 3d5dca128..3adf15ada 100644 --- a/NEWS.md +++ b/NEWS.md @@ -5,6 +5,7 @@ User-visible changes worth mentioning. ## master - [#970] Escape certain attributes in authorization forms. +- [#974] Redirect URI is checked without query params within AuthorizationCodeRequest. ## 4.2.5 diff --git a/lib/doorkeeper/oauth/authorization_code_request.rb b/lib/doorkeeper/oauth/authorization_code_request.rb index 62d9ea99c..d6a7ebcd8 100644 --- a/lib/doorkeeper/oauth/authorization_code_request.rb +++ b/lib/doorkeeper/oauth/authorization_code_request.rb @@ -44,7 +44,10 @@ def validate_grant end def validate_redirect_uri - grant.redirect_uri == redirect_uri + Helpers::URIChecker.valid_for_authorization?( + redirect_uri, + grant.redirect_uri + ) end end end diff --git a/spec/lib/oauth/authorization_code_request_spec.rb b/spec/lib/oauth/authorization_code_request_spec.rb index 325c4b66a..6c0832238 100644 --- a/spec/lib/oauth/authorization_code_request_spec.rb +++ b/spec/lib/oauth/authorization_code_request_spec.rb @@ -10,9 +10,11 @@ module Doorkeeper::OAuth end let(:grant) { FactoryGirl.create :access_grant } let(:client) { grant.application } + let(:redirect_uri) { client.redirect_uri } + let(:params) { { redirect_uri: redirect_uri } } subject do - AuthorizationCodeRequest.new server, grant, client, redirect_uri: client.redirect_uri + AuthorizationCodeRequest.new server, grant, client, params end it 'issues a new token for the client' do @@ -76,5 +78,14 @@ module Doorkeeper::OAuth subject.authorize end.to_not change { Doorkeeper::AccessToken.count } end + + context "when redirect_uri contains some query params" do + let(:redirect_uri) { client.redirect_uri + "?query=q" } + + it "compares only host part with grant's redirect_uri" do + subject.validate + expect(subject.error).to eq(nil) + end + end end end