diff --git a/.envrc b/.envrc index 1164e64..9ec6ade 100644 --- a/.envrc +++ b/.envrc @@ -22,7 +22,7 @@ export K_SOUP_COV_COMMAND_NAME="Test Coverage" # Available formats are html, xml, rcov, lcov, json, tty export K_SOUP_COV_FORMATTERS="html,xml,rcov,lcov,json,tty" export K_SOUP_COV_MIN_BRANCH=78 # Means you want to enforce X% branch coverage -export K_SOUP_COV_MIN_LINE=98 # Means you want to enforce X% line coverage +export K_SOUP_COV_MIN_LINE=97 # Means you want to enforce X% line coverage export K_SOUP_COV_MIN_HARD=true # Means you want the build to fail if the coverage thresholds are not met export K_SOUP_COV_MULTI_FORMATTERS=true export K_SOUP_COV_OPEN_BIN= # Means don't try to open coverage results in browser diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 1d7963a..ee45a04 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -7,7 +7,7 @@ permissions: env: K_SOUP_COV_MIN_BRANCH: 78 - K_SOUP_COV_MIN_LINE: 98 + K_SOUP_COV_MIN_LINE: 97 K_SOUP_COV_MIN_HARD: true K_SOUP_COV_FORMATTERS: "xml,rcov,lcov,tty" K_SOUP_COV_DO: true @@ -115,7 +115,7 @@ jobs: hide_complexity: true indicators: true output: both - thresholds: '98 78' + thresholds: '97 78' continue-on-error: ${{ matrix.experimental != 'false' }} - name: Add Coverage PR Comment diff --git a/.rubocop_gradual.lock b/.rubocop_gradual.lock index a43bf2c..23e729b 100644 --- a/.rubocop_gradual.lock +++ b/.rubocop_gradual.lock @@ -30,7 +30,7 @@ [47, 7, 38, "RSpec/AnyInstance: Avoid stubbing using `allow_any_instance_of`.", 3627954156], [84, 7, 48, "RSpec/AnyInstance: Avoid stubbing using `allow_any_instance_of`.", 2759780562] ], - "spec/omniauth/strategies/ldap_spec.rb:1003951887": [ + "spec/omniauth/strategies/ldap_spec.rb:1788355205": [ [14, 3, 54, "RSpec/LeakyConstantDeclaration: Stub class constant instead of declaring explicitly.", 2419068710], [90, 13, 9, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1130140517], [145, 17, 28, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 3444838747], diff --git a/CHANGELOG.md b/CHANGELOG.md index 069ba29..1b73744 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,8 @@ Please file a bug if you notice a violation of semantic versioning. - Improved code coverage to 98% lines and 78% branches - Added integration tests with a complete Roda-based demo app for specs - Well tested support for all versions of OmniAuth >= v1 and Rack >= v1 via appraisals +- Documentation for why auth.uid == dn +- Support for LDAP-based SSO identity via HTTP Header ### Changed diff --git a/README.md b/README.md index 54c853d..ba11cab 100644 --- a/README.md +++ b/README.md @@ -200,8 +200,8 @@ The following options are available for configuring the OmniAuth LDAP strategy: Why DN for `auth.uid`? -- DN is the canonical, globally unique identifier for an LDAP entry and is always present in search results. See LDAPv3 and DN syntax: RFC 4511 (LDAP protocol) and RFC 4514 (String Representation of Distinguished Names). -- Attributes like `uid` (defined in RFC 4519) or `sAMAccountName` (Active Directory–specific) may be absent, duplicated across parts of the DIT, or vary between directories. Using DN ensures consistent behavior across AD, OpenLDAP, and other servers. +- DN is the canonical, globally unique identifier for an LDAP entry and is always present in search results. See LDAPv3 and DN syntax: [RFC 4511][rfc4511] (LDAP protocol) and [RFC 4514][rfc4514] (String Representation of Distinguished Names). +- Attributes like `uid` (defined in [RFC 4519][rfc4519]) or `sAMAccountName` (Active Directory–specific) may be absent, duplicated across parts of the DIT, or vary between directories. Using DN ensures consistent behavior across AD, OpenLDAP, and other servers. - This trade-off favors cross-directory interoperability and stability for apps that need a unique identifier. Where to find the "username"-style value @@ -341,6 +341,67 @@ provider :ldap, This trims `alice@example.com` to `alice` before searching. +### Trusted header SSO (REMOTE_USER and friends) + +Some deployments terminate SSO at a reverse proxy or portal and forward the already-authenticated user identity via an HTTP header such as `REMOTE_USER`. +When you enable this mode, the LDAP strategy will trust the upstream header, perform a directory lookup for that user, and complete OmniAuth without asking the user for a password. + +Important: Only enable this behind a trusted front-end that strips and sets the header itself. Never enable on a public endpoint without such a gateway, or an attacker could spoof the header. + +Configuration options: + +- `:header_auth` (Boolean, default: false) — Enable trusted header SSO. +- `:header_name` (String, default: "REMOTE_USER") — The env/header key to read. The strategy checks both `env["REMOTE_USER"]` and the Rack variant `env["HTTP_REMOTE_USER"]`. +- `:name_proc` is applied to the header value before search (e.g., to strip a domain part). +- Search is done using your configured `:uid` or `:filter` and the service bind (`:bind_dn`/`:password`) or anonymous bind if allowed. + +Minimal Rack example: + +```ruby +use OmniAuth::Builder do + provider :ldap, + host: "ldap.example.com", + base: "dc=example,dc=com", + uid: "uid", + bind_dn: "cn=search,dc=example,dc=com", + password: ENV["LDAP_SEARCH_PASSWORD"], + header_auth: true, # trust REMOTE_USER + header_name: "REMOTE_USER", # default + name_proc: proc { |n| n.split("@").first } +end +``` + +Rails initializer example: + +```ruby +Rails.application.config.middleware.use(OmniAuth::Builder) do + provider :ldap, + title: "Acme LDAP", + host: "ldap.acme.internal", + base: "dc=acme,dc=corp", + uid: "sAMAccountName", + bind_dn: "cn=search,dc=acme,dc=corp", + password: ENV["LDAP_SEARCH_PASSWORD"], + header_auth: true, + header_name: "REMOTE_USER", + # Optionally restrict with a group filter while using the header value + filter: "(&(sAMAccountName=%{username})(memberOf=cn=myapp-users,ou=groups,dc=acme,dc=corp))", + name_proc: proc { |n| n.gsub(/@.*$/, "") } +end +``` + +Flow: + +- If `header_auth` is on and the header is present when the request hits `/auth/ldap`, the strategy immediately redirects to `/auth/ldap/callback`. +- In the callback, the strategy searches the directory for that user and maps their attributes; no user password bind is attempted. +- If the header is missing (or `header_auth` is false), the normal username/password form flow is used. + +Security checklist: + +- Ensure your reverse proxy strips user-controlled copies of the header and sets the canonical `REMOTE_USER` itself. +- Prefer TLS-secured internal links between the proxy and your app. +- Consider also restricting with a group-based `:filter` so only authorized users can sign in. + ## 🦷 FLOSS Funding While these tools are free software and will always be, the project would benefit immensely from some funding. diff --git a/lib/omniauth/strategies/ldap.rb b/lib/omniauth/strategies/ldap.rb index ddadbd6..a00e7e7 100644 --- a/lib/omniauth/strategies/ldap.rb +++ b/lib/omniauth/strategies/ldap.rb @@ -39,6 +39,13 @@ class LDAP option :ssl_version, nil # use OpenSSL default if nil option :uid, "sAMAccountName" option :name_proc, lambda { |n| n } + # Trusted header SSO support (disabled by default) + # :header_auth - when true and the header is present, the strategy trusts the upstream gateway + # and searches the directory for the user without requiring a user password. + # :header_name - which header/env key to read (default: "REMOTE_USER"). We will also check the + # standard Rack "HTTP_" variant automatically. + option :header_auth, false + option :header_name, "REMOTE_USER" def request_phase # OmniAuth >= 2.0 expects the request phase to be POST-only for /auth/:provider. @@ -47,6 +54,12 @@ def request_phase return Rack::Response.new("", 404, {"Content-Type" => "text/plain"}).finish end + # Fast-path: if a trusted identity header is present, skip the login form + # and jump to the callback where we will complete using directory lookup. + if header_username + return Rack::Response.new([], 302, "Location" => callback_path).finish + end + # If credentials were POSTed directly to /auth/:provider, redirect to the callback path. # This mirrors the behavior of many OmniAuth providers and allows test helpers (like # OmniAuth::Test::PhonySession) to populate `env['omniauth.auth']` on the callback request. @@ -66,6 +79,22 @@ def callback_phase @adaptor = OmniAuth::LDAP::Adaptor.new(@options) return fail!(:invalid_request_method) unless valid_request_method? + + # Header-based SSO (REMOTE_USER-style) path + if (hu = header_username) + begin + entry = directory_lookup(@adaptor, hu) + unless entry + return fail!(:invalid_credentials, InvalidCredentialsError.new("User not found for header #{hu}")) + end + @ldap_user_info = entry + @user_info = self.class.map_user(CONFIG, @ldap_user_info) + return super + rescue => e + return fail!(:ldap_error, e) + end + end + return fail!(:missing_credentials) if missing_credentials? begin @ldap_user_info = @adaptor.bind_as(filter: filter(@adaptor), size: 1, password: request.params["password"]) @@ -81,12 +110,12 @@ def callback_phase end end - def filter(adaptor) + def filter(adaptor, username_override = nil) if adaptor.filter && !adaptor.filter.empty? - username = Net::LDAP::Filter.escape(@options[:name_proc].call(request.params["username"])) + username = Net::LDAP::Filter.escape(@options[:name_proc].call(username_override || request.params["username"])) Net::LDAP::Filter.construct(adaptor.filter % {username: username}) else - Net::LDAP::Filter.equals(adaptor.uid, @options[:name_proc].call(request.params["username"])) + Net::LDAP::Filter.equals(adaptor.uid, @options[:name_proc].call(username_override || request.params["username"])) end end @@ -146,6 +175,31 @@ def valid_request_method? def missing_credentials? request.params["username"].nil? || request.params["username"].empty? || request.params["password"].nil? || request.params["password"].empty? end # missing_credentials? + + # Extract a normalized username from a trusted header when enabled. + # Returns nil when not configured or not present. + def header_username + return unless options[:header_auth] + + name = options[:header_name] || "REMOTE_USER" + # Try both the raw env var (e.g., REMOTE_USER) and the Rack HTTP_ variant (e.g., HTTP_REMOTE_USER or HTTP_X_REMOTE_USER) + raw = request.env[name] || request.env["HTTP_#{name.upcase.tr("-", "_")}"] + return if raw.nil? || raw.to_s.strip.empty? + + options[:name_proc].call(raw.to_s) + end + + # Perform a directory lookup for the given username using the strategy configuration + # (bind_dn/password or anonymous). Does not attempt to bind as the user. + def directory_lookup(adaptor, username) + entry = nil + filter = filter(adaptor, username) + adaptor.connection.open do |conn| + rs = conn.search(filter: filter, size: 1) + entry = rs.first if rs && rs.first + end + entry + end end end end diff --git a/sig/omniauth/strategies/ldap.rbs b/sig/omniauth/strategies/ldap.rbs index 04a629c..58be1f6 100644 --- a/sig/omniauth/strategies/ldap.rbs +++ b/sig/omniauth/strategies/ldap.rbs @@ -13,12 +13,20 @@ module OmniAuth def callback_phase: () -> untyped # Accepts an adaptor and returns a Net::LDAP::Filter or similar + # Optional second argument allows overriding the username (used for header-based SSO) def filter: (OmniAuth::LDAP::Adaptor) -> Net::LDAP::Filter + | (OmniAuth::LDAP::Adaptor, String?) -> Net::LDAP::Filter # Map a user object (Net::LDAP::Entry-like) into a Hash for the auth info def self.map_user: (Hash[String, untyped], untyped) -> Hash[String, untyped] def missing_credentials?: () -> bool + + # Extract username from a trusted header when enabled + def header_username: () -> (String | nil) + + # Perform a directory lookup for a given username; returns an Entry or nil + def directory_lookup: (OmniAuth::LDAP::Adaptor, String) -> untyped end end end diff --git a/spec/omniauth/strategies/ldap_spec.rb b/spec/omniauth/strategies/ldap_spec.rb index d438b3a..9f1ebb8 100644 --- a/spec/omniauth/strategies/ldap_spec.rb +++ b/spec/omniauth/strategies/ldap_spec.rb @@ -357,4 +357,94 @@ def make_env(path = "/auth/ldap", props = {}) expect(auth.info.nickname).to eq "ping" # comes from sAMAccountName end end + + # Header-based SSO (REMOTE_USER) support + describe "trusted header SSO" do + let(:app) do + Rack::Builder.new do + use OmniAuth::Test::PhonySession + use MyHeaderProvider, + name: "ldap", + title: "Header LDAP", + host: "ldap.example.com", + base: "dc=example,dc=com", + uid: "uid", + header_auth: true, + header_name: "REMOTE_USER", + name_proc: proc { |n| n.gsub(/@.*$/, "") } + run lambda { |env| [404, {"Content-Type" => "text/plain"}, [env.key?("omniauth.auth").to_s]] } + end.to_app + end + + before do + ldap_strategy = Class.new(OmniAuth::Strategies::LDAP) + stub_const("MyHeaderProvider", ldap_strategy) + @adaptor = double(OmniAuth::LDAP::Adaptor, {uid: "uid", filter: nil}) + allow(OmniAuth::LDAP::Adaptor).to receive(:new) { @adaptor } + end + + def connection_returning(entry) + searcher = double("ldap search conn") + allow(searcher).to receive(:search).and_return(entry ? [entry] : []) + conn = double("ldap connection") + allow(conn).to receive(:open).and_yield(searcher) + conn + end + + it "redirects from request phase when header present" do + env = {"rack.session" => {}, "REQUEST_METHOD" => "POST", "PATH_INFO" => "/auth/ldap", "REMOTE_USER" => "alice"} + post "/auth/ldap", nil, env + expect(last_response).to be_redirect + expect(last_response.headers["Location"]).to eq "/auth/ldap/callback" + end + + it "authenticates on callback without password using REMOTE_USER" do + entry = Net::LDAP::Entry.from_single_ldif_string(%{dn: cn=alice, dc=example, dc=com +uid: alice +mail: alice@example.com +}) + allow(@adaptor).to receive(:connection).and_return(connection_returning(entry)) + + post "/auth/ldap/callback", nil, {"REMOTE_USER" => "alice"} + + expect(last_response).not_to be_redirect + auth = last_request.env["omniauth.auth"] + expect(auth.uid).to eq "cn=alice, dc=example, dc=com" + expect(auth.info.nickname).to eq "alice" + end + + it "authenticates on callback with HTTP_ header variant" do + entry = Net::LDAP::Entry.from_single_ldif_string(%{dn: cn=alice, dc=example, dc=com +uid: alice +}) + allow(@adaptor).to receive(:connection).and_return(connection_returning(entry)) + + post "/auth/ldap/callback", nil, {"HTTP_REMOTE_USER" => "alice"} + expect(last_response).not_to be_redirect + auth = last_request.env["omniauth.auth"] + expect(auth.info.nickname).to eq "alice" + end + + it "applies name_proc and filter mapping when provided" do + # search result + entry = Net::LDAP::Entry.from_single_ldif_string(%{dn: cn=alice, dc=example, dc=com + uid: alice + }) + allow(@adaptor).to receive_messages( + filter: "uid=%{username}", + connection: connection_returning(entry), + ) + expect(Net::LDAP::Filter).to receive(:construct).with("uid=alice").and_call_original + + post "/auth/ldap/callback", nil, {"REMOTE_USER" => "alice@example.com"} + expect(last_response).not_to be_redirect + end + + it "fails when directory lookup returns no entry" do + allow(@adaptor).to receive(:connection).and_return(connection_returning(nil)) + post "/auth/ldap/callback", nil, {"REMOTE_USER" => "missing"} + expect(last_response).to be_redirect + expect(last_response.headers["Location"]).to match(/invalid_credentials/) + end + end end