Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add config to enforce content type #1076

Merged
merged 1 commit into from
May 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ User-visible changes worth mentioning.
customized Token Info route).
- [#1086, #1088] Fix bug with receiving default scopes in the token even if they are
not present in the application scopes (use scopes intersection).
- [#1076] Add config to enforce content type to application/x-www-form-urlencoded

## 4.3.2

Expand Down
4 changes: 4 additions & 0 deletions app/controllers/doorkeeper/application_metal_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,17 @@ class ApplicationMetalController < ActionController::Metal
AbstractController::Rendering,
ActionController::Rendering,
ActionController::Renderers::All,
AbstractController::Callbacks,
Helpers::Controller
].freeze

MODULES.each do |mod|
include mod
end

before_action :enforce_content_type,
if: -> { Doorkeeper.configuration.enforce_content_type }

ActiveSupport.run_load_hooks(:doorkeeper_metal_controller, self)
end
end
7 changes: 7 additions & 0 deletions lib/doorkeeper/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,12 @@ def api_only
def enforce_configured_scopes
@config.instance_variable_set(:@enforce_configured_scopes, true)
end

# Enforce request content type as the spec requires:
# disabled by default for backward compatibility.
def enforce_content_type
@config.instance_variable_set(:@enforce_content_type, true)
end
end

module Option
Expand Down Expand Up @@ -284,6 +290,7 @@ def option(name, options = {})

attr_reader :reuse_access_token
attr_reader :api_only
attr_reader :enforce_content_type

def refresh_token_enabled?
defined?(@refresh_token_enabled) && @refresh_token_enabled
Expand Down
5 changes: 5 additions & 0 deletions lib/doorkeeper/helpers/controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ def handle_token_exception(exception)
def skip_authorization?
!!instance_exec([@server.current_resource_owner, @pre_auth.client], &Doorkeeper.configuration.skip_authorization)
end

def enforce_content_type
return if request.content_type == 'application/x-www-form-urlencoded'
render json: {}, status: :unsupported_media_type
end
end
end
end
5 changes: 5 additions & 0 deletions lib/generators/doorkeeper/templates/initializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@
#
# api_only

# Enforce token request content type to application/x-www-form-urlencoded.
# It is not enabled by default to not break prior versions of the gem.
#
# enforce_content_type

# Authorization Code expiration time (default 10 minutes).
#
# authorization_code_expires_in 10.minutes
Expand Down
10 changes: 0 additions & 10 deletions spec/controllers/application_metal_controller.rb

This file was deleted.

50 changes: 50 additions & 0 deletions spec/controllers/application_metal_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# frozen_string_literal: true

require 'spec_helper_integration'

describe Doorkeeper::ApplicationMetalController do
controller(Doorkeeper::ApplicationMetalController) do
def index
render json: {}, status: 200
end
end

it "lazy run hooks" do
i = 0
ActiveSupport.on_load(:doorkeeper_metal_controller) { i += 1 }

expect(i).to eq 1
end

describe 'enforce_content_type' do
before { allow(Doorkeeper.configuration).to receive(:enforce_content_type).and_return(flag) }

context 'enabled' do
let(:flag) { true }

it '200 for the correct media type' do
get :index, params: {}, as: :url_encoded_form
expect(response).to have_http_status 200
end

it 'returns a 415 for an incorrect media type' do
get :index, as: :json
expect(response).to have_http_status 415
end
end

context 'disabled' do
let(:flag) { false }

it 'returns a 200 for the correct media type' do
get :index, as: :url_encoded_form
expect(response).to have_http_status 200
end

it 'returns a 200 for an incorrect media type' do
get :index, as: :json
expect(response).to have_http_status 200
end
end
end
end
43 changes: 29 additions & 14 deletions spec/controllers/applications_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,13 @@ module Doorkeeper

it 'does not create application' do
expect do
post :create, doorkeeper_application: {
name: 'Example',
redirect_uri: 'https://example.com' }
post :create,
params: {
doorkeeper_application: {
name: 'Example',
redirect_uri: 'https://example.com'
}
}
end.not_to change { Doorkeeper::Application.count }
end
end
Expand All @@ -43,30 +47,41 @@ module Doorkeeper

it 'creates application' do
expect do
post :create, doorkeeper_application: {
name: 'Example',
redirect_uri: 'https://example.com' }
post :create,
params: {
doorkeeper_application: {
name: 'Example',
redirect_uri: 'https://example.com'
}
}
end.to change { Doorkeeper::Application.count }.by(1)

expect(response).to be_redirect
end

it 'does not allow mass assignment of uid or secret' do
application = FactoryBot.create(:application)
put :update, id: application.id, doorkeeper_application: {
uid: '1A2B3C4D',
secret: '1A2B3C4D'
}
put :update,
params: {
id: application.id,
doorkeeper_application: {
uid: '1A2B3C4D',
secret: '1A2B3C4D'
}
}

expect(application.reload.uid).not_to eq '1A2B3C4D'
end

it 'updates application' do
application = FactoryBot.create(:application)
put :update, id: application.id, doorkeeper_application: {
name: 'Example',
redirect_uri: 'https://example.com'
}
put :update,
params: {
id: application.id, doorkeeper_application: {
name: 'Example',
redirect_uri: 'https://example.com'
}
}

expect(application.reload.name).to eq 'Example'
end
Expand Down
28 changes: 14 additions & 14 deletions spec/controllers/authorizations_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def translated_error_message(key)

describe 'POST #create' do
before do
post :create, client_id: client.uid, response_type: 'token', redirect_uri: client.redirect_uri
post :create, params: { client_id: client.uid, response_type: 'token', redirect_uri: client.redirect_uri }
end

it 'redirects after authorization' do
Expand Down Expand Up @@ -76,7 +76,7 @@ def translated_error_message(key)
describe "POST #create in API mode" do
before do
allow(Doorkeeper.configuration).to receive(:api_only).and_return(true)
post :create, client_id: client.uid, response_type: "token", redirect_uri: client.redirect_uri
post :create, params: { client_id: client.uid, response_type: "token", redirect_uri: client.redirect_uri }
end

let(:response_json_body) { JSON.parse(response.body) }
Expand Down Expand Up @@ -114,7 +114,7 @@ def translated_error_message(key)
describe 'POST #create with errors' do
before do
default_scopes_exist :public
post :create, client_id: client.uid, response_type: 'token', scope: 'invalid', redirect_uri: client.redirect_uri
post :create, params: { client_id: client.uid, response_type: 'token', scope: 'invalid', redirect_uri: client.redirect_uri }
end

it 'redirects after authorization' do
Expand Down Expand Up @@ -146,7 +146,7 @@ def translated_error_message(key)
before do
allow(Doorkeeper.configuration).to receive(:api_only).and_return(true)
default_scopes_exist :public
post :create, client_id: client.uid, response_type: 'token', scope: 'invalid', redirect_uri: client.redirect_uri
post :create, params: { client_id: client.uid, response_type: 'token', scope: 'invalid', redirect_uri: client.redirect_uri }
end

let(:response_json_body) { JSON.parse(response.body) }
Expand Down Expand Up @@ -182,7 +182,7 @@ def translated_error_message(key)
allow(Doorkeeper.configuration).to receive(:reuse_access_token).and_return(true)

access_token.save!
post :create, client_id: client.uid, response_type: 'token', redirect_uri: client.redirect_uri
post :create, params: { client_id: client.uid, response_type: 'token', redirect_uri: client.redirect_uri }
end

it 'returns the existing access token in a fragment' do
Expand All @@ -201,7 +201,7 @@ def translated_error_message(key)

describe 'when successful' do
after do
post :create, client_id: client.uid, response_type: 'token', redirect_uri: client.redirect_uri
post :create, params: { client_id: client.uid, response_type: 'token', redirect_uri: client.redirect_uri }
end

it 'should call :before_successful_authorization callback' do
Expand All @@ -215,7 +215,7 @@ def translated_error_message(key)

describe 'with errors' do
after do
post :create, client_id: client.uid, response_type: 'token', redirect_uri: 'bad_uri'
post :create, params: { client_id: client.uid, response_type: 'token', redirect_uri: 'bad_uri' }
end

it 'should not call :before_successful_authorization callback' do
Expand All @@ -234,7 +234,7 @@ def translated_error_message(key)
true
end)
client.update_attribute :redirect_uri, 'urn:ietf:wg:oauth:2.0:oob'
get :new, client_id: client.uid, response_type: 'token', redirect_uri: client.redirect_uri
get :new, params: { client_id: client.uid, response_type: 'token', redirect_uri: client.redirect_uri }
end

it 'should redirect immediately' do
Expand All @@ -258,7 +258,7 @@ def translated_error_message(key)
true
end)
client.update_attribute :redirect_uri, 'urn:ietf:wg:oauth:2.0:oob'
get :new, client_id: client.uid, response_type: 'code', redirect_uri: client.redirect_uri
get :new, params: { client_id: client.uid, response_type: 'code', redirect_uri: client.redirect_uri }
end

it 'should redirect immediately' do
Expand All @@ -280,7 +280,7 @@ def translated_error_message(key)
allow(Doorkeeper.configuration).to receive(:skip_authorization).and_return(proc do
true
end)
get :new, client_id: client.uid, response_type: 'token', redirect_uri: client.redirect_uri
get :new, params: { client_id: client.uid, response_type: 'token', redirect_uri: client.redirect_uri }
end

it 'should redirect immediately' do
Expand Down Expand Up @@ -312,7 +312,7 @@ def translated_error_message(key)
describe 'GET #new in API mode' do
before do
allow(Doorkeeper.configuration).to receive(:api_only).and_return(true)
get :new, client_id: client.uid, response_type: 'token', redirect_uri: client.redirect_uri
get :new, params: { client_id: client.uid, response_type: 'token', redirect_uri: client.redirect_uri }
end

it 'should render success' do
Expand All @@ -337,7 +337,7 @@ def translated_error_message(key)
allow(Doorkeeper.configuration).to receive(:skip_authorization).and_return(proc { true })
allow(Doorkeeper.configuration).to receive(:api_only).and_return(true)

get :new, client_id: client.uid, response_type: 'token', redirect_uri: client.redirect_uri
get :new, params: { client_id: client.uid, response_type: 'token', redirect_uri: client.redirect_uri }
end

it 'should render success' do
Expand Down Expand Up @@ -374,7 +374,7 @@ def translated_error_message(key)
describe 'GET #new with errors' do
before do
default_scopes_exist :public
get :new, an_invalid: 'request'
get :new, params: { an_invalid: 'request' }
end

it 'does not redirect' do
Expand All @@ -390,7 +390,7 @@ def translated_error_message(key)
describe 'GET #new with callbacks' do
after do
client.update_attribute :redirect_uri, 'urn:ietf:wg:oauth:2.0:oob'
get :new, client_id: client.uid, response_type: 'token', redirect_uri: client.redirect_uri
get :new, params: { client_id: client.uid, response_type: 'token', redirect_uri: client.redirect_uri }
end

describe 'when authorizing' do
Expand Down
Loading