Merge pull request #19738 from opf/fix/oidc-attribute-mapping-66242

Fix/OIDC attribute mapping 66242

modules/openid_connect/app/models/openid_connect/provider/hash_builder.rb

@@ -55,8 +55,11 @@ module OpenIDConnect
         token_endpoint:,
         limit_self_registration:,
         end_session_endpoint:,
-        attribute_map:
-      }.merge(attribute_map)
+        attribute_map:,
+        post_logout_redirect_uri:,
+        claims:,
+        acr_values:
+      }
        .merge(provider_specific_to_h)
        .compact_blank
     end

modules/openid_connect/app/services/openid_connect/configuration_mapper.rb

@@ -62,9 +62,9 @@ module OpenIDConnect
         "end_session_endpoint" => extract_url(options, "end_session_endpoint"),
         "jwks_uri" => extract_url(options, "jwks_uri"),
         "mapping_login" => options.dig("attribute_map", "login"),
-        "mapping_mail" => options.dig("attribute_map", "email"),
-        "mapping_firstname" => options.dig("attribute_map", "first_name"),
-        "mapping_lastname" => options.dig("attribute_map", "last_name"),
+        "mapping_email" => options.dig("attribute_map", "email"),
+        "mapping_first_name" => options.dig("attribute_map", "first_name"),
+        "mapping_last_name" => options.dig("attribute_map", "last_name"),
         "mapping_admin" => options.dig("attribute_map", "admin")
       }.compact
     end

modules/openid_connect/spec/models/provider_spec.rb

@@ -54,4 +54,71 @@ RSpec.describe OpenIDConnect::Provider do
       it { is_expected.to be_falsey }
     end
   end
+
+  describe "#to_h" do
+    subject { provider.to_h }
+
+    let(:options) { raise "define me!" }
+
+    before do
+      options.stringify_keys.each do |opt, value|
+        provider.options[opt] = value
+      end
+    end
+
+    describe "with claims" do
+      let(:options) { { claims: "login" } }
+
+      it "includes the claims" do
+        expect(subject[:claims]).to eq "login"
+      end
+    end
+
+    describe "with acr_values" do
+      let(:options) { { acr_values: "phr" } }
+
+      it "includes the acr values" do
+        expect(subject[:acr_values]).to eq "phr"
+      end
+    end
+
+    describe "with mapped attributes" do
+      let(:options) do
+        {
+          mapping_email: :address,
+          mapping_login: :logout,
+          mapping_first_name: :given_name,
+          mapping_last_name: :surname
+        }
+      end
+
+      let(:expected_value) do
+        {
+          email: :address,
+          login: :logout,
+          first_name: :given_name,
+          last_name: :surname
+        }
+      end
+
+      it "contains the resulting attribute map being passed to omniauth-openid-connect" do
+        expect(subject[:attribute_map]).to eq expected_value
+      end
+
+      it "does not turn them into superfluous attributes" do
+        expect(subject).not_to include :email
+        expect(subject).not_to include :login
+        expect(subject).not_to include :first_name
+        expect(subject).not_to include :last_name
+      end
+    end
+
+    describe "with post_logout_redirect_uri" do
+      let(:options) { { post_logout_redirect_uri: "https://www.openproject.org" } }
+
+      it "contains the option" do
+        expect(subject[:post_logout_redirect_uri]).to eq options[:post_logout_redirect_uri]
+      end
+    end
+  end
 end