Merge pull request #10 from handnot2/sp_entity_id_not_set

SP entity_id missing in SP metadata when it is not specified in config
handnot2 · Nov 16, 2017 · 3e5b231 · 3e5b231
2 parents c3aca8c + 1cb1b2b
commit 3e5b231
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 3 deletions.
diff --git a/lib/samly/idp_data.ex b/lib/samly/idp_data.ex
@@ -246,6 +246,12 @@ defmodule Samly.IdpData do
     idp_id_from = Application.get_env(:samly, :idp_id_from)
     path_segment_idp_id = if idp_id_from == :subdomain, do: nil, else: idp_data.id
 
+    sp_entity_id =
+      case sp_data.entity_id do
+        "" -> :undefined
+        id -> String.to_charlist(id)
+      end
+
     Esaml.esaml_sp(
       org:
         Esaml.esaml_org(
@@ -268,7 +274,7 @@ defmodule Samly.IdpData do
       metadata_uri: Helper.get_metadata_uri(idp_data.base_url, path_segment_idp_id),
       consume_uri: Helper.get_consume_uri(idp_data.base_url, path_segment_idp_id),
       logout_uri: Helper.get_logout_uri(idp_data.base_url, path_segment_idp_id),
-      entity_id: String.to_charlist(sp_data.entity_id)
+      entity_id: sp_entity_id
     )
   end
 

diff --git a/test/samly_idp_data_test.exs b/test/samly_idp_data_test.exs
@@ -10,16 +10,30 @@ defmodule SamlyIdpDataTest do
     keyfile: "test/data/test.pem"
   }
 
+  @sp_config2 %{
+    id: "sp2",
+    certfile: "test/data/test.crt",
+    keyfile: "test/data/test.pem"
+  }
+
   @idp_config1 %{
     id: "idp1",
     sp_id: "sp1",
     base_url: "http://samly.howto:4003/sso",
     metadata_file: "test/data/idp_metadata.xml"
   }
 
+  @idp_config2 %{
+    id: "idp2",
+    sp_id: "sp2",
+    base_url: "http://samly.howto:4003/sso",
+    metadata_file: "test/data/idp_metadata.xml"
+  }
+
   setup context do
-    sp_data = SpData.load_provider(@sp_config1)
-    [sps: %{sp_data.id => sp_data}] |> Enum.into(context)
+    sp_data1 = SpData.load_provider(@sp_config1)
+    sp_data2 = SpData.load_provider(@sp_config2)
+    [sps: %{sp_data1.id => sp_data1, sp_data2.id => sp_data2}] |> Enum.into(context)
   end
 
   test "valid-idp-config-1", %{sps: sps} do
@@ -146,6 +160,13 @@ defmodule SamlyIdpDataTest do
     assert slo_url |> List.to_string() |> String.ends_with?("/SAML2/Redirect/SLO")
   end
 
+  test "sp entity_id test-1", %{sps: sps} do
+    %IdpData{} = idp_data = IdpData.load_provider(@idp_config2, sps)
+    assert idp_data.valid?
+    Esaml.esaml_sp(entity_id: entity_id) = idp_data.esaml_sp_rec
+    assert entity_id == :undefined
+  end
+
   @tag :skip
   test "invalid-idp-config-1", %{sps: sps} do
     idp_config = %{@idp_config1 | id: ""}