Merge pull request #366 from yetanalytics/profile-scope

[SQL-170] Implement profile scopes

.gitignore

@@ -22,7 +22,8 @@ results/
 Test.java
 .DS_Store
 # VSCode
-.clj-kondo/.cache/
+.clj-kondo/
+!.clj-kondo/config.edn
 .calva/
 .lsp/
 .vscode/

Makefile

@@ -2,7 +2,7 @@
 
 # Version of LRS Admin UI to use
 
-LRS_ADMIN_UI_VERSION ?= v0.1.16
+LRS_ADMIN_UI_VERSION ?= v0.1.17
 LRS_ADMIN_UI_LOCATION ?= https://github.com/yetanalytics/lrs-admin-ui/releases/download/${LRS_ADMIN_UI_VERSION}/lrs-admin-ui.zip
 LRS_ADMIN_ZIPFILE ?= lrs-admin-ui-${LRS_ADMIN_UI_VERSION}.zip
 

doc/oidc.md

@@ -5,7 +5,7 @@
 SQL LRS supports [OpenID Connect (OIDC)](https://openid.net/connect/) on top of [OAuth 2.0](https://oauth.net/2/) for authentication and authorization to access xAPI resources and administrative functions. OIDC support enables several integration use cases:
 
 - Send xAPI statements to the LRS from OIDC-authenticated client applications
-- Provision LRS credentials programatically via the API
+- Provision LRS credentials programmatically via the API
 - Log in to the LRS Admin UI with a foreign identity (SSO)
 
 ### Resource Server
@@ -28,7 +28,9 @@ xAPI resources accept tokens with the following scopes:
 | ---------------------- | ------------------------------------------------------------------------------------------------------ |
 | `all`                  | Full read/write access to all xAPI resources.                                                          |
 | `all/read`             | Read-only access to all xAPI resources.                                                                |
-| `state`                | Read/Write access to the xAPI State Resource.                                                          |
+| `state`                | Read/write access to xAPI State Documents.                                                          |
+| `activities_profile`   | Read/write access to xAPI Activity Profile Documents.
+| `agents_profile`       | Read/write access to xAPI Agent Profile Documents.
 | `statements/read`      | Read-only access to all xAPI Statements (but not non-Statement resources).                             |
 | `statements/read/mine` | Read-only access to xAPI Statements whose `authority` value matches the authority of the current user. |
 | `statements/write`     | Write-only access to xAPI Statements.                                                                  |

src/db/postgres/lrsql/postgres/record.clj

@@ -53,7 +53,7 @@
   (-update-all! [_ tx]
     (alter-admin-account-passhash-optional! tx)
     (alter-admin-account-add-openid-issuer! tx)
-    (alter-scope-enum-type! tx)
+    (alter-scope-enum-type-v2! tx)
     (when-not (some? (query-xapi-statement-timestamp-exists tx))
       (alter-xapi-statement-add-timestamp! tx)
       (migrate-xapi-statement-timestamps! tx))

src/db/postgres/lrsql/postgres/sql/ddl.sql

@@ -260,10 +260,9 @@ ALTER TABLE IF EXISTS admin_account ADD COLUMN IF NOT EXISTS oidc_issuer VARCHAR
    fail inside a transaction. Therefore we have to use this circuitous route:
    https://stackoverflow.com/a/56376907 */
 
-
 -- :name alter-scope-enum-type!
 -- :command :execute
--- :doc Add `statements/read/mine` to `credential-to-scope.scope` enum.
+-- :doc DEPRECATED. Add `statements/read/mine` to `credential-to-scope.scope` enum.
 ALTER TABLE IF EXISTS credential_to_scope ALTER COLUMN scope TYPE VARCHAR(255);
 DROP TYPE IF EXISTS scope_enum;
 CREATE TYPE scope_enum AS ENUM (
@@ -281,6 +280,35 @@ CREATE TYPE scope_enum AS ENUM (
   'profile/read');
 ALTER TABLE IF EXISTS credential_to_scope ALTER COLUMN scope TYPE scope_enum USING (scope::scope_enum);
 
+/* Migration 2024-01-24 - Add profile and profile/read scopes */
+
+/* Simply adding the profile scope will cause a name clash with the reserved
+   OIDC profile scope. As a result, we add prefixes to create activity_profile
+   and agent_profile (and their read-only versions); this has the additional
+   benefit of narrowing the scope to just activity and agent profiles,
+   respectively. Since profile and profile/read have always been unused, we
+   are safe to remove them as enums. */
+
+-- :name alter-scope-enum-type-v2!
+-- :command :execute
+-- :doc Add `activity_profile`, `activity_profile/read`, `agent_profile`, and `agent_profile/read` to `credential-to-scope.scope` enum. Supersedes `alter-scope-enum-type!`
+ALTER TABLE IF EXISTS credential_to_scope ALTER COLUMN scope TYPE VARCHAR(255);
+DROP TYPE IF EXISTS scope_enum;
+CREATE TYPE scope_enum AS ENUM (
+  'statements/write',
+  'statements/read',
+  'statements/read/mine', -- new
+  'all/read',
+  'all',
+  'state',
+  'state/read',
+  'define',
+  -- NEW: add `activities_profile` + `agents_profile` scopes and remove unused `profile` scope
+  'activities_profile',
+  'activities_profile/read',
+  'agents_profile',
+  'agents_profile/read');
+ALTER TABLE IF EXISTS credential_to_scope ALTER COLUMN scope TYPE scope_enum USING (scope::scope_enum);
 
 /* Migration 2023-05-08-00 - Add timestamp to xapi_statement */
 

src/db/sqlite/lrsql/sqlite/record.clj

@@ -104,7 +104,7 @@
       (migrate-timestamps-activity-profile-02! tx)
       (migrate-timestamps-activity-profile-03! tx)
       (migrate-timestamps-activity-profile-04! tx))
-    (update-schema-simple! tx alter-credential-to-scope-scope-datatype!)
+    (update-schema-simple! tx alter-credential-to-scope-scope-datatype-v2!)
     (create-reaction-table! tx)
     (when-not (some? (query-xapi-statement-reaction-id-exists tx))
       (xapi-statement-add-reaction-id! tx)

src/db/sqlite/lrsql/sqlite/sql/ddl.sql

@@ -305,7 +305,7 @@ ALTER TABLE admin_account ADD COLUMN oidc_issuer TEXT
 
 -- :name alter-credential-to-scope-scope-datatype!
 -- :command :execute
--- :doc Change the enum datatype of the `credential_to_scope.scope` column.
+-- :doc DEPRECATED. Change the enum datatype of the `credential_to_scope.scope` column.
 UPDATE sqlite_schema
 SET sql = 'CREATE TABLE credential_to_scope (
   id         TEXT NOT NULL PRIMARY KEY,
@@ -329,6 +329,40 @@ SET sql = 'CREATE TABLE credential_to_scope (
 )'
 WHERE type = 'table' AND name = 'credential_to_scope'
 
+/* Migration 2024-01-24 - Add document/profile and document/profile/read scopes */
+
+/* The suggested scope name would simply be profile, but that would clash with
+   the reserved OIDC profile scope. Since they have always remained unused, we
+   are safe to remove them from the enum table. */
+
+-- :name alter-credential-to-scope-scope-datatype-v2!
+-- :command :execute
+-- :doc Change the enum datatype of the `credential_to_scope.scope` column. Supersedes `alter-credential-to-scope-scope-datatype!`
+UPDATE sqlite_schema
+SET sql = 'CREATE TABLE credential_to_scope (
+  id         TEXT NOT NULL PRIMARY KEY,
+  api_key    TEXT NOT NULL,
+  secret_key TEXT NOT NULL,
+  scope      TEXT CHECK (
+               scope IN (''statements/write'',
+                         ''statements/read'',
+                         ''statements/read/mine'',
+                         ''all/read'',
+                         ''all'',
+                         ''define'',
+                         ''state'',
+                         ''state/read'',
+                         ''activities_profile'',
+                         ''activities_profile/read'',
+                         ''agents_profile'',
+                         ''agents_profile/read'')
+             ),
+  FOREIGN KEY (api_key, secret_key)
+    REFERENCES lrs_credential(api_key, secret_key)
+    ON DELETE CASCADE
+)'
+WHERE type = 'table' AND name = 'credential_to_scope'
+
 /* Migration 2023-05-08-00 - Add timestamp to xapi_statement */
 
 -- :name query-xapi-statement-timestamp-exists

src/main/lrsql/ops/query/auth.clj

@@ -52,9 +52,9 @@
                   authority-url]} input
           ;; NOTE: According to the xAPI spec, the LRS MUST assume scopes
           ;; "statements/write" and "statements/read/mine" if no scope is
-          ;; specified, if OAuth 1.0 is used. Since we are using Basic
-          ;; Auth and since the above behavior is confusing, we simply
-          ;; return empty scope sets as-is.
+          ;; specified, if OAuth 1.0 is used. Since we are not using OAuth 1.0
+          ;; and since the above behavior is confusing, we simply return empty
+          ;; scope sets as-is.
           scope-set (->> scopes
                          (map au/scope-str->kw)
                          (into #{}))]

src/main/lrsql/spec/auth.clj

@@ -46,6 +46,8 @@
   #{"all"
     "all/read"
     "state"
+    "activities_profile"
+    "agents_profile"
     "statements/read"
     "statements/read/mine"
     "statements/write"})
@@ -54,6 +56,8 @@
   #{:scope/all
     :scope/all.read
     :scope/state
+    :scope/activities_profile
+    :scope/agents_profile
     :scope/statements.read
     :scope/statements.read.mine
     :scope/statements.write})

src/main/lrsql/util/auth.clj

@@ -14,6 +14,8 @@
   {"all"                  :scope/all
    "all/read"             :scope/all.read
    "state"                :scope/state
+   "activities_profile"   :scope/activities_profile
+   "agents_profile"       :scope/agents_profile
    "statements/read"      :scope/statements.read
    "statements/write"     :scope/statements.write
    "statements/read/mine" :scope/statements.read.mine})
@@ -135,7 +137,7 @@
       (cond
         (#{:get :head} request-method)
         (some? (most-permissive-statement-read-scope auth-identity))
-        (#{:put :post} request-method) ; No statement delete implemented
+        (#{:put :post} request-method) ; No statement delete for /statements resource
         (some? (most-permissive-statement-write-scope auth-identity))
         :else
         (do (log-scope-error scopes)
@@ -147,6 +149,18 @@
        (contains? scopes :scope/state))
       true
 
+      ;; activities/profile
+      (and
+       (cstr/ends-with? path "activities/profile")
+       (contains? scopes :scope/activities_profile))
+      true
+
+      ;; agents/profile
+      (and
+       (cstr/ends-with? path "agents/profile")
+       (contains? scopes :scope/agents_profile))
+      true
+
       ;; all other paths (e.g. /about, /activities)
       :else
       (cond

src/test/logback-test.xml

@@ -9,4 +9,5 @@
   </root>
   <!-- silence the selfie warnings -->
   <logger name="lrsql.util.cert" level="error" />
+  <logger name="lrsql.ops.query.reaction" level="off" />
 </configuration>