Refactor SAML structs with separate groups of attributes

LauraBeatris · Mar 24, 2024 · 98c4ff5 · 98c4ff5
1 parent 931e0c0
commit 98c4ff5
Show file tree

Hide file tree

Showing 6 changed files with 147 additions and 39 deletions.
diff --git a/lib/saml.ex b/lib/saml.ex
@@ -32,7 +32,7 @@ defmodule ShinAuth.SAML do
     if valid_xml?(saml_request) do
       case DataSchema.to_struct(saml_request, Request) do
         {:ok, parsed_saml_request} -> {:ok, parsed_saml_request}
-        {:error, _} = error -> error
+        _ -> {:error, error}
       end
     else
       {:error, error}
@@ -63,7 +63,7 @@ defmodule ShinAuth.SAML do
     if valid_xml?(saml_response) do
       case DataSchema.to_struct(saml_response, Response) do
         {:ok, parsed_saml_response} -> {:ok, parsed_saml_response}
-        {:error, _} = error -> error
+        _ -> {:error, error}
       end
     else
       {:error, error}

diff --git a/lib/saml/request/request.ex b/lib/saml/request/request.ex
@@ -7,6 +7,20 @@ defmodule ShinAuth.SAML.Request do
 
   alias ShinAuth.SAML.Request.Utils
 
+  @type t ::
+          {:common, ShinAuth.SAML.Request.Common.t()}
+
+  @data_accessor ShinAuth.SAML.XMLHandler
+  data_schema(has_one: {:common, "/samlp:AuthnRequest", ShinAuth.SAML.Request.Common})
+end
+
+defmodule ShinAuth.SAML.Request.Common do
+  @moduledoc false
+
+  import DataSchema, only: [data_schema: 1]
+
+  alias ShinAuth.SAML.Request.Utils
+
   @type t ::
           {:id, String.t()}
           | {:version, String.t()}
@@ -16,19 +30,13 @@ defmodule ShinAuth.SAML.Request do
 
   @data_accessor ShinAuth.SAML.XMLHandler
   data_schema(
-    field: {:id, "/samlp:AuthnRequest/@ID", &{:ok, Utils.maybe_to_string(&1)}, optional: false},
+    field: {:id, "./@ID", &{:ok, Utils.maybe_to_string(&1)}, optional: false},
+    field: {:version, "./@Version", &{:ok, Utils.maybe_to_string(&1)}, optional: false},
     field:
-      {:version, "/samlp:AuthnRequest/@Version", &{:ok, Utils.maybe_to_string(&1)},
-       optional: false},
-    field:
-      {:assertion_consumer_service_url, "/samlp:AuthnRequest/@AssertionConsumerServiceURL",
+      {:assertion_consumer_service_url, "./@AssertionConsumerServiceURL",
        &{:ok, Utils.maybe_to_string(&1)}, optional: false},
-    field:
-      {:issuer, "/samlp:AuthnRequest/saml:Issuer/text()", &{:ok, Utils.maybe_to_string(&1)},
-       optional: false},
-    field:
-      {:issue_instant, "/samlp:AuthnRequest/@IssueInstant", &{:ok, Utils.maybe_to_string(&1)},
-       optional: false}
+    field: {:issuer, "./saml:Issuer/text()", &{:ok, Utils.maybe_to_string(&1)}, optional: false},
+    field: {:issue_instant, "./@IssueInstant", &{:ok, Utils.maybe_to_string(&1)}, optional: false}
   )
 end
 

diff --git a/lib/saml/response/response.ex b/lib/saml/response/response.ex
@@ -3,31 +3,124 @@ defmodule ShinAuth.SAML.Response do
   Parsed XML struct of a SAML response
   """
 
+  @type t ::
+          {:common, ShinAuth.SAML.Response.Common.t()}
+          | {:conditions, ShinAuth.SAML.Response.Conditions.t()}
+
+  import DataSchema, only: [data_schema: 1]
+
+  alias ShinAuth.SAML.Response.Utils
+
+  @data_accessor ShinAuth.SAML.XMLHandler
+  data_schema(
+    has_one: {:common, "/saml2p:Response", ShinAuth.SAML.Response.Common},
+    has_one: {:status, "/saml2p:Response/saml2p:Status", ShinAuth.SAML.Response.Status},
+    has_one:
+      {:conditions, "/saml2p:Response/saml2:Assertion/saml2:Conditions",
+       ShinAuth.SAML.Response.Conditions},
+    has_many:
+      {:attributes, "/saml2p:Response/saml2:Assertion/saml2:AttributeStatement/saml2:Attribute",
+       ShinAuth.SAML.Response.Attribute}
+  )
+end
+
+defmodule ShinAuth.SAML.Response.Common do
+  @moduledoc false
+
+  @type t ::
+          {:id, String.t()}
+          | {:version, String.t()}
+          | {:destination, String.t()}
+          | {:issuer, String.t()}
+          | {:issue_instant, String.t()}
+
   import DataSchema, only: [data_schema: 1]
 
   alias ShinAuth.SAML.Response.Utils
 
   @data_accessor ShinAuth.SAML.XMLHandler
   data_schema(
-    field: {:id, "/saml2p:Response/@ID", &{:ok, Utils.maybe_to_string(&1)}, optional: false},
+    field: {:id, "./@ID", &{:ok, Utils.maybe_to_string(&1)}, optional: false},
+    field: {:version, "./@Version", &{:ok, Utils.maybe_to_string(&1)}, optional: false},
+    field: {:destination, "./@Destination", &{:ok, Utils.maybe_to_string(&1)}, optional: false},
     field:
-      {:version, "/saml2p:Response/@Version", &{:ok, Utils.maybe_to_string(&1)}, optional: false},
+      {:issuer, "./saml2p:Issuer/text()", &{:ok, Utils.maybe_to_string(&1)}, optional: false},
+    field: {:issue_instant, "./@IssueInstant", &{:ok, Utils.maybe_to_string(&1)}, optional: false}
+  )
+end
+
+defmodule ShinAuth.SAML.Response.Conditions do
+  @moduledoc false
+
+  @type t ::
+          {:not_before, String.t()}
+          | {:not_on_or_after, String.t()}
+
+  import DataSchema, only: [data_schema: 1]
+
+  alias ShinAuth.SAML.Response.Utils
+
+  @data_accessor ShinAuth.SAML.XMLHandler
+  data_schema(
+    field: {:not_before, "./@NotBefore", &{:ok, Utils.maybe_to_string(&1)}, optional: false},
     field:
-      {:destination, "/saml2p:Response/@Destination", &{:ok, Utils.maybe_to_string(&1)},
-       optional: false},
+      {:not_on_or_after, "./@NotOnOrAfter", &{:ok, Utils.maybe_to_string(&1)}, optional: false}
+  )
+end
+
+defmodule ShinAuth.SAML.Response.Status do
+  @moduledoc false
+
+  @type t ::
+          {:status, :failure, :successful}
+          | {:status_code, String.t()}
+
+  import DataSchema, only: [data_schema: 1]
+
+  alias ShinAuth.SAML.Response.Utils
+
+  @data_accessor ShinAuth.SAML.XMLHandler
+  data_schema(
     field:
-      {:issuer, "/saml2p:Response/saml2p:Issuer/text()", &{:ok, Utils.maybe_to_string(&1)},
+      {:status, "./saml2p:StatusCode/@Value", &{:ok, Utils.map_status_code_value(&1)},
        optional: false},
     field:
-      {:issue_instant, "/saml2p:Response/@IssueInstant", &{:ok, Utils.maybe_to_string(&1)},
+      {:status_code, "./saml2p:StatusCode/@Value", &{:ok, Utils.maybe_to_string(&1)},
        optional: false}
   )
 end
 
+defmodule ShinAuth.SAML.Response.Attribute do
+  @moduledoc false
+
+  @type t ::
+          {:name, String.t()}
+          | {:value, String.t()}
+
+  import DataSchema, only: [data_schema: 1]
+
+  alias ShinAuth.SAML.Response.Utils
+
+  @data_accessor ShinAuth.SAML.XMLHandler
+  data_schema(
+    field: {:name, "./@Name", &{:ok, Utils.maybe_to_string(&1)}, optional: false},
+    field:
+      {:value, "./saml2:AttributeValue/text()", &{:ok, to_string(&1) |> String.trim()},
+       optional: true}
+  )
+end
+
 defmodule ShinAuth.SAML.Response.Utils do
   @moduledoc false
 
   def maybe_to_string(""), do: nil
   def maybe_to_string(nil), do: nil
   def maybe_to_string(value), do: to_string(value) |> String.trim()
+
+  def map_status_code_value(value) do
+    case value do
+      "urn:oasis:names:tc:SAML:2.0:status:Success" -> :success
+      _ -> :failure
+    end
+  end
 end
diff --git a/test/saml/request_test.exs b/test/saml/request_test.exs
@@ -30,17 +30,11 @@ defmodule ShinAuth.SAML.RequestTest do
 
     context "when missing required element" do
       it "returns error" do
-        saml_request_mock = """
-        <?xml version="1.0"?>
-        <samlp:AuthnRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" ID="_123" Version="2.0" IssueInstant="2023-09-27T17:20:42.746Z" ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Destination="https://accounts.google.com/o/saml2/idp?idpid=C03gu405z" AssertionConsumerServiceURL="https://auth.example.com/sso/saml/acs/123">
-        <samlp:NameIDPolicy Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress" AllowCreate="true"/>
-        </samlp:AuthnRequest>
-        """
-
         {:error,
-         %DataSchema.Errors{
-           errors: [issuer: "Field was required but value supplied is considered empty"]
-         }} = SAML.decode_saml_request(saml_request_mock)
+         %Request.Error{
+           tag: :malformed_saml_request,
+           message: "Verify if the SAML request is structured correctly by the Service Provider."
+         }} = SAML.decode_saml_request(get_xml("saml_request_missing_element"))
       end
     end
   end
@@ -49,11 +43,13 @@ defmodule ShinAuth.SAML.RequestTest do
     it "returns parsed request struct" do
       {:ok,
        %Request{
-         id: "_123",
-         version: "2.0",
-         issue_instant: "2023-09-27T17:20:42.746Z",
-         issuer: "https://example.com/123",
-         assertion_consumer_service_url: "https://auth.example.com/sso/saml/acs/123"
+         common: %{
+           id: "_123",
+           version: "2.0",
+           issue_instant: "2023-09-27T17:20:42.746Z",
+           issuer: "https://example.com/123",
+           assertion_consumer_service_url: "https://auth.example.com/sso/saml/acs/123"
+         }
        }} = SAML.decode_saml_request(get_xml("valid_saml_request"))
     end
   end

diff --git a/test/saml/response_test.exs b/test/saml/response_test.exs
@@ -46,11 +46,13 @@ defmodule ShinAuth.SAML.ResponseTest do
     it "returns parsed response struct" do
       {:ok,
        %Response{
-         id: "_123",
-         version: "2.0",
-         destination: "https://api.example.com/sso/saml/acs/123",
-         issuer: "https://example.com/1234/issuer/1234",
-         issue_instant: "2024-03-23T20:56:56.768Z"
+         common: %{
+           id: "_123",
+           version: "2.0",
+           destination: "https://api.example.com/sso/saml/acs/123",
+           issuer: "https://example.com/1234/issuer/1234",
+           issue_instant: "2024-03-23T20:56:56.768Z"
+         }
        }} = SAML.decode_saml_response(get_xml("valid_saml_response"))
     end
   end

diff --git a/test/support/saml/saml_request_missing_element.xml b/test/support/saml/saml_request_missing_element.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0"?>
+<samlp:AuthnRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" ID="_123" Version="2.0"
+  IssueInstant="2023-09-27T17:20:42.746Z"
+  ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"
+  Destination="https://accounts.google.com/o/saml2/idp?idpid=C03gu405z"
+  AssertionConsumerServiceURL="https://auth.example.com/sso/saml/acs/123">
+  <samlp:NameIDPolicy Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress"
+    AllowCreate="true" />
+</samlp:AuthnRequest>