Merge pull request #107 from omniauth/feat/custom-mapping

Author: pboling

.rubocop_gradual.lock

@@ -23,7 +23,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:1834231975": [
+  "spec/omniauth/strategies/ldap_spec.rb:4166458344": [
     [126, 13, 9, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1130140517],
     [181, 17, 28, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 3444838747],
     [190, 17, 23, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1584148894],

README.md

@@ -69,6 +69,11 @@ use OmniAuth::Strategies::LDAP,
   tls_options: {
     ssl_version: "TLSv1_2",
     ciphers: ["AES-128-CBC", "AES-128-CBC-HMAC-SHA1", "AES-128-CBC-HMAC-SHA256"],
+  },
+  mapping: {
+    "name" => "cn;lang-en",
+    "email" => ["preferredEmail", "mail"],
+    "nickname" => ["uid", "userid", "sAMAccountName"],
   }
 # Or, alternatively:
 # use OmniAuth::Strategies::LDAP, filter: '(&(uid=%{username})(memberOf=cn=myapp-users,ou=groups,dc=example,dc=com))'
@@ -199,6 +204,7 @@ The following options are available for configuring the OmniAuth LDAP strategy:
   - `adaptor.last_password_policy_response` — the matching password policy response control (implementation-specific object). This can indicate conditions such as password expired, account locked, reset required, or grace logins remaining (per the draft RFC).
 - `:connect_timeout` - Maximum time in seconds to wait when establishing the TCP connection to the LDAP server. Forwarded to `Net::LDAP`.
 - `:read_timeout` - Maximum time in seconds to wait for reads during LDAP operations (search/bind). Forwarded to `Net::LDAP`.
+- `:mapping` - Customize how LDAP attributes map to the returned `auth.info` hash. A sensible default mapping is built into the strategy and will be merged with your overrides. See `lib/omniauth/strategies/ldap.rb` for the default keys and behavior; values can be a String (single attribute), an Array (first present attribute wins), or a Hash (string pattern with placeholders like `%0` combined from multiple attributes).
 
 Example enabling password policy:
 

lib/omniauth/strategies/ldap.rb

@@ -9,7 +9,7 @@ class LDAP
 
       InvalidCredentialsError = Class.new(StandardError)
 
-      CONFIG = {
+      option :mapping, {
         "name" => "cn",
         "first_name" => "givenName",
         "last_name" => "sn",
@@ -23,7 +23,7 @@ class LDAP
         "url" => ["wwwhomepage"],
         "image" => "jpegPhoto",
         "description" => "description",
-      }.freeze
+      }
       option :title, "LDAP Authentication" # default title for authentication form
       # For OmniAuth >= 2.0 the default allowed request method is POST only.
       # Ensure the strategy follows that default so GET /auth/:provider returns 404 as expected in tests.
@@ -91,7 +91,7 @@ def callback_phase
               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)
+            @user_info = self.class.map_user(@options[:mapping], @ldap_user_info)
             return super
           rescue => e
             return fail!(:ldap_error, e)
@@ -111,7 +111,7 @@ def callback_phase
           # Optionally attach policy info even on success (e.g., timeBeforeExpiration)
           attach_password_policy_env(@adaptor)
 
-          @user_info = self.class.map_user(CONFIG, @ldap_user_info)
+          @user_info = self.class.map_user(@options[:mapping], @ldap_user_info)
           super
         rescue => e
           fail!(:ldap_error, e)

sig/omniauth/strategies/ldap.rbs

@@ -3,9 +3,6 @@ module OmniAuth
     class LDAP
       OMNIAUTH_GTE_V2: bool
 
-      # CONFIG is a read-only mapping of string keys to mapping definitions
-      CONFIG: Hash[String, untyped]
-
       # The request_phase either returns a Rack-compatible response or the form response.
       def request_phase: () -> (Rack::Response | Array[untyped] | String)
 

spec/omniauth/strategies/ldap_spec.rb

@@ -347,7 +347,7 @@ def make_env(path = "/auth/ldap", props = {})
         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")
+          expect(Net::LDAP::Filter).to receive(:construct).with('uid=al\28ice\29')
           post("/auth/ldap/callback", {username: "al(ice)", password: "secret"})
         end
 
@@ -432,6 +432,22 @@ def make_env(path = "/auth/ldap", props = {})
         expect(info.image).to eq "http://www.intridea.com/ping.jpg"
         expect(info.description).to eq "omniauth-ldap"
       end
+
+      context "when mapping is set" do
+        let(:app) do
+          Rack::Builder.new {
+            use OmniAuth::Test::PhonySession
+            use MyLdapProvider, name: "ldap", host: "192.168.1.145", base: "dc=score, dc=local", mapping: {"phone" => "mobile"}
+            run lambda { |env| [404, {"Content-Type" => "text/plain"}, [env.key?("omniauth.auth").to_s]] }
+          }.to_app
+        end
+
+        it "maps user info according to customized mapping" do
+          post("/auth/ldap/callback", {username: "ping", password: "password"})
+          expect(auth_hash.info.phone).to eq "444-444-4444"
+          expect(auth_hash.info.mobile).to eq "444-444-4444"
+        end
+      end
     end
   end
 
@@ -588,7 +604,7 @@ def connection_returning(entry)
         connection: connection_returning(entry),
         filter: "uid=%{username}",
       )
-      expect(Net::LDAP::Filter).to receive(:construct).with("uid=al\\28ice\\29").and_call_original
+      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
@@ -616,5 +632,41 @@ def connection_returning(entry)
       post "/auth/ldap/callback", nil, {"REMOTE_USER" => "[email protected]"}
       expect(last_response).not_to be_redirect
     end
+
+    context "with custom mapping option" 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 },
+            mapping: {"phone" => "mobile"}
+          run lambda { |env| [404, {"Content-Type" => "text/plain"}, [env.key?("omniauth.auth").to_s]] }
+        end.to_app
+      end
+
+      it "applies the custom mapping in header SSO path" do
+        entry = Net::LDAP::Entry.from_single_ldif_string(%{dn: cn=bob, dc=example, dc=com
+uid: bob
+mobile: 444-444-4444
+telephonenumber: 555-555-5555
+})
+        allow(@adaptor).to receive(:connection).and_return(connection_returning(entry))
+
+        post "/auth/ldap/callback", nil, {"REMOTE_USER" => "bob"}
+        expect(last_response).not_to be_redirect
+        auth = last_request.env["omniauth.auth"]
+        # phone should come from mobile due to custom mapping override
+        expect(auth.info.phone).to eq "444-444-4444"
+        # mobile remains available as well
+        expect(auth.info.mobile).to eq "444-444-4444"
+      end
+    end
   end
 end