Skip to content

Commit

Permalink
Merge branch 'main' into reactions
Browse files Browse the repository at this point in the history
  • Loading branch information
milt committed Sep 21, 2023
2 parents 6ffc9ea + ba1168b commit d5a783a
Show file tree
Hide file tree
Showing 23 changed files with 402 additions and 28 deletions.
9 changes: 9 additions & 0 deletions .nvd/suppression.xml
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,13 @@
<cpe>cpe:/a:postgresql:postgresql_jdbc_driver</cpe>
<cve>CVE-2020-21469</cve>
</suppress>
<!-- This CVE is actually listed has having "no known exploit scenario"
(plus LRS validation should prevent issues anyways) -->
<suppress>
<notes><![CDATA[
file name: jetty-http-9.4.51.v20230217.jar
]]></notes>
<packageUrl regex="true">^pkg:maven/org\.eclipse\.jetty/jetty\-http@.*$</packageUrl>
<vulnerabilityName>CVE-2023-40167</vulnerabilityName>
</suppress>
</suppressions>
1 change: 1 addition & 0 deletions doc/endpoints.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ The following examples use `http://example.org` as the URL body. All methods ret
#### Misc Admin Routes

- `GET http://example.org/admin/env`: Get select environment variables about the configuration which may aid in client-side operations. Currently returns a map containing the configuration variables `urlPrefix` and `enableStmtHtml`.
- `DELETE http://example.org/admin/agents`: Runs a *hard delete* of all records of an actor, and associated records (statements, attachments, etc). Intended for privacy purposes like GDPR. Body should be a JSON object of form `{"actor-ifi":<actor-ifi>}`. Disabled unless the configuration variable enableAdminDeleteActor to be set to `true`.

### Reaction Management Routes

Expand Down
1 change: 1 addition & 0 deletions doc/env_vars.md
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ These config vars enable and configure browser security response headers from th

| Env Var | Config | Description | Default |
| --------------------------------- | ------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------ |
| `LRSQL_ENABLE_ADMIN_DELETE_ACTOR` | `enableAdminDeleteActor` | Whether or not to enable the route and UI for deleting actors and related documents | `false` |
| `LRSQL_ENABLE_ADMIN_UI` | `enableAdminUi` | Whether or not to serve the administrative UI at `/admin` | `true` |
| `LRSQL_ENABLE_ADMIN_STATUS` | `enableAdminStatus` | Whether or not to serve admin system status data that queries the database. | `true` |
| `LRSQL_ENABLE_STMT_HTML` | `enableStmtHtml` | Whether or not HTML data is returned in the LRS HTTP response. If `false` disables HTML rendering even if `LRSQL_ENABLE_ADMIN_UI` is `true`. In that case the UI will not display the Statement Browser feature. | `true` |
Expand Down
1 change: 1 addition & 0 deletions resources/lrsql/config/prod/default/webserver.edn
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
:allowed-origins #array #or [#env LRSQL_ALLOWED_ORIGINS nil]
:ssl-port #long #or [#env LRSQL_SSL_PORT 8443]
:url-prefix #or [#env LRSQL_URL_PREFIX "/xapi"]
:enable-admin-delete-actor #boolean #or [#env LRSQL_ENABLE_ADMIN_DELETE_ACTOR false]
:enable-admin-ui #boolean #or [#env LRSQL_ENABLE_ADMIN_UI true]
:enable-admin-status #boolean #or [#env LRSQL_ENABLE_ADMIN_STATUS true]
:enable-stmt-html #boolean #or [#env LRSQL_ENABLE_STMT_HTML true]
Expand Down
1 change: 1 addition & 0 deletions resources/lrsql/config/test/default/webserver.edn
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
;; Allow access from admin-ui dev
"http://localhost:9500"]
:url-prefix "/xapi"
:enable-admin-delete-actor true
:enable-admin-ui true
:enable-admin-status true
:enable-stmt-html true
Expand Down
6 changes: 5 additions & 1 deletion src/db/postgres/lrsql/postgres/record.clj
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@
(xapi-statement-add-trigger-id! tx))
(if (-> tuning :config :enable-jsonb)
(migrate-to-jsonb! tx)
(migrate-to-json! tx)))
(migrate-to-json! tx))
(when (nil? (check-statement-to-actor-cascading-delete tx))
(add-statement-to-actor-cascading-delete! tx)))

bp/BackendUtil
(-txn-retry? [_ ex]
Expand Down Expand Up @@ -105,6 +107,8 @@
(insert-statement-to-actor! tx input))
(-update-actor! [_ tx input]
(update-actor! tx input))
(-delete-actor! [_ tx input]
(delete-actor-and-dependents! tx input))
(-query-actor [_ tx input]
(query-actor tx input))

Expand Down
14 changes: 14 additions & 0 deletions src/db/postgres/lrsql/postgres/sql/ddl.sql
Original file line number Diff line number Diff line change
Expand Up @@ -399,3 +399,17 @@ SELECT 1 FROM information_schema.columns WHERE table_name = 'xapi_statement' AND
ALTER TABLE xapi_statement ADD COLUMN trigger_id UUID;
ALTER TABLE xapi_statement ADD CONSTRAINT stmt_trigger_id_fk FOREIGN KEY (trigger_id) REFERENCES xapi_statement(statement_id);
CREATE INDEX IF NOT EXISTS stmt_trigger_id_idx ON xapi_statement(trigger_id);

-- :name check-statement-to-actor-cascading-delete
-- :result :one
-- :command :execute
SELECT 1
FROM pg_constraint
WHERE conname = 'statement_fk'
AND pg_get_constraintdef(oid) LIKE '%ON DELETE CASCADE%'

-- :name add-statement-to-actor-cascading-delete!
-- :command :execute
-- :doc Adds a cascading delete to delete st2actor entries when corresponding statements are deleted
ALTER TABLE statement_to_actor DROP CONSTRAINT statement_fk;
ALTER TABLE statement_to_actor ADD CONSTRAINT statement_fk FOREIGN KEY (statement_id) REFERENCES xapi_statement(statement_id) ON DELETE CASCADE;
36 changes: 36 additions & 0 deletions src/db/postgres/lrsql/postgres/sql/delete.sql
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,39 @@ SET
modified = :modified
WHERE
id = :reaction-id

----------------------begin components of delete-actor-----
-- :name delete-actor-and-dependents!
-- :command :execute
-- :result :affected
DELETE FROM statement_to_statement
WHERE ancestor_id IN (
SELECT statement_id FROM statement_to_actor
WHERE actor_ifi = :actor-ifi
)
OR descendant_id IN (
SELECT statement_id FROM statement_to_actor
WHERE actor_ifi = :actor-ifi
);

DELETE FROM statement_to_activity WHERE statement_id IN (
SELECT statement_id FROM statement_to_actor
WHERE actor_ifi = :actor-ifi
);

DELETE FROM attachment WHERE statement_id IN (
SELECT statement_id FROM statement_to_actor
WHERE actor_ifi = :actor-ifi
);

DELETE FROM xapi_statement WHERE statement_id IN (
SELECT statement_id FROM statement_to_actor
WHERE actor_ifi = :actor-ifi
);

DELETE FROM agent_profile_document WHERE agent_ifi = :actor-ifi;

DELETE FROM state_document WHERE agent_ifi = :actor-ifi;

DELETE FROM actor WHERE actor_ifi = :actor-ifi;
------------------end delete-actor--------------------
11 changes: 11 additions & 0 deletions src/db/sqlite/lrsql/sqlite/record.clj
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@
(when-not (some? (query-xapi-statement-reaction-id-exists tx))
(xapi-statement-add-reaction-id! tx)
(xapi-statement-add-trigger-id! tx))
(when-not (= "CASCADE" (:on-delete (first (query-statement-to-actor-has-cascade-delete? tx))))
(update-schema-simple! tx alter-statement-to-actor-add-cascade-delete!))
(log/infof "sqlite schema_version: %d"
(:schema_version (query-schema-version tx))))

Expand Down Expand Up @@ -140,6 +142,15 @@
(insert-statement-to-actor! tx input))
(-update-actor! [_ tx input]
(update-actor! tx input))
(-delete-actor! [_ tx input]
(delete-actor-st2st tx input)
(delete-actor-st2activ tx input)
(delete-actor-attachments tx input)
(delete-actor-statements tx input)
(delete-actor-agent-profile tx input)
(delete-actor-state-document tx input)
(delete-actor-actor tx input))

(-query-actor [_ tx input]
(query-actor tx input))

Expand Down
25 changes: 24 additions & 1 deletion src/db/sqlite/lrsql/sqlite/sql/ddl.sql
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,6 @@ ALTER TABLE activity_profile_document DROP COLUMN last_modified;
-- :doc Convert `activity_profile_document.last_modified` to timestamp - 04
ALTER TABLE activity_profile_document RENAME COLUMN last_modified_tmp TO last_modified;


/* Migration 2023-07-21-00 - Add Reaction Table */

-- :name create-reaction-table!
Expand Down Expand Up @@ -458,3 +457,27 @@ ALTER TABLE xapi_statement ADD COLUMN reaction_id TEXT REFERENCES reaction(id);
-- :command :execute
-- :doc Adds `xapi_statement.trigger_id`
ALTER TABLE xapi_statement ADD COLUMN trigger_id TEXT REFERENCES xapi_statement(statement_id);

--:name query-statement-to-actor-has-cascade-delete?
SELECT on_delete FROM pragma_foreign_key_list("statement_to_actor") WHERE "table" = "xapi_statement"

-- :name alter-statement-to-actor-add-cascade-delete!
-- :command :execute
UPDATE sqlite_schema
SET sql = 'CREATE TABLE statement_to_actor (
id TEXT NOT NULL PRIMARY KEY, -- uuid
statement_id TEXT NOT NULL, -- uuid
usage TEXT NOT NULL CHECK (
usage IN (''Actor'', ''Object'', ''Authority'', ''Instructor'', ''Team'',
''SubActor'', ''SubObject'', ''SubInstructor'', ''SubTeam'')
), -- enum
actor_ifi TEXT NOT NULL, -- ifi string
actor_type TEXT NOT NULL CHECK (
actor_type IN (''Agent'', ''Group'')
), -- enum
FOREIGN KEY (statement_id) REFERENCES xapi_statement(statement_id)
ON DELETE CASCADE,
FOREIGN KEY (actor_ifi, actor_type) REFERENCES actor(actor_ifi, actor_type)
)'
WHERE type = 'table' AND name = 'statement_to_actor'
53 changes: 53 additions & 0 deletions src/db/sqlite/lrsql/sqlite/sql/delete.sql
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,56 @@ SET
modified = :modified
WHERE
id = :reaction-id

----------------------begin components of delete-actor-----
-- :name delete-actor-st2st
-- :command :execute
-- :result :affected
DELETE FROM statement_to_statement
WHERE ancestor_id IN (
SELECT statement_id FROM statement_to_actor
WHERE actor_ifi = :actor-ifi
)
OR descendant_id IN (
SELECT statement_id FROM statement_to_actor
WHERE actor_ifi = :actor-ifi
);

-- :name delete-actor-st2activ
-- :command :execute
-- :result :affected
DELETE FROM statement_to_activity WHERE statement_id IN (
SELECT statement_id FROM statement_to_actor
WHERE actor_ifi = :actor-ifi
);

-- :name delete-actor-attachments
-- :command :execute
-- :result :affected
DELETE FROM attachment WHERE statement_id IN (
SELECT statement_id FROM statement_to_actor
WHERE actor_ifi = :actor-ifi
);

-- :name delete-actor-statements
-- :command :execute
-- :result :affected
DELETE FROM xapi_statement WHERE statement_id IN (
SELECT statement_id FROM statement_to_actor
WHERE actor_ifi = :actor-ifi
);

-- :name delete-actor-agent-profile
-- :command :execute
-- :result :affected
DELETE FROM agent_profile_document WHERE agent_ifi = :actor-ifi

-- :name delete-actor-state-document
-- :command :execute
-- :result :affected
DELETE FROM state_document WHERE agent_ifi = :actor-ifi

-- :name delete-actor-actor
-- :command :execute
-- :result :affected
DELETE FROM actor WHERE actor_ifi = :actor-ifi
33 changes: 33 additions & 0 deletions src/main/lrsql/admin/interceptors/lrs_management.clj
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
(ns lrsql.admin.interceptors.lrs-management
(:require [clojure.spec.alpha :as s]
[io.pedestal.interceptor :refer [interceptor]]
[io.pedestal.interceptor.chain :as chain]
[lrsql.admin.protocol :as adp]
[lrsql.spec.admin :as ads]))

(def validate-delete-actor-params
(interceptor
{:name ::validate-delete-actor-params
:enter (fn validate-delete-params [ctx]
(let [params (get-in ctx [:request :json-params])]
(if-some [err (s/explain-data
ads/delete-actor-spec
params)]
(assoc (chain/terminate ctx)
:response
{:status 400
:body {:error (format "Invalid parameters:\n%s"
(-> err s/explain-out with-out-str))}})
(assoc ctx ::data params))))}))

(def delete-actor
(interceptor
{:name ::delete-actor
:enter (fn delete-actor [ctx]
(let [{lrs :com.yetanalytics/lrs
params ::data}
ctx]
(adp/-delete-actor lrs params)
(assoc ctx
:response {:status 200
:body params})))}))
7 changes: 5 additions & 2 deletions src/main/lrsql/admin/interceptors/ui.clj
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@
config to inject:
:enable-admin-status - boolean, determines if the admin status endpoint is
enabled."
[{:keys [enable-admin-status
[{:keys [enable-admin-delete-actor
enable-admin-status
enable-reactions
no-val?]
:or {enable-admin-status false
:or {enable-admin-delete-actor false
enable-admin-status false
enable-reactions false
no-val? false}}]
(interceptor
Expand All @@ -34,6 +36,7 @@
(merge
{:url-prefix url-prefix
:enable-stmt-html (some? enable-stmt-html)
:enable-admin-delete-actor enable-admin-delete-actor
:enable-admin-status enable-admin-status
:enable-reactions enable-reactions
:no-val? no-val?}
Expand Down
3 changes: 3 additions & 0 deletions src/main/lrsql/admin/protocol.clj
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,6 @@
"Update a reaction with a new title, ruleset and/or active status")
(-delete-reaction [this reaction-id]
"Soft-delete a reaction."))

(defprotocol AdminLRSManager
(-delete-actor [this params]))
22 changes: 17 additions & 5 deletions src/main/lrsql/admin/routes.clj
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
[com.yetanalytics.lrs.pedestal.interceptor :as i]
[lrsql.admin.interceptors.account :as ai]
[lrsql.admin.interceptors.credentials :as ci]
[lrsql.admin.interceptors.lrs-management :as lm]
[lrsql.admin.interceptors.ui :as ui]
[lrsql.admin.interceptors.jwt :as ji]
[lrsql.admin.interceptors.status :as si]
Expand Down Expand Up @@ -114,7 +115,6 @@
si/get-status)
:route-name :lrsql.admin.status/get]})


(defn admin-ui-routes
[common-interceptors inject-config]
#{;; Redirect root to admin UI
Expand Down Expand Up @@ -164,6 +164,14 @@
ri/delete-reaction)
:route-name :lrsql.admin.reaction/delete]})

(defn admin-lrs-management-routes [common-interceptors jwt-secret jwt-leeway no-val-opts]
#{["/admin/agents" :delete (conj common-interceptors
lm/validate-delete-actor-params
(ji/validate-jwt jwt-secret jwt-leeway no-val-opts)
ji/validate-jwt-account
lm/delete-actor)
:route-name :lrsql.lrs-management/delete-actor]})

(defn add-admin-routes
"Given a set of routes `routes` for a default LRS implementation,
add additional routes specific to creating and updating admin
Expand All @@ -177,6 +185,7 @@
no-val-uname
no-val-role-key
no-val-role
enable-admin-delete-actor
enable-admin-ui
enable-admin-status
enable-account-routes
Expand Down Expand Up @@ -205,12 +214,15 @@
(admin-ui-routes
(into common-interceptors
oidc-ui-interceptors)
{:enable-admin-status enable-admin-status
:enable-reactions enable-reaction-routes
:no-val? no-val?}))
{:enable-admin-status enable-admin-status
:enable-reactions enable-reaction-routes
:enable-admin-delete-actor enable-admin-delete-actor
:no-val? no-val?}))
(when enable-admin-status
(admin-status-routes
common-interceptors-oidc secret leeway no-val-opts))
(when enable-reaction-routes
(admin-reaction-routes
common-interceptors secret leeway no-val-opts)))))
common-interceptors secret leeway no-val-opts))
(when enable-admin-delete-actor
(admin-lrs-management-routes common-interceptors-oidc secret leeway no-val-opts)))))
1 change: 1 addition & 0 deletions src/main/lrsql/backend/protocol.clj
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
(-insert-actor! [this tx input])
(-insert-statement-to-actor! [this tx input])
(-update-actor! [this tx input])
(-delete-actor! [this tx input])
;; Queries
(-query-actor [this tx input]))

Expand Down
11 changes: 11 additions & 0 deletions src/main/lrsql/input/actor.clj
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,17 @@
:actor-ifi actor-ifi
:actor-type actor-type})

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Actor Delete
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

(s/fdef delete-actor-input
:args (s/cat :actor-ifi :lrsql.spec.actor/actor-ifi)
:ret (s/keys :req-un [:lrsql.spec.actor/actor-ifi]))

(defn delete-actor-input [actor-ifi]
{:actor-ifi actor-ifi})

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Actor Query
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
Expand Down
Loading

0 comments on commit d5a783a

Please sign in to comment.