diff --git a/app/controllers/api/v1/apps_controller.rb b/app/controllers/api/v1/apps_controller.rb index 068d57aae57..fbfae2937a0 100644 --- a/app/controllers/api/v1/apps_controller.rb +++ b/app/controllers/api/v1/apps_controller.rb @@ -2,6 +2,9 @@ class Api::V1::AppsController < Api::BaseController skip_before_action :require_authenticated_user! + before_action :validate_token_endpoint_auth_method! + + TOKEN_ENDPOINT_AUTH_METHODS = %w(none client_secret_basic client_secret_post).freeze def create @app = Doorkeeper::Application.create!(application_options) @@ -16,12 +19,18 @@ class Api::V1::AppsController < Api::BaseController redirect_uri: app_params[:redirect_uris], scopes: app_scopes_or_default, website: app_params[:website], - confidential: app_confidential?, + confidential: !app_public?, } end - def app_confidential? - !app_params[:token_endpoint_auth_method] || app_params[:token_endpoint_auth_method] != 'none' + def validate_token_endpoint_auth_method! + return unless app_params.include? :token_endpoint_auth_method + + bad_request unless TOKEN_ENDPOINT_AUTH_METHODS.include? app_params[:token_endpoint_auth_method] + end + + def app_public? + app_params[:token_endpoint_auth_method] == 'none' end def app_scopes_or_default diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index b427a2f34ab..d7084647ff7 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -129,7 +129,9 @@ Doorkeeper.configure do # By default it retrieves first from the `HTTP_AUTHORIZATION` header, then # falls back to the `:client_id` and `:client_secret` params from the `params` object. # Check out the wiki for more information on customization - # client_credentials :from_basic, :from_params + # + # This is the default value: + client_credentials :from_basic, :from_params # Change the way access token is authenticated from the request object. # By default it retrieves first from the `HTTP_AUTHORIZATION` header, then diff --git a/spec/requests/api/v1/apps_spec.rb b/spec/requests/api/v1/apps_spec.rb index 3120ab9c646..c54a3710605 100644 --- a/spec/requests/api/v1/apps_spec.rb +++ b/spec/requests/api/v1/apps_spec.rb @@ -36,6 +36,7 @@ RSpec.describe 'Apps' do expect(app).to be_present expect(app.scopes.to_s).to eq scopes expect(app.redirect_uris).to eq redirect_uris + expect(app.confidential).to be true expect(response.parsed_body).to match( a_hash_including( @@ -55,6 +56,76 @@ RSpec.describe 'Apps' do end end + context 'without being a confidential application' do + let(:client_name) { 'Test confidential app' } + let(:params) do + { + client_name: client_name, + redirect_uris: redirect_uris, + scopes: scopes, + website: website, + token_endpoint_auth_method: 'none', + } + end + + it 'creates an public OAuth app', :aggregate_failures do + subject + + expect(response).to have_http_status(200) + expect(response.content_type) + .to start_with('application/json') + + app = Doorkeeper::Application.find_by(name: client_name) + + expect(app).to be_present + expect(app.scopes.to_s).to eq scopes + expect(app.redirect_uris).to eq redirect_uris + expect(app.confidential).to be false + + expect(response.parsed_body).to match( + a_hash_including( + id: app.id.to_s, + client_id: app.uid, + client_secret: nil, + client_secret_expires_at: 0, + name: client_name, + website: website, + scopes: ['read', 'write'], + redirect_uris: redirect_uris, + # Deprecated properties as of 4.3: + redirect_uri: redirect_uri, + vapid_key: Rails.configuration.x.vapid_public_key + ) + ) + end + end + + context 'when token_endpoint_auth_method is unknown' do + let(:client_name) { 'Test unknown auth app' } + let(:params) do + { + client_name: client_name, + redirect_uris: redirect_uris, + scopes: scopes, + website: website, + # Not yet supported: + token_endpoint_auth_method: 'private_key_jwt', + } + end + + it 'does not create an OAuth app', :aggregate_failures do + subject + + expect(response).to have_http_status(400) + expect(response.content_type) + .to start_with('application/json') + + app = Doorkeeper::Application.find_by(name: client_name) + + expect(app).to_not be_present + end + end + context 'without scopes being supplied' do let(:scopes) { nil }