discourse
/
discourse-openid-connect
mirror of https://github.com/discourse/discourse-openid-connect.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

DEV: Introduce syntax_tree for ruby formatting (#51)

This commit is contained in:
David Taylor 2022-12-29 12:33:26 +00:00 committed by GitHub
parent 43a30dde4a
commit 030f82f880
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 310 additions and 201 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
.github/workflows/plugin-linting.yml vendored
View File

@ -55,3 +55,12 @@ jobs:
- name: Rubocop
if: ${{ !cancelled() }}
run: bundle exec rubocop .
- name: Syntax Tree
if: ${{ !cancelled() }}
run: |
if test -f .streerc; then
bundle exec stree check Gemfile $(git ls-files '*.rb') $(git ls-files '*.rake')
else
echo "Stree config not detected for this repository. Skipping."
fi

6
.github/workflows/plugin-tests.yml vendored
View File

@ -80,7 +80,7 @@ jobs:
- name: Get yarn cache directory
id: yarn-cache-dir
run: echo "::set-output name=dir::$(yarn cache dir)"
run: echo "dir=$(yarn cache dir)" >> $GITHUB_OUTPUT
- name: Yarn cache
uses: actions/cache@v3
@ -130,7 +130,7 @@ jobs:
shell: bash
run: |
if [ 0 -lt $(find plugins/${{ github.event.repository.name }}/spec -type f -name "*.rb" 2> /dev/null | wc -l) ]; then
echo "::set-output name=files_exist::true"
echo "files_exist=true" >> $GITHUB_OUTPUT
fi
- name: Plugin RSpec
@ -142,7 +142,7 @@ jobs:
shell: bash
run: |
if [ 0 -lt $(find plugins/${{ github.event.repository.name }}/test/javascripts -type f \( -name "*.js" -or -name "*.es6" \) 2> /dev/null | wc -l) ]; then
echo "::set-output name=files_exist::true"
echo "files_exist=true" >> $GITHUB_OUTPUT
fi
- name: Plugin QUnit

2
.rubocop.yml
View File

@ -1,2 +1,2 @@
inherit_gem:
rubocop-discourse: default.yml
rubocop-discourse: stree-compat.yml

2
.streerc Normal file
View File

@ -0,0 +1,2 @@
--print-width=100
--plugins=plugin/trailing_comma

5
Gemfile
View File

@ -1,7 +1,8 @@
# frozen_string_literal: true
source 'https://rubygems.org'
source "https://rubygems.org"
group :development do
gem 'rubocop-discourse'
gem "rubocop-discourse"
gem "syntax_tree"
end

4
Gemfile.lock
View File

@ -6,6 +6,7 @@ GEM
parallel (1.22.1)
parser (3.1.2.1)
ast (~> 2.4.1)
prettier_print (1.2.0)
rainbow (3.1.1)
regexp_parser (2.6.0)
rexml (3.2.5)
@ -27,6 +28,8 @@ GEM
rubocop-rspec (2.13.2)
rubocop (~> 1.33)
ruby-progressbar (1.11.0)
syntax_tree (5.1.0)
prettier_print (>= 1.2.0)
unicode-display_width (2.3.0)
PLATFORMS
@ -39,6 +42,7 @@ PLATFORMS
DEPENDENCIES
rubocop-discourse
syntax_tree
BUNDLED WITH
2.3.10

161
lib/omniauth_open_id_connect.rb
View File

@ -1,16 +1,19 @@
# frozen_string_literal: true
require 'omniauth-oauth2'
require "omniauth-oauth2"
module ::OmniAuth
module OpenIDConnect
class DiscoveryError < Error; end
class DiscoveryError < Error
end
end
module Strategies
class OpenIDConnect < OmniAuth::Strategies::OAuth2
class NonceVerifyError < StandardError; end
class SubVerifyError < StandardError; end
class NonceVerifyError < StandardError
end
class SubVerifyError < StandardError
end
option :scope, "openid"
option :discovery, true
@ -23,11 +26,11 @@ module ::OmniAuth
option :claims, nil
option :client_options,
site: nil,
authorize_url: nil,
token_url: nil,
userinfo_endpoint: nil,
auth_scheme: :basic_auth
site: nil,
authorize_url: nil,
token_url: nil,
userinfo_endpoint: nil,
auth_scheme: :basic_auth
def verbose_log(message)
options.verbose_logger.call(message)
@ -35,24 +38,39 @@ module ::OmniAuth
def discover!
discovery_document = options[:discovery_document]
raise ::OmniAuth::OpenIDConnect::DiscoveryError.new("Discovery document is missing") if discovery_document.nil?
if discovery_document.nil?
raise ::OmniAuth::OpenIDConnect::DiscoveryError.new("Discovery document is missing")
end
discovery_params = {
authorize_url: "authorization_endpoint",
token_url: "token_endpoint",
site: "issuer"
site: "issuer",
}
discovery_params.each do |internal_key, external_key|
val = discovery_document[external_key]
raise ::OmniAuth::OpenIDConnect::DiscoveryError.new("missing discovery parameter #{external_key}") if val.nil? || val.empty?
if val.nil? || val.empty?
raise ::OmniAuth::OpenIDConnect::DiscoveryError.new(
"missing discovery parameter #{external_key}",
)
end
options[:client_options][internal_key] = val
end
userinfo_endpoint = options[:client_options][:userinfo_endpoint] = discovery_document["userinfo_endpoint"].to_s
userinfo_endpoint =
options[:client_options][:userinfo_endpoint] = discovery_document[
"userinfo_endpoint"
].to_s
options.use_userinfo = false if userinfo_endpoint.nil? || userinfo_endpoint.empty?
if discovery_document["token_endpoint_auth_methods_supported"] && !discovery_document["token_endpoint_auth_methods_supported"].include?("client_secret_basic") && discovery_document["token_endpoint_auth_methods_supported"].include?("client_secret_post")
if discovery_document["token_endpoint_auth_methods_supported"] &&
!discovery_document["token_endpoint_auth_methods_supported"].include?(
"client_secret_basic",
) &&
discovery_document["token_endpoint_auth_methods_supported"].include?(
"client_secret_post",
)
options[:client_options][:auth_scheme] = :request_body
end
end
@ -70,18 +88,18 @@ module ::OmniAuth
def authorize_params
super.tap do |params|
options[:passthrough_authorize_options].each do |k|
params[k] = request.params[k.to_s] unless [nil, ''].include?(request.params[k.to_s])
params[k] = request.params[k.to_s] unless [nil, ""].include?(request.params[k.to_s])
end
if options[:claims].present?
params[:claims] = options[:claims]
end
params[:claims] = options[:claims] if options[:claims].present?
params[:scope] = options[:scope]
session['omniauth.nonce'] = params[:nonce] = SecureRandom.hex(32)
session["omniauth.nonce"] = params[:nonce] = SecureRandom.hex(32)
options[:passthrough_token_options].each do |k|
session["omniauth.param.#{k}"] = request.params[k.to_s] unless [nil, ''].include?(request.params[k.to_s])
session["omniauth.param.#{k}"] = request.params[k.to_s] unless [nil, ""].include?(
request.params[k.to_s],
)
end
end
end
@ -90,13 +108,18 @@ module ::OmniAuth
params = {}
options[:passthrough_token_options].each do |k|
val = session.delete("omniauth.param.#{k}")
params[k] = val unless [nil, ''].include?(val)
params[k] = val unless [nil, ""].include?(val)
end
super.merge(params)
end
def callback_phase
if request.params["error"] && request.params["error_description"] && response = options.error_handler.call(request.params["error"], request.params["error_description"])
if request.params["error"] && request.params["error_description"] &&
response =
options.error_handler.call(
request.params["error"],
request.params["error_description"],
)
verbose_log("Error handled, redirecting\n\n#{response.to_yaml}")
return redirect(response)
end
@ -105,7 +128,7 @@ module ::OmniAuth
discover! if options[:discovery]
oauth2_callback_phase = super
return oauth2_callback_phase if env['omniauth.error']
return oauth2_callback_phase if env["omniauth.error"]
oauth2_callback_phase
rescue ::OmniAuth::OpenIDConnect::DiscoveryError => e
@ -123,66 +146,70 @@ module ::OmniAuth
# Verify the claims in the JWT
# The signature does not need to be verified because the
# token was acquired via a direct server-server connection to the issuer
@id_token_info ||= begin
decoded = ::JWT.decode(access_token['id_token'], nil, false).first
verbose_log("Loaded JWT\n\n#{decoded.to_yaml}")
::JWT::Verify.verify_claims(decoded,
verify_iss: true,
iss: options[:client_options][:site],
verify_aud: true,
aud: options.client_id,
verify_sub: false,
verify_expiration: true,
verify_not_before: true,
verify_iat: false,
verify_jti: false
)
@id_token_info ||=
begin
decoded = ::JWT.decode(access_token["id_token"], nil, false).first
verbose_log("Loaded JWT\n\n#{decoded.to_yaml}")
::JWT::Verify.verify_claims(
decoded,
verify_iss: true,
iss: options[:client_options][:site],
verify_aud: true,
aud: options.client_id,
verify_sub: false,
verify_expiration: true,
verify_not_before: true,
verify_iat: false,
verify_jti: false,
)
if decoded["nonce"].nil? || decoded["nonce"].empty? || decoded["nonce"] != session.delete("omniauth.nonce")
raise NonceVerifyError.new "JWT nonce is missing, or does not match"
if decoded["nonce"].nil? || decoded["nonce"].empty? ||
decoded["nonce"] != session.delete("omniauth.nonce")
raise NonceVerifyError.new "JWT nonce is missing, or does not match"
end
verbose_log("Verified JWT\n\n#{decoded.to_yaml}")
decoded
end
verbose_log("Verified JWT\n\n#{decoded.to_yaml}")
decoded
end
end
def userinfo_response
@raw_info ||= begin
info = access_token.get(options[:client_options][:userinfo_endpoint]).parsed
verbose_log("Fetched userinfo response\n\n#{info.to_yaml}")
info
end
@raw_info ||=
begin
info = access_token.get(options[:client_options][:userinfo_endpoint]).parsed
verbose_log("Fetched userinfo response\n\n#{info.to_yaml}")
info
end
userinfo_sub = @raw_info['sub']
id_token_sub = id_token_info['sub']
userinfo_sub = @raw_info["sub"]
id_token_sub = id_token_info["sub"]
if userinfo_sub != id_token_sub
raise SubVerifyError.new(
"OIDC `sub` mismatch. ID Token value: #{id_token_sub.inspect}. UserInfo value: #{userinfo_sub.inspect}"
)
"OIDC `sub` mismatch. ID Token value: #{id_token_sub.inspect}. UserInfo value: #{userinfo_sub.inspect}",
)
end
@raw_info
end
uid { id_token_info['sub'] }
uid { id_token_info["sub"] }
info do
data_source = options.use_userinfo ? userinfo_response : id_token_info
prune!(
name: data_source['name'],
email: data_source['email'],
first_name: data_source['given_name'],
last_name: data_source['family_name'],
nickname: data_source['preferred_username'],
image: data_source['picture']
name: data_source["name"],
email: data_source["email"],
first_name: data_source["given_name"],
last_name: data_source["family_name"],
nickname: data_source["preferred_username"],
image: data_source["picture"],
)
end
extra do
hash = {}
hash[:raw_info] = options.use_userinfo ? userinfo_response : id_token_info
hash[:id_token] = access_token['id_token']
hash[:id_token] = access_token["id_token"]
prune! hash
end
@ -193,11 +220,12 @@ module ::OmniAuth
end
def get_token_options
{ redirect_uri: callback_url,
grant_type: 'authorization_code',
{
redirect_uri: callback_url,
grant_type: "authorization_code",
code: request.params["code"],
client_id: options[:client_id],
client_secret: options[:client_secret]
client_secret: options[:client_secret],
}.merge(token_params.to_hash(symbolize_keys: true))
end
@ -212,11 +240,12 @@ module ::OmniAuth
def build_access_token
return super if options.use_userinfo
response = client.request(:post, options[:client_options][:token_url], body: get_token_options)
response =
client.request(:post, options[:client_options][:token_url], body: get_token_options)
::OAuth2::AccessToken.from_hash(client, response.parsed)
end
end
end
end
OmniAuth.config.add_camelization 'openid_connect', 'OpenIDConnect'
OmniAuth.config.add_camelization "openid_connect", "OpenIDConnect"

109
lib/openid_connect_authenticator.rb
View File

@ -2,7 +2,7 @@
class OpenIDConnectAuthenticator < Auth::ManagedAuthenticator
def name
'oidc'
"oidc"
end
def can_revoke?
@ -18,13 +18,13 @@ class OpenIDConnectAuthenticator < Auth::ManagedAuthenticator
end
def primary_email_verified?(auth)
supplied_verified_boolean = auth['extra']['raw_info']['email_verified']
supplied_verified_boolean = auth["extra"]["raw_info"]["email_verified"]
# If the payload includes the email_verified boolean, use it. Otherwise assume true
if supplied_verified_boolean.nil?
true
else
# Many providers violate the spec, and send this as a string rather than a boolean
supplied_verified_boolean == true || supplied_verified_boolean == 'true'
supplied_verified_boolean == true || supplied_verified_boolean == "true"
end
end
@ -44,18 +44,22 @@ class OpenIDConnectAuthenticator < Auth::ManagedAuthenticator
end
from_cache = true
result = Discourse.cache.fetch("openid-connect-discovery-#{document_url}", expires_in: 10.minutes) do
from_cache = false
oidc_log("Fetching discovery document from #{document_url}")
connection = Faraday.new(request: { timeout: request_timeout_seconds }) do |c|
c.use Faraday::Response::RaiseError
c.adapter FinalDestination::FaradayAdapter
end
JSON.parse(connection.get(document_url).body)
rescue Faraday::Error, JSON::ParserError => e
oidc_log("Fetching discovery document raised error #{e.class} #{e.message}", error: true)
nil
end
result =
Discourse
.cache
.fetch("openid-connect-discovery-#{document_url}", expires_in: 10.minutes) do
from_cache = false
oidc_log("Fetching discovery document from #{document_url}")
connection =
Faraday.new(request: { timeout: request_timeout_seconds }) do |c|
c.use Faraday::Response::RaiseError
c.adapter FinalDestination::FaradayAdapter
end
JSON.parse(connection.get(document_url).body)
rescue Faraday::Error, JSON::ParserError => e
oidc_log("Fetching discovery document raised error #{e.class} #{e.message}", error: true)
nil
end
oidc_log("Discovery document loaded from cache") if from_cache
oidc_log("Discovery document is\n\n#{result.to_yaml}")
@ -73,43 +77,54 @@ class OpenIDConnectAuthenticator < Auth::ManagedAuthenticator
def register_middleware(omniauth)
omniauth.provider :openid_connect,
name: :oidc,
error_handler: lambda { |error, message|
handlers = SiteSetting.openid_connect_error_redirects.split("\n")
handlers.each do |row|
parts = row.split("|")
return parts[1] if message.include? parts[0]
end
nil
},
verbose_logger: lambda { |message| oidc_log(message) },
setup: lambda { |env|
opts = env['omniauth.strategy'].options
name: :oidc,
error_handler:
lambda { |error, message|
handlers = SiteSetting.openid_connect_error_redirects.split("\n")
handlers.each do |row|
parts = row.split("|")
return parts[1] if message.include? parts[0]
end
nil
},
verbose_logger: lambda { |message| oidc_log(message) },
setup:
lambda { |env|
opts = env["omniauth.strategy"].options
token_params = {}
token_params[:scope] = SiteSetting.openid_connect_token_scope if SiteSetting.openid_connect_token_scope.present?
token_params = {}
token_params[
:scope
] = SiteSetting.openid_connect_token_scope if SiteSetting.openid_connect_token_scope.present?
opts.deep_merge!(
client_id: SiteSetting.openid_connect_client_id,
client_secret: SiteSetting.openid_connect_client_secret,
discovery_document: discovery_document,
scope: SiteSetting.openid_connect_authorize_scope,
token_params: token_params,
passthrough_authorize_options: SiteSetting.openid_connect_authorize_parameters.split("|"),
claims: SiteSetting.openid_connect_claims
)
opts.deep_merge!(
client_id: SiteSetting.openid_connect_client_id,
client_secret: SiteSetting.openid_connect_client_secret,
discovery_document: discovery_document,
scope: SiteSetting.openid_connect_authorize_scope,
token_params: token_params,
passthrough_authorize_options:
SiteSetting.openid_connect_authorize_parameters.split("|"),
claims: SiteSetting.openid_connect_claims,
)
opts[:client_options][:connection_opts] = { request: { timeout: request_timeout_seconds } }
opts[:client_options][:connection_opts] = {
request: {
timeout: request_timeout_seconds,
},
}
opts[:client_options][:connection_build] = lambda { |builder|
if SiteSetting.openid_connect_verbose_logging
builder.response :logger, Rails.logger, { bodies: true, formatter: OIDCFaradayFormatter }
end
opts[:client_options][:connection_build] = lambda do |builder|
if SiteSetting.openid_connect_verbose_logging
builder.response :logger,
Rails.logger,
{ bodies: true, formatter: OIDCFaradayFormatter }
end
builder.request :url_encoded # form-encode POST params
builder.adapter FinalDestination::FaradayAdapter # make requests with FinalDestination::HTTP
}
}
builder.request :url_encoded # form-encode POST params
builder.adapter FinalDestination::FaradayAdapter # make requests with FinalDestination::HTTP
end
}
end
def request_timeout_seconds

2
lib/openid_connect_faraday_formatter.rb
View File

@ -1,6 +1,6 @@
# frozen_string_literal: true
require 'faraday/logging/formatter'
require "faraday/logging/formatter"
class OIDCFaradayFormatter < Faraday::Logging::Formatter
def request(env)

6
plugin.rb
View File

@ -36,14 +36,16 @@ on(:before_session_destroy) do |data|
end_session_endpoint = authenticator.discovery_document["end_session_endpoint"].presence
if !end_session_endpoint
authenticator.oidc_log "Logout: No end_session_endpoint found in discovery document", error: true
authenticator.oidc_log "Logout: No end_session_endpoint found in discovery document",
error: true
next
end
begin
uri = URI.parse(end_session_endpoint)
rescue URI::Error
authenticator.oidc_log "Logout: unable to parse end_session_endpoint #{end_session_endpoint}", error: true
authenticator.oidc_log "Logout: unable to parse end_session_endpoint #{end_session_endpoint}",
error: true
end
authenticator.oidc_log "Logout: Redirecting user_id=#{data[:user].id} to end_session_endpoint"

118
spec/lib/omniauth_open_id_connect_spec.rb
View File

@ -1,14 +1,14 @@
# frozen_string_literal: true
require_relative '../../lib/omniauth_open_id_connect'
require 'rails_helper'
require_relative "../../lib/omniauth_open_id_connect"
require "rails_helper"
describe OmniAuth::Strategies::OpenIDConnect do
let(:app) do
@app_called = false
lambda do |*args|
@app_called = true
[200, {}, ['Hello.']]
[200, {}, ["Hello."]]
end
end
@ -22,7 +22,12 @@ describe OmniAuth::Strategies::OpenIDConnect do
end
subject do
OmniAuth::Strategies::OpenIDConnect.new(app, 'appid', 'secret', discovery_document: discovery_document)
OmniAuth::Strategies::OpenIDConnect.new(
app,
"appid",
"secret",
discovery_document: discovery_document,
)
end
before { OmniAuth.config.test_mode = true }
@ -30,7 +35,8 @@ describe OmniAuth::Strategies::OpenIDConnect do
after { OmniAuth.config.test_mode = false }
it "throws error for missing discovery document" do
strategy = OmniAuth::Strategies::OpenIDConnect.new(app, 'appid', 'secret', discovery_document: nil)
strategy =
OmniAuth::Strategies::OpenIDConnect.new(app, "appid", "secret", discovery_document: nil)
expect { strategy.discover! }.to raise_error(::OmniAuth::OpenIDConnect::DiscoveryError)
end
@ -51,7 +57,9 @@ describe OmniAuth::Strategies::OpenIDConnect do
end
it "uses basic authentication when both client_secret_basic and client_secret_post are provided" do
discovery_document.merge!({ "token_endpoint_auth_methods_supported" => ["client_secret_basic", "client_secret_post"] })
discovery_document.merge!(
{ "token_endpoint_auth_methods_supported" => %w[client_secret_basic client_secret_post] },
)
subject.discover!
expect(subject.options.client_options.auth_scheme).to eq(:basic_auth)
end
@ -62,10 +70,13 @@ describe OmniAuth::Strategies::OpenIDConnect do
expect(subject.options.client_options.auth_scheme).to eq(:request_body)
end
context 'with valid discovery document loaded' do
context "with valid discovery document loaded" do
before do
subject.stubs(:request).returns(mock('object'))
subject.request.stubs(:params).returns("p" => "someallowedvalue", "somethingelse" => "notallowed")
subject.stubs(:request).returns(mock("object"))
subject
.request
.stubs(:params)
.returns("p" => "someallowedvalue", "somethingelse" => "notallowed")
subject.options.claims = '{"userinfo":{"email":null,"email_verified":null}'
subject.discover!
end
@ -74,7 +85,9 @@ describe OmniAuth::Strategies::OpenIDConnect do
expect(subject.options.client_options.site).to eq("https://id.example.com/")
expect(subject.options.client_options.authorize_url).to eq("https://id.example.com/authorize")
expect(subject.options.client_options.token_url).to eq("https://id.example.com/token")
expect(subject.options.client_options.userinfo_endpoint).to eq("https://id.example.com/userinfo")
expect(subject.options.client_options.userinfo_endpoint).to eq(
"https://id.example.com/userinfo",
)
end
describe "authorize parameters" do
@ -87,11 +100,13 @@ describe OmniAuth::Strategies::OpenIDConnect do
it "sets a nonce" do
expect((nonce = subject.authorize_params[:nonce]).size).to eq(64)
expect(subject.session['omniauth.nonce']).to eq(nonce)
expect(subject.session["omniauth.nonce"]).to eq(nonce)
end
it "passes claims through to authorize endpoint if present" do
expect(subject.authorize_params[:claims]).to eq('{"userinfo":{"email":null,"email_verified":null}')
expect(subject.authorize_params[:claims]).to eq(
'{"userinfo":{"email":null,"email_verified":null}',
)
end
it "does not pass claims if empty string" do
@ -103,7 +118,7 @@ describe OmniAuth::Strategies::OpenIDConnect do
describe "token parameters" do
it "passes through parameters from authorize phase" do
expect(subject.authorize_params[:p]).to eq("someallowedvalue")
subject.stubs(:request).returns(mock())
subject.stubs(:request).returns(mock)
subject.request.stubs(:params).returns({})
expect(subject.token_params[:p]).to eq("someallowedvalue")
end
@ -115,8 +130,11 @@ describe OmniAuth::Strategies::OpenIDConnect do
subject.stubs(:full_host).returns("https://example.com")
subject.stubs(:request).returns(mock())
subject.request.stubs(:params).returns("state" => auth_params[:state], "code" => "supersecretcode")
subject.stubs(:request).returns(mock)
subject
.request
.stubs(:params)
.returns("state" => auth_params[:state], "code" => "supersecretcode")
payload = {
iss: "https://id.example.com/",
@ -126,15 +144,21 @@ describe OmniAuth::Strategies::OpenIDConnect do
exp: Time.now.to_i + 120,
nonce: auth_params[:nonce],
name: "My Auth Token Name",
email: "tokenemail@example.com"
email: "tokenemail@example.com",
}
@token = ::JWT.encode payload, nil, 'none'
@token = ::JWT.encode payload, nil, "none"
end
it "handles error redirects correctly" do
subject.stubs(:request).returns(mock())
subject.request.stubs(:params).returns("error" => true, "error_description" => "User forgot password")
subject.options.error_handler = lambda { |error, message| return "https://example.com/error_redirect" if message.include?("forgot password") }
subject.stubs(:request).returns(mock)
subject
.request
.stubs(:params)
.returns("error" => true, "error_description" => "User forgot password")
subject.options.error_handler =
lambda do |error, message|
return "https://example.com/error_redirect" if message.include?("forgot password")
end
expect(subject.callback_phase[0]).to eq(302)
expect(subject.callback_phase[1]["Location"]).to eq("https://example.com/error_redirect")
expect(@app_called).to eq(false)
@ -142,11 +166,15 @@ describe OmniAuth::Strategies::OpenIDConnect do
context "with userinfo disabled" do
before do
stub_request(:post, "https://id.example.com/token").
with(body: hash_including("code" => "supersecretcode", "p" => "someallowedvalue")).
to_return(status: 200, body: {
"id_token": @token,
}.to_json, headers: { "Content-Type" => "application/json" })
stub_request(:post, "https://id.example.com/token").with(
body: hash_including("code" => "supersecretcode", "p" => "someallowedvalue"),
).to_return(
status: 200,
body: { id_token: @token }.to_json,
headers: {
"Content-Type" => "application/json",
},
)
subject.options.use_userinfo = false
end
@ -174,30 +202,30 @@ describe OmniAuth::Strategies::OpenIDConnect do
end
context "with userinfo enabled" do
let(:userinfo_response) {
{
sub: "someuserid",
name: "My Userinfo Name",
email: "userinfoemail@example.com",
}
}
let(:userinfo_response) do
{ sub: "someuserid", name: "My Userinfo Name", email: "userinfoemail@example.com" }
end
before do
stub_request(:post, "https://id.example.com/token").
with(body: hash_including("code" => "supersecretcode", "p" => "someallowedvalue")).
to_return(status: 200, body: {
"access_token": "AnAccessToken",
"expires_in": 3600,
"id_token": @token,
}.to_json, headers: { "Content-Type" => "application/json" })
stub_request(:post, "https://id.example.com/token").with(
body: hash_including("code" => "supersecretcode", "p" => "someallowedvalue"),
).to_return(
status: 200,
body: { access_token: "AnAccessToken", expires_in: 3600, id_token: @token }.to_json,
headers: {
"Content-Type" => "application/json",
},
)
stub_request(:get, "https://id.example.com/userinfo").
with(headers: { 'Authorization' => 'Bearer AnAccessToken' }).
to_return do |request|
stub_request(:get, "https://id.example.com/userinfo")
.with(headers: { "Authorization" => "Bearer AnAccessToken" })
.to_return do |request|
{
status: 200,
body: userinfo_response.to_json,
headers: { "Content-Type" => "application/json" }
headers: {
"Content-Type" => "application/json",
},
}
end
end
@ -214,7 +242,9 @@ describe OmniAuth::Strategies::OpenIDConnect do
userinfo_response["sub"] = "someothersub"
callback_response = subject.callback_phase
expect(callback_response[0]).to eq(302)
expect(callback_response[1]["Location"]).to eq("/auth/failure?message=openid_connect_sub_mismatch&strategy=openidconnect")
expect(callback_response[1]["Location"]).to eq(
"/auth/failure?message=openid_connect_sub_mismatch&strategy=openidconnect",
)
expect(@app_called).to eq(false)
end
end

45
spec/lib/openid_connect_authenticator_spec.rb
View File

@ -1,7 +1,7 @@
# frozen_string_literal: true
require 'rails_helper'
require_relative '../../lib/omniauth_open_id_connect'
require "rails_helper"
require_relative "../../lib/omniauth_open_id_connect"
describe OpenIDConnectAuthenticator do
let(:authenticator) { described_class.new }
@ -11,22 +11,22 @@ describe OpenIDConnectAuthenticator do
provider: "oidc",
uid: "123456789",
info: {
name: "John Doe",
email: user.email
name: "John Doe",
email: user.email,
},
extra: {
raw_info: {
email: user.email,
name: "John Doe"
}
}
name: "John Doe",
},
},
)
end
context "when email_verified is not supplied" do
# Some IDPs do not supply this information
# In this case we trust that they have verified the address
it 'matches the user' do
it "matches the user" do
result = authenticator.after_authenticate(hash)
expect(result.user).to eq(user)
@ -34,35 +34,35 @@ describe OpenIDConnectAuthenticator do
end
context "when email_verified is true" do
it 'matches the user' do
it "matches the user" do
hash[:extra][:raw_info][:email_verified] = true
result = authenticator.after_authenticate(hash)
expect(result.user).to eq(user)
end
it 'matches the user as a true string' do
hash[:extra][:raw_info][:email_verified] = 'true'
it "matches the user as a true string" do
hash[:extra][:raw_info][:email_verified] = "true"
result = authenticator.after_authenticate(hash)
expect(result.user).to eq(user)
end
end
context "when email_verified is false" do
it 'does not match the user' do
it "does not match the user" do
hash[:extra][:raw_info][:email_verified] = false
result = authenticator.after_authenticate(hash)
expect(result.user).to eq(nil)
end
it 'does not match the user as a false string' do
hash[:extra][:raw_info][:email_verified] = 'false'
it "does not match the user as a false string" do
hash[:extra][:raw_info][:email_verified] = "false"
result = authenticator.after_authenticate(hash)
expect(result.user).to eq(nil)
end
end
context "when match_by_email is false" do
it 'does not match the user' do
it "does not match the user" do
SiteSetting.openid_connect_match_by_email = false
result = authenticator.after_authenticate(hash)
expect(result.user).to eq(nil)
@ -70,13 +70,16 @@ describe OpenIDConnectAuthenticator do
end
describe "discovery document fetching" do
let(:document_url) { SiteSetting.openid_connect_discovery_document = "https://id.example.com/.well-known/openid-configuration" }
let(:document_url) do
SiteSetting.openid_connect_discovery_document =
"https://id.example.com/.well-known/openid-configuration"
end
let(:document) do
{
"issuer": "https://id.example.com/",
"authorization_endpoint": "https://id.example.com/authorize",
"token_endpoint": "https://id.example.com/token",
"userinfo_endpoint": "https://id.example.com/userinfo",
issuer: "https://id.example.com/",
authorization_endpoint: "https://id.example.com/authorize",
token_endpoint: "https://id.example.com/token",
userinfo_endpoint: "https://id.example.com/userinfo",
}.to_json
end
after { Discourse.cache.delete("openid-connect-discovery-#{document_url}") }
@ -87,7 +90,7 @@ describe OpenIDConnectAuthenticator do
"issuer",
"authorization_endpoint",
"token_endpoint",
"userinfo_endpoint"
"userinfo_endpoint",
)
end

42
spec/requests/rp_initiated_logout_spec.rb
View File

@ -1,16 +1,19 @@
# frozen_string_literal: true
require 'rails_helper'
require "rails_helper"
describe "OIDC RP-Initiated Logout" do
let(:document_url) { SiteSetting.openid_connect_discovery_document = "https://id.example.com/.well-known/openid-configuration" }
let(:document_url) do
SiteSetting.openid_connect_discovery_document =
"https://id.example.com/.well-known/openid-configuration"
end
let(:document) do
{
"issuer": "https://id.example.com/",
"authorization_endpoint": "https://id.example.com/authorize",
"token_endpoint": "https://id.example.com/token",
"userinfo_endpoint": "https://id.example.com/userinfo",
"end_session_endpoint": "https://id.example.com/endsession",
issuer: "https://id.example.com/",
authorization_endpoint: "https://id.example.com/authorize",
token_endpoint: "https://id.example.com/token",
userinfo_endpoint: "https://id.example.com/userinfo",
end_session_endpoint: "https://id.example.com/endsession",
}
end
fab!(:user) { Fabricate(:user) }
@ -21,9 +24,7 @@ describe "OIDC RP-Initiated Logout" do
stub_request(:get, document_url).to_return(body: lambda { |r| document.to_json })
end
after do
Discourse.cache.delete("openid-connect-discovery-#{document_url}")
end
after { Discourse.cache.delete("openid-connect-discovery-#{document_url}") }
it "does nothing for a user with no oidc record" do
sign_in(user)
@ -43,13 +44,22 @@ describe "OIDC RP-Initiated Logout" do
context "with user and token" do
before do
sign_in(user)
UserAssociatedAccount.create!(provider_name: "oidc", user: user, provider_uid: "myuid", extra: { id_token: "myoidctoken" })
UserAssociatedAccount.create!(
provider_name: "oidc",
user: user,
provider_uid: "myuid",
extra: {
id_token: "myoidctoken",
},
)
end
it "redirects the user to the logout endpoint" do
delete "/session/#{user.username}", xhr: true
expect(response.status).to eq(200)
expect(response.parsed_body["redirect_url"]).to eq("https://id.example.com/endsession?id_token_hint=myoidctoken")
expect(response.parsed_body["redirect_url"]).to eq(
"https://id.example.com/endsession?id_token_hint=myoidctoken",
)
end
it "correctly handles logout urls with existing query params" do
@ -57,14 +67,18 @@ describe "OIDC RP-Initiated Logout" do
delete "/session/#{user.username}", xhr: true
expect(response.status).to eq(200)
expect(response.parsed_body["redirect_url"]).to eq("https://id.example.com/endsession?param=true&id_token_hint=myoidctoken")
expect(response.parsed_body["redirect_url"]).to eq(
"https://id.example.com/endsession?param=true&id_token_hint=myoidctoken",
)
end
it "includes the redirect URI if set" do
SiteSetting.openid_connect_rp_initiated_logout_redirect = "https://example.com"
delete "/session/#{user.username}", xhr: true
expect(response.status).to eq(200)
expect(response.parsed_body["redirect_url"]).to eq("https://id.example.com/endsession?id_token_hint=myoidctoken&post_logout_redirect_uri=https%3A%2F%2Fexample.com")
expect(response.parsed_body["redirect_url"]).to eq(
"https://id.example.com/endsession?id_token_hint=myoidctoken&post_logout_redirect_uri=https%3A%2F%2Fexample.com",
)
end
it "does not redirect if plugin disabled" do