From b690e0ab4556dc1a1df11bf917da7c980cea3e96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20K=C3=B6rber?= Date: Mon, 12 Feb 2018 15:59:30 +0100 Subject: [PATCH] Session contains account location instead of abstract key identifier --- .../java/org/shredzone/acme4j/Account.java | 2 +- .../org/shredzone/acme4j/AccountBuilder.java | 2 +- .../java/org/shredzone/acme4j/Session.java | 14 ++++----- .../acme4j/connector/DefaultConnection.java | 8 ++--- .../shredzone/acme4j/AccountBuilderTest.java | 4 +-- .../org/shredzone/acme4j/AccountTest.java | 2 +- .../org/shredzone/acme4j/CertificateTest.java | 2 +- .../org/shredzone/acme4j/SessionTest.java | 12 +++---- .../connector/DefaultConnectionTest.java | 31 ++++++------------- .../shredzone/acme4j/it/pebble/AccountIT.java | 6 ++-- 10 files changed, 34 insertions(+), 49 deletions(-) diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/Account.java b/acme4j-client/src/main/java/org/shredzone/acme4j/Account.java index 1bb45622..43ba00f2 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/Account.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/Account.java @@ -57,7 +57,7 @@ public class Account extends AcmeJsonResource { protected Account(Session session, URL location) { super(session); setLocation(location); - session.setKeyIdentifier(location.toString()); + session.setAccountLocation(location); } /** diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/AccountBuilder.java b/acme4j-client/src/main/java/org/shredzone/acme4j/AccountBuilder.java index ea60218a..8ce195a3 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/AccountBuilder.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/AccountBuilder.java @@ -155,7 +155,7 @@ public class AccountBuilder { public Account create(Session session) throws AcmeException { LOG.debug("create"); - if (session.getKeyIdentifier() != null) { + if (session.getAccountLocation() != null) { throw new IllegalArgumentException("session already seems to have an Account"); } diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/Session.java b/acme4j-client/src/main/java/org/shredzone/acme4j/Session.java index dc6c0480..72355fde 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/Session.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/Session.java @@ -47,7 +47,7 @@ public class Session { private final AcmeProvider provider; private KeyPair keyPair; - private String keyIdentifier; + private URL accountLocation; private byte[] nonce; private JSON directoryJson; private Locale locale = Locale.getDefault(); @@ -115,17 +115,17 @@ public class Session { } /** - * Gets the key identifier of the ACME account. + * Gets the location {@link URL} of the account logged into this session. */ - public String getKeyIdentifier() { - return keyIdentifier; + public URL getAccountLocation() { + return accountLocation; } /** - * Sets the key identifier of the ACME account. + * Sets the location {@link URL} of the account logged into this session. */ - public void setKeyIdentifier(String keyIdentifier) { - this.keyIdentifier = keyIdentifier; + public void setAccountLocation(URL accountLocation) { + this.accountLocation = accountLocation; } /** diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java index 2ba2448f..e0fd9487 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java @@ -155,10 +155,6 @@ public class DefaultConnection implements Connection { @Override public int sendSignedRequest(URL url, JSONBuilder claims, Session session, int... httpStatus) throws AcmeException { - if (session.getKeyIdentifier() == null) { - throw new IllegalStateException("session has no KeyIdentifier set"); - } - return sendSignedRequest(url, claims, session, false, httpStatus); } @@ -326,10 +322,10 @@ public class DefaultConnection implements Connection { jws.setPayload(claims.toString()); jws.getHeaders().setObjectHeaderValue("nonce", Base64Url.encode(session.getNonce())); jws.getHeaders().setObjectHeaderValue("url", url); - if (enforceJwk || session.getKeyIdentifier() == null) { + if (enforceJwk || session.getAccountLocation() == null) { jws.getHeaders().setJwkHeaderValue("jwk", jwk); } else { - jws.getHeaders().setObjectHeaderValue("kid", session.getKeyIdentifier()); + jws.getHeaders().setObjectHeaderValue("kid", session.getAccountLocation()); } jws.setAlgorithmHeaderValue(keyAlgorithm(jwk)); diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/AccountBuilderTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/AccountBuilderTest.java index ba2a8058..84a04bf4 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/AccountBuilderTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/AccountBuilderTest.java @@ -93,7 +93,7 @@ public class AccountBuilderTest { assertThat(account.getLocation(), is(locationUrl)); assertThat(account.getTermsOfServiceAgreed(), is(true)); - assertThat(session.getKeyIdentifier(), is(locationUrl.toString())); + assertThat(session.getAccountLocation(), is(locationUrl)); try { AccountBuilder builder2 = new AccountBuilder(); @@ -219,7 +219,7 @@ public class AccountBuilderTest { Account account = builder.create(session); assertThat(account.getLocation(), is(locationUrl)); - assertThat(session.getKeyIdentifier(), is(locationUrl.toString())); + assertThat(session.getAccountLocation(), is(locationUrl)); provider.close(); } diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/AccountTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/AccountTest.java index f8f384ce..32d0dc58 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/AccountTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/AccountTest.java @@ -101,7 +101,7 @@ public class AccountTest { Account account = new Account(session, locationUrl); account.update(); - assertThat(session.getKeyIdentifier(), is(locationUrl.toString())); + assertThat(session.getAccountLocation(), is(locationUrl)); assertThat(account.getLocation(), is(locationUrl)); assertThat(account.getTermsOfServiceAgreed(), is(true)); assertThat(account.getContacts(), hasSize(1)); diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/CertificateTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/CertificateTest.java index b13e552f..9bf99777 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/CertificateTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/CertificateTest.java @@ -136,7 +136,7 @@ public class CertificateTest { assertThat(url, is(resourceUrl)); assertThat(claims.toString(), sameJSONAs(getJSON("revokeCertificateRequest").toString())); assertThat(session, is(notNullValue())); - assertThat(session.getKeyIdentifier(), is(nullValue())); + assertThat(session.getAccountLocation(), is(nullValue())); assertThat(enforceJwk, is(true)); certRequested = false; assertThat(httpStatus, isIntArrayContainingInAnyOrder()); diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/SessionTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/SessionTest.java index eb16b44f..3da8573d 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/SessionTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/SessionTest.java @@ -73,13 +73,13 @@ public class SessionTest { assertThat(session, not(nullValue())); assertThat(session.getServerUri(), is(serverUri)); assertThat(session.getKeyPair(), is(keyPair)); - assertThat(session.getKeyIdentifier(), is(nullValue())); + assertThat(session.getAccountLocation(), is(nullValue())); Session session2 = new Session("https://example.com/acme", keyPair); assertThat(session2, not(nullValue())); assertThat(session2.getServerUri(), is(serverUri)); assertThat(session2.getKeyPair(), is(keyPair)); - assertThat(session2.getKeyIdentifier(), is(nullValue())); + assertThat(session2.getAccountLocation(), is(nullValue())); try { new Session("#*aBaDuRi*#", keyPair); @@ -97,7 +97,7 @@ public class SessionTest { KeyPair kp1 = TestUtils.createKeyPair(); KeyPair kp2 = TestUtils.createDomainKeyPair(); URI serverUri = URI.create(TestUtils.ACME_SERVER_URI); - String keyIdentifier = TestUtils.ACME_SERVER_URI + "/acct/1"; + URL accountUrl = TestUtils.url(TestUtils.ACME_SERVER_URI + "/acct/1"); Session session = new Session(serverUri, kp1); @@ -110,9 +110,9 @@ public class SessionTest { session.setKeyPair(kp2); assertThat(session.getKeyPair(), is(kp2)); - assertThat(session.getKeyIdentifier(), is(nullValue())); - session.setKeyIdentifier(keyIdentifier); - assertThat(session.getKeyIdentifier(), is(keyIdentifier)); + assertThat(session.getAccountLocation(), is(nullValue())); + session.setAccountLocation(accountUrl); + assertThat(session.getAccountLocation(), is(accountUrl)); assertThat(session.getServerUri(), is(serverUri)); } diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DefaultConnectionTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DefaultConnectionTest.java index 3faebe75..98341334 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DefaultConnectionTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DefaultConnectionTest.java @@ -64,7 +64,7 @@ import org.shredzone.acme4j.toolbox.TestUtils; public class DefaultConnectionTest { private URL requestUrl = TestUtils.url("http://example.com/acme/"); - private String keyIdentifier = TestUtils.ACME_SERVER_URI + "/acct/1"; + private URL accountUrl = TestUtils.url(TestUtils.ACME_SERVER_URI + "/acct/1"); private HttpURLConnection mockUrlConnection; private HttpConnector mockHttpConnection; private Session session; @@ -402,7 +402,7 @@ public class DefaultConnectionTest { when(mockUrlConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_OK); when(mockUrlConnection.getOutputStream()).thenReturn(new ByteArrayOutputStream()); - session.setKeyIdentifier(keyIdentifier); + session.setAccountLocation(accountUrl); session.setNonce(TestUtils.DUMMY_NONCE); try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { @@ -426,7 +426,7 @@ public class DefaultConnectionTest { when(mockUrlConnection.getErrorStream()).thenReturn(new ByteArrayInputStream(jsonData.getBytes("utf-8"))); when(mockUrlConnection.getURL()).thenReturn(url("https://example.com/acme/1")); - session.setKeyIdentifier(keyIdentifier); + session.setAccountLocation(accountUrl); session.setNonce(TestUtils.DUMMY_NONCE); try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { @@ -462,7 +462,7 @@ public class DefaultConnectionTest { when(mockUrlConnection.getErrorStream()).thenReturn(new ByteArrayInputStream(jsonData.getBytes("utf-8"))); when(mockUrlConnection.getURL()).thenReturn(url("https://example.com/acme/1")); - session.setKeyIdentifier(keyIdentifier); + session.setAccountLocation(accountUrl); session.setNonce(TestUtils.DUMMY_NONCE); try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { @@ -504,7 +504,7 @@ public class DefaultConnectionTest { when(mockUrlConnection.getErrorStream()).thenReturn(new ByteArrayInputStream(jsonData.getBytes("utf-8"))); when(mockUrlConnection.getURL()).thenReturn(url("https://example.com/acme/1")); - session.setKeyIdentifier(keyIdentifier); + session.setAccountLocation(accountUrl); session.setNonce(TestUtils.DUMMY_NONCE); try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { @@ -544,7 +544,7 @@ public class DefaultConnectionTest { when(mockUrlConnection.getOutputStream()) .thenReturn(new ByteArrayOutputStream()); - session.setKeyIdentifier(keyIdentifier); + session.setAccountLocation(accountUrl); session.setNonce(TestUtils.DUMMY_NONCE); try (DefaultConnection conn = new DefaultConnection(mockHttpConnection) { @@ -584,7 +584,7 @@ public class DefaultConnectionTest { when(mockUrlConnection.getOutputStream()) .thenReturn(new ByteArrayOutputStream()); - session.setKeyIdentifier(keyIdentifier); + session.setAccountLocation(accountUrl); session.setNonce(TestUtils.DUMMY_NONCE); try (DefaultConnection conn = new DefaultConnection(mockHttpConnection) { @@ -620,7 +620,7 @@ public class DefaultConnectionTest { when(mockUrlConnection.getOutputStream()) .thenReturn(new ByteArrayOutputStream()); - session.setKeyIdentifier(keyIdentifier); + session.setAccountLocation(accountUrl); session.setNonce(TestUtils.DUMMY_NONCE); try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { @@ -691,7 +691,7 @@ public class DefaultConnectionTest { }) { JSONBuilder cb = new JSONBuilder(); cb.put("foo", 123).put("bar", "a-string"); - session.setKeyIdentifier(keyIdentifier); + session.setAccountLocation(accountUrl); conn.sendSignedRequest(requestUrl, cb, session); } @@ -718,7 +718,7 @@ public class DefaultConnectionTest { expectedHeader.append("\"nonce\":\"").append(Base64Url.encode(nonce1)).append("\","); expectedHeader.append("\"url\":\"").append(requestUrl).append("\","); expectedHeader.append("\"alg\":\"RS256\","); - expectedHeader.append("\"kid\":\"").append(keyIdentifier).append('"'); + expectedHeader.append("\"kid\":\"").append(accountUrl).append('"'); expectedHeader.append('}'); assertThat(Base64Url.decodeToUtf8String(encodedHeader), sameJSONAs(expectedHeader.toString())); @@ -808,17 +808,6 @@ public class DefaultConnectionTest { assertThat(jws.verifySignature(), is(true)); } - /** - * Test signed POST requests without a required KeyIdentifier. - */ - @Test(expected = IllegalStateException.class) - public void testSendSignedRequestNoKidFailed() throws Exception { - try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { - JSONBuilder cb = new JSONBuilder(); - conn.sendSignedRequest(requestUrl, cb, session); - } - } - /** * Test signed POST requests if there is no nonce. */ diff --git a/acme4j-it/src/test/java/org/shredzone/acme4j/it/pebble/AccountIT.java b/acme4j-it/src/test/java/org/shredzone/acme4j/it/pebble/AccountIT.java index 174d4e3d..a9788b82 100644 --- a/acme4j-it/src/test/java/org/shredzone/acme4j/it/pebble/AccountIT.java +++ b/acme4j-it/src/test/java/org/shredzone/acme4j/it/pebble/AccountIT.java @@ -48,7 +48,7 @@ public class AccountIT extends PebbleITBase { Account acct = ab.create(session); URL location = acct.getLocation(); assertIsPebbleUrl(location); - assertThat(session.getKeyIdentifier(), is(location.toString())); + assertThat(session.getAccountLocation(), is(location)); // Check registered data assertThat(acct.getContacts(), contains(URI.create("mailto:acme@example.com"))); @@ -73,7 +73,7 @@ public class AccountIT extends PebbleITBase { .create(session1); URL location1 = acct1.getLocation(); assertIsPebbleUrl(location1); - assertThat(session1.getKeyIdentifier(), is(location1.toString())); + assertThat(session1.getAccountLocation(), is(location1)); Session session2 = new Session(pebbleURI(), keyPair); Account acct2 = new AccountBuilder() @@ -81,7 +81,7 @@ public class AccountIT extends PebbleITBase { .create(session2); URL location2 = acct2.getLocation(); assertIsPebbleUrl(location2); - assertThat(session2.getKeyIdentifier(), is(location2.toString())); + assertThat(session2.getAccountLocation(), is(location2)); assertThat(location1, is(location2)); }