Clean out challenge API

acme4j-client/src/main/java/org/shredzone/acme4j/challenge/Challenge.java

@@ -56,12 +56,11 @@ public interface Challenge extends Serializable {
     void unmarshall(Map<String, Object> map);
 
     /**
-     * Copies the current challenge state to the claim builder, as preparation for
+     * Exports the response state, as preparation for triggering the challenge.
-     * triggering it.
      *
      * @param cb
-     *            {@link ClaimBuilder} to copy the challenge state to
+     *            {@link ClaimBuilder} to copy the response to
      */
-    void marshall(ClaimBuilder cb);
+    void respond(ClaimBuilder cb);
 
 }

acme4j-client/src/main/java/org/shredzone/acme4j/challenge/DnsChallenge.java

@@ -37,27 +37,17 @@ public class DnsChallenge extends GenericChallenge {
     private String authorization = null;
 
     /**
-     * Returns the token to be used for this challenge.
+     * Authorizes the {@link Challenge} by signing it with an {@link Account}.
+     *
+     * @param account
+     *            {@link Account} to sign the challenge with
      */
-    public String getToken() {
+    public void authorize(Account account) {
-        return get(KEY_TOKEN);
+        if (account == null) {
+            throw new NullPointerException("account must not be null");
         }
 
-    /**
+        authorization = getToken() + '.' + Base64Url.encode(jwkThumbprint(account.getKeyPair().getPublic()));
-     * Sets the token to be used.
-     */
-    public void setToken(String token) {
-        put(KEY_TOKEN, token);
-    }
-
-    /**
-     * Returns the authorization string to be used for the response.
-     */
-    public String getAuthorization() {
-        if (authorization == null) {
-            throw new IllegalStateException("Challenge is not authorized yet");
-        }
-        return authorization;
     }
 
     /**
@@ -76,25 +66,15 @@ public class DnsChallenge extends GenericChallenge {
         }
     }
 
-    /**
-     * Authorizes the {@link Challenge} by signing it with an {@link Account}.
-     *
-     * @param account
-     *            {@link Account} to sign the challenge with
-     */
-    public void authorize(Account account) {
-        if (account == null) {
-            throw new NullPointerException("account must not be null");
-        }
-
-        authorization = getToken() + '.' + Base64Url.encode(jwkThumbprint(account.getKeyPair().getPublic()));
-    }
-
     @Override
-    public void marshall(ClaimBuilder cb) {
+    public void respond(ClaimBuilder cb) {
-        cb.put(KEY_KEY_AUTHORIZSATION, getAuthorization());
+        if (authorization == null) {
-        cb.put(KEY_TYPE, getType());
+            throw new IllegalStateException("Challenge is not authorized yet");
+        }
+
+        super.respond(cb);
         cb.put(KEY_TOKEN, getToken());
+        cb.put(KEY_KEY_AUTHORIZATION, getAuthorization());
     }
 
     @Override
@@ -102,4 +82,16 @@ public class DnsChallenge extends GenericChallenge {
         return TYPE.equals(type);
     }
 
+    private String getToken() {
+        return get(KEY_TOKEN);
+    }
+
+    private String getAuthorization() {
+        if (authorization == null) {
+            throw new IllegalStateException("Challenge is not authorized yet");
+        }
+
+        return authorization;
+    }
+
 }

acme4j-client/src/main/java/org/shredzone/acme4j/challenge/GenericChallenge.java

@@ -36,6 +36,10 @@ import org.shredzone.acme4j.util.ClaimBuilder;
  * A generic implementation of {@link Challenge}. It can be used as a base class for
  * actual challenge implemenation, but it is also used if the ACME server offers a
  * proprietary challenge that is unknown to acme4j.
+ * <p>
+ * Subclasses must override {@link GenericChallenge#acceptable(String)} so it only
+ * accepts the own type. {@link GenericChallenge#respond(ClaimBuilder)} should be
+ * overridden to put all required data to the response.
  *
  * @author Richard "Shred" Körber
  */
@@ -47,7 +51,7 @@ public class GenericChallenge implements Challenge {
     protected static final String KEY_URI = "uri";
     protected static final String KEY_VALIDATED = "validated";
     protected static final String KEY_TOKEN = "token";
-    protected static final String KEY_KEY_AUTHORIZSATION = "keyAuthorization";
+    protected static final String KEY_KEY_AUTHORIZATION = "keyAuthorization";
 
     private transient Map<String, Object> data = new HashMap<>();
 
@@ -95,8 +99,8 @@ public class GenericChallenge implements Challenge {
     }
 
     @Override
-    public void marshall(ClaimBuilder cb) {
+    public void respond(ClaimBuilder cb) {
-        cb.putAll(data);
+        cb.put(KEY_TYPE, getType());
     }
 
     /**
@@ -122,18 +126,6 @@ public class GenericChallenge implements Challenge {
         return (T) data.get(key);
     }
 
-    /**
-     * Puts a value to the challenge state.
-     *
-     * @param key
-     *            Key
-     * @param value
-     *            Value, may be {@code null}
-     */
-    protected void put(String key, Object value) {
-        data.put(key, value);
-    }
-
     /**
      * Computes a JWK Thumbprint. It is frequently used in responses.
      *

acme4j-client/src/main/java/org/shredzone/acme4j/challenge/HttpChallenge.java

@@ -32,6 +32,20 @@ public class HttpChallenge extends GenericChallenge {
 
     private String authorization = null;
 
+    /**
+     * Authorizes the {@link Challenge} by signing it with an {@link Account}.
+     *
+     * @param account
+     *            {@link Account} to sign the challenge with
+     */
+    public void authorize(Account account) {
+        if (account == null) {
+            throw new NullPointerException("account must not be null");
+        }
+
+        authorization = getToken() + '.' + Base64Url.encode(jwkThumbprint(account.getKeyPair().getPublic()));
+    }
+
     /**
      * Returns the token to be used for this challenge.
      */
@@ -39,13 +53,6 @@ public class HttpChallenge extends GenericChallenge {
         return get(KEY_TOKEN);
     }
 
-    /**
-     * Sets the token to be used.
-     */
-    public void setToken(String token) {
-        put(KEY_TOKEN, token);
-    }
-
     /**
      * Returns the authorization string to be used for the response.
      * <p>
@@ -60,25 +67,15 @@ public class HttpChallenge extends GenericChallenge {
         return authorization;
     }
 
-    /**
-     * Authorizes the {@link Challenge} by signing it with an {@link Account}.
-     *
-     * @param account
-     *            {@link Account} to sign the challenge with
-     */
-    public void authorize(Account account) {
-        if (account == null) {
-            throw new NullPointerException("account must not be null");
-        }
-
-        authorization = getToken() + '.' + Base64Url.encode(jwkThumbprint(account.getKeyPair().getPublic()));
-    }
-
     @Override
-    public void marshall(ClaimBuilder cb) {
+    public void respond(ClaimBuilder cb) {
-        cb.put(KEY_KEY_AUTHORIZSATION, getAuthorization());
+        if (authorization == null) {
-        cb.put(KEY_TYPE, getType());
+            throw new IllegalStateException("Challenge is not authorized yet");
+        }
+
+        super.respond(cb);
         cb.put(KEY_TOKEN, getToken());
+        cb.put(KEY_KEY_AUTHORIZATION, getAuthorization());
     }
 
     @Override

acme4j-client/src/main/java/org/shredzone/acme4j/challenge/ProofOfPossessionChallenge.java

@@ -115,13 +115,14 @@ public class ProofOfPossessionChallenge extends GenericChallenge {
     }
 
     @Override
-    public void marshall(ClaimBuilder cb) {
+    public void respond(ClaimBuilder cb) {
         if (validation == null) {
             throw new IllegalStateException("not validated");
         }
 
+        super.respond(cb);
+
         try {
-            cb.put(KEY_TYPE, getType());
             cb.put("authorization", JsonUtil.parseJson(validation));
         } catch (JoseException ex) {
             // should not happen, as the JSON is prevalidated in the setter

acme4j-client/src/main/java/org/shredzone/acme4j/challenge/TlsSniChallenge.java

@@ -28,51 +28,16 @@ import org.shredzone.acme4j.util.ClaimBuilder;
  */
 public class TlsSniChallenge extends GenericChallenge {
     private static final long serialVersionUID = 7370329525205430573L;
+    private static final char[] HEX = "0123456789abcdef".toCharArray();
 
     /**
      * Challenge type name: {@value}
      */
     public static final String TYPE = "tls-sni-01";
 
-    private static final char[] HEX = "0123456789abcdef".toCharArray();
-
     private String authorization = null;
     private String subject = null;
 
-    /**
-     * Returns the token to be used for this challenge.
-     */
-    public String getToken() {
-        return get(KEY_TOKEN);
-    }
-
-    /**
-     * Sets the token to be used.
-     */
-    public void setToken(String token) {
-        put(KEY_TOKEN, token);
-    }
-
-    /**
-     * Returns the authorization string.
-     */
-    public String getAuthorization() {
-        if (authorization == null) {
-            throw new IllegalStateException("Challenge is not authorized yet");
-        }
-        return authorization;
-    }
-
-    /**
-     * Return the subject to generate a self-signed certificate for.
-     */
-    public String getSubject() {
-        if (authorization == null) {
-            throw new IllegalStateException("Challenge is not authorized yet");
-        }
-        return subject;
-    }
-
     /**
      * Authorizes the {@link Challenge} by signing it with an {@link Account}.
      *
@@ -90,11 +55,22 @@ public class TlsSniChallenge extends GenericChallenge {
         subject = hash.substring(0, 32) + '.' + hash.substring(32) + ".acme.invalid";
     }
 
+    /**
+     * Return the subject to generate a self-signed certificate for.
+     */
+    public String getSubject() {
+        if (authorization == null) {
+            throw new IllegalStateException("Challenge is not authorized yet");
+        }
+        return subject;
+    }
+
+
     @Override
-    public void marshall(ClaimBuilder cb) {
+    public void respond(ClaimBuilder cb) {
-        cb.put(KEY_KEY_AUTHORIZSATION, getAuthorization());
+        super.respond(cb);
-        cb.put(KEY_TYPE, getType());
         cb.put(KEY_TOKEN, getToken());
+        cb.put(KEY_KEY_AUTHORIZATION, getAuthorization());
     }
 
     @Override
@@ -127,4 +103,16 @@ public class TlsSniChallenge extends GenericChallenge {
         }
     }
 
+    private String getToken() {
+        return get(KEY_TOKEN);
+    }
+
+    private String getAuthorization() {
+        if (authorization == null) {
+            throw new IllegalStateException("Challenge is not authorized yet");
+        }
+
+        return authorization;
+    }
+
 }

acme4j-client/src/main/java/org/shredzone/acme4j/impl/AbstractAcmeClient.java

@@ -272,7 +272,7 @@ public abstract class AbstractAcmeClient implements AcmeClient {
         try (Connection conn = createConnection()) {
             ClaimBuilder claims = new ClaimBuilder();
             claims.putResource("challenge");
-            challenge.marshall(claims);
+            challenge.respond(claims);
 
             int rc = conn.sendSignedRequest(challenge.getLocation(), claims, session, account);
             if (rc != HttpURLConnection.HTTP_OK && rc != HttpURLConnection.HTTP_ACCEPTED) {

acme4j-client/src/test/java/org/shredzone/acme4j/challenge/DnsChallengeTest.java

@@ -33,8 +33,6 @@ import org.shredzone.acme4j.util.TestUtils;
  */
 public class DnsChallengeTest {
 
-    private static final String TOKEN =
-            "pNvmJivs0WCko2suV7fhe-59oFqyYx_yB7tx6kIMAyE";
     private static final String KEY_AUTHORIZATION =
             "pNvmJivs0WCko2suV7fhe-59oFqyYx_yB7tx6kIMAyE.HnWjTDnyqlCrm6tZ-6wX-TrEXgRdeNu9G71gqxSO6o0";
 
@@ -53,20 +51,18 @@ public class DnsChallengeTest {
         assertThat(challenge.getStatus(), is(Status.PENDING));
 
         try {
-            challenge.getAuthorization();
+            challenge.getDigest();
-            fail("getAuthorization() without previous authorize()");
+            fail("getDigest() without previous authorize()");
         } catch (IllegalStateException ex) {
             // expected
         }
 
         challenge.authorize(account);
 
-        assertThat(challenge.getToken(), is(TOKEN));
-        assertThat(challenge.getAuthorization(), is(KEY_AUTHORIZATION));
         assertThat(challenge.getDigest(), is("rzMmotrIgsithyBYc0vgiLUEEKYx0WetQRgEF2JIozA"));
 
         ClaimBuilder cb = new ClaimBuilder();
-        challenge.marshall(cb);
+        challenge.respond(cb);
 
         assertThat(cb.toString(), sameJSONAs("{\"keyAuthorization\"=\""
             + KEY_AUTHORIZATION + "\"}").allowingExtraUnexpectedFields());

acme4j-client/src/test/java/org/shredzone/acme4j/challenge/GenericChallengeTest.java

@@ -67,38 +67,19 @@ public class GenericChallengeTest {
     }
 
     /**
-     * Test get and put methods.
+     * Test that {@link GenericChallenge#respond(ClaimBuilder)} contains the type.
-     */
-    public void testGetPut() {
-        GenericChallenge challenge = new GenericChallenge();
-
-        challenge.put("a-string", "foo");
-        challenge.put("a-number", 1234);
-
-        assertThat((String) challenge.get("a-string"), is("foo"));
-        assertThat((Integer) challenge.get("a-number"), is(1234));
-
-        ClaimBuilder cb = new ClaimBuilder();
-        challenge.marshall(cb);
-        assertThat(cb.toString(), sameJSONAs("{\"a-string\":\"foo\",\"a-number\":1234}")
-                .allowingExtraUnexpectedFields());
-    }
-
-    /**
-     * Test that marshalling results in an identical JSON like the one that was
-     * unmarshalled.
      */
     @Test
-    public void testMarshall() throws JoseException {
+    public void testRespond() throws JoseException {
         String json = TestUtils.getJson("genericChallenge");
 
         GenericChallenge challenge = new GenericChallenge();
         challenge.unmarshall(JsonUtil.parseJson(json));
 
         ClaimBuilder cb = new ClaimBuilder();
-        challenge.marshall(cb);
+        challenge.respond(cb);
 
-        assertThat(cb.toString(), sameJSONAs(json));
+        assertThat(cb.toString(), sameJSONAs("{\"type\"=\"generic-01\"}"));
     }
 
     /**

acme4j-client/src/test/java/org/shredzone/acme4j/challenge/HttpChallengeTest.java

@@ -65,7 +65,7 @@ public class HttpChallengeTest {
         assertThat(challenge.getAuthorization(), is(KEY_AUTHORIZATION));
 
         ClaimBuilder cb = new ClaimBuilder();
-        challenge.marshall(cb);
+        challenge.respond(cb);
 
         assertThat(cb.toString(), sameJSONAs("{\"keyAuthorization\"=\""
             + KEY_AUTHORIZATION + "\"}").allowingExtraUnexpectedFields());

acme4j-client/src/test/java/org/shredzone/acme4j/challenge/ProofOfPossessionChallengeTest.java

@@ -54,7 +54,7 @@ public class ProofOfPossessionChallengeTest {
         assertThat(challenge.getStatus(), is(Status.PENDING));
 
         try {
-            challenge.marshall(new ClaimBuilder());
+            challenge.respond(new ClaimBuilder());
             fail("marshall() without previous authorize()");
         } catch (IllegalStateException ex) {
             // expected
@@ -63,7 +63,7 @@ public class ProofOfPossessionChallengeTest {
         challenge.authorize(account, domainKeyPair, "example.org");
 
         ClaimBuilder cb = new ClaimBuilder();
-        challenge.marshall(cb);
+        challenge.respond(cb);
 
         assertThat(cb.toString(), sameJSONAs("{\"type\"=\""
             + ProofOfPossessionChallenge.TYPE + "\",\"authorization\"="
@@ -90,7 +90,7 @@ public class ProofOfPossessionChallengeTest {
         challenge.importValidation(validation);
 
         ClaimBuilder cb = new ClaimBuilder();
-        challenge.marshall(cb);
+        challenge.respond(cb);
 
         assertThat(cb.toString(), sameJSONAs("{\"type\"=\""
             + ProofOfPossessionChallenge.TYPE + "\",\"authorization\"=" + validation

acme4j-client/src/test/java/org/shredzone/acme4j/challenge/TlsSniChallengeTest.java

@@ -33,8 +33,6 @@ import org.shredzone.acme4j.util.TestUtils;
  */
 public class TlsSniChallengeTest {
 
-    private static final String TOKEN =
-            "VNLBdSiZ3LppU2CRG8bilqlwq4DuApJMg3ZJowU6JhQ";
     private static final String KEY_AUTHORIZATION =
             "VNLBdSiZ3LppU2CRG8bilqlwq4DuApJMg3ZJowU6JhQ.HnWjTDnyqlCrm6tZ-6wX-TrEXgRdeNu9G71gqxSO6o0";
 
@@ -52,13 +50,6 @@ public class TlsSniChallengeTest {
         assertThat(challenge.getType(), is(TlsSniChallenge.TYPE));
         assertThat(challenge.getStatus(), is(Status.PENDING));
 
-        try {
-            challenge.getAuthorization();
-            fail("getAuthorization() without previous authorize()");
-        } catch (IllegalStateException ex) {
-            // expected
-        }
-
         try {
             challenge.getSubject();
             fail("getSubject() without previous authorize()");
@@ -68,12 +59,10 @@ public class TlsSniChallengeTest {
 
         challenge.authorize(account);
 
-        assertThat(challenge.getToken(), is(TOKEN));
-        assertThat(challenge.getAuthorization(), is(KEY_AUTHORIZATION));
         assertThat(challenge.getSubject(), is("14e2350a04434f93c2e0b6012968d99d.ed459b6a7a019d9695609b8514f9d63d.acme.invalid"));
 
         ClaimBuilder cb = new ClaimBuilder();
-        challenge.marshall(cb);
+        challenge.respond(cb);
 
         assertThat(cb.toString(), sameJSONAs("{\"keyAuthorization\"=\""
             + KEY_AUTHORIZATION + "\"}").allowingExtraUnexpectedFields());