Skip to content

Commit

Permalink
SQL-187 Feature: Update Local Admin Password (#295)
Browse files Browse the repository at this point in the history
* SQL-187 working backend for update password

* SQL-187 remove redundant do

* SQL-187 add params validator for update-password

* SQL-187 add api

* SQL-187 derive user for password change from JWT

* SQL-187 move params spec to be with its siblings

* SQL-187 UI release v0.1.9
  • Loading branch information
milt authored Apr 12, 2023
1 parent 681a05b commit 6ce8a47
Show file tree
Hide file tree
Showing 21 changed files with 280 additions and 12 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

# Version of LRS Admin UI to use

LRS_ADMIN_UI_VERSION ?= v0.1.8
LRS_ADMIN_UI_VERSION ?= v0.1.9
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

Expand Down
2 changes: 2 additions & 0 deletions src/db/h2/lrsql/h2/record.clj
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@
(query-all-accounts tx))
(-delete-admin-account! [_ tx input]
(delete-admin-account! tx input))
(-update-admin-password! [_ tx input]
(update-admin-password! tx input))
(-query-account [_ tx input]
(query-account tx input))
(-query-account-oidc [_ tx input]
Expand Down
5 changes: 3 additions & 2 deletions src/db/h2/lrsql/h2/sql/query.sql
Original file line number Diff line number Diff line change
Expand Up @@ -243,9 +243,10 @@ WHERE activity_iri = :activity-iri
-- :name query-account
-- :command :query
-- :result :one
-- :doc Given an account `username`, return the ID and the hashed password, which can be used to verify the account.
-- :doc Given an account `username` or `account-id`, return the ID and the hashed password, which can be used to verify the account.
SELECT id, passhash FROM admin_account
WHERE username = :username
--~ (when (:username params) "WHERE username = :username")
--~ (when (:account-id params) "WHERE id = :account-id")

-- :name query-account-oidc
-- :command :query
Expand Down
9 changes: 9 additions & 0 deletions src/db/h2/lrsql/h2/sql/update.sql
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,12 @@ SET
last_modified = :last-modified
WHERE profile_id = :profile-id
AND activity_iri = :activity-iri

-- :name update-admin-password!
-- :command :execute
-- :result :affected
-- :doc Update the `passhash` of an admin account.
UPDATE admin_account
SET
passhash = :new-passhash
WHERE id = :account-id
2 changes: 2 additions & 0 deletions src/db/postgres/lrsql/postgres/record.clj
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@
(query-all-accounts tx))
(-delete-admin-account! [_ tx input]
(delete-admin-account! tx input))
(-update-admin-password! [_ tx input]
(update-admin-password! tx input))
(-query-account [_ tx input]
(query-account tx input))
(-query-account-oidc [_ tx input]
Expand Down
6 changes: 4 additions & 2 deletions src/db/postgres/lrsql/postgres/sql/query.sql
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,11 @@ WHERE activity_iri = :activity-iri
-- :name query-account
-- :command :query
-- :result :one
-- :doc Given an account `username`, return the ID and the hashed password, which can be used to verify the account.
-- :doc Given an account `username` or `account-id`, return the ID and the hashed password, which can be used to verify the account.
SELECT id, passhash FROM admin_account
WHERE username = :username;
--~ (when (:username params) "WHERE username = :username")
--~ (when (:account-id params) "WHERE id = :account-id")
;

-- :name query-account-oidc
-- :command :query
Expand Down
9 changes: 9 additions & 0 deletions src/db/postgres/lrsql/postgres/sql/update.sql
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,12 @@ SET
last_modified = :last-modified
WHERE profile_id = :profile-id
AND activity_iri = :activity-iri;

-- :name update-admin-password!
-- :command :execute
-- :result :affected
-- :doc Update the `passhash` of an admin account.
UPDATE admin_account
SET
passhash = :new-passhash
WHERE id = :account-id;
2 changes: 2 additions & 0 deletions src/db/sqlite/lrsql/sqlite/record.clj
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,8 @@
(query-all-accounts tx))
(-delete-admin-account! [_ tx input]
(delete-admin-account! tx input))
(-update-admin-password! [_ tx input]
(update-admin-password! tx input))
(-query-account [_ tx input]
(query-account tx input))
(-query-account-oidc [_ tx input]
Expand Down
5 changes: 3 additions & 2 deletions src/db/sqlite/lrsql/sqlite/sql/query.sql
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,10 @@ WHERE activity_iri = :activity-iri
-- :name query-account
-- :command :query
-- :result :one
-- :doc Given an account `username`, return the ID and the hashed password, which can be used to verify the account.
-- :doc Given an account `username` or `account-id`, return the ID and the hashed password, which can be used to verify the account.
SELECT id, passhash FROM admin_account
WHERE username = :username
--~ (when (:username params) "WHERE username = :username")
--~ (when (:account-id params) "WHERE id = :account-id")

-- :name query-account-oidc
-- :command :query
Expand Down
9 changes: 9 additions & 0 deletions src/db/sqlite/lrsql/sqlite/sql/update.sql
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,12 @@ SET
last_modified = :last-modified
WHERE profile_id = :profile-id
AND activity_iri = :activity-iri

-- :name update-admin-password!
-- :command :execute
-- :result :affected
-- :doc Update the `passhash` of an admin account.
UPDATE admin_account
SET
passhash = :new-passhash
WHERE id = :account-id
62 changes: 62 additions & 0 deletions src/main/lrsql/admin/interceptors/account.clj
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,31 @@
(assoc ::data acc-info)
(assoc-in [:request :session ::data] acc-info))))))}))


(def validate-update-password-params
"Validate that the JSON params contain the params `old-password`
and `new-password` for password update. Also validates that `old-password`
`new-password` do not match."
(interceptor
{:name ::validate-update-password-params
:enter
(fn validate-params [ctx]
(let [params (get-in ctx [:request :json-params])]
(if-some [err (s/explain-data
ads/update-admin-password-params-spec params)]
;; Invalid parameters - Bad Request
(assoc (chain/terminate ctx)
:response
{:status 400
:body {:error (format "Invalid parameters:\n%s"
(-> err s/explain-out with-out-str))}})
;; Valid params - continue
(let [update-info (select-keys params
[:old-password :new-password])]
(-> ctx
(assoc ::data update-info)
(assoc-in [:request :session ::data] update-info))))))}))

(def validate-delete-params
"Validate that the JSON params contain `account-id` for delete."
(interceptor
Expand Down Expand Up @@ -121,6 +146,43 @@
:body {:error (format "An account \"%s\" already exists!"
username)}}))))}))

(def update-admin-password
"Set a new password for an admin account."
(interceptor
{:name ::update-admin-password
:enter
(fn update-admin-password [ctx]
(let [{lrs
:com.yetanalytics/lrs
{:keys [old-password new-password]}
::data
{:keys [account-id]}
:lrsql.admin.interceptors.jwt/data}
ctx
{:keys [result]}
(adp/-update-admin-password lrs account-id old-password new-password)]
(cond
;; The result is the account ID - success!
(uuid? result)
(assoc ctx
:response
{:status 200 :body {:account-id result}})

;; The given account-id does not belong to a known account
(= :lrsql.admin/missing-account-error result)
(assoc (chain/terminate ctx)
:response
{:status 404
:body {:error (format "The account \"%s\" does not exist!"
(u/uuid->str account-id))}})

;; The old password is not correct.
(= :lrsql.admin/invalid-password-error result)
(assoc (chain/terminate ctx)
:response
{:status 401
:body {:error "Invalid Account Credentials"}}))))}))

(def delete-admin
"Delete the selected admin account. This is a hard delete."
(interceptor
Expand Down
4 changes: 3 additions & 1 deletion src/main/lrsql/admin/protocol.clj
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
(-delete-account [this account-id oidc-enabled?]
"Delete the account and all associated creds. Assumes the account has already been authenticated. Requires OIDC status to prevent sole account deletion.")
(-ensure-account-oidc [this username oidc-issuer]
"Create or verify an existing admin account with the given username and oidc-issuer."))
"Create or verify an existing admin account with the given username and oidc-issuer.")
(-update-admin-password [this account-id old-password new-password]
"Update the password for an admin account given old and new passwords."))

(defprotocol APIKeyManager
(-create-api-keys [this account-id scopes]
Expand Down
7 changes: 7 additions & 0 deletions src/main/lrsql/admin/routes.clj
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@
ji/validate-jwt-account
ai/create-admin)
:route-name :lrsql.admin.account/create]
;; Update account password
["/admin/account/password"
:put (conj common-interceptors
ai/validate-update-password-params
(ji/validate-jwt jwt-secret jwt-leeway)
ji/validate-jwt-account
ai/update-admin-password)]
;; Get all accounts
["/admin/account" :get (conj common-interceptors
(ji/validate-jwt jwt-secret jwt-leeway)
Expand Down
1 change: 1 addition & 0 deletions src/main/lrsql/backend/protocol.clj
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
(-insert-admin-account! [this tx input])
(-insert-admin-account-oidc! [this tx input])
(-delete-admin-account! [this tx input])
(-update-admin-password! [this tx input])
;; Queries
(-query-account [this tx input])
(-query-account-oidc [this tx input])
Expand Down
22 changes: 22 additions & 0 deletions src/main/lrsql/input/admin.clj
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,25 @@
[username password]
{:username username
:password password})

(s/fdef query-validate-admin-by-id-input
:args (s/cat :account-id ::ads/account-id :password ::ads/password)
:ret ads/query-validate-admin-by-id-input-spec)

(defn query-validate-admin-by-id-input
"Given `account-id` and `password`, construct the input param map for
`query-validate-admin-by-id`."
[account-id password]
{:account-id account-id
:password password})

(s/fdef update-admin-password-input
:args (s/cat :account-id ::ads/account-id :new-password ::ads/new-password)
:ret ads/update-admin-password-input-spec)

(defn update-admin-password-input
"Given `account-id` and `new-password`, construct the input
param map for `update-admin-password`."
[account-id new-password]
{:account-id account-id
:new-passhash (adu/hash-password new-password)})
19 changes: 19 additions & 0 deletions src/main/lrsql/ops/command/admin.clj
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,25 @@
:lrsql.admin/sole-admin-delete-error)
:lrsql.admin/missing-account-error)})

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Admin Account Update Password
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

(s/fdef update-admin-password!
:args (s/cat :bk ads/admin-backend?
:tx transaction?
:input ads/update-admin-password-input-spec)
:ret ads/update-admin-password-ret-spec)

(defn update-admin-password!
"Update the password of an admin account. Returns a map where `:result` is the
account ID."
[bk tx input]
{:result
(let [{:keys [id]} (bp/-query-account bk tx input)]
(bp/-update-admin-password! bk tx input)
id)})

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Ensure Admin Account from OIDC
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
Expand Down
12 changes: 8 additions & 4 deletions src/main/lrsql/ops/query/admin.clj
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@
(s/fdef query-validate-admin
:args (s/cat :bk ads/admin-backend?
:tx transaction?
:input ads/query-validate-admin-input-spec)
:input (s/or
:by-username ads/query-admin-input-spec
:by-id ads/query-admin-by-id-input-spec))
:ret ads/query-admin-ret-spec)

(defn query-admin
"Query an admin account with the given username and password. Returns
a map containing `:account-id` and `:passhash` on success, or nil on
"Query an admin account with the given username (or account-id) and password.
Returns a map containing `:account-id` and `:passhash` on success, or nil on
failure."
[bk tx input]
(when-some [{account-id :id
Expand All @@ -33,7 +35,9 @@
(s/fdef query-validate-admin
:args (s/cat :bk ads/admin-backend?
:tx transaction?
:input ads/query-validate-admin-input-spec)
:input (s/or
:by-username ads/query-validate-admin-input-spec
:by-id ads/query-validate-admin-by-id-input-spec))
:ret ads/query-validate-admin-ret-spec)

(defn query-validate-admin
Expand Down
35 changes: 35 additions & 0 deletions src/main/lrsql/spec/admin.clj
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@
(s/def :lrsql.spec.admin.ret/oidc-issuer (s/nilable string?))
;; boolean to indicate whether OIDC is enabled
(s/def :lrsql.spec.admin.input/oidc-enabled? boolean?)
;; Update password params
(s/def ::old-password ::password)
(s/def ::new-password ::password)
;; Update password input
(s/def :lrsql.spec.admin.input/new-passhash :lrsql.spec.admin.input/passhash)

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Inputs
Expand All @@ -43,6 +48,15 @@
(def admin-delete-params-spec
(s/keys :req-un [::account-id]))

(def update-admin-password-params-spec
(s/and
(s/keys :req-un [::old-password
::new-password])
(fn new-pass-noteq-old-pass
[{:keys [old-password
new-password]}]
(not= old-password new-password))))

(def insert-admin-input-spec
(s/keys :req-un [::c/primary-key
::username
Expand All @@ -57,17 +71,33 @@
::username
:lrsql.spec.admin.input/oidc-issuer]))

(def query-admin-input-spec
(s/keys :req-un [::username
::password]))

(def query-admin-by-id-input-spec
(s/keys :req-un [::account-id
::password]))

(def query-validate-admin-input-spec
(s/keys :req-un [::username
::password]))

(def query-validate-admin-by-id-input-spec
(s/keys :req-un [::account-id
::password]))

(def admin-id-input-spec
(s/keys :req-un [::account-id]))

(def delete-admin-input-spec
(s/merge admin-id-input-spec
(s/keys :req-un [:lrsql.spec.admin.input/oidc-enabled?])))

(def update-admin-password-input-spec
(s/keys :req-un [::account-id
:lrsql.spec.admin.input/new-passhash]))

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Results
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
Expand Down Expand Up @@ -113,3 +143,8 @@

(def query-validate-admin-ret-spec
(s/keys :req-un [:lrsql.spec.admin.query/result]))

(s/def :lrsql.spec.admin.update-password/result uuid?)

(def update-admin-password-ret-spec
(s/keys :req-un [:lrsql.spec.admin.update-password/result]))
13 changes: 13 additions & 0 deletions src/main/lrsql/system/lrs.clj
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,19 @@
input (admin-input/ensure-admin-oidc-input username oidc-issuer)]
(jdbc/with-transaction [tx conn]
(admin-cmd/ensure-admin-oidc! backend tx input))))
(-update-admin-password
[this account-id old-password new-password]
(let [conn (lrs-conn this)
auth-input (admin-input/query-validate-admin-by-id-input
account-id old-password)]
(jdbc/with-transaction [tx conn]
(let [{:keys [result]} (admin-q/query-validate-admin
backend tx auth-input)]
(if (uuid? result)
(let [input (admin-input/update-admin-password-input
account-id new-password)]
(admin-cmd/update-admin-password! backend tx input))
{:result result})))))

adp/APIKeyManager
(-create-api-keys
Expand Down
Loading

0 comments on commit 6ce8a47

Please sign in to comment.