From 5d2deb6c230b08504addffe2a22216d2784a8a50 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Thu, 20 May 2021 10:47:00 -0400 Subject: [PATCH 1/3] disallow anon groups from agent params --- src/xapi_schema/spec.cljc | 108 ++++++++++++---------- src/xapi_schema/spec/resources.cljc | 22 +++-- test/xapi_schema/spec/resources_test.cljc | 11 ++- 3 files changed, 80 insertions(+), 61 deletions(-) diff --git a/src/xapi_schema/spec.cljc b/src/xapi_schema/spec.cljc index 226bc0b..2152466 100644 --- a/src/xapi_schema/spec.cljc +++ b/src/xapi_schema/spec.cljc @@ -659,55 +659,26 @@ (s/coll-of ::agent :kind vector? :into [] :gen-max 3)) (s/def ::identified-group - (s/keys :req [:group/objectType - (or :group/mbox + (s/with-gen + (s/and + (s/conformer + (partial conform-ns-map "group") + unform-ns-map) + (s/keys :req [:group/objectType + (or :group/mbox + :group/mbox_sha1sum + :group/openid + :group/account)] + :opt [:group/name + :group/member]) + (restrict-keys :group/mbox :group/mbox_sha1sum :group/openid - :group/account)] - :opt [:group/name - :group/member])) - -(s/def ::anonymous-group - (s/and - (s/keys :req [:group/objectType - :group/member] - :opt [:group/name]) - #(-> % :group/member seq))) - -(def identified-group? - (comp - some? - (some-fn :group/mbox - :group/mbox_sha1sum - :group/openid - :group/account))) - -(defmulti group-type #(if (identified-group? %) - :group/identified - :group/anonymous)) - -(defmethod group-type :group/identified [_] - ::identified-group) - -(defmethod group-type :group/anonymous [_] - ::anonymous-group) - - -(s/def ::group - (s/with-gen (s/and - (s/conformer - (partial conform-ns-map "group") - unform-ns-map) - (s/multi-spec group-type (fn [gen-val _] - gen-val)) - (restrict-keys :group/mbox - :group/mbox_sha1sum - :group/openid - :group/account - :group/name - :group/objectType - :group/member) - max-one-ifi) + :group/account + :group/name + :group/objectType + :group/member) + max-one-ifi) #(sgen/fmap unform-ns-map (s/gen (s/or :ifi-mbox @@ -729,12 +700,47 @@ (s/keys :req [:group/account] :opt [:group/member :group/name - :group/objectType]) - :anon - (s/keys :req [:group/member] - :opt [:group/name :group/objectType])))))) +(s/def ::anonymous-group + (s/with-gen + (s/and + (s/conformer + (partial conform-ns-map "group") + unform-ns-map) + (s/keys :req [:group/objectType + :group/member] + :opt [:group/name]) + (restrict-keys :group/objectType + :group/member + :group/name) + #(-> % :group/member not-empty)) + #(sgen/fmap + unform-ns-map + (s/gen (s/keys :req [:group/member] + :opt [:group/name + :group/objectType]))))) + +(defn identified-group? + [group] + (-> group + (select-keys ["mbox" "mbox_sha1sum" "openid" "account"]) + not-empty + boolean)) + +(defmulti group-type #(if (identified-group? %) + :group/identified + :group/anonymous)) + +(defmethod group-type :group/identified [_] + ::identified-group) + +(defmethod group-type :group/anonymous [_] + ::anonymous-group) + +(s/def ::group + (s/multi-spec group-type (fn [gen-val _] gen-val))) + ;; Actor (defmulti actor-type (fn [a] diff --git a/src/xapi_schema/spec/resources.cljc b/src/xapi_schema/spec/resources.cljc index d9e94ab..cc08dc2 100644 --- a/src/xapi_schema/spec/resources.cljc +++ b/src/xapi_schema/spec/resources.cljc @@ -57,13 +57,18 @@ ;; xAPI Resources -;; common +;; Common + (s/def :xapi.common.param/agent (json - (s/nonconforming ::xs/actor))) + (s/nonconforming + ;; anonymous groups are not allowed as agent params + (s/or :agent ::xs/agent + :group ::xs/identified-group)))) ;; Statements ;; GET https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Communication.md#213-get-statements + (s/def :xapi.statements.GET.request.params/statementId :statement/id) @@ -108,7 +113,7 @@ (s/def :xapi.statements.GET.request.params/ascending (json boolean?)) -;; +;; Putting it all together (def singular-query? (comp @@ -194,8 +199,7 @@ :activity/id) (s/def :xapi.document.params/agent - (json - (s/nonconforming ::xs/agent))) + :xapi.common.param/agent) (s/def :xapi.document.params/registration ::xs/uuid) @@ -248,13 +252,15 @@ :xapi.document.state/context-params)) ;; Agents https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Communication.md#24-agents-resource + (s/def :xapi.agents.GET.request.params/agent - :xapi.document.params/agent) + :xapi.common.param/agent) (s/def :xapi.agents.GET.request/params (s/keys :req-un [:xapi.agents.GET.request.params/agent])) ;; Person https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Communication.md#person-properties + (s/def :xapi.agents.GET.response.person/objectType #{"Person"}) @@ -281,8 +287,7 @@ :xapi.agents.GET.response.person/mbox :xapi.agents.GET.response.person/mbox_sha1sum :xapi.agents.GET.response.person/openid - :xapi.agents.GET.response.person/account - ]) + :xapi.agents.GET.response.person/account]) walk/keywordize-keys walk/stringify-keys))) @@ -295,6 +300,7 @@ (s/keys :req-un [:xapi.activities.GET.request.params/activityId])) ;; Agent Profile https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Communication.md#26-agent-profile-resource + (s/def :xapi.document.agent-profile/context-params (s/keys :req-un [:xapi.document.params/agent])) diff --git a/test/xapi_schema/spec/resources_test.cljc b/test/xapi_schema/spec/resources_test.cljc index e3f7ec8..7d9dc7b 100644 --- a/test/xapi_schema/spec/resources_test.cljc +++ b/test/xapi_schema/spec/resources_test.cljc @@ -1,5 +1,5 @@ (ns xapi-schema.spec.resources-test - (:require [clojure.test :refer [deftest is testing] :include-macros true] + (:require [clojure.test :refer [deftest is] :include-macros true] [clojure.spec.alpha :as s :include-macros true] [xapi-schema.spec.resources :as xsr :refer [*read-json-fn* *write-json-fn* @@ -39,7 +39,14 @@ (is (not (s/valid? :xapi.common.param/agent "{\"mbox\":\"milt@yetanalytics.com\"}"))) (is (not (s/valid? :xapi.common.param/agent - "{\"email\":\"mailto:milt@yetanalytics.com\"}")))) + "{\"email\":\"mailto:milt@yetanalytics.com\"}"))) + (is (s/valid? :xapi.common.param/agent + "{\"objectType\": \"Group\", + \"mbox\": \"mailto:group@example.com\", + \"member\": [{\"mbox\": \"mailto:foo@example.com\"}]}")) + (is (not (s/valid? :xapi.common.param/agent + "{\"objectType\": \"Group\", + \"member\": [{\"mbox\": \"mailto:foo@example.com\"}]}")))) (deftest statements-get-params-test (is (s/valid? :xapi.statements.GET.request/params From 596d18c61da4fd24add8f6012a50a919904426dc Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Thu, 20 May 2021 11:00:04 -0400 Subject: [PATCH 2/3] Disallow groups from agent and agent profile resources --- src/xapi_schema/spec/resources.cljc | 12 ++++++++---- test/xapi_schema/spec/resources_test.cljc | 18 ++++++++++++------ 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/xapi_schema/spec/resources.cljc b/src/xapi_schema/spec/resources.cljc index cc08dc2..e777bc2 100644 --- a/src/xapi_schema/spec/resources.cljc +++ b/src/xapi_schema/spec/resources.cljc @@ -62,9 +62,8 @@ (s/def :xapi.common.param/agent (json (s/nonconforming - ;; anonymous groups are not allowed as agent params - (s/or :agent ::xs/agent - :group ::xs/identified-group)))) + ;; except for statement queries, groups are not allowed as agent params + ::xs/agent))) ;; Statements ;; GET https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Communication.md#213-get-statements @@ -76,7 +75,12 @@ :statement/id) (s/def :xapi.statements.GET.request.params/agent - :xapi.common.param/agent) + (json + (s/nonconforming + ;; anonymous groups are not allowed as agent params + ;; identified gorups, on the other hand, are allowed + (s/or :agent ::xs/agent + :group ::xs/identified-group)))) (s/def :xapi.statements.GET.request.params/verb ::xs/iri) diff --git a/test/xapi_schema/spec/resources_test.cljc b/test/xapi_schema/spec/resources_test.cljc index 7d9dc7b..f3e3cf8 100644 --- a/test/xapi_schema/spec/resources_test.cljc +++ b/test/xapi_schema/spec/resources_test.cljc @@ -30,8 +30,7 @@ {"foo" "bar"}))) (is (= "{\"foo\":\"bar\"}" (s/unform json-string-conformer - "{\"foo\":\"bar\"}"))) - ) + "{\"foo\":\"bar\"}")))) (deftest agent-param-test (is (s/valid? :xapi.common.param/agent @@ -40,15 +39,22 @@ "{\"mbox\":\"milt@yetanalytics.com\"}"))) (is (not (s/valid? :xapi.common.param/agent "{\"email\":\"mailto:milt@yetanalytics.com\"}"))) - (is (s/valid? :xapi.common.param/agent - "{\"objectType\": \"Group\", - \"mbox\": \"mailto:group@example.com\", - \"member\": [{\"mbox\": \"mailto:foo@example.com\"}]}")) + (is (not (s/valid? :xapi.common.param/agent + "{\"objectType\": \"Group\", + \"mbox\": \"mailto:group@example.com\", + \"member\": [{\"mbox\": \"mailto:foo@example.com\"}]}"))) (is (not (s/valid? :xapi.common.param/agent "{\"objectType\": \"Group\", \"member\": [{\"mbox\": \"mailto:foo@example.com\"}]}")))) (deftest statements-get-params-test + (is (s/valid? :xapi.statements.GET.request.params/agent + "{\"objectType\": \"Group\", + \"mbox\": \"mailto:group@example.com\", + \"member\": [{\"mbox\": \"mailto:foo@example.com\"}]}")) + (is (not (s/valid? :xapi.statements.GET.request.params/agent + "{\"objectType\": \"Group\", + \"member\": [{\"mbox\": \"mailto:foo@example.com\"}]}"))) (is (s/valid? :xapi.statements.GET.request/params {:statementId (str #?(:clj (java.util.UUID/randomUUID) :cljs (random-uuid))) From 401691e28b9e07cc8a8a708ad4e19029dbd21952 Mon Sep 17 00:00:00 2001 From: kelvinqian00 Date: Thu, 20 May 2021 11:03:19 -0400 Subject: [PATCH 3/3] Undo formatting --- src/xapi_schema/spec/resources.cljc | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/xapi_schema/spec/resources.cljc b/src/xapi_schema/spec/resources.cljc index e777bc2..92625d4 100644 --- a/src/xapi_schema/spec/resources.cljc +++ b/src/xapi_schema/spec/resources.cljc @@ -57,7 +57,7 @@ ;; xAPI Resources -;; Common +;; common (s/def :xapi.common.param/agent (json @@ -67,7 +67,6 @@ ;; Statements ;; GET https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Communication.md#213-get-statements - (s/def :xapi.statements.GET.request.params/statementId :statement/id) @@ -117,7 +116,7 @@ (s/def :xapi.statements.GET.request.params/ascending (json boolean?)) -;; Putting it all together +;; (def singular-query? (comp @@ -256,7 +255,6 @@ :xapi.document.state/context-params)) ;; Agents https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Communication.md#24-agents-resource - (s/def :xapi.agents.GET.request.params/agent :xapi.common.param/agent) @@ -264,7 +262,6 @@ (s/keys :req-un [:xapi.agents.GET.request.params/agent])) ;; Person https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Communication.md#person-properties - (s/def :xapi.agents.GET.response.person/objectType #{"Person"}) @@ -291,7 +288,8 @@ :xapi.agents.GET.response.person/mbox :xapi.agents.GET.response.person/mbox_sha1sum :xapi.agents.GET.response.person/openid - :xapi.agents.GET.response.person/account]) + :xapi.agents.GET.response.person/account + ]) walk/keywordize-keys walk/stringify-keys))) @@ -304,7 +302,6 @@ (s/keys :req-un [:xapi.activities.GET.request.params/activityId])) ;; Agent Profile https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Communication.md#26-agent-profile-resource - (s/def :xapi.document.agent-profile/context-params (s/keys :req-un [:xapi.document.params/agent]))