cst1/openstreetmap-website
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge remote-tracking branch 'upstream/pull/4226'

Browse source
This commit is contained in:
Tom Hughes 2023-10-03 19:39:41 +01:00
commit c8fc2218e5
15 changed files with 289 additions and 13 deletions
Download patch file Download diff file Expand all files Collapse all files

2
Gemfile
View file

@ -81,6 +81,7 @@ gem "omniauth-rails_csrf_protection", "~> 1.0"
# Doorkeeper for OAuth2
gem "doorkeeper"
gem "doorkeeper-i18n"
gem "doorkeeper-openid_connect"
# Markdown formatting support
gem "kramdown"
@ -150,6 +151,7 @@ group :test do
gem "capybara", ">= 2.15"
gem "erb_lint", :require => false
gem "factory_bot_rails"
gem "jwt"
gem "minitest", "~> 5.1"
gem "puma", "~> 5.6"
gem "rails-controller-testing"

5
Gemfile.lock
View file

@ -179,6 +179,9 @@ GEM
railties (>= 5)
doorkeeper-i18n (5.2.7)
doorkeeper (>= 5.2)
doorkeeper-openid_connect (1.8.7)
doorkeeper (>= 5.5, < 5.7)
jwt (>= 2.5)
dry-configurable (1.1.0)
dry-core (~> 1.0, < 2)
zeitwerk (~> 2.6)
@ -577,6 +580,7 @@ DEPENDENCIES
delayed_job_active_record
doorkeeper
doorkeeper-i18n
doorkeeper-openid_connect
erb_lint
factory_bot_rails
faraday
@ -592,6 +596,7 @@ DEPENDENCIES
jbuilder (~> 2.7)
jquery-rails
json
jwt
kgio
kramdown
libxml-ruby (>= 2.0.5)

2
app/views/oauth2_applications/_form.html.erb
View file

@ -3,5 +3,5 @@
<%= f.form_group :confidential do %>
<%= f.check_box :confidential %>
<% end %>
<%= f.collection_check_boxes :scopes, Oauth.scopes(:privileged => current_user.administrator?), :name, :description %>
<%= f.collection_check_boxes :scopes, Oauth.scopes(:oauth2 => true, :privileged => current_user.administrator?), :name, :description %>
<%= f.primary %>

2
app/views/oauth2_authorizations/new.html.erb
View file

@ -18,6 +18,7 @@
<%= f.hidden_field :state, :value => @pre_auth.state %>
<%= f.hidden_field :response_type, :value => @pre_auth.response_type %>
<%= f.hidden_field :scope, :value => @pre_auth.scope %>
<%= f.hidden_field :nonce, :value => @pre_auth.nonce %>
<%= f.hidden_field :code_challenge, :value => @pre_auth.code_challenge %>
<%= f.hidden_field :code_challenge_method, :value => @pre_auth.code_challenge_method %>
<%= f.primary t(".authorize") %>
@ -30,6 +31,7 @@
<%= f.hidden_field :state, :value => @pre_auth.state %>
<%= f.hidden_field :response_type, :value => @pre_auth.response_type %>
<%= f.hidden_field :scope, :value => @pre_auth.scope %>
<%= f.hidden_field :nonce, :value => @pre_auth.nonce %>
<%= f.hidden_field :code_challenge, :value => @pre_auth.code_challenge %>
<%= f.hidden_field :code_challenge_method, :value => @pre_auth.code_challenge_method %>
<%= f.submit t(".deny") %>

2
config/initializers/doorkeeper.rb
View file

@ -225,7 +225,7 @@ Doorkeeper.configure do
# https://doorkeeper.gitbook.io/guides/ruby-on-rails/scopes
# default_scopes :public
optional_scopes(*Oauth::SCOPES, *Oauth::PRIVILEGED_SCOPES)
optional_scopes(*Oauth::SCOPES, *Oauth::PRIVILEGED_SCOPES, *Oauth::OAUTH2_SCOPES)
# Allows to restrict only certain scopes for grant_type.
# By default, all the scopes will be available for all the grant types.

38
config/initializers/doorkeeper_openid_connect.rb Normal file
View file

@ -0,0 +1,38 @@
# frozen_string_literal: true
Doorkeeper::OpenidConnect.configure do
issuer do |_resource_owner, _application|
"#{Settings.server_protocol}://#{Settings.server_url}"
end
signing_key Settings.doorkeeper_signing_key
subject_types_supported [:public]
resource_owner_from_access_token do |access_token|
User.find_by(:id => access_token.resource_owner_id)
end
auth_time_from_resource_owner do |resource_owner|
# empty block necessary as a workaround to missing configuration
# when no auth_time claim is provided
end
subject do |resource_owner, _application|
resource_owner.id
end
protocol do
Settings.server_protocol.to_sym
end
claims do
claim :preferred_username, :scope => :openid do |resource_owner, _scopes, _access_token|
resource_owner.display_name
end
claim :email, :scope => :read_email, :response => [:id_token, :user_info] do |resource_owner, _scopes, _access_token|
resource_owner.email
end
end
end

22
config/locales/en.yml
View file

@ -559,10 +559,31 @@ en:
newer_comments: "Newer Comments"
older_comments: "Older Comments"
doorkeeper:
errors:
messages:
account_selection_required: "The authorization server requires end-user account selection"
consent_required: "The authorization server requires end-user consent"
interaction_required: "The authorization server requires end-user interaction"
login_required: "The authorization server requires end-user authentication"
flash:
applications:
create:
notice: Application Registered.
openid_connect:
errors:
messages:
# Configuration error messages
auth_time_from_resource_owner_not_configured: "Failure due to Doorkeeper::OpenidConnect.configure.auth_time_from_resource_owner missing configuration."
reauthenticate_resource_owner_not_configured: "Failure due to Doorkeeper::OpenidConnect.configure.reauthenticate_resource_owner missing configuration."
resource_owner_from_access_token_not_configured: "Failure due to Doorkeeper::OpenidConnect.configure.resource_owner_from_access_token missing configuration."
select_account_for_resource_owner_not_configured: "Failure due to Doorkeeper::OpenidConnect.configure.select_account_for_resource_owner missing configuration."
subject_not_configured: "ID Token generation failed due to Doorkeeper::OpenidConnect.configure.subject missing configuration."
scopes:
address: "View your physical address"
email: "View your email address"
openid: "Authenticate your account"
phone: "View your phone number"
profile: "View your profile information"
errors:
contact:
contact_url: https://wiki.openstreetmap.org/wiki/Contact
@ -2530,6 +2551,7 @@ en:
permissions:
missing: "You have not permitted the application access to this facility"
scopes:
openid: Sign-in using OpenStreetMap
read_prefs: Read user preferences
write_prefs: Modify user preferences
write_diary: Create diary entries, comments and make friends

2
config/routes.rb
View file

@ -5,6 +5,8 @@ OpenStreetMap::Application.routes.draw do
:authorized_applications => "oauth2_authorized_applications"
end
use_doorkeeper_openid_connect :scope => "oauth2" if Settings.key?(:doorkeeper_signing_key)
# API
namespace :api do
get "capabilities" => "capabilities#show" # Deprecated, remove when 0.6 support is removed

5
config/settings.yml
View file

@ -154,3 +154,8 @@ smtp_password: null
#signup_ip_max_burst:
#signup_email_per_day:
#signup_email_max_burst:
# Private key for signing id_tokens
#doorkeeper_signing_key: |
# -----BEGIN PRIVATE KEY-----
# ...
# -----END PRIVATE KEY-----

30
config/settings/test.yml
View file

@ -22,3 +22,33 @@ trace_icon_storage: "test"
# Lower some rate limits for testing
max_changeset_comments_per_hour: 30
moderator_changeset_comments_per_hour: 60
# Private key for signing id_tokens
doorkeeper_signing_key: |
-----BEGIN PRIVATE KEY-----
MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDA4pSRHIicerJQ
BvIy9lGJ6ZQA7SAGVM8QeNMBaQftS+ROMY+6CFCJ0kiwb9oDdtFNyo3gpgmlULMC
q0C96r0UllKnTSkHSntkKM0wy3TX0pa8QBaJbbcOXU13xu5cR7ffvtn2kQX8RZc9
eZtE/bSybNBDSiS4mbP31cSQ71EYsbfD3UiWOpOTbc6Xlw9kCkUjwXk36Jnim7gF
1kFjD3Vq94ij4OVNxcFp+btrfhq2tsiXa9IPBlt1xTetHwj5HrxseDu2qNQNgPxY
ivFAA3t4BuIuou1HjAdzfqp7Ylsr2b7qx+w+Y9TqhH6AcKd0i1qxh6bYnJezH9JU
BjlJvJMhAgMBAAECggEAAID2/gldiqRqunkc1n48iJ2CufKPRAT3r3rT+OvNzf8F
6csJAuWKVE8ndR0trBb6L/eooYloJWA4aiLes0BIMyQQs4go5HB7hwTw7ZYycsKF
i0NS676iHO2odKN2iZN/CvIO1AKH9KM35GdgvPA5XG1FU/pUbeOqNn+pQ5mkaWnt
kt+ndBpJQDPSS7nTY8g2BCh97SJSbxEPAccAqNLSvKQED4QVygC63jYZNPDxkJWI
guzNn4wv1AfM0DU4W5fI0UtNSxcWSsefWBJTOKO/uQr/XJglxVh6uKof1dnBZiJD
KU6/+bR1cXoKQ05HAcEcf/mtjJGwnze41p1EI22gYQKBgQDB+VZJwvxlME1MgEGJ
WFPPKiQspKjS0kgbfBw7Iny+mYM6YLpQyF0NFNRloALW2rHH2QLNSerHMlytZUAd
1SluQZ4We6P3hLDi2J3p37lkIdBXhjJi8gdoEfQ1YVcCbPGbR2ZVwYms7BP3yiQY
ZLcHLUKPKG6hOZztY1gBYqoKqQKBgQD+kBtR8krdJHPEU3m+d/6NWlGk4KZgCFx5
ouN/aHtxE6Ge+mUwbrJun/oVrFjbX7ySYTYYb6SdKUrchyKfJL4Z89WHGwrFTV1/
6J2ShXmoeUeic1TS4btcnFmZyCXlADk1eyHZm9wtkwd5e2lBfdRxzErKC42lWdaQ
rreP2nZHuQKBgQCiNbVgB6vznrn1kIe9qFylsJMBtkzryCe+vEILfaKd7VhdOEh2
h6ew6ctYlL/rFoV3H1YFgJvSKp5v7mz4xapY5oyiNpD+yzr06LrdulaZkuFcX//A
2K8y61iyTw1pHNvKw6Gjcy6DqgRkwej/cTHR0ZqIhwJE1x4RMnOE7RJPyQKBgQCM
SLYFjtSa0b/KbYYl5NKu6xsbFYIaYgE0NwPP7rA4PG1QwwSIkDhcpmSXFQdSvYuZ
z2CUTtIUmfDbXs1BjmoEu07syYZB/MSN/I75c/z7TvqfF5ejLyqlerQV/yqC7ICa
bGTXGwFXTDNOSyhSIxm0LLT6ayt/9+Y6jU4zRFzyYQKBgGiScevkv/XNz9MXswJ+
2bEIJNIJn0wIeuopifcDQrOTeCK+037t1AQ3lxMXisJABwG1jfw7WTjF3zz4dSUX
cK1+/2V+OkM/0nXjxPwPj7LiOediUyZNUn48r29uGOL1S83PSUdyST207CP6mZjc
K8aJmnGsVEAcWPzbpNh14q/c
-----END PRIVATE KEY-----

18
db/migrate/20230830115219_create_doorkeeper_openid_connect_tables.rb Normal file
View file

@ -0,0 +1,18 @@
class CreateDoorkeeperOpenidConnectTables < ActiveRecord::Migration[7.0]
def change
create_table :oauth_openid_requests do |t|
t.references :access_grant, :null => false, :index => true
t.string :nonce, :null => false
end
# Avoid validating foreign keys doe to possible deadlock
# create a separate migration instead, as suggested by db:migrate
add_foreign_key(
:oauth_openid_requests,
:oauth_access_grants,
:column => :access_grant_id,
:on_delete => :cascade, :validate => false
)
end
end

6
db/migrate/20230830115220_validate_create_doorkeeper_openid_connect_tables.rb Normal file
View file

@ -0,0 +1,6 @@
class ValidateCreateDoorkeeperOpenidConnectTables < ActiveRecord::Migration[7.0]
# Validate foreign key created by CreateDoorkeeperOpenidConnectTables
def change
validate_foreign_key :oauth_openid_requests, :oauth_access_grants
end
end

62
db/structure.sql
View file

@ -1152,6 +1152,36 @@ CREATE SEQUENCE public.oauth_nonces_id_seq
ALTER SEQUENCE public.oauth_nonces_id_seq OWNED BY public.oauth_nonces.id;
--
-- Name: oauth_openid_requests; Type: TABLE; Schema: public; Owner: -
--
CREATE TABLE public.oauth_openid_requests (
id bigint NOT NULL,
access_grant_id bigint NOT NULL,
nonce character varying NOT NULL
);
--
-- Name: oauth_openid_requests_id_seq; Type: SEQUENCE; Schema: public; Owner: -
--
CREATE SEQUENCE public.oauth_openid_requests_id_seq
START WITH 1
INCREMENT BY 1
NO MINVALUE
NO MAXVALUE
CACHE 1;
--
-- Name: oauth_openid_requests_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: -
--
ALTER SEQUENCE public.oauth_openid_requests_id_seq OWNED BY public.oauth_openid_requests.id;
--
-- Name: oauth_tokens; Type: TABLE; Schema: public; Owner: -
--
@ -1704,6 +1734,13 @@ ALTER TABLE ONLY public.oauth_applications ALTER COLUMN id SET DEFAULT nextval('
ALTER TABLE ONLY public.oauth_nonces ALTER COLUMN id SET DEFAULT nextval('public.oauth_nonces_id_seq'::regclass);
--
-- Name: oauth_openid_requests id; Type: DEFAULT; Schema: public; Owner: -
--
ALTER TABLE ONLY public.oauth_openid_requests ALTER COLUMN id SET DEFAULT nextval('public.oauth_openid_requests_id_seq'::regclass);
--
-- Name: oauth_tokens id; Type: DEFAULT; Schema: public; Owner: -
--
@ -2033,6 +2070,14 @@ ALTER TABLE ONLY public.oauth_nonces
ADD CONSTRAINT oauth_nonces_pkey PRIMARY KEY (id);
--
-- Name: oauth_openid_requests oauth_openid_requests_pkey; Type: CONSTRAINT; Schema: public; Owner: -
--
ALTER TABLE ONLY public.oauth_openid_requests
ADD CONSTRAINT oauth_openid_requests_pkey PRIMARY KEY (id);
--
-- Name: oauth_tokens oauth_tokens_pkey; Type: CONSTRAINT; Schema: public; Owner: -
--
@ -2573,6 +2618,13 @@ CREATE UNIQUE INDEX index_oauth_applications_on_uid ON public.oauth_applications
CREATE UNIQUE INDEX index_oauth_nonces_on_nonce_and_timestamp ON public.oauth_nonces USING btree (nonce, "timestamp");
--
-- Name: index_oauth_openid_requests_on_access_grant_id; Type: INDEX; Schema: public; Owner: -
--
CREATE INDEX index_oauth_openid_requests_on_access_grant_id ON public.oauth_openid_requests USING btree (access_grant_id);
--
-- Name: index_oauth_tokens_on_token; Type: INDEX; Schema: public; Owner: -
--
@ -2989,6 +3041,14 @@ ALTER TABLE ONLY public.oauth_access_tokens
ADD CONSTRAINT fk_rails_732cb83ab7 FOREIGN KEY (application_id) REFERENCES public.oauth_applications(id) NOT VALID;
--
-- Name: oauth_openid_requests fk_rails_77114b3b09; Type: FK CONSTRAINT; Schema: public; Owner: -
--
ALTER TABLE ONLY public.oauth_openid_requests
ADD CONSTRAINT fk_rails_77114b3b09 FOREIGN KEY (access_grant_id) REFERENCES public.oauth_access_grants(id) ON DELETE CASCADE;
--
-- Name: active_storage_variant_records fk_rails_993965df05; Type: FK CONSTRAINT; Schema: public; Owner: -
--
@ -3404,6 +3464,8 @@ INSERT INTO "schema_migrations" (version) VALUES
('20220223140543'),
('20230816135800'),
('20230825162137'),
('20230830115219'),
('20230830115220'),
('21'),
('22'),
('23'),

4
lib/oauth.rb
View file

@ -1,6 +1,7 @@
module Oauth
SCOPES = %w[read_prefs write_prefs write_diary write_api read_gpx write_gpx write_notes].freeze
PRIVILEGED_SCOPES = %w[read_email skip_authorization].freeze
OAUTH2_SCOPES = %w[openid].freeze
class Scope
attr_reader :name
@ -14,9 +15,10 @@ module Oauth
end
end
def self.scopes(privileged: false)
def self.scopes(oauth2: false, privileged: false)
scopes = SCOPES
scopes += PRIVILEGED_SCOPES if privileged
scopes += OAUTH2_SCOPES if oauth2
scopes.collect { |s| Scope.new(s) }
end
end

102
test/integration/oauth2_test.rb
View file

@ -1,4 +1,5 @@
require "test_helper"
require "jwt"
class OAuth2Test < ActionDispatch::IntegrationTest
def test_oauth2
@ -12,7 +13,8 @@ class OAuth2Test < ActionDispatch::IntegrationTest
token = request_token(client, code)
test_token(token, user, client)
assert_equal "read_prefs", token["scope"]
test_token(token["access_token"], user, client)
end
def test_oauth2_oob
@ -30,7 +32,8 @@ class OAuth2Test < ActionDispatch::IntegrationTest
token = request_token(client, code)
test_token(token, user, client)
assert_equal "read_prefs", token["scope"]
test_token(token["access_token"], user, client)
end
def test_oauth2_pkce_plain
@ -46,7 +49,8 @@ class OAuth2Test < ActionDispatch::IntegrationTest
token = request_token(client, code, verifier)
test_token(token, user, client)
assert_equal "read_prefs", token["scope"]
test_token(token["access_token"], user, client)
end
def test_oauth2_pkce_s256
@ -62,16 +66,95 @@ class OAuth2Test < ActionDispatch::IntegrationTest
token = request_token(client, code, verifier)
test_token(token, user, client)
assert_equal "read_prefs", token["scope"]
test_token(token["access_token"], user, client)
end
def test_openid_connect
user = create(:user)
client = create(:oauth_application, :redirect_uri => "https://some.web.app.example.org/callback", :scopes => "openid read_prefs")
state = SecureRandom.urlsafe_base64(16)
verifier = SecureRandom.urlsafe_base64(48)
challenge = Base64.urlsafe_encode64(Digest::SHA256.digest(verifier), :padding => false)
authorize_client(user, client, :state => state, :code_challenge => challenge, :code_challenge_method => "S256", :scope => "openid read_prefs")
assert_response :redirect
code = validate_redirect(client, state)
token = request_token(client, code, verifier)
assert_equal "openid read_prefs", token["scope"]
access_token = token["access_token"]
assert_not_nil access_token
id_token = token["id_token"]
assert_not_nil id_token
data, _headers = JWT.decode id_token, nil, true, {
:algorithm => [Doorkeeper::OpenidConnect.signing_algorithm.to_s],
:verify_iss => true,
:iss => "#{Settings.server_protocol}://#{Settings.server_url}",
:verify_sub => true,
:sub => user.id,
:verify_aud => true,
:aud => client.uid
} do |headers, _payload|
kid = headers["kid"]
get oauth_discovery_keys_path
keys = response.parsed_body["keys"]
jwk = keys&.detect { |e| e["kid"] == kid }
jwk && JWT::JWK::RSA.import(jwk).public_key
end
assert_equal user.id.to_s, data["sub"]
assert_not data.key?("preferred_username")
get oauth_userinfo_path
assert_response :unauthorized
auth_header = bearer_authorization_header(access_token)
get oauth_userinfo_path, :headers => auth_header
assert_response :success
userinfo = response.parsed_body
assert_not_nil userinfo
assert_equal user.id.to_s, userinfo["sub"]
assert_equal user.display_name, userinfo["preferred_username"]
end
def test_openid_discovery
get oauth_discovery_provider_path
assert_response :success
openid_config = response.parsed_body
assert_equal "#{Settings.server_protocol}://#{Settings.server_url}", openid_config["issuer"]
assert_equal oauth_authorization_path, URI(openid_config["authorization_endpoint"]).path
assert_equal oauth_token_path, URI(openid_config["token_endpoint"]).path
assert_equal oauth_userinfo_path, URI(openid_config["userinfo_endpoint"]).path
assert_equal oauth_discovery_keys_path, URI(openid_config["jwks_uri"]).path
end
def test_openid_key
get oauth_discovery_keys_path
assert_response :success
key_info = response.parsed_body
assert key_info.key?("keys")
assert_equal 1, key_info["keys"].size
assert_equal Doorkeeper::OpenidConnect.signing_key.kid, key_info["keys"][0]["kid"]
end
private
def authorize_client(user, client, options = {})
options = options.merge(:client_id => client.uid,
:redirect_uri => client.redirect_uri,
:response_type => "code",
:scope => "read_prefs")
options = {
:client_id => client.uid,
:redirect_uri => client.redirect_uri,
:response_type => "code",
:scope => "read_prefs"
}.merge(options)
get oauth_authorization_path(options)
assert_response :redirect
@ -135,9 +218,8 @@ class OAuth2Test < ActionDispatch::IntegrationTest
assert_response :success
token = response.parsed_body
assert_equal "Bearer", token["token_type"]
assert_equal "read_prefs", token["scope"]
token["access_token"]
token
end
def test_token(token, user, client)