📝 Document usage of filter config

Author: pboling

.rubocop_gradual.lock

@@ -30,15 +30,14 @@
     [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: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],
-    [154, 17, 23, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1584148894],
-    [165, 17, 32, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1515076977],
-    [174, 19, 19, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 2526348694],
-    [187, 17, 56, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 2413495789],
-    [202, 13, 9, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 3182939526],
-    [235, 15, 19, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 2526348694]
+  "spec/omniauth/strategies/ldap_spec.rb:783052937": [
+    [93, 13, 9, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1130140517],
+    [148, 17, 28, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 3444838747],
+    [157, 17, 23, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1584148894],
+    [168, 17, 32, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1515076977],
+    [177, 19, 19, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 2526348694],
+    [203, 17, 56, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 2413495789],
+    [218, 13, 9, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 3182939526],
+    [251, 15, 19, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 2526348694]
   ]
 }

README.md

@@ -312,6 +312,66 @@ provider :ldap,
   password: ENV["LDAP_SEARCH_PASSWORD"]
 ```
 
+What `:filter` actually does
+
+- If `:filter` is provided, the strategy constructs an LDAP filter string by substituting `%{username}` with the submitted username after applying `:name_proc`, escaping special characters per RFC 4515, and passes it to the directory search.
+- In the normal password flow, a successful search returns the user's DN and we then bind as that DN with the submitted password.
+- In trusted header SSO flow (`header_auth: true`), we only perform the search and skip the user password bind; if the search returns no entry, authentication fails.
+- If `:filter` is not provided, the strategy falls back to a simple equality filter using `:uid` (e.g. `(uid=alice)`).
+
+Notes on escaping and safety
+
+- We escape the interpolated username with `Net::LDAP::Filter.escape`, which protects against LDAP injection and handles special characters like `(`, `)`, `*`, and `\`.
+- Your static filter text is used as-is — keep it to a valid LDAP filter expression and only use `%{username}` for substitution.
+
+Group-based recipes
+
+- Active Directory (simple group):
+
+  ```text
+  (&(sAMAccountName=%{username})(memberOf=cn=myapp-users,ou=groups,dc=example,dc=com))
+  ```
+
+- Active Directory (nested groups via matchingRuleInChain):
+
+  ```text
+  (&(sAMAccountName=%{username})(memberOf:1.2.840.113556.1.4.1941:=cn=myapp-users,ou=groups,dc=example,dc=com))
+  ```
+
+- OpenLDAP (groupOfNames):
+
+  ```text
+  (&(uid=%{username})(memberOf=cn=myapp-users,ou=groups,dc=example,dc=com))
+  ```
+
+  or, if you can't use `memberOf` overlays, filter on the group and member DN:
+
+  ```text
+  (&(uid=%{username})(|(uniqueMember=uid=%{username},ou=people,dc=example,dc=com)(member=uid=%{username},ou=people,dc=example,dc=com)))
+  ```
+
+Username normalization examples
+
+- If your users sign in with an email but the directory expects a short name, combine `:name_proc` with `:filter`:
+
+  ```ruby
+  provider :ldap,
+    name_proc: proc { |n| n.split("@").first },
+    filter: "(&(sAMAccountName=%{username})(memberOf=cn=myapp-users,ou=groups,dc=example,dc=com))"
+    # other settings...
+  ```
+
+Discourse plugin (jonmbake/discourse-ldap-auth)
+
+- That plugin forwards its `filter` setting to this gem. You can therefore paste the same filter strings shown above.
+- Example (allow only members of `forum-users`):
+
+  ```text
+  (&(uid=%{username})(memberOf=cn=forum-users,ou=groups,dc=example,dc=com))
+  ```
+
+- If users type an email address but your directory matches on a short user id, also configure `name_proc` accordingly in your app (or the plugin, if supported).
+
 ### SASL (advanced)
 
 SASL enables alternative bind mechanisms. Only enable if you understand the server-side requirements.
@@ -325,7 +385,7 @@ provider :ldap,
   uid: "uid"
 ```
 
-Supported mechanisms include `"DIGEST-MD5"` and `"GSS-SPNEGO"` depending on your environment and gems.
+Supported mechanisms include "DIGEST-MD5" and "GSS-SPNEGO" depending on your environment and gems.
 
 ### Name processing and examples
 

spec/omniauth/strategies/ldap_spec.rb

@@ -11,7 +11,10 @@
   # :name_proc => Proc.new {|name| name.gsub(/@.*$/,'')}
   # :bind_dn => 'default_bind_dn'
   # :password => 'password'
-  class MyLdapProvider < OmniAuth::Strategies::LDAP; end
+  before do
+    ldap_strategy = Class.new(OmniAuth::Strategies::LDAP)
+    stub_const("MyLdapProvider", ldap_strategy)
+  end
 
   let(:app) do
     Rack::Builder.new {
@@ -181,6 +184,19 @@ def make_env(path = "/auth/ldap", props = {})
               expect(last_response.headers["Location"]).to match("invalid_credentials")
               expect(last_request.env["omniauth.error"].message).to eq("Invalid credentials for ping")
             end
+
+            it "supports group restriction filters and applies name_proc" do
+              # Complex filter with %{username} placeholder and group membership
+              group_filter = "(&(uid=%{username})(memberOf=cn=forum-users,ou=groups,dc=example,dc=com))"
+              allow(@adaptor).to receive(:filter).and_return(group_filter)
+              # username has a domain part; name_proc on strategy under test strips it
+              expect(Net::LDAP::Filter).to receive(:construct).with("(&(uid=alice)(memberOf=cn=forum-users,ou=groups,dc=example,dc=com))")
+
+              post("/auth/ldap/callback", {username: "[email protected]", password: "password"})
+
+              expect(last_response).to be_redirect
+              expect(last_response.headers["Location"]).to match("invalid_credentials")
+            end
           end
         end
 
@@ -240,6 +256,29 @@ def make_env(path = "/auth/ldap", props = {})
 
           expect(last_response).not_to be_redirect
         end
+
+        it "escapes special characters in username when building filter" do
+          allow(@adaptor).to receive(:filter).and_return("uid=%{username}")
+          # '(' => \28 and ')' => \29 per RFC 4515 escaping
+          expect(Net::LDAP::Filter).to receive(:construct).with("uid=al\\28ice\\29")
+          post("/auth/ldap/callback", {username: "al(ice)", password: "secret"})
+        end
+
+        it "binds with complex group filter and applies name_proc" do
+          allow(@adaptor).to receive(:bind_as) {
+            Net::LDAP::Entry.from_single_ldif_string(
+              %{dn: cn=alice, dc=example, dc=com
+uid: alice
+},
+            )
+          }
+          allow(@adaptor).to receive(:filter).and_return("(&(uid=%{username})(memberOf=cn=forum-users,ou=groups,dc=example,dc=com))")
+          expect(Net::LDAP::Filter).to receive(:construct).with("(&(uid=alice)(memberOf=cn=forum-users,ou=groups,dc=example,dc=com))")
+
+          post("/auth/ldap/callback", {username: "[email protected]", password: "secret"})
+          expect(last_response).not_to be_redirect
+          expect(last_request.env["omniauth.auth"].info.nickname).to eq "alice"
+        end
       end
 
       it "maps user info to Auth Hash" do
@@ -440,11 +479,41 @@ def connection_returning(entry)
       expect(last_response).not_to be_redirect
     end
 
+    it "escapes special characters in header SSO username when building filter" do
+      entry = Net::LDAP::Entry.from_single_ldif_string(%{dn: cn=al\\28ice\\29, dc=example, dc=com
+uid: al(ice)
+})
+      allow(@adaptor).to receive_messages(
+        connection: connection_returning(entry),
+        filter: "uid=%{username}",
+      )
+      expect(Net::LDAP::Filter).to receive(:construct).with("uid=al\\28ice\\29").and_call_original
+
+      post "/auth/ldap/callback", nil, {"REMOTE_USER" => "al(ice)"}
+      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
+
+    it "supports complex group filter with %{username} in header SSO path" do
+      # Expect that the complex filter string is constructed with the processed username
+      expect(Net::LDAP::Filter).to receive(:construct).with("(&(uid=alice)(memberOf=cn=forum-users,ou=groups,dc=example,dc=com))").and_call_original
+
+      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})(memberOf=cn=forum-users,ou=groups,dc=example,dc=com))",
+        connection: connection_returning(entry),
+      )
+
+      post "/auth/ldap/callback", nil, {"REMOTE_USER" => "[email protected]"}
+      expect(last_response).not_to be_redirect
+    end
   end
 end