Merge pull request #101 from omniauth/feat/spec-auth-uid-as-dn

Author: pboling

.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:86189447": [
+  "spec/omniauth/strategies/ldap_spec.rb:1003951887": [
     [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],

README.md

@@ -179,7 +179,7 @@ The following options are available for configuring the OmniAuth LDAP strategy:
 - `:port` - The port number of the LDAP server (default: 389).
 - `:method` - The connection method. Allowed values: `:plain`, `:ssl`, `:tls` (default: `:plain`).
 - `:base` - The base DN for the LDAP search.
-- `:uid` or `:filter` - Either `:uid` (the LDAP attribute for username, default: "sAMAccountName") or `:filter` (LDAP filter for searching user entries). If `:filter` is provided, `:uid` is not required.
+- `:uid` or `:filter` - Either `:uid` (the LDAP attribute for username, default: "sAMAccountName") or `:filter` (LDAP filter for searching user entries). If `:filter` is provided, `:uid` is not required. Note: This `:uid` option is the search attribute, not the top-level `auth.uid` in the OmniAuth result.
 
 ### Optional Options
 
@@ -192,6 +192,37 @@ The following options are available for configuring the OmniAuth LDAP strategy:
 - `:allow_anonymous` - Whether to allow anonymous binding (default: false).
 - `:logger` - A logger instance for debugging (optional, for internal use).
 
+### Auth Hash UID vs LDAP :uid (search attribute)
+
+- By design, the top-level `auth.uid` returned by this strategy is the entry's Distinguished Name (DN).
+- The configuration option `:uid` controls which LDAP attribute is used to locate the entry (or to build the filter), not the value exposed as `auth.uid`.
+- Your LDAP "account name" (for example, `sAMAccountName` on Active Directory or `uid` on many schemas) is exposed via `auth.info.nickname` and is also available in `auth.extra.raw_info`.
+
+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.
+- This trade-off favors cross-directory interoperability and stability for apps that need a unique identifier.
+
+Where to find the "username"-style value
+
+- `auth.info.nickname` maps from the first present of: `uid`, `userid`, or `sAMAccountName`.
+- You can also read the raw attribute from `auth.extra.raw_info` (a `Net::LDAP::Entry`):
+
+```ruby
+get "/auth/ldap/callback" do
+  auth = request.env["omniauth.auth"]
+  dn = auth.uid                                # => "cn=alice,ou=users,dc=example,dc=com"
+  username = auth.info.nickname                # => "alice" (from uid/sAMAccountName)
+  # Or, directly from raw_info (case-insensitive keys):
+  sams = auth.extra.raw_info[:samaccountname]
+  sam = sams.first if sams
+  # ...
+end
+```
+
+If you need top-level `auth.uid` to be something other than the DN (for example, `sAMAccountName`), you'll currently need to read it from `auth.info.nickname` (or `raw_info`) in your app. Changing the top-level `uid` mapping would be a breaking behavior change for existing users; if you have a use-case, please open an issue to discuss a configurable mapping.
+
 ## 🔧 Basic Usage
 
 The strategy exposes a simple Rack middleware and can be used in plain Rack apps, Sinatra, or Rails.
@@ -655,3 +686,8 @@ Thanks for RTFM. ☺️
 [💎appraisal2]: https://github.com/appraisal-rb/appraisal2
 [💎appraisal2-img]: https://img.shields.io/badge/appraised_by-appraisal2-34495e.svg?plastic&logo=ruby&logoColor=white
 [💎d-in-dvcs]: https://railsbling.com/posts/dvcs/put_the_d_in_dvcs/
+
+[//]: # (LDAP RFC references)
+[rfc4511]: https://datatracker.ietf.org/doc/html/rfc4511
+[rfc4514]: https://datatracker.ietf.org/doc/html/rfc4514
+[rfc4519]: https://datatracker.ietf.org/doc/html/rfc4519

spec/omniauth/strategies/ldap_spec.rb

@@ -308,4 +308,53 @@ def make_env(path = "/auth/ldap", props = {})
       end
     end
   end
+
+  # Validate uid behavior specifically when using sAMAccountName
+  describe "uid behavior with sAMAccountName option" do
+    let(:app) do
+      Rack::Builder.new do
+        use OmniAuth::Test::PhonySession
+        use MySamaccountnameProvider,
+          name: "ldap",
+          title: "My LDAP",
+          host: "1.2.3.4",
+          port: 636,
+          method: "ssl",
+          base: "ou=snip,dc=snip,dc=example,dc=com",
+          uid: "sAMAccountName",
+          bind_dn: "snip",
+          password: "snip"
+        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("MySamaccountnameProvider", ldap_strategy)
+      @adaptor = double(OmniAuth::LDAP::Adaptor, {uid: "sAMAccountName"})
+      allow(@adaptor).to receive(:filter)
+      allow(OmniAuth::LDAP::Adaptor).to receive(:new) { @adaptor }
+      # Return an entry that includes sAMAccountName but not uid, so nickname maps from sAMAccountName
+      allow(@adaptor).to receive(:bind_as).and_return(
+        Net::LDAP::Entry.from_single_ldif_string(
+          %{dn: cn=ping, dc=snip, dc=example, dc=com
+samaccountname: ping
+mail: [email protected]
+givenname: Ping
+sn: User
+},
+        ),
+      )
+    end
+
+    it "sets auth.uid to the DN (not the sAMAccountName attribute) and maps nickname from sAMAccountName" do
+      post("/auth/ldap/callback", {username: "ping", password: "secret"})
+
+      expect(last_response).not_to be_redirect
+
+      auth = last_request.env["omniauth.auth"]
+      expect(auth.uid).to eq "cn=ping, dc=snip, dc=example, dc=com"
+      expect(auth.info.nickname).to eq "ping" # comes from sAMAccountName
+    end
+  end
 end