Skip to content

Commit

Permalink
DEV: Update ruby linting (#69)
Browse files Browse the repository at this point in the history 
* DEV: Update ruby linting

* Use named subjects in specs

---------

Co-authored-by: Loïc Guitaut <[email protected]>
  • Loading branch information
@CvX @Flink
CvX and Flink authored Nov 30, 2023
1 parent de0831a commit c7e89b2
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 83 deletions.
45 changes: 28 additions & 17 deletions Gemfile.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,38 +2,49 @@ GEM
remote: https://rubygems.org/
specs:
ast (2.4.2)
json (2.6.2)
parallel (1.22.1)
parser (3.1.2.1)
json (2.6.3)
language_server-protocol (3.17.0.3)
parallel (1.23.0)
parser (3.2.2.4)
ast (~> 2.4.1)
prettier_print (1.2.0)
racc
prettier_print (1.2.1)
racc (1.7.3)
rainbow (3.1.1)
regexp_parser (2.6.0)
rexml (3.2.5)
rubocop (1.36.0)
regexp_parser (2.8.2)
rexml (3.2.6)
rubocop (1.57.2)
json (~> 2.3)
language_server-protocol (>= 3.17.0)
parallel (~> 1.10)
parser (>= 3.1.2.1)
parser (>= 3.2.2.4)
rainbow (>= 2.2.2, < 4.0)
regexp_parser (>= 1.8, < 3.0)
rexml (>= 3.2.5, < 4.0)
rubocop-ast (>= 1.20.1, < 2.0)
rubocop-ast (>= 1.28.1, < 2.0)
ruby-progressbar (~> 1.7)
unicode-display_width (>= 1.4.0, < 3.0)
rubocop-ast (1.21.0)
parser (>= 3.1.1.0)
rubocop-discourse (3.0)
unicode-display_width (>= 2.4.0, < 3.0)
rubocop-ast (1.30.0)
parser (>= 3.2.1.0)
rubocop-capybara (2.19.0)
rubocop (~> 1.41)
rubocop-discourse (3.4.1)
rubocop (>= 1.1.0)
rubocop-rspec (>= 2.0.0)
rubocop-rspec (2.13.2)
rubocop-factory_bot (2.24.0)
rubocop (~> 1.33)
ruby-progressbar (1.11.0)
syntax_tree (5.1.0)
rubocop-rspec (2.25.0)
rubocop (~> 1.40)
rubocop-capybara (~> 2.17)
rubocop-factory_bot (~> 2.22)
ruby-progressbar (1.13.0)
syntax_tree (6.2.0)
prettier_print (>= 1.2.0)
unicode-display_width (2.3.0)
unicode-display_width (2.5.0)

PLATFORMS
arm64-darwin-20
arm64-darwin-22
ruby
x86_64-darwin-18
x86_64-darwin-19
Expand Down
136 changes: 70 additions & 66 deletions spec/lib/omniauth_open_id_connect_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@
require "rails_helper"

describe OmniAuth::Strategies::OpenIDConnect do
subject(:strategy) do
OmniAuth::Strategies::OpenIDConnect.new(
app,
"appid",
"secret",
discovery_document: discovery_document,
)
end

let(:app) do
@app_called = false
lambda do |*args|
Expand All @@ -21,117 +30,112 @@
}
end

subject do
OmniAuth::Strategies::OpenIDConnect.new(
app,
"appid",
"secret",
discovery_document: discovery_document,
)
end

before { OmniAuth.config.test_mode = true }

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)
expect { strategy.discover! }.to raise_error(::OmniAuth::OpenIDConnect::DiscoveryError)
context "when discovery document is missing" do
let(:discovery_document) { nil }

it "throws an error" do
expect { strategy.discover! }.to raise_error(::OmniAuth::OpenIDConnect::DiscoveryError)
end
end

it "throws error for invalid discovery document" do
discovery_document.delete("authorization_endpoint")
expect { subject.discover! }.to raise_error(::OmniAuth::OpenIDConnect::DiscoveryError)
expect { strategy.discover! }.to raise_error(::OmniAuth::OpenIDConnect::DiscoveryError)
end

it "disables userinfo if not included in discovery document" do
discovery_document.delete("userinfo_endpoint")
subject.discover!
expect(subject.options.use_userinfo).to eq(false)
strategy.discover!
expect(strategy.options.use_userinfo).to eq(false)
end

it "uses basic authentication when no endpoint auth methods are provided" do
subject.discover!
expect(subject.options.client_options.auth_scheme).to eq(:basic_auth)
strategy.discover!
expect(strategy.options.client_options.auth_scheme).to eq(:basic_auth)
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" => %w[client_secret_basic client_secret_post] },
)
subject.discover!
expect(subject.options.client_options.auth_scheme).to eq(:basic_auth)
strategy.discover!
expect(strategy.options.client_options.auth_scheme).to eq(:basic_auth)
end

it "uses request_body authentication when client_secret_post is provided only" do
discovery_document.merge!({ "token_endpoint_auth_methods_supported" => ["client_secret_post"] })
subject.discover!
expect(subject.options.client_options.auth_scheme).to eq(:request_body)
strategy.discover!
expect(strategy.options.client_options.auth_scheme).to eq(:request_body)
end

context "with valid discovery document loaded" do
before do
subject.stubs(:request).returns(mock("object"))
subject
strategy.stubs(:request).returns(mock("object"))
strategy
.request
.stubs(:params)
.returns("p" => "someallowedvalue", "somethingelse" => "notallowed")
subject.options.claims = '{"userinfo":{"email":null,"email_verified":null}'
subject.discover!
strategy.options.claims = '{"userinfo":{"email":null,"email_verified":null}'
strategy.discover!
end

it "loads parameters correctly" 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(
expect(strategy.options.client_options.site).to eq("https://id.example.com/")
expect(strategy.options.client_options.authorize_url).to eq(
"https://id.example.com/authorize",
)
expect(strategy.options.client_options.token_url).to eq("https://id.example.com/token")
expect(strategy.options.client_options.userinfo_endpoint).to eq(
"https://id.example.com/userinfo",
)
end

describe "authorize parameters" do
it "passes through allowed parameters" do
expect(subject.authorize_params[:p]).to eq("someallowedvalue")
expect(subject.authorize_params[:somethingelse]).to eq(nil)
expect(strategy.authorize_params[:p]).to eq("someallowedvalue")
expect(strategy.authorize_params[:somethingelse]).to eq(nil)

expect(subject.session["omniauth.param.p"]).to eq("someallowedvalue")
expect(strategy.session["omniauth.param.p"]).to eq("someallowedvalue")
end

it "sets a nonce" do
expect((nonce = subject.authorize_params[:nonce]).size).to eq(64)
expect(subject.session["omniauth.nonce"]).to eq(nonce)
expect((nonce = strategy.authorize_params[:nonce]).size).to eq(64)
expect(strategy.session["omniauth.nonce"]).to eq(nonce)
end

it "passes claims through to authorize endpoint if present" do
expect(subject.authorize_params[:claims]).to eq(
expect(strategy.authorize_params[:claims]).to eq(
'{"userinfo":{"email":null,"email_verified":null}',
)
end

it "does not pass claims if empty string" do
subject.options.claims = ""
expect(subject.authorize_params[:claims]).to eq(nil)
strategy.options.claims = ""
expect(strategy.authorize_params[:claims]).to eq(nil)
end
end

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.request.stubs(:params).returns({})
expect(subject.token_params[:p]).to eq("someallowedvalue")
expect(strategy.authorize_params[:p]).to eq("someallowedvalue")
strategy.stubs(:request).returns(mock)
strategy.request.stubs(:params).returns({})
expect(strategy.token_params[:p]).to eq("someallowedvalue")
end
end

describe "callback_phase" do
before do
auth_params = subject.authorize_params
auth_params = strategy.authorize_params

subject.stubs(:full_host).returns("https://example.com")
strategy.stubs(:full_host).returns("https://example.com")

subject.stubs(:request).returns(mock)
subject
strategy.stubs(:request).returns(mock)
strategy
.request
.stubs(:params)
.returns("state" => auth_params[:state], "code" => "supersecretcode")
Expand All @@ -150,17 +154,17 @@
end

it "handles error redirects correctly" do
subject.stubs(:request).returns(mock)
subject
strategy.stubs(:request).returns(mock)
strategy
.request
.stubs(:params)
.returns("error" => true, "error_description" => "User forgot password")
subject.options.error_handler =
strategy.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(strategy.callback_phase[0]).to eq(302)
expect(strategy.callback_phase[1]["Location"]).to eq("https://example.com/error_redirect")
expect(@app_called).to eq(false)
end

Expand All @@ -176,27 +180,27 @@
},
)

subject.options.use_userinfo = false
strategy.options.use_userinfo = false
end

it "fetches auth token correctly, and uses it for user info" do
expect(subject.callback_phase[0]).to eq(200)
expect(subject.uid).to eq("someuserid")
expect(subject.info[:name]).to eq("My Auth Token Name")
expect(subject.info[:email]).to eq("[email protected]")
expect(subject.extra[:id_token]).to eq(@token)
expect(strategy.callback_phase[0]).to eq(200)
expect(strategy.uid).to eq("someuserid")
expect(strategy.info[:name]).to eq("My Auth Token Name")
expect(strategy.info[:email]).to eq("[email protected]")
expect(strategy.extra[:id_token]).to eq(@token)
expect(@app_called).to eq(true)
end

it "checks the nonce" do
subject.session["omniauth.nonce"] = "overriddenNonce"
expect(subject.callback_phase[0]).to eq(302)
strategy.session["omniauth.nonce"] = "overriddenNonce"
expect(strategy.callback_phase[0]).to eq(302)
expect(@app_called).to eq(false)
end

it "checks the issuer" do
subject.options.client_id = "overriddenclientid"
expect(subject.callback_phase[0]).to eq(302)
strategy.options.client_id = "overriddenclientid"
expect(strategy.callback_phase[0]).to eq(302)
expect(@app_called).to eq(false)
end
end
Expand Down Expand Up @@ -231,16 +235,16 @@
end

it "fetches credentials and auth token correctly" do
expect(subject.callback_phase[0]).to eq(200)
expect(subject.uid).to eq("someuserid")
expect(subject.info[:name]).to eq("My Userinfo Name")
expect(subject.info[:email]).to eq("[email protected]")
expect(strategy.callback_phase[0]).to eq(200)
expect(strategy.uid).to eq("someuserid")
expect(strategy.info[:name]).to eq("My Userinfo Name")
expect(strategy.info[:email]).to eq("[email protected]")
expect(@app_called).to eq(true)
end

it "handles mismatching `sub` correctly" do
userinfo_response["sub"] = "someothersub"
callback_response = subject.callback_phase
callback_response = strategy.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",
Expand Down

0 comments on commit c7e89b2

Please sign in to comment.