improved: upload supervisor picture

The url is now stored in the database. While the url can still be derived from the supervisor.id, we now know if an picture was uploaded without trying to load it.
johannesloetzsch · Feb 22, 2022 · d99ae55 · d99ae55
1 parent bed535d
commit d99ae55
Show file tree

Hide file tree

Showing 8 changed files with 105 additions and 28 deletions.
diff --git a/backend/src/swlkup/db/seed/example.edn b/backend/src/swlkup/db/seed/example.edn
@@ -132,7 +132,6 @@
   :xt/spec :swlkup.model.supervisor/doc
   :swlkup.model.login/login:ids "login_max_mueller"
   :name_full "Max Müller"
-  :photo "https://raw.githubusercontent.com/wiki/community-garden/community-garden.github.io/images/team_avatar_joe.png"
   :languages [:de :en]
   :offers #{"counselling_refugees" "crisis_intervention_refugees"}
   :contacts {:phone "0351 1234 5678"
@@ -151,7 +150,6 @@
   :xt/spec :swlkup.model.supervisor/doc
   :swlkup.model.login/login:ids "[email protected]"
   :name_full "Maria Musterfrau"
-  :photo "https://raw.githubusercontent.com/wiki/community-garden/community-garden.github.io/images/team_avatar_stephanie.png"
   :languages [:it :es :fr :ar]
   :offers #{"counseling_refugees" "crisis_intervention_refugees"
             "mediation" "moderation" "supervision" "workshops"}

diff --git a/backend/src/swlkup/resolver/root/supervisor/deactivate.clj b/backend/src/swlkup/resolver/root/supervisor/deactivate.clj
@@ -3,8 +3,7 @@
             [specialist-server.type :as t]
             [swlkup.auth.core :refer [auth+role->entity]]
             [swlkup.model.auth :as auth]
-            [swlkup.model.supervisor :as supervisor]
-            [xtdb.api :as xt]))
+            [swlkup.model.supervisor :as supervisor]))
 
 (s/fdef supervisor_deactivate
         :args (s/tuple map? (s/keys :req-un [::auth/auth ::supervisor/deactivated]) map? map?)
@@ -21,7 +20,7 @@
                                    '(fn [ctx eid deactivated]
                                         (let [db (xtdb.api/db ctx)
                                               entity (xtdb.api/entity db eid)]
-                                             [[::xt/put (assoc entity :deactivated deactivated)]])))
+                                             [[:xtdb.api/put (assoc entity :deactivated deactivated)]])))
                         (tx-fn-call :deactivate-supervisor supervisor:id (:deactivated opt)))]
        (sync)
        (boolean (:xtdb.api/tx-id tx_result))))

diff --git a/backend/src/swlkup/resolver/root/supervisor/update.clj b/backend/src/swlkup/resolver/root/supervisor/update.clj
@@ -12,15 +12,50 @@
         :ret t/boolean)
 
 (defn supervisor_update
-  "Update a supervisors data"
+  "Update a supervisors profile
+
+   (:supervisor_input opt) will be merged into an existing database entry when existing.
+   To delete an entry, it must explicitly be in the dictionary.
+   This allows editing a profile without overwriting :photo and :deactivated.
+  "
   [_node opt ctx _info]
-  (let [{:keys [tx_sync]} (:db_ctx ctx)
+  (let [{:keys [tx-fn-put tx-fn-call sync]} (:db_ctx ctx)
         [supervisor:id login:id] (auth+role->entity ctx (:auth opt) ::supervisor/doc)
         tx_result (when login:id  ;; any login, independent of role can be used
-                        (tx_sync [[:xtdb.api/put (assoc (:supervisor_input opt)
-                                                         :xt/id (or supervisor:id (uuid))
-                                                         :xt/spec ::supervisor/doc
-                                                         ::login/login:ids login:id)]]))]
+                        (tx-fn-put :merge-supervisor-profile
+                                   '(fn [ctx eid doc login:id]
+                                        (let [db (xtdb.api/db ctx)
+                                              entity (xtdb.api/entity db eid)]
+                                             [[:xtdb.api/put (assoc (merge entity doc)
+                                                                    :xt/id eid
+                                                                    :xt/spec ::supervisor/doc
+                                                                    ::login/login:ids login:id)]])))
+                        (tx-fn-call :merge-supervisor-profile (or supervisor:id (uuid))
+                                                              (:supervisor_input opt)
+                                                              login:id))]
+       (sync)
        (boolean (:xtdb.api/tx-id tx_result))))
 
 (s/def ::supervisor_update (t/resolver #'supervisor_update))
+
+
+(defn supervisor_update_photo
+  "Update a supervisors profile picture
+
+   This mutation is not directly usable (via graphql API), but only called from the `upload-supervisor-picture` handler.
+   That gives us full control over the url and avoids:
+   * potential XSS via SVG
+   * tracking of users by images embedded from other servers
+  "
+  [_node opt ctx _info]
+  (let [{:keys [tx-fn-put tx-fn-call sync]} (:db_ctx ctx)
+        [supervisor:id _login:id] (auth+role->entity ctx (:auth opt) ::supervisor/doc)
+        tx_result (when supervisor:id
+                        (tx-fn-put :update-supervisor-photo
+                                   '(fn [ctx eid photo]
+                                        (let [db (xtdb.api/db ctx)
+                                              entity (xtdb.api/entity db eid)]
+                                             [[:xtdb.api/put (assoc entity :photo photo)]])))
+                        (tx-fn-call :update-supervisor-photo supervisor:id (:photo opt)))]
+       (sync)
+       (boolean (:xtdb.api/tx-id tx_result))))
diff --git a/backend/src/swlkup/webserver/upload/state.clj b/backend/src/swlkup/webserver/upload/state.clj
@@ -7,7 +7,7 @@
             [clojure.string :as string :refer [split]]
             [swlkup.auth.core :refer [auth+role->entity]]
             [swlkup.db.state :refer [db_ctx]]
-            [swlkup.resolver.root.supervisor.update]
+            [swlkup.resolver.root.supervisor.update :refer [supervisor_update supervisor_update_photo]]
             [swlkup.model.supervisor :as supervisor]))
 
 (defn -upload-supervisor-picture
@@ -23,20 +23,24 @@
                                       (if supervisor:id
                                           [supervisor:id login:id]
                                           (when login:id
-                                                #_(println "create profile")
                                                 (let [opt {:auth auth
                                                            :supervisor_input supervisor/empty}]
-                                                     (swlkup.resolver.root.supervisor.update/supervisor_update nil opt ctx nil))
+                                                     (supervisor_update nil opt ctx nil))
                                                 (auth+role->entity ctx auth ::supervisor/doc))))
 
-        dest (str (:upload-dir env) "/" supervisor:id ".jpeg")]
+        dest (str (:upload-dir env) "/" supervisor:id ".jpeg")
+        public_path (str "/uploads/" supervisor:id ".jpeg")]
        (if-not supervisor:id
                {:status 403
                 :body "Invalid login:id, ensure you are logged in!"}
                (if (> (.length (io/file source)) (* 1024 1024 (:upload-limit-mb env)))
                    {:status 413
                     :body (str "Upload exceeds maximum size of " (:upload-limit-mb env) "MB")}
                    (do (io/copy (io/file source) (io/file dest))
+                       (let [simple_hash (hash (slurp (io/file dest)))  ;; This is not an cryptographic integrety check, but only to determine caching.
+                             opt {:auth auth
+                                  :photo (str public_path "?" simple_hash)}]
+                            (supervisor_update_photo nil opt ctx nil))
                        (response "Upload Successful"))))))
 
 (defn -serve-uploaded-supervisor-picture

diff --git a/frontend/codegen/generates.ts b/frontend/codegen/generates.ts
@@ -63,7 +63,14 @@ export type MutationType = {
   supervisor_delete: Scalars['Boolean'];
   /** Add a new supervisor account to the database and send a mail containing the password via mail */
   supervisor_register: Scalars['Boolean'];
-  /** Update a supervisors data */
+  /**
+   * Update a supervisors profile
+   *
+   *    (:supervisor_input opt) will be merged into an existing database entry when existing.
+   *    To delete an entry, it must explicitly be in the dictionary.
+   *    This allows editing a profile without overwriting :photo and :deactivated.
+   *
+   */
   supervisor_update: Scalars['Boolean'];
 };
 
@@ -167,6 +174,7 @@ export type QueryTypeSupervisors_RegisteredArgs = {
 export type SupervisorInput = {
   /** Self descriptive. */
   contacts: ContactsInput;
+  deactivated?: InputMaybe<Scalars['Boolean']>;
   languages: Array<Scalars['String']>;
   /** Self descriptive. */
   location: LocationInput;
@@ -320,7 +328,14 @@ export type SupervisorGetQueryVariables = Exact<{
 }>;
 
 
-export type SupervisorGetQuery = { __typename?: 'QueryType', supervisor_get?: { __typename?: 'supervisor_get', id: string, deactivated?: boolean | null | undefined, ngos: any, name_full: string, languages: Array<string>, offers: Array<string>, photo?: string | null | undefined, text_specialization?: string | null | undefined, text?: string | null | undefined, contacts: { __typename?: 'Contacts', phone?: string | null | undefined, website?: string | null | undefined, email?: string | null | undefined }, location: { __typename?: 'Location', zip?: string | null | undefined } } | null | undefined, languages: Array<{ __typename?: 'languages', id: string, name: string, flag_url: string, idx: number }>, offers: Array<{ __typename?: 'offers', id: string, target: string, idx: number }>, ngos: Array<{ __typename?: 'ngos', id?: string | null | undefined, name?: string | null | undefined }> };
+export type SupervisorGetQuery = { __typename?: 'QueryType', supervisor_get?: { __typename?: 'supervisor_get', id: string, deactivated?: boolean | null | undefined, ngos: any, name_full: string, languages: Array<string>, offers: Array<string>, text_specialization?: string | null | undefined, text?: string | null | undefined, contacts: { __typename?: 'Contacts', phone?: string | null | undefined, website?: string | null | undefined, email?: string | null | undefined }, location: { __typename?: 'Location', zip?: string | null | undefined } } | null | undefined, languages: Array<{ __typename?: 'languages', id: string, name: string, flag_url: string, idx: number }>, offers: Array<{ __typename?: 'offers', id: string, target: string, idx: number }>, ngos: Array<{ __typename?: 'ngos', id?: string | null | undefined, name?: string | null | undefined }> };
+
+export type SupervisorGetPhotoQueryVariables = Exact<{
+  auth: Auth;
+}>;
+
+
+export type SupervisorGetPhotoQuery = { __typename?: 'QueryType', supervisor_get?: { __typename?: 'supervisor_get', photo?: string | null | undefined } | null | undefined };
 
 export type NgoQueryVariables = Exact<{
   auth: Auth;
@@ -434,7 +449,6 @@ export const SupervisorGetDocument = `
     location {
       zip
     }
-    photo
     text_specialization
     text
   }
@@ -467,6 +481,25 @@ export const useSupervisorGetQuery = <
       fetcher<SupervisorGetQuery, SupervisorGetQueryVariables>(SupervisorGetDocument, variables),
       options
     );
+export const SupervisorGetPhotoDocument = `
+    query SupervisorGetPhoto($auth: Auth!) {
+  supervisor_get(auth: $auth) {
+    photo
+  }
+}
+    `;
+export const useSupervisorGetPhotoQuery = <
+      TData = SupervisorGetPhotoQuery,
+      TError = unknown
+    >(
+      variables: SupervisorGetPhotoQueryVariables,
+      options?: UseQueryOptions<SupervisorGetPhotoQuery, TError, TData>
+    ) =>
+    useQuery<SupervisorGetPhotoQuery, TError, TData>(
+      ['SupervisorGetPhoto', variables],
+      fetcher<SupervisorGetPhotoQuery, SupervisorGetPhotoQueryVariables>(SupervisorGetPhotoDocument, variables),
+      options
+    );
 export const NgoDocument = `
     query Ngo($auth: Auth!) {
   created_tokens(auth: $auth) {

diff --git a/frontend/codegen/queries.ts b/frontend/codegen/queries.ts
@@ -68,7 +68,6 @@ export const supervisor_get = gql`
       location {
         zip
       }
-      photo
       text_specialization
       text
     }
@@ -92,6 +91,13 @@ export const supervisor_get = gql`
     }
   }`
 
+export const supervisor_get_photo = gql`
+  query SupervisorGetPhoto($auth: Auth!) {
+    supervisor_get(auth: $auth) {
+      photo
+    }
+  }`
+
 export const ngo = gql`
   query Ngo($auth: Auth!) {
     created_tokens(auth: $auth) {token purpose valid}

diff --git a/frontend/components/supervisor/ProfilePictureUpload.tsx b/frontend/components/supervisor/ProfilePictureUpload.tsx
@@ -1,7 +1,7 @@
 import { useEffect } from 'react'
 import { useTranslation } from 'react-i18next';
 import { useAuthStore } from '../../components/Login'
-import { useSupervisorGetQuery } from '../../codegen/generates'
+import { useSupervisorGetPhotoQuery } from '../../codegen/generates'
 import { config, fetch_config } from "../../config";
 
 function upload_profile_picture(jwt: String, upload_url: URL, refetch: any) {
@@ -26,19 +26,20 @@ export function ProfilePictureUpload() {
   const {t} = useTranslation()
   const auth = useAuthStore()
 
-  const {data, refetch} = useSupervisorGetQuery({auth}, {enabled: Boolean(auth.jwt)})
-  const supervisor = data?.supervisor_get
+  /** We use a separate query for the profile picture. This allows refetching without compromising unsubmitted changes in the form. **/
+  const {data, refetch} = useSupervisorGetPhotoQuery({auth}, {enabled: Boolean(auth.jwt)})
+  const photo = data?.supervisor_get?.photo
 
-  const img_url = `${config.backend_base_url}/uploads/${supervisor?.id}.jpeg?${new Date().getTime()}`  // TODO: url from supervisor.photo
   const upload_url = `${config.backend_base_url}/api/upload-supervisor-picture` as any as URL
+  const img_src = `${config.backend_base_url}${photo}`
 
   return (
     <>
       <div style={{display: "inline-block"}}>
         <input type="file" id="profilePicture" name="upload" accept="image/jpeg"/><br/>
         <input type="button" value={ t('Upload') as string } onClick={ () => upload_profile_picture(auth.jwt, upload_url, refetch) } />
       </div>
-      <img src={img_url} style={{maxHeight: "100px", maxWidth: "50%", verticalAlign: "middle"}}/>
+      { photo && <img src={img_src} style={{maxHeight: "100px", maxWidth: "50%", verticalAlign: "middle"}}/> }
     </>
   )
 }
diff --git a/frontend/components/user/LookupResult.tsx b/frontend/components/user/LookupResult.tsx
@@ -132,7 +132,7 @@ function FilterForm({languages, offers, selections}:
 
 function Supervisor({supervisor, languages, backend_base_url}:
                     {supervisor: Supervisors, languages: Languages[], backend_base_url: URL}) {
-  const img_url = `${backend_base_url}/uploads/${supervisor?.id}.jpeg`  // TODO supervisor.photo
+  const img_src = `${config.backend_base_url}${supervisor?.photo}`
 
   return (
     <div className={styles.card}>
@@ -150,9 +150,10 @@ function Supervisor({supervisor, languages, backend_base_url}:
           <tr>
             <td>{supervisor.text}</td>
             <td style={{textAlign: "right"}}>
-              { // supervisor.photo &&  /** TODO **/
-		<img src={img_url} style={{width: "110px", minWidth: "110px",  /** enough to display 4 flags above it **/
-                                           maxHeight: "200px"}}/> }
+              { supervisor?.photo && <img src={img_src}
+                                          style={{width: "110px", minWidth: "110px",  /** enough to display 4 flags above it **/
+                                                  maxHeight: "200px"}}/>
+              }
             </td>
           </tr>
         </tbody>