Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

1582 Fix experts invitations related permissions handling #1583

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions backend/src/gpml/db/stakeholder.sql
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ update stakeholder set
--~ (when (contains? params :idp_usernames) "idp_usernames= :idp_usernames::jsonb,")
--~ (when (contains? params :picture_id) "picture_id= :picture_id,")
--~ (when (contains? params :cv_id) "cv_id= :cv_id, ")
--~ (when (contains? params :review_status) "review_status= :review_status::review_status,")
modified = now()
where id = :id;

Expand Down
11 changes: 6 additions & 5 deletions backend/src/gpml/handler/stakeholder.clj
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,12 @@
(fn [{:keys [body-params headers user]}]
(try
(let [tags (handler.stakeholder.tag/api-stakeholder-tags->stakeholder-tags body-params)]
(update-stakeholder config (assoc body-params
:id (:id user)
:picture {:payload (:picture body-params)
:user-agent (get headers "user-agent")}
:tags tags))
(update-stakeholder config (-> body-params
(assoc :id (:id user)
:picture {:payload (:picture body-params)
:user-agent (get headers "user-agent")}
:tags tags)
(dissoc :review_status)))
(resp/status {:success? true} 204))
(catch Exception e
(log logger :error ::failed-to-update-stakeholder {:exception-message (.getMessage e)})
Expand Down
58 changes: 44 additions & 14 deletions backend/src/gpml/handler/stakeholder/expert.clj
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
[gpml.handler.resource.permission :as h.r.permission]
[gpml.handler.responses :as r]
[gpml.handler.stakeholder.tag :as handler.stakeholder.tag]
[gpml.service.permissions :as srv.permissions]
[gpml.util :as util]
[gpml.util.email :as email]
[gpml.util.postgresql :as pg-util]
Expand Down Expand Up @@ -179,6 +180,8 @@
(log logger :error ::send-invitation-emails-failed {:exception-message (.getMessage e)
:context-data {:invitations invitations}}))))

;; TODO: Improve how we deal with errors here, since we should rollback invitation processes one by one, as otherwise
;; we might rollback all of them while the notifications or some of them have been sent already.
(defn- invite-experts
[{:keys [db mailjet-config logger] :as config}
{{:keys [body]} :parameters}]
Expand All @@ -202,21 +205,48 @@
logger
mailjet-config
{:tags (handler.stakeholder.tag/api-stakeholder-tags->stakeholder-tags {:expertise expertise})
:stakeholder-id stakeholder-id}))
:stakeholder-id stakeholder-id})
(let [{:keys [success?]} (srv.permissions/create-resource-context
{:conn conn
:logger logger}
{:context-type :stakeholder
:resource-id stakeholder-id})]
(if-not success?
(throw (ex-info
"Failing creating stakeholder rbac context"
{:reason :failing-creating-stakeholder-rbac-context
:stakeholder-id stakeholder-id}))
(let [role-assignments [{:role-name :unapproved-user
:context-type :application
:resource-id srv.permissions/root-app-resource-id
:user-id stakeholder-id}]
result (first (srv.permissions/assign-roles-to-users
{:conn conn
:logger logger}
role-assignments))]
(when-not (:success? result)
(throw (ex-info
"Failing assigning roles to stakeholder"
{:reason :failing-assigning-roles-to-stakeholder
:role-assignments role-assignments})))))))
(future (send-invitation-emails config invitations))
(resp/response {:success? true
:invited-experts (map #(update % :id str) invitations)})))
(catch Exception e
(log logger :error ::invite-experts-error {:exception-message (.getMessage e)})
(if (instance? SQLException e)
{:status 500
:body {:success? false
:reason (if (= :unique-constraint-violation (pg-util/get-sql-state e))
:stakeholder-email-already-exists
(pg-util/get-sql-state e))}}
{:status 500
:body {:success? false
:reason :could-not-create-expert}}))))
(r/ok {:success? true
:invited-experts (map #(update % :id str) invitations)})))
(catch Throwable t
(log logger :error ::invite-experts-error {:exception-message (ex-message t)
:exception-data (ex-data t)
:stack-trace (.getStackTrace t)
:context-data body})
(if (instance? SQLException t)
(r/server-error
{:success? false
:reason (if (= :unique-constraint-violation (pg-util/get-sql-state t))
:stakeholder-email-already-exists
(pg-util/get-sql-state t))})
(let [{:keys [reason]} (ex-data t)]
(r/server-error
{:success? false
:reason reason}))))))

(defn- generate-admins-expert-suggestion-text
[{:keys [email expertise suggested_expertise] :as expert}
Expand Down
89 changes: 83 additions & 6 deletions backend/src/gpml/service/stakeholder.clj
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
(ns gpml.service.stakeholder
(:require [gpml.db.organisation :as db.organisation]
(:require [duct.logger :refer [log]]
[gpml.db.organisation :as db.organisation]
[gpml.db.resource.tag :as db.resource.tag]
[gpml.db.stakeholder :as db.stakeholder]
[gpml.domain.file :as dom.file]
Expand Down Expand Up @@ -318,12 +319,89 @@
(fn get-experts
[{:keys [old-stakeholder] :as context}]
(let [expert? (seq (db.stakeholder/get-experts conn {:filters {:ids [(:id old-stakeholder)]}
:page-size 0
:page-size 1
:offset 0}))]
(assoc context :expert? expert?)))}
(assoc context :expert? expert?)))
:rollback-fn
(fn rollback-unassign-unapproved-user-role
[{:keys [old-stakeholder invited-expert?] :as context}]
(if invited-expert?
(let [role-assignments [{:role-name :unapproved-user
:context-type :application
:resource-id srv.permissions/root-app-resource-id
:user-id (:id old-stakeholder)}]
result (first (srv.permissions/assign-roles-to-users
{:conn conn
:logger logger}
role-assignments))]
(when-not (:success? result)
(log logger :error ::rollback-unassign-unapproved-user-role {:reason result})))
(dissoc context :invited-expert?)))}
{:txn-fn
(fn unassign-unapproved-user-role
[{:keys [old-stakeholder expert?] :as context}]
(if (and expert?
(= (:review-status old-stakeholder) "INVITED"))
(let [role-unassignments [{:role-name :unapproved-user
:context-type :application
:resource-id srv.permissions/root-app-resource-id
:user-id (:id old-stakeholder)}]
result (first (srv.permissions/unassign-roles-from-users
{:conn conn
:logger logger}
role-unassignments))]
(if (:success? result)
(assoc context :invited-expert? true)
(assoc context
:success? false
:reason :failed-to-remove-unapproved-user-role
:error-details {:result result})))
context))
:rollback-fn
(fn rollback-assign-approved-user-role
[{:keys [old-stakeholder invited-expert?] :as context}]
(if invited-expert?
(let [role-unassignments [{:role-name :approved-user
:context-type :application
:resource-id srv.permissions/root-app-resource-id
:user-id (:id old-stakeholder)}]
result (first (srv.permissions/unassign-roles-from-users
{:conn conn
:logger logger}
role-unassignments))]
(when-not (:success? result)
(log logger :error ::rollback-assign-approved-user-role {:reason result}))
context)
context))}
{:txn-fn
(fn assign-approved-user-role
[{:keys [old-stakeholder invited-expert?] :as context}]
(if invited-expert?
(let [role-assignments [{:role-name :approved-user
:context-type :application
:resource-id srv.permissions/root-app-resource-id
:user-id (:id old-stakeholder)}]
result (first (srv.permissions/assign-roles-to-users
{:conn conn
:logger logger}
role-assignments))]
(if (:success? result)
context
(assoc context
:success? false
:reason :failed-to-add-approved-user-role
:error-details {:result result})))
context))
:rollback-fn
(fn rollback-update-stakeholder
[{:keys [old-stakeholder] :as context}]
(let [affected (db.stakeholder/update-stakeholder conn old-stakeholder)]
(when-not (= 1 affected)
(log logger :error ::rollback-update-stakeholder {:id (:id old-stakeholder)})))
context)}
{:txn-fn
(fn update-stakeholder
[{:keys [stakeholder old-stakeholder expert? picture-file cv-file] :as context}]
[{:keys [stakeholder old-stakeholder invited-expert? picture-file cv-file] :as context}]
(let [idp-usernames (-> (concat (:idp-usernames old-stakeholder)
(:idp_usernames stakeholder))
distinct
Expand All @@ -342,8 +420,7 @@
cv-id
(assoc :cv_id cv-id)

(and expert?
(= (:review-status old-stakeholder) "INVITED"))
invited-expert?
(assoc :review_status "APPROVED")))]
(if (= 1 affected)
context
Expand Down