diff --git a/Dockerfile b/Dockerfile index 6949da3fb..d1360160c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -28,21 +28,23 @@ ENV LANG=en_US.UTF-8 ENV LANGUAGE=en_US:en ENV LC_ALL=en_US.UTF-8 -ENV BUNDLER_VERSION=2.5.11 -RUN gem install bundler -v ${BUNDLER_VERSION} -i /usr/local/lib/ruby/gems/$(ls /usr/local/lib/ruby/gems) --force - WORKDIR /srv -COPY Gemfile doorkeeper.gemspec /srv/ -COPY lib/doorkeeper/version.rb /srv/lib/doorkeeper/version.rb - -RUN bundle install - COPY . /srv/ -RUN chown -R doorkeeper:doorkeeper /srv/coverage +RUN chown -R doorkeeper:doorkeeper /srv # Set the running user for resulting container USER doorkeeper -CMD ["rake"] +RUN mkdir -p /srv/.local/gem/share +ENV GEM_HOME=/srv/.local/gem/share + +ENV BUNDLER_VERSION=2.5.11 +RUN gem install bundler -v ${BUNDLER_VERSION} + +# This is a fix for sqlite alpine issues +RUN bundle config force_ruby_platform true +RUN bundle install + +CMD ["bundle", "exec", "rake"] diff --git a/Gemfile b/Gemfile index 346bc323d..03399e08e 100644 --- a/Gemfile +++ b/Gemfile @@ -27,3 +27,8 @@ gem "sqlite3", "~> 1.4", platform: [:ruby, :mswin, :mingw, :x64_mingw] gem "tzinfo-data", platforms: %i[mingw mswin x64_mingw] gem "timecop" + +gem 'irb', '~> 1.8' + +# Interactive Debugging tools +gem 'debug', '~> 1.8' diff --git a/app/controllers/doorkeeper/discovery_controller.rb b/app/controllers/doorkeeper/discovery_controller.rb new file mode 100644 index 000000000..d58cd9fbb --- /dev/null +++ b/app/controllers/doorkeeper/discovery_controller.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Doorkeeper + class DiscoveryController < Doorkeeper::ApplicationMetalController + def show + render json: {}, status: 200 + end + end +end diff --git a/bin/console b/bin/console index 019ffb0df..df716fecf 100755 --- a/bin/console +++ b/bin/console @@ -4,15 +4,10 @@ require "bundler/setup" require "rails/all" require "active_support/all" +require "irb" +require "debug" require "doorkeeper" -# You can add fixtures and/or initialization code here to make experimenting -# with your gem easier. You can also use a different console, if you like. - -# (If you use this, don't forget to add pry to your Gemfile!) -# require "pry" -# Pry.start - Rails.logger = Logger.new(STDOUT) Rails.logger.info("Doorkeeper version: #{Doorkeeper::VERSION::STRING}") @@ -32,5 +27,4 @@ ActiveRecord::Base.establish_connection( # Load database schema load File.expand_path("../spec/dummy/db/schema.rb", __dir__) -require "irb" IRB.start(__FILE__) diff --git a/lib/doorkeeper/config.rb b/lib/doorkeeper/config.rb index d8d8ce88a..eaa226c75 100644 --- a/lib/doorkeeper/config.rb +++ b/lib/doorkeeper/config.rb @@ -243,6 +243,7 @@ def configure_secrets_for(type, using:, fallback:) option :orm, default: :active_record option :native_redirect_uri, default: "urn:ietf:wg:oauth:2.0:oob", deprecated: true option :grant_flows, default: %w[authorization_code client_credentials] + option :pkce_code_challenge_methods, default: %w[plain S256] option :handle_auth_errors, default: :render option :token_lookup_batch_size, default: 10_000 # Sets the token_reuse_limit @@ -554,6 +555,12 @@ def scopes_by_grant_type @scopes_by_grant_type ||= {} end + def pkce_code_challenge_methods_supported + return [] unless access_grant_model.pkce_supported? + + pkce_code_challenge_methods + end + def client_credentials_methods @client_credentials_methods ||= %i[from_basic from_params] end diff --git a/lib/doorkeeper/config/validations.rb b/lib/doorkeeper/config/validations.rb index bf9760014..d7ee68659 100644 --- a/lib/doorkeeper/config/validations.rb +++ b/lib/doorkeeper/config/validations.rb @@ -11,6 +11,7 @@ def validate! validate_reuse_access_token_value validate_token_reuse_limit validate_secret_strategies + validate_pkce_code_challenge_methods end private @@ -48,6 +49,17 @@ def validate_token_reuse_limit ) @token_reuse_limit = 100 end + + def validate_pkce_code_challenge_methods + return if pkce_code_challenge_methods.all? {|method| method =~ /^plain$|^S256$/ } + + ::Rails.logger.warn( + "[DOORKEEPER] You have configured an invalid value for pkce_code_challenge_methods option. " \ + "It will be set to default ['plain', 'S256']", + ) + + @pkce_code_challenge_methods = ['plain', 'S256'] + end end end end diff --git a/lib/doorkeeper/oauth/pre_authorization.rb b/lib/doorkeeper/oauth/pre_authorization.rb index 88f744bb4..050bc2d4f 100644 --- a/lib/doorkeeper/oauth/pre_authorization.rb +++ b/lib/doorkeeper/oauth/pre_authorization.rb @@ -154,7 +154,7 @@ def validate_code_challenge_method return true unless Doorkeeper.config.access_grant_model.pkce_supported? code_challenge.blank? || - (code_challenge_method.present? && code_challenge_method =~ /^plain$|^S256$/) + (code_challenge_method.present? && Doorkeeper.config.pkce_code_challenge_methods_supported.include?(code_challenge_method)) end def response_on_fragment? diff --git a/lib/doorkeeper/rails/routes.rb b/lib/doorkeeper/rails/routes.rb index 1a2bf7c48..3d0cf58d4 100644 --- a/lib/doorkeeper/rails/routes.rb +++ b/lib/doorkeeper/rails/routes.rb @@ -41,10 +41,16 @@ def generate_routes!(options) map_route(:authorized_applications, :authorized_applications_routes) map_route(:token_info, :token_info_routes) end + + map_route(:discovery, :discovery_routes) end private + def discovery_routes(mapping) + routes.get ".well-known/oauth-authorization-server", controller: mapping[:controllers], action: :show + end + def authorization_routes(mapping) routes.resource( :authorization, diff --git a/lib/doorkeeper/rails/routes/mapping.rb b/lib/doorkeeper/rails/routes/mapping.rb index eac2f12b1..90e6d85cb 100644 --- a/lib/doorkeeper/rails/routes/mapping.rb +++ b/lib/doorkeeper/rails/routes/mapping.rb @@ -11,6 +11,7 @@ def initialize authorizations: "doorkeeper/authorizations", applications: "doorkeeper/applications", authorized_applications: "doorkeeper/authorized_applications", + discovery: "doorkeeper/discovery", tokens: "doorkeeper/tokens", token_info: "doorkeeper/token_info", } diff --git a/spec/dummy/app/controllers/custom_discovery_controller.rb b/spec/dummy/app/controllers/custom_discovery_controller.rb new file mode 100644 index 000000000..97a0a77b7 --- /dev/null +++ b/spec/dummy/app/controllers/custom_discovery_controller.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class CustomDiscoveryController < ::ApplicationController + def show + render nothing: true + end +end diff --git a/spec/lib/oauth/authorization_code_request_spec.rb b/spec/lib/oauth/authorization_code_request_spec.rb index ae5cf66ab..cca21867f 100644 --- a/spec/lib/oauth/authorization_code_request_spec.rb +++ b/spec/lib/oauth/authorization_code_request_spec.rb @@ -231,6 +231,28 @@ expect(request.error).to eq(Doorkeeper::Errors::InvalidGrant) end + + context "when PKCE code challenge methods is set to only S256" do + before do + Doorkeeper.configure do + pkce_code_challenge_methods ["S256"] + end + end + + it "validates when code_verifier is S256" do + params[:code_verifier] = grant.code_challenge = "S256" + request.validate + + expect(request.error).to eq(nil) + end + + it "invalidates when code_verifier is plain" do + params[:code_verifier] = "plain" + request.validate + + expect(request.error).to eq(Doorkeeper::Errors::InvalidGrant) + end + end end context "when PKCE is not supported" do diff --git a/spec/lib/oauth/pre_authorization_spec.rb b/spec/lib/oauth/pre_authorization_spec.rb index 9aa561ffd..3ee631a4b 100644 --- a/spec/lib/oauth/pre_authorization_spec.rb +++ b/spec/lib/oauth/pre_authorization_spec.rb @@ -330,6 +330,28 @@ expect(pre_auth).not_to be_authorizable end + + context "when pkce_code_challenge_methods is set to only S256" do + before do + Doorkeeper.configure do + pkce_code_challenge_methods ["S256"] + end + end + + it "accepts S256 as a code_challenge_method" do + attributes[:code_challenge] = "a45a9fea-0676-477e-95b1-a40f72ac3cfb" + attributes[:code_challenge_method] = "S256" + + expect(pre_auth).to be_authorizable + end + + it "rejects plain as a code_challenge_method" do + attributes[:code_challenge] = "a45a9fea-0676-477e-95b1-a40f72ac3cfb" + attributes[:code_challenge_method] = "plain" + + expect(pre_auth).to_not be_authorizable + end + end end context "when PKCE is not supported" do diff --git a/spec/requests/endpoints/discovery_spec.rb b/spec/requests/endpoints/discovery_spec.rb new file mode 100644 index 000000000..295d011b1 --- /dev/null +++ b/spec/requests/endpoints/discovery_spec.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe "Discovery endpoint" do + it "returns json" do + get "/.well-known/oauth-authorization-server" + + response_status_should_be(200) + + # WIP: currently empty + expect(json_response).to eq({}) + end +end diff --git a/spec/routing/custom_controller_routes_spec.rb b/spec/routing/custom_controller_routes_spec.rb index 48788f532..8cd2a9f0e 100644 --- a/spec/routing/custom_controller_routes_spec.rb +++ b/spec/routing/custom_controller_routes_spec.rb @@ -16,7 +16,8 @@ controllers authorizations: "custom_authorizations", tokens: "custom_authorizations", applications: "custom_authorizations", - token_info: "custom_authorizations" + token_info: "custom_authorizations", + discovery: "custom_discovery" as authorizations: "custom_auth", tokens: "custom_token", @@ -29,7 +30,8 @@ controllers authorizations: "custom_authorizations", tokens: "custom_authorizations", applications: "custom_authorizations", - token_info: "custom_authorizations" + token_info: "custom_authorizations", + discovery: "custom_discovery" as authorizations: "custom_auth", tokens: "custom_token", @@ -41,7 +43,8 @@ use_doorkeeper do controllers authorizations: "custom_authorizations", tokens: "custom_authorizations", - token_info: "custom_authorizations" + token_info: "custom_authorizations", + discovery: "custom_discovery" as authorizations: "custom_auth", tokens: "custom_token", @@ -83,6 +86,11 @@ expect(get("/inner_space/scope/token/info")).to route_to("custom_authorizations#show") end + it "GET /inner_space/.well-known/oauth-authorization-server route to show Discovery controller" do + expect(get("/space/.well-known/oauth-authorization-server")).to route_to("custom_discovery#show") + end + + it "GET /space/oauth/authorize routes to custom authorizations controller" do expect(get("/space/oauth/authorize")).to route_to("custom_authorizations#new") end @@ -115,6 +123,10 @@ expect(get("/space/oauth/token/info")).to route_to("custom_authorizations#show") end + it "GET /space/.well-known/oauth-authorization-server route to show Discovery controller" do + expect(get("/space/.well-known/oauth-authorization-server")).to route_to("custom_discovery#show") + end + it "POST /outer_space/oauth/token is not be routable" do expect(post("/outer_space/oauth/token")).not_to be_routable end @@ -130,4 +142,8 @@ it "GET /outer_space/oauth/token_info is not routable" do expect(get("/outer_space/oauth/token/info")).not_to be_routable end + + it "GET /outer_space/.well-known/oauth-authorization-server route to show Discovery controller" do + expect(get("/outer_space/.well-known/oauth-authorization-server")).to route_to("custom_discovery#show") + end end diff --git a/spec/routing/default_routes_spec.rb b/spec/routing/default_routes_spec.rb index 0793b5341..ca6f16f54 100644 --- a/spec/routing/default_routes_spec.rb +++ b/spec/routing/default_routes_spec.rb @@ -38,4 +38,8 @@ it "GET /oauth/token/info route to authorized TokenInfo controller" do expect(get("/oauth/token/info")).to route_to("doorkeeper/token_info#show") end + + it "GET /.well-known/oauth-authorization-server route to show Discovery controller" do + expect(get("/.well-known/oauth-authorization-server")).to route_to("doorkeeper/discovery#show") + end end diff --git a/spec/routing/scoped_routes_spec.rb b/spec/routing/scoped_routes_spec.rb index 3a0be6eb4..8f3584a6d 100644 --- a/spec/routing/scoped_routes_spec.rb +++ b/spec/routing/scoped_routes_spec.rb @@ -53,4 +53,8 @@ it "POST /scope/introspect routes not to exist" do expect(post("/scope/introspect")).not_to be_routable end + + it "GET /.well-known/oauth-authorization-server route to show Discovery controller" do + expect(get("/.well-known/oauth-authorization-server")).to route_to("doorkeeper/discovery#show") + end end