JAMES-1779 document jmap download limitations

server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/JmapAuthentication.java

@@ -32,6 +32,10 @@ public static AccessToken authenticateJamesUser(String username, String password
             with()
                 .body("{\"token\": \"" + continuationToken + "\", \"method\": \"password\", \"password\": \"" + password + "\"}")
             .post("/authentication")
+            .then()
+                .statusCode(201)
+                .log().ifError()
+            .extract()
                 .body()
                 .jsonPath()
                 .getString("accessToken")

server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/DownloadStepdefs.java

@@ -24,13 +24,18 @@
 import static org.hamcrest.Matchers.notNullValue;
 
 import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
 
 import javax.inject.Inject;
 import javax.mail.Flags;
 
+import org.apache.james.jmap.api.access.AccessToken;
 import org.apache.james.mailbox.model.MailboxConstants;
 import org.apache.james.mailbox.model.MailboxPath;
 
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.Multimap;
 import com.jayway.restassured.response.Response;
 
 import cucumber.api.java.en.Given;
@@ -44,58 +49,63 @@ public class DownloadStepdefs {
     private final UserStepdefs userStepdefs;
     private final MainStepdefs mainStepdefs;
     private Response response;
+    private Multimap<String, String> attachmentsByMessageId;
+    private Map<String, String> blobIdByAttachmentId;
 
     @Inject
     private DownloadStepdefs(MainStepdefs mainStepdefs, UserStepdefs userStepdefs) {
         this.mainStepdefs = mainStepdefs;
         this.userStepdefs = userStepdefs;
+        this.attachmentsByMessageId = ArrayListMultimap.create();
+        this.blobIdByAttachmentId = new HashMap<>();
     }
 
-    @Given("^a message containing an attachment$")
-    public void appendMessageWithAttachment() throws Exception {
-        mainStepdefs.jmapServer.serverProbe().createMailbox(MailboxConstants.USER_NAMESPACE, userStepdefs.username, "INBOX");
-        MailboxPath mailboxPath = new MailboxPath(MailboxConstants.USER_NAMESPACE, userStepdefs.username, "INBOX");
+    @Given("^\"([^\"]*)\" mailbox \"([^\"]*)\" contains a message \"([^\"]*)\" with an attachment \"([^\"]*)\"$")
+    public void appendMessageWithAttachmentToMailbox(String user, String mailbox, String messageId, String attachmentId) throws Throwable {
+        MailboxPath mailboxPath = new MailboxPath(MailboxConstants.USER_NAMESPACE, user, mailbox);
 
-        mainStepdefs.jmapServer.serverProbe().appendMessage(userStepdefs.username, mailboxPath,
+        mainStepdefs.jmapServer.serverProbe().appendMessage(user, mailboxPath,
                 ClassLoader.getSystemResourceAsStream("eml/oneAttachment.eml"), new Date(), false, new Flags());
+
+        attachmentsByMessageId.put(messageId, attachmentId);
+        blobIdByAttachmentId.put(attachmentId, "4000c5145f633410b80be368c44e1c394bff9437");
     }
 
-    @When("^checking for the availability of the attachment endpoint$")
-    public void optionDownload() throws Throwable {
-        if (userStepdefs.accessToken != null) {
-            with().header("Authorization", userStepdefs.accessToken.serialize());
+    @When("^\"([^\"]*)\" checks for the availability of the attachment endpoint$")
+    public void optionDownload(String username) throws Throwable {
+        AccessToken accessToken = userStepdefs.tokenByUser.get(username);
+        if (accessToken != null) {
+            with().header("Authorization", accessToken.serialize());
         }
 
         response = with().options("/download/myBlob");
     }
 
-    @When("^asking for an attachment$")
-    public void getDownload() throws Exception {
-        if (userStepdefs.accessToken != null) {
-            with().header("Authorization", userStepdefs.accessToken.serialize());
+    @When("^\"([^\"]*)\" downloads \"([^\"]*)\"$")
+    public void downloads(String username, String attachmentId) throws Throwable {
+        String blobId = blobIdByAttachmentId.get(attachmentId);
+        AccessToken accessToken = userStepdefs.tokenByUser.get(username);
+        if (accessToken != null) {
+            with().header("Authorization", accessToken.serialize());
         }
-
-        response = with().get("/download/myBlob");
+        response = with().get("/download/" + blobId);
     }
+
 
-    @When("^asking for an attachment without blobId parameter$")
-    public void getDownloadWithoutBlobId() throws Throwable {
+    @When("^\"([^\"]*)\" asks for an attachment without blobId parameter$")
+    public void getDownloadWithoutBlobId(String username) throws Throwable {
+        AccessToken accessToken = userStepdefs.tokenByUser.get(username);
         response = with()
-            .header("Authorization", userStepdefs.accessToken.serialize())
+            .header("Authorization", accessToken.serialize())
             .get("/download/");
     }
+
 
-    @When("^getting the attachment with its correct blobId$")
-    public void getDownloadWithKnownBlobId() throws Throwable {
-        response = with()
-                .header("Authorization", userStepdefs.accessToken.serialize())
-                .get("/download/4000c5145f633410b80be368c44e1c394bff9437");
-    }
-
-    @When("^getting the attachment with an unknown blobId$")
-    public void getDownloadWithUnknownBlobId() throws Throwable {
+    @When("^\"([^\"]*)\" asks for an attachment with wrong blobId$")
+    public void getDownloadWithWrongBlobId(String username) throws Throwable {
+        AccessToken accessToken = userStepdefs.tokenByUser.get(username);
         response = with()
-                .header("Authorization", userStepdefs.accessToken.serialize())
+                .header("Authorization", accessToken.serialize())
                 .get("/download/badbadbadbadbadbadbadbadbadbadbadbadbadb");
     }
 

server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/SetMailboxesMethodStepdefs.java

@@ -37,7 +37,6 @@
 import org.apache.james.mailbox.store.mail.model.Mailbox;
 
 import com.github.fge.lambdas.Throwing;
-import com.jayway.restassured.http.ContentType;
 
 import cucumber.api.java.en.Given;
 import cucumber.api.java.en.Then;
@@ -61,8 +60,8 @@ private SetMailboxesMethodStepdefs(MainStepdefs mainStepdefs, UserStepdefs userS
 
     @Given("^mailbox \"([^\"]*)\" with (\\d+) messages$")
     public void mailboxWithMessages(String mailboxName, int messageCount) throws Throwable {
-        mainStepdefs.jmapServer.serverProbe().createMailbox("#private", userStepdefs.username, mailboxName);
-        MailboxPath mailboxPath = new MailboxPath(MailboxConstants.USER_NAMESPACE, userStepdefs.username, mailboxName);
+        mainStepdefs.jmapServer.serverProbe().createMailbox("#private", userStepdefs.lastConnectedUser, mailboxName);
+        MailboxPath mailboxPath = new MailboxPath(MailboxConstants.USER_NAMESPACE, userStepdefs.lastConnectedUser, mailboxName);
         IntStream
             .range(0, messageCount)
             .forEach(Throwing.intConsumer(i -> appendMessage(mailboxPath, i)));
@@ -72,13 +71,14 @@ public void mailboxWithMessages(String mailboxName, int messageCount) throws Thr
     private void appendMessage(MailboxPath mailboxPath, int i) throws MailboxException {
         String content = "Subject: test" + i + "\r\n\r\n"
                 + "testBody" + i;
-        mainStepdefs.jmapServer.serverProbe().appendMessage(userStepdefs.username, mailboxPath,
+        mainStepdefs.jmapServer.serverProbe().appendMessage(userStepdefs.lastConnectedUser, mailboxPath,
                 new ByteArrayInputStream(content.getBytes()), new Date(), false, new Flags());
     }
 
     @When("^renaming mailbox \"([^\"]*)\" to \"([^\"]*)\"")
     public void renamingMailbox(String actualMailboxName, String newMailboxName) throws Throwable {
-        Mailbox mailbox = mainStepdefs.jmapServer.serverProbe().getMailbox("#private", userStepdefs.username, actualMailboxName);
+        String username = userStepdefs.lastConnectedUser;
+        Mailbox mailbox = mainStepdefs.jmapServer.serverProbe().getMailbox("#private", userStepdefs.lastConnectedUser, actualMailboxName);
         String mailboxId = mailbox.getMailboxId().serialize();
         String requestBody =
                 "[" +
@@ -95,16 +95,17 @@ public void renamingMailbox(String actualMailboxName, String newMailboxName) thr
                     "]";
 
         with()
-            .header("Authorization", userStepdefs.accessToken.serialize())
+            .header("Authorization", userStepdefs.tokenByUser.get(username).serialize())
             .body(requestBody)
             .post("/jmap");
     }
 
     @When("^moving mailbox \"([^\"]*)\" to \"([^\"]*)\"$")
     public void movingMailbox(String actualMailboxPath, String newParentMailboxPath) throws Throwable {
-        Mailbox mailbox = mainStepdefs.jmapServer.serverProbe().getMailbox("#private", userStepdefs.username, actualMailboxPath);
+        String username = userStepdefs.lastConnectedUser;
+        Mailbox mailbox = mainStepdefs.jmapServer.serverProbe().getMailbox("#private", username, actualMailboxPath);
         String mailboxId = mailbox.getMailboxId().serialize();
-        Mailbox parent = mainStepdefs.jmapServer.serverProbe().getMailbox("#private", userStepdefs.username, newParentMailboxPath);
+        Mailbox parent = mainStepdefs.jmapServer.serverProbe().getMailbox("#private", username, newParentMailboxPath);
         String parentId = parent.getMailboxId().serialize();
 
         String requestBody =
@@ -122,17 +123,18 @@ public void movingMailbox(String actualMailboxPath, String newParentMailboxPath)
                     "]";
 
         with()
-            .header("Authorization", userStepdefs.accessToken.serialize())
+            .header("Authorization", userStepdefs.tokenByUser.get(username).serialize())
             .body(requestBody)
             .post("/jmap");
     }
 
     @Then("^mailbox \"([^\"]*)\" contains (\\d+) messages$")
     public void mailboxContainsMessages(String mailboxName, int messageCount) throws Throwable {
-        Mailbox mailbox = mainStepdefs.jmapServer.serverProbe().getMailbox("#private", userStepdefs.username, mailboxName);
+        String username = userStepdefs.lastConnectedUser;
+        Mailbox mailbox = mainStepdefs.jmapServer.serverProbe().getMailbox("#private", username, mailboxName);
         String mailboxId = mailbox.getMailboxId().serialize();
         given()
-            .header("Authorization", userStepdefs.accessToken.serialize())
+            .header("Authorization", userStepdefs.tokenByUser.get(username).serialize())
             .body("[[\"getMessageList\", {\"filter\":{\"inMailboxes\":[\"" + mailboxId + "\"]}}, \"#0\"]]")
         .when()
             .post("/jmap")

server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/cucumber/UserStepdefs.java

@@ -19,36 +19,97 @@
 
 package org.apache.james.jmap.methods.integration.cucumber;
 
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
 import javax.inject.Inject;
 
 import org.apache.james.jmap.JmapAuthentication;
 import org.apache.james.jmap.api.access.AccessToken;
+import org.apache.james.mailbox.model.MailboxConstants;
+
+import com.github.fge.lambdas.Throwing;
+import com.google.common.base.Charsets;
+import com.google.common.base.Preconditions;
+import com.google.common.hash.Hashing;
 
+import cucumber.api.PendingException;
 import cucumber.api.java.en.Given;
 import cucumber.runtime.java.guice.ScenarioScoped;
 
 @ScenarioScoped
 public class UserStepdefs {
 
     private final MainStepdefs mainStepdefs;
-    protected String username;
-    protected AccessToken accessToken;
-
+
+    protected Map<String, String> passwordByUser;
+    protected Set<String> domains;
+    protected Map<String, AccessToken> tokenByUser;
+    protected String lastConnectedUser;
+
     @Inject
     private UserStepdefs(MainStepdefs mainStepdefs) {
         this.mainStepdefs = mainStepdefs;
+        this.domains = new HashSet<>();
+        this.passwordByUser = new HashMap<>();
+        this.tokenByUser = new HashMap<>();
     }
 
     @Given("^a domain named \"([^\"]*)\"$")
     public void createDomain(String domain) throws Throwable {
         mainStepdefs.jmapServer.serverProbe().addDomain(domain);
+        domains.add(domain);
     }
 
-    @Given("^a current user with username \"([^\"]*)\" and password \"([^\"]*)\"$")
-    public void createUserWithPasswordAndAuthenticate(String username, String password) throws Throwable {
-        this.username = username;
+    @Given("^some users (.*)$")
+    public void createUsers(List<String> users) throws Throwable {
+        users.stream()
+            .map(this::unquote)
+            .forEach(Throwing.consumer(this::createUser));
+    }
+
+    private String unquote(String quotedString) {
+        return quotedString.substring(1, quotedString.length() - 1);
+    }
+
+    @Given("^a user \"([^\"]*)\"$")
+    public void createUser(String username) throws Exception {
+        String password = generatePassword(username);
         mainStepdefs.jmapServer.serverProbe().addUser(username, password);
-        accessToken = JmapAuthentication.authenticateJamesUser(username, password);
+        passwordByUser.put(username, password);
+    }
+
+    @Given("^a connected user \"([^\"]*)\"$")
+    public void createConnectedUser(String username) throws Throwable {
+        createUser(username);
+        connectUser(username);
+    }
+
+    @Given("^\"([^\"]*)\" has a mailbox \"([^\"]*)\"$")
+    public void createMailbox(String username, String mailbox) throws Throwable {
+        mainStepdefs.jmapServer.serverProbe().createMailbox(MailboxConstants.USER_NAMESPACE, username, mailbox);
+    }
+
+
+    @Given("^\"([^\"]*)\" is connected$")
+    public void connectUser(String username) throws Throwable {
+        String password = passwordByUser.get(username);
+        Preconditions.checkState(password != null, "unknown user " + username);
+        AccessToken accessToken = JmapAuthentication.authenticateJamesUser(username, password);
+        tokenByUser.put(username, accessToken);
+        lastConnectedUser = username;
+    }
+
+    @Given("^\"([^\"]*)\" shares its mailbox \"([^\"]*)\" with \"([^\"]*)\"$")
+    public void shareMailbox(String owner, String mailbox, String shareTo) throws Throwable {
+        throw new PendingException();
+    }
+
+    private String generatePassword(String username) {
+        return Hashing.murmur3_128().hashString(username, Charsets.UTF_8).toString();
     }
 
 }

server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/DownloadEndpoint.feature

@@ -4,27 +4,49 @@ Feature: Download endpoint
 
   Background:
     Given a domain named "domain.tld"
-
-  Scenario: A known user should initiate the access to the download endpoint
-    Given a current user with username "username@domain.tld" and password "secret"
-    When checking for the availability of the attachment endpoint
+    And some users "usera@domain.tld", "userb@domain.tld", "userc@domain.tld"
+    And "usera@domain.tld" has a mailbox "INBOX"
+    And "usera@domain.tld" mailbox "INBOX" contains a message "m1" with an attachment "a1"
+
+  Scenario: An authenticated user should initiate the access to the download endpoint
+    Given "usera@domain.tld" is connected
+    When "usera@domain.tld" checks for the availability of the attachment endpoint
     Then the user should be authorized
 
   Scenario: An unauthenticated user should initiate the access to the download endpoint
-    When checking for the availability of the attachment endpoint
+    When "usera@domain.tld" checks for the availability of the attachment endpoint
     Then the user should be authorized
 
-  Scenario: A known user should have access to the download endpoint
-    Given a current user with username "username@domain.tld" and password "secret"
-    When asking for an attachment
+  Scenario: An authenticated user should have access to the download endpoint
+    Given "usera@domain.tld" is connected
+    When "usera@domain.tld" downloads "a1"
     Then the user should be authorized
 
   @Ignore
   Scenario: An unauthenticated user should not have access to the download endpoint
-    When asking for an attachment
+    When "usera@domain.tld" downloads "a1"
     Then the user should not be authorized
 
-  Scenario: A known user should not have access to the download endpoint without a blobId
-    Given a current user with username "username@domain.tld" and password "secret"
-    When asking for an attachment without blobId parameter
+  Scenario: A authenticated user should not have access to the download endpoint without a blobId
+    Given "usera@domain.tld" is connected
+    When "usera@domain.tld" asks for an attachment without blobId parameter
     Then the user should receive a bad request response
+
+
+  Scenario: A user should not retrieve anything when using wrong blobId
+    Given "usera@domain.tld" is connected
+    When "usera@domain.tld" asks for an attachment with wrong blobId
+    Then the user should receive a not found response
+
+  @Ignore
+  Scenario: A user should not have access to someone else attachment
+    Given "userb@domain.tld" is connected
+    When "userb@domain.tld" downloads "a1"
+    Then the user should receive a not found response
+
+  @Ignore
+  Scenario: A user should have access to a shared attachment
+    Given "usera@domain.tld" shares its mailbox "INBOX" with "userb@domain.tld"
+    And "userb@domain.tld" is connected
+    When "userb@domain.tld" downloads "a1"
+    Then the user should be authorized

server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/resources/cucumber/MailboxModification.feature

@@ -5,7 +5,7 @@ Feature: Mailbox modification
 
   Background:
     Given a domain named "domain.tld"
-    And a current user with username "username@domain.tld" and password "secret"
+    And a connected user "username@domain.tld"
 
   Scenario: Renaming a mailbox should keep messages
     Given mailbox "A" with 2 messages

server/protocols/jmap/doc/specs/spec/authentication.mdwn

@@ -107,8 +107,7 @@ The response body will be a single JSON object with the following properties.
   The URL endpoint to use when uploading files (see the Upload section of this spec).
 - **download**: `String`
   <aside class="warning">
-  Not implemented
-  </aside>
+  Download endpoint does not handle name, it's not secured by any access protection and any authenticated user can get any attachment without restriction.</aside>
   The URL endpoint to use when downloading files, in [RFC6570 URI Template](https://tools.ietf.org/html/rfc6570) (level 1) format. The URL MUST contain a variable called `blobId`. The URL SHOULD contain a variable called `name`. The client may use this template in combination with a blobId to download any binary data (files) referenced by other objects. Since a blob is not associated with a particular name, the template SHOULD allow a name to be substituted in as well; the server will return this as the filename if it sets a `Content-Disposition` header. To download the data the client MUST make an authenticated GET request (see below for how to authenticate requests) to the expanded URL, and then follow any redirects.
 
 URLs are returned only after logging in. This allows different URLs to be used for users located in different geographic datacentres within the same service.