Merge branch 'master' of github.com:CSCfi/rems into event-push-2095

CHANGELOG.md

@@ -8,12 +8,21 @@ have notable changes.
 
 Changes since v2.11
 
-### Additions
-- Event notifications over HTTP. See [docs/event-notification.md](docs/event-notification.md) for details. (#2095)
+### Changes
+- Improvements to PDFs (#2114)
+  - show attachment file names
+  - list instead of table for events
+  - hide draft-saved events
+  - vertical space around form fields
+  - PDF button moved to Actions pane
 
 ### Fixes
 - Long attachment filenames are now truncated in the UI (#2118)
 
+### Additions
+- `api/applications/:id/attachments` api for downloading all attachments as a zip file (#2075)
+- Event notifications over HTTP. See [docs/event-notification.md](docs/event-notification.md) for details. (#2095)
+
 ## v2.11 "Kotitontuntie" 2020-04-07
 
 ### Additions

dev-config.edn

@@ -23,7 +23,9 @@
                 :translations {:fi {:title "Info"
                                     :filename "about-fi.md"}
                                :en {:title "About"
-                                    :filename "about-en.md"}}}]
+                                    :filename "about-en.md"}
+                               :sv {:title "Info"
+                                    :filename "about-sv.md"}}}]
  :extra-pages-path "./test-data/extra-pages"
  :application-deadline-days 4
  :enable-pdf-api true

docs/architecture/010-transactions.md

@@ -1,6 +1,6 @@
 # 010: Database transactions
 
-Authors: @opqdonut
+Authors: @opqdonut @Macroz
 
 ## Introduction
 

docs/architecture/011-api-key-access-control.md

@@ -0,0 +1,92 @@
+# 011: API Key Access Control
+
+Authors: @opqdonut @Macroz @foxlynx
+
+## Background
+
+REMS access control has always been user-based. For example all
+application commands must have an actor, and the code checks whether
+this actor is allowed to perform this command. API authentication has
+been done with two headers: `x-rems-api-key` which must be a valid api
+key, and `x-rems-user-id`, which specifies which user is being
+impersonated.
+
+Some users have had a need to limit the access available with API
+keys. In response, we added a list of allowed roles to each API key.
+The roles were a whitelist, with which the roles of the impersonated
+user were filtered.
+
+This approach works fine for so-called _explicit roles_ that are
+granted using the `role` table in the database. However, we also have
+_implicit roles_ that are granted per application, based on the
+application state. For example, a user has the `:reviewer` role for an
+application if they have been invited to review the application. When
+the API key roles were implemented, they were (accidentally?) only
+implemented for explicit roles. Extending the implementation for
+implicit roles would require more code.
+
+As a concrete example, an API key with only the role `:logged-in`
+would still be able to impersonate the handler of an application and
+perform handler commands like approving the application.
+
+In order to support the use cases at the end of this document, we need
+to either fix the interaction between API key roles & implicit roles,
+or choose a new approach.
+
+## Proposed approach
+
+Let's dismantle the API key role support, and instead associate with
+each API key:
+
+- an optional whitelist of HTTP paths and methods that the API key is allowed to access
+- an optional whitelist of users that the API key is allowed to impersonate
+
+## Use cases
+
+Here are some use cases for API key access control, and a comparison
+of the current approach with the proposed approach.
+
+These are from the NKR project, see
+[Issue #1910](https://github.com/CSCfi/rems/issues/1910).
+
+### Creating users and applications
+
+Use case: a proxy service creates a user, and and applies for a
+resource as the created user.
+
+Current approach: API key with the `:user-owner` and `:applicant`
+roles. Additional work would be required to actually support the
+`:applicant` role for an API key (it's an implicit role).
+
+Proposed approach: Two API keys:
+
+- One API key for creating users. It can only impersonate a fixed user
+  with the `:user-owner` role. (Can also be limited to the
+  `/api/users/create` endpoint.)
+- Another API key for creating applications. It can impersonate
+  anyone, but is limited to the `/api/application/create`,
+  `/api/application/save-draft` and `/api/application/submit`
+  endpoints.
+
+### Fetching entitlements and applications
+
+Use case: fetching entitlements and applications for any applicant.
+
+Current approach: API key with the `:reporter` role.
+
+Proposed approach: API key associated with a user with the `:reporter`
+role. (Additional path restrictions possible if needed.)
+
+### Closing applications
+
+Use case: closing applications (and thus ending entitlements) that are no longer needed.
+
+Current approach: A new `:application-closer` role, and an API key
+associated with it. Also needs fixes to the handling of implicit
+roles.
+
+Proposed approach: An API key associated with a user that is a handler
+for suitable workflows. The API key is limited to the
+`/api/applications/close` POST request, and some additional GET
+requests as needed.
+

docs/configuration.md

@@ -81,7 +81,7 @@ create the following <b>en.edn</b> file to the new translations folder.
 
 Custom themes can be used by creating a file, for example `my-custom-theme.edn`, and specifying its location in the `:theme-path` configuration parameter. The theme file can override some or all of the theme attributes (see `:theme` in [config-defaults.edn](https://github.com/CSCfi/rems/blob/master/resources/config-defaults.edn)). Static resources can be placed in a `public` directory next to the theme configuration file. See [example-theme/theme.edn](https://github.com/CSCfi/rems/blob/master/example-theme/theme.edn) for an example.
 
-To quickly validate that all UI components look right navigate to `/guide`. See it in action at <https://rems2demo.csc.fi/guide>.
+To quickly validate that all UI components look right navigate to `/guide`. See it in action at <https://rems-demo.rahtiapp.fi/guide>.
 
 ## Extra pages
 

docs/linking.md

@@ -5,25 +5,25 @@ REMS supports linking users directly to application forms, pre-existing applicat
 ## Linking into catalogue
 
 ```
-https://rems2demo.csc.fi/catalogue
+https://rems-demo.rahtiapp.fi/catalogue
 ```
 
 ## Linking into a new application
 
-This application has items with catalogue item ids 2 and 3.
+This creates a draft for an application for catalogue item ids 2 and 3.
 
 ```
-https://rems2demo.csc.fi/application?items=2,3
+https://rems-demo.rahtiapp.fi/application?items=2,3
 ```
 
 If only the resource ID is known, this will find out which catalogue item matches it and will redirect to the new application page for it.
 
 ```
-https://rems2demo.csc.fi/apply-for?resource=urn:nbn:fi:lb-123456789
+https://rems-demo.rahtiapp.fi/apply-for?resource=urn:nbn:fi:lb-123456789
 ```
 
 ## Linking into an existing application
 
 ```
-https://rems2demo.csc.fi/application/2
+https://rems-demo.rahtiapp.fi/application/2
 ```

docs/usingtheapi.md

@@ -3,7 +3,7 @@
 These examples assume that the REMS instance you want to talk to is running locally at `localhost:3000`.
 
 ## Authentication
- 
+
 To call the API programmatically, you will first need to add an API key to the `api_key` database table. The API key must be provided in the `x-rems-api-key` header when calling the API.
 
 Some API endpoints also require `x-rems-user-id` header to contain the REMS user ID for the user that is being represented, i.e. the user which applies for a resource or approves an application.
@@ -52,6 +52,6 @@ Returns the list of catalogue items as a JSON response:
 
 ## Learn More
 
-See the [REMS API documentation](https://rems2demo.csc.fi/swagger-ui) for a list of all available operations.
+See the [REMS API documentation](https://rems-demo.rahtiapp.fi/swagger-ui) for a list of all available operations.
 
-You may also inspect what API request the REMS UI does using your web browser's developer tools. The REMS UI does its requests using the `application/transit+json` content-type, but all the APIs work also using `application/json` (which is the default). 
+You may also inspect what API request the REMS UI does using your web browser's developer tools. The REMS UI does its requests using the `application/transit+json` content-type, but all the APIs work also using `application/json` (which is the default).

example-theme/theme.edn

@@ -35,6 +35,8 @@
  :logo-name-sm-fi "rems_logo_fi.png"
  :logo-name-en "rems_logo_en.png"
  :logo-name-sm-en "rems_logo_en.png"
+ :logo-name-sv "rems_logo_sv.png"
+ :logo-name-sm-sv "rems_logo_sv.png"
  :logo-content-origin "initial"
  :phase-color "#111"
  :phase-bgcolor "#f8f8f8"

project.clj

@@ -7,7 +7,7 @@
                  [ch.qos.logback/logback-classic "1.2.3"]
                  [clj-commons/secretary "1.2.4"]
                  [clj-http "3.10.0"]
-                 [clj-pdf "2.4.0"]
+                 [clj-pdf "2.4.3"]
                  [clj-time "0.15.2"]
                  [cljs-ajax "0.8.0"]
                  [cljsjs/react "16.9.0-1"]

resources/translations/en.edn

@@ -319,15 +319,14 @@
          :alert-disabled-resources "Resources no longer available:"
          :application "Application"
          :attachment-remove "Remove"
+         :attachments "Attachments"
          :checkbox-checked "Yes"
          :checkbox-unchecked "No"
          :comment "Comment"
          :copy-as-new "Copy as a new application"
          :current-value "Current"
-         :date "Time"
          :diff-hide "Hide changes"
          :diff-show "Show changes"
-         :event "Event"
          :events "Events"
          :failed "Failed"
          :has-accepted-licenses "Terms of use accepted."

resources/translations/fi.edn

@@ -65,7 +65,7 @@
                    :create-workflow "Uusi työvuo"
                    :created "Luotu"
                    :disable "Poista käytöstä"
-                   :disabled-and-archived-explanation "Käytöstä poistaminen piilottaa kielivaran käyttäjiltä. Arkistoiminen piilottaa sen myös ylläpitonäkymästä."
+                   :disabled-and-archived-explanation "Käytöstä poistaminen piilottaa asian käyttäjiltä. Arkistoiminen piilottaa sen myös ylläpitonäkymästä."
                    :display-archived "Näytä arkistoidut"
                    :edit "Muokkaa"
                    :edit-catalogue-item "Muokkaa aineistoa"
@@ -317,15 +317,14 @@
          :alert-disabled-resources "Resurssit eivät ole enää saatavilla:"
          :application "Tiedot"
          :attachment-remove "Poista"
+         :attachments "Liitteet"
          :checkbox-checked "Kyllä"
          :checkbox-unchecked "Ei"
          :comment "Kommentti"
          :copy-as-new "Kopioi uudeksi hakemukseksi"
          :current-value "Nykyinen"
-         :date "Aika"
          :diff-hide "Piilota muutokset"
          :diff-show "Näytä muutokset"
-         :event "Tapahtuma"
          :events "Tapahtumat"
          :failed "Epäonnistui"
          :has-accepted-licenses "Käyttöehdot hyväksytty."

resources/translations/sv.edn

@@ -65,7 +65,7 @@
                    :create-workflow "Nytt arbetsflöde"
                    :created "Skapad"
                    :disable "Avaktivera"
-                   :disabled-and-archived-explanation "Avaktiveringen döljer resursen för användarna. Arkiveringen döljer den också från administrationsvyn."
+                   :disabled-and-archived-explanation "Avaktiveringen döljer föremål för användarna. Arkiveringen döljer den också från administrationsvyn."
                    :display-archived "Visa arkiverade"
                    :edit "Ändra"
                    :edit-catalogue-item "Ändra katalogpost"
@@ -317,15 +317,14 @@
          :alert-disabled-resources "Resurser som inte längre är tillgängliga:"
          :application "Ansökning"
          :attachment-remove "Ta bort"
+         :attachments "Bilagor"
          :checkbox-checked "Ja"
          :checkbox-unchecked "Nej"
          :comment "Komment"
          :copy-as-new "Kopiera som nytt"
          :current-value "Nuvarande"
-         :date "Datum"
          :diff-hide "Dölja ändringar"
          :diff-show "Visa ändringar"
-         :event "Händelse"
          :events "Händelser"
          :failed "Misslyckades"
          :has-accepted-licenses "Har accepterat licenserna."

src/clj/rems/api/applications.clj

@@ -266,6 +266,16 @@
         (ok app)
         (api-util/not-found-json-response)))
 
+    (GET "/:application-id/attachments" []
+      :summary "Get all attachments as a zip file"
+      :roles #{:logged-in}
+      :path-params [application-id :- (describe s/Int "application id")]
+      :responses {200 {}
+                  404 {:schema s/Str :description "Not found"}}
+      (if-let [app (applications/get-application-for-user (getx-user-id) application-id)]
+        (attachment/zip-attachments app)
+        (api-util/not-found-json-response)))
+
     (GET "/:application-id/experimental/pdf" request
       :summary "PDF export of application (EXPERIMENTAL)"
       :roles #{:logged-in :api-key}

src/clj/rems/api/services/attachment.clj

@@ -1,13 +1,16 @@
 (ns rems.api.services.attachment
   (:require [clojure.set :as set]
-            [clojure.test :refer :all]
+            [clojure.tools.logging :as log]
             [rems.application.commands :as commands]
             [rems.common.application-util :as application-util]
             [rems.auth.util :refer [throw-forbidden]]
             [rems.db.applications :as applications]
             [rems.db.attachments :as attachments]
-            [ring.util.http-response :refer [ok content-type header]])
-  (:import [java.io ByteArrayInputStream]))
+            [rems.util :refer [getx]]
+            [ring.util.http-response :refer [ok content-type header]]
+            [ring.util.io :as ring-io])
+  (:import [java.io ByteArrayInputStream ByteArrayOutputStream]
+           [java.util.zip ZipOutputStream ZipEntry ZipException]))
 
 (defn download [attachment]
   (-> (ok (ByteArrayInputStream. (:attachment/data attachment)))
@@ -41,3 +44,23 @@
                     (:application/permissions application))
       (throw-forbidden))
     (attachments/save-attachment! file user-id application-id)))
+
+(defn zip-attachments [application]
+  (let [zip-input (ring-io/piped-input-stream
+                   (fn [out]
+                     (with-open [zip (ZipOutputStream. out)]
+                       (doseq [metadata (getx application :application/attachments)]
+                         (let [id (getx metadata :attachment/id)
+                               attachment (attachments/get-attachment id)]
+                           ;; we deduplicate filenames when uploading, but here's a
+                           ;; failsafe in case we have duplicate filenames in old
+                           ;; applications
+                           (try
+                             (.putNextEntry zip (ZipEntry. (getx attachment :attachment/filename)))
+                             (.write zip (getx attachment :attachment/data))
+                             (.closeEntry zip)
+                             (catch ZipException e
+                               (log/warn "Ignoring attachment" (pr-str metadata) "when generating zip. Cause:" e))))))))]
+    (-> (ok zip-input)
+        (header "Content-Disposition" (str "attachment;filename=attachments-" (getx application :application/id) ".zip"))
+        (content-type "application/zip"))))

src/clj/rems/db/applications.clj

@@ -4,6 +4,7 @@
             [clojure.java.jdbc :as jdbc]
             [clojure.set :as set]
             [clojure.test :refer [deftest is]]
+            [clojure.tools.logging :as log]
             [conman.core :as conman]
             [medley.core :refer [map-vals]]
             [mount.core :as mount]
@@ -259,10 +260,12 @@
     (csv/applications-to-csv filtered-applications user-id)))
 
 (defn reload-cache! []
+  (log/info "Start rems.db.applications/reload-cache!")
   (empty-injections-cache!)
   ;; TODO: Here is a small chance that a user will experience a cache miss. Consider rebuilding the cache asynchronously and then `reset!` the cache.
   (events-cache/empty! all-applications-cache)
-  (refresh-all-applications-cache!))
+  (refresh-all-applications-cache!)
+  (log/info "Finished rems.db.applications/reload-cache!"))
 
 ;; empty the cache occasionally in case some of the injected entities are changed
 (mount/defstate all-applications-cache-reloader