Skip to content

Commit db3fc78

Browse files
committed
Delete dead code: Discovery, OmniauthOptions, clock_skew
Three pieces of dead code surfaced during a post-testing code review: 1. The Discovery class (~100 LoC + spec) implemented OIDC discovery from scratch but was never wired into any production code path. The real discovery is delegated to omniauth_openid_connect via discovery: true in the Devise omniauth provider config, which the library handles itself. Dropping Discovery also removes DiscoveryError. 2. OmniauthOptions.build (~25 LoC + its only caller, self.omniauth_options) was defined but never called. Hosts wire the OmniAuth provider directly in their Devise initializer; the helper was a leftover from an earlier iteration that built the options hash in-gem. 3. Configuration#clock_skew was settable but never read — the value was never forwarded to any OIDC verification step. Removed along with its specs. Configuration#reset! is kept: it is legitimately used by the test helper (spec_helper before(:each)) to isolate state between examples. Removing it would force a clunkier 'replace the @config ivar' helper, which is strictly worse than a one-line method on the class.
1 parent 759f49b commit db3fc78

6 files changed

Lines changed: 1 addition & 240 deletions

File tree

lib/activeadmin-oidc.rb

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ module ActiveAdmin
2222
module Oidc
2323
class Error < StandardError; end
2424
class ConfigurationError < Error; end
25-
class DiscoveryError < Error; end
2625
class ProvisioningError < Error; end
2726

2827
class << self
@@ -43,17 +42,7 @@ def reset!
4342
end
4443

4544
require "activeadmin/oidc/configuration"
46-
require "activeadmin/oidc/discovery"
4745
require "activeadmin/oidc/presets/zitadel"
4846
require "activeadmin/oidc/user_provisioner"
4947
require "rails/engine"
5048
require "activeadmin/oidc/engine"
51-
require "activeadmin/oidc/omniauth_options"
52-
53-
module ActiveAdmin
54-
module Oidc
55-
def self.omniauth_options
56-
OmniauthOptions.build(config)
57-
end
58-
end
59-
end

lib/activeadmin/oidc/configuration.rb

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ module ActiveAdmin
44
module Oidc
55
class Configuration
66
DEFAULT_SCOPE = "openid email profile"
7-
DEFAULT_CLOCK_SKEW = 30
87
DEFAULT_TIMEOUT = 5
98
DEFAULT_IDENTITY_ATTRIBUTE = :email
109
DEFAULT_IDENTITY_CLAIM = :email
@@ -13,7 +12,7 @@ class Configuration
1312
"Your account has no permission to access this admin panel."
1413

1514
attr_accessor :issuer, :client_id, :client_secret, :scope,
16-
:login_button_label, :clock_skew, :timeout,
15+
:login_button_label, :timeout,
1716
:identity_attribute, :identity_claim,
1817
:access_denied_message, :on_login
1918

@@ -27,7 +26,6 @@ def reset!
2726
@client_secret = nil
2827
@scope = DEFAULT_SCOPE
2928
@login_button_label = DEFAULT_LOGIN_BUTTON_LABEL
30-
@clock_skew = DEFAULT_CLOCK_SKEW
3129
@timeout = DEFAULT_TIMEOUT
3230
@identity_attribute = DEFAULT_IDENTITY_ATTRIBUTE
3331
@identity_claim = DEFAULT_IDENTITY_CLAIM

lib/activeadmin/oidc/discovery.rb

Lines changed: 0 additions & 99 deletions
This file was deleted.

lib/activeadmin/oidc/omniauth_options.rb

Lines changed: 0 additions & 32 deletions
This file was deleted.

spec/unit/configuration_spec.rb

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,6 @@
1010
expect(config.scope).to eq("openid email profile")
1111
end
1212

13-
it "has a default clock_skew of 30 seconds" do
14-
expect(config.clock_skew).to eq(30)
15-
end
16-
1713
it "has a default timeout of 5 seconds" do
1814
expect(config.timeout).to eq(5)
1915
end
@@ -162,7 +158,6 @@
162158
config.client_secret = "sec"
163159
config.scope = "openid email"
164160
config.login_button_label = "Sign in"
165-
config.clock_skew = 60
166161
config.timeout = 10
167162
config.identity_attribute = :username
168163
config.identity_claim = :preferred_username
@@ -175,7 +170,6 @@
175170
expect(config.client_secret).to eq("sec")
176171
expect(config.scope).to eq("openid email")
177172
expect(config.login_button_label).to eq("Sign in")
178-
expect(config.clock_skew).to eq(60)
179173
expect(config.timeout).to eq(10)
180174
expect(config.identity_attribute).to eq(:username)
181175
expect(config.identity_claim).to eq(:preferred_username)

spec/unit/discovery_spec.rb

Lines changed: 0 additions & 89 deletions
This file was deleted.

0 commit comments

Comments
 (0)