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 pkce_code_challenge_methods option #1735

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
22 changes: 12 additions & 10 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
7 changes: 7 additions & 0 deletions lib/doorkeeper/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions lib/doorkeeper/config/validations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion lib/doorkeeper/oauth/pre_authorization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
22 changes: 22 additions & 0 deletions spec/lib/oauth/authorization_code_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 22 additions & 0 deletions spec/lib/oauth/pre_authorization_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down