Unit test for the right kind of cryptography key

Question

I have a quite large unit test case for one class that currently does not exist, I am going to write it after finishing the test case.

I am wondering if my unit test doesn't lack something important, or alternatively, if it is not too complex. I have to describe what the tested class is supposed to do, although some details are left out, and I can answer questions about them if needed.

This class is a key matcher, part of a library implementing things like json web signing/json web key.

The key matcher will take input from some other classes like JWS processors, that will process json objects. The json representation of, for example, a signed object, can contain a key used to verify the signatures on the object. This key can be given as a json web key or certificate, or can be given by key id. A key set can also be provided, that would mean the key identified by the key id is to be looked up in that set. The protocol does not specify the policy of determining which key to use for verification, so in theory all those fields can be present at once.

The key matcher will match keys, trying in some predetermined order, like explicit key has priority over a certificate, and it has priority over matching in key set, last is an external source of keys or certificates that is application specific. However, usually when the matcher looks for keys in different sources, and a higher priority source is found, a lower priority source is usually not tried even if keys from the higher priority source do not match. It stems from the fact that if an application sends a signed or encrypted object containing multiple incompatible keys, it is an application error and such cases should not be handled, because they make no sense.

This test case tests matching in case of each possible source of key material, and most but not all cases of match failure. However, it does not test for cases where matcher would throw NullPointerException or IllegalArgumentException or possibly IllegalStateException, for example if a key type is not specified. I am not sure if I should test for good reaction to bugs in the user of the matcher class. The last test will test if the order of matching is correct.

I would like to know if my unit test too complex, or if it misses something important.

/**
* Copyright (c) 2016-2017, acme-client developers
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met:
*
* 1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer.
*
* 2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
package io.github.webczat.acmeClient.jws.keyMatching;

import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.security.cert.X509Certificate;
import java.util.*;

import org.junit.Test;

import io.github.webczat.acmeClient.jws.KeyType;
import io.github.webczat.acmeClient.jws.NoMatchingKeyException;
import io.github.webczat.acmeClient.jws.WebKey;
import io.github.webczat.acmeClient.jws.WebPublicKey;
import io.github.webczat.acmeClient.testUtil.CertificateTestUtils;

/**
 * This class tests key matcher.
 * 
 * @author webczat
 */
@SuppressWarnings({ "javadoc" })
public class KeyMatcherTest {
    /**
     * Test for matching an explicitly given key.
     */
    @Test
    public void testExplicitKeyMatchWithAlgorithm() {
        WebPublicKey key = mock(WebPublicKey.class);
        when(key.getKeyType()).thenReturn(KeyType.RSA);
        when(key.getAlgorithm()).thenReturn("test");
        assertEquals(new KeyMatcher().setKeyType(KeyType.RSA).setAlgorithm("test").setWebKey(key).match(), key);
    }

    /*
     * Test for explicitly given key without match without algorithm on the key.
     */
    @Test
    public void testExplicitKeyMatchWithoutAlgorithm() {
        WebPublicKey key = mock(WebPublicKey.class);
        when(key.getKeyType()).thenReturn(KeyType.RSA);
        assertEquals(new KeyMatcher().setKeyType(KeyType.RSA).setAlgorithm("test").setWebKey(key).match(), key);
    }

    /**
     * Test explicit key match with bad algorithm.
     */
    @Test(expected = NoMatchingKeyException.class)
    public void testExplicitKeyMatchWithBadAlgorithm() throws NoMatchingKeyException {
        WebPublicKey key = mock(WebPublicKey.class);
        when(key.getKeyType()).thenReturn(KeyType.RSA);
        when(key.getAlgorithm()).thenReturn("test2");
        new KeyMatcher().setKeyType(KeyType.RSA).setAlgorithm("test2").setWebKey(key).match();
    }

    /**
     * Test for explicit key with bad key type.
     */
    @Test(expected = NoMatchingKeyException.class)
    public void testExplicitKeyMatchWithBadType() throws NoMatchingKeyException {
        WebPublicKey key = mock(WebPublicKey.class);
        when(key.getKeyType()).thenReturn(KeyType.EC);
        new KeyMatcher().setKeyType(KeyType.RSA).setAlgorithm("test").match();
    }

    /**
     * Tests for explicit key match with a key validator passing.
     */
    @Test
    public void testExplicitKeyMatchWithPassingValidator() {
        WebPublicKey key = mock(WebPublicKey.class);
        when(key.getKeyType()).thenReturn(KeyType.RSA);
        assertEquals(
                new KeyMatcher().setKeyType(KeyType.RSA).setAlgorithm("test").setKeyValidator((k) -> true).setWebKey(
                        key).match(),
                key);
    }

    /**
     * Test for explicit key match with failing validator.
     */
    @Test(expected = NoMatchingKeyException.class)
    public void testExplicitKeyMatchWithFailingValidator() throws NoMatchingKeyException {
        WebPublicKey key = mock(WebPublicKey.class);
        when(key.getKeyType()).thenReturn(KeyType.RSA);
        new KeyMatcher().setWebKey(key).setKeyValidator((k) -> false).setAlgorithm("test").setKeyType(
                KeyType.RSA).match();
    }

    /**
     * Test for matching a key from the given set of keys, with given key
     * identifier.
     * 
     */
    @Test
    public void testSetKeyMatchWithKeyId() {
        WebPublicKey key1 = mock(WebPublicKey.class), key2 = mock(WebPublicKey.class), key3 = mock(WebPublicKey.class);
        when(key1.getKeyType()).thenReturn(KeyType.RSA);
        when(key1.getKeyId()).thenReturn("test");
        when(key2.getKeyType()).thenReturn(KeyType.EC);
        when(key2.getKeyId()).thenReturn("test");
        when(key3.getKeyType()).thenReturn(KeyType.RSA);
        LinkedHashSet<WebKey> keySet = new LinkedHashSet<WebKey>(
                Arrays.asList(new WebKey[] { key3, key2, key1, key1 }));
        assertEquals(new KeyMatcher().setKeyId("test").setKeyType(KeyType.RSA).setAlgorithm("test").setWebKeySet(
                keySet).match(), key1);
    }

    /**
     * Test for no matching keys for key id when matching by key set.
     */
    @Test(expected = NoMatchingKeyException.class)
    public void testSetKeyMatchWithKeyIdAndNoCandidates() throws NoMatchingKeyException {
        WebPublicKey key = mock(WebPublicKey.class);
        when(key.getKeyType()).thenReturn(KeyType.RSA);
        LinkedHashSet<WebKey> keySet = new LinkedHashSet<WebKey>(Arrays.asList(new WebKey[] { key }));
        new KeyMatcher().setAlgorithm("test").setKeyId("test").setKeyType(KeyType.RSA).setWebKeySet(keySet).match();
    }

    /**
     * Tests key matching using a key set, with no key id given.
     */
    @Test
    public void testSetKeyMatchWithoutKeyId() {
        WebPublicKey key1 = mock(WebPublicKey.class), key2 = mock(WebPublicKey.class);
        when(key1.getKeyType()).thenReturn(KeyType.RSA);
        when(key2.getKeyType()).thenReturn(KeyType.EC);
        LinkedHashSet<WebKey> keySet = new LinkedHashSet<>(Arrays.asList(new WebKey[] { key2, key1 }));
        assertEquals(new KeyMatcher().setAlgorithm("test").setKeyType(KeyType.RSA).setWebKeySet(keySet).match(), key1);
    }

    /**
     * Test for matching keys from set with no key id and no candidates.
     */
    @Test(expected = NoMatchingKeyException.class)
    public void testSetKeyMatchWithoutKeyIdAndCandidates() throws NoMatchingKeyException {
        WebPublicKey key = mock(WebPublicKey.class);
        when(key.getKeyType()).thenReturn(KeyType.EC);
        HashSet<WebKey> keySet = new HashSet<>(Arrays.asList(new WebKey[] { key }));
        new KeyMatcher().setAlgorithm("test").setKeyType(KeyType.RSA).setWebKeySet(keySet).match();
    }

    /**
     * Test for matching key from external source.
     */
    @Test
    public void testExternalKeyMatch() {
        WebPublicKey key1 = mock(WebPublicKey.class), key2 = mock(WebPublicKey.class);
        when(key1.getKeyType()).thenReturn(KeyType.RSA);
        when(key2.getKeyType()).thenReturn(KeyType.EC);
        KeyProvider kp = mock(KeyProvider.class);
        when(kp.lookupKey("test")).thenReturn(Arrays.asList(new WebKey[] { key2, key1 }));
        assertEquals(new KeyMatcher().setKeyType(KeyType.RSA).setAlgorithm("test").setKeyId("test").setKeyProvider(
                kp).match(), key1);
    }

    /**
     * Test matching keys from external source when no keys match.
     */
    @Test(expected = NoMatchingKeyException.class)
    public void testExternalKeyMatchWithNoCandidates() throws NoMatchingKeyException {
        WebPublicKey key = mock(WebPublicKey.class);
        when(key.getKeyType()).thenReturn(KeyType.EC);
        KeyProvider kp = mock(KeyProvider.class);
        when(kp.lookupKey("test")).thenReturn(Arrays.asList(new WebKey[] { key }));
        new KeyMatcher().setKeyType(KeyType.RSA).setAlgorithm("test").setKeyId("test").setKeyProvider(kp).match();
    }

    /**
     * Test for external key matching when no key id specified, it should not
     * work at all.
     */
    @Test(expected = NoMatchingKeyException.class)
    public void testExternalKeyMatchWithNoKeyId() throws NoMatchingKeyException {
        WebPublicKey key = mock(WebPublicKey.class);
        when(key.getKeyType()).thenReturn(KeyType.RSA);
        KeyProvider kp = mock(KeyProvider.class);
        when(kp.lookupKey("test")).thenReturn(Arrays.asList(new WebKey[] { key }));
        new KeyMatcher().setKeyType(KeyType.RSA).setAlgorithm("test").setKeyProvider(kp).match();
    }

    /**
     * Test for certificate matching when cert chain is explicitly given.
     */
    @Test
    public void testExplicitCertificateMatch() {
        List<X509Certificate> certs = CertificateTestUtils.newChain(3, null).getCertificateChain();
        assertEquals(new KeyMatcher().setAlgorithm("test").setKeyType(KeyType.RSA).setCertificateChain(certs).match(),
                certs);
    }

    /**
     * Test explicit certificate match that fails.
     */
    @Test(expected = NoMatchingKeyException.class)
    public void testExplicitCertificateMatchFailure() throws NoMatchingKeyException {
        List<X509Certificate> certs = CertificateTestUtils.newChain(3, null).getCertificateChain();
        new KeyMatcher().setKeyType(KeyType.EC).setAlgorithm("test").setCertificateChain(certs).match();
    }

    /**
     * Test SHA256 fingerprint matching.
     */
    @Test
    public void testFingerprintCertificateMatchWithSha256() throws NoSuchAlgorithmException {
        List<X509Certificate> certs = CertificateTestUtils.newChain(3, null).getCertificateChain();
        byte[] fingerprint = MessageDigest.getInstance("SHA2-256").digest(certs.get(0).getEncoded());
        CertificateProvider cp = mock(CertificateProvider.class);
        when(cp.lookupCertificateBySha256Fingerprint(fingerprint)).thenReturn(certs);
        assertEquals(new KeyMatcher().setKeyType(KeyType.RSA).setAlgorithm("test").setSha256Fingerprint(
                fingerprint).setCertificateProvider(cp).match(), certs);
    }

    /**
     * Test certificate matching using SHA1 fingerprint.
     */
    @Test
    public void testFingerprintCertificateMatchWithSha1() throws NoSuchAlgorithmException {
        List<X509Certificate> certs = CertificateTestUtils.newChain(3, null).getCertificateChain();
        byte[] fingerprint = MessageDigest.getInstance("SHA1").digest(certs.get(0).getEncoded());
        CertificateProvider cp = mock(CertificateProvider.class);
        when(cp.lookupCertificateBySha1Fingerprint(fingerprint)).thenReturn(certs);
        assertEquals(new KeyMatcher().setKeyType(KeyType.RSA).setAlgorithm("test").setCertificateProvider(
                cp).setSha1Fingerprint(fingerprint).match(), certs);
    }

    /**
     * Test for matching by sha256 fingerprint when key type is invalid.
     */
    @Test(expected = NoMatchingKeyException.class)
    public void testFingerprintCertificateMatchWithSha256AndBadKeyType()
            throws NoMatchingKeyException, NoSuchAlgorithmException {
        List<X509Certificate> certs = CertificateTestUtils.newChain(3, null).getCertificateChain();
        byte[] fingerprint = MessageDigest.getInstance("SHA2-256").digest(certs.get(0).getEncoded());
        CertificateProvider cp = mock(CertificateProvider.class);
        when(cp.lookupCertificateBySha256Fingerprint(fingerprint)).thenReturn(certs);
        new KeyMatcher().setKeyType(KeyType.EC).setCertificateProvider(cp).setAlgorithm("test").setSha256Fingerprint(
                fingerprint).match();
    }

    /**
     * Test for sha1 fingerprint matching when wrong key type is given.
     */
    @Test(expected = NoMatchingKeyException.class)
    public void testFingerprintCertificateMatchWithSha1AndBadKeyType()
            throws NoMatchingKeyException, NoSuchAlgorithmException {
        List<X509Certificate> certs = CertificateTestUtils.newChain(3, null).getCertificateChain();
        byte[] fingerprint = MessageDigest.getInstance("SHA1").digest(certs.get(0).getEncoded());
        CertificateProvider cp = mock(CertificateProvider.class);
        when(cp.lookupCertificateBySha1Fingerprint(fingerprint)).thenReturn(certs);
        new KeyMatcher().setCertificateProvider(cp).setKeyType(KeyType.EC).setAlgorithm("test").setSha1Fingerprint(
                fingerprint).match();
    }

    /**
     * Test for fingerprint matching with no fingerprints set.
     */
    @Test(expected = NoMatchingKeyException.class)
    public void testFingerprintCertificateMatchWithNoFingerprint() {
        new KeyMatcher().setKeyType(KeyType.RSA).setAlgorithm("test").setCertificateProvider(
                mock(CertificateProvider.class)).match();
    }

    /**
     * Test for matching when no parameters are set.
     */
    @Test(expected = NoMatchingKeyException.class)
    public void testKeyMatchWithNoData() throws NoMatchingKeyException {
        new KeyMatcher().setKeyType(KeyType.RSA).setAlgorithm("test").match();
    }

    /**
     * Test for matching order.
     */
    @Test
    public void testMatchingOrder() {
        WebPublicKey key1 = mock(WebPublicKey.class);
        when(key1.getKeyType()).thenReturn(KeyType.RSA);
        KeyMatcher km = new KeyMatcher();
        km.setKeyType(KeyType.RSA).setAlgorithm("TEST").setWebKey(key1);
        List<X509Certificate> certs1 = CertificateTestUtils.newChain(1, null).getCertificateChain();
        km.setCertificateChain(certs1);
        WebPublicKey key2 = mock(WebPublicKey.class);
        when(key2.getKeyType()).thenReturn(KeyType.RSA);
        when(key2.getKeyId()).thenReturn("test");
        Set<WebKey> keySet = new HashSet<>();
        keySet.add(key2);
        km.setWebKeySet(keySet);
        km.setKeyId("test");
        List<X509Certificate> certs2 = CertificateTestUtils.newChain(1, null).getCertificateChain();
        byte[] fingerprint1 = MessageDigest.getInstance("SHA2-256").digest(certs2.get(0).getEncoded());
        km.setSha256Fingerprint(fingerprint1);
        List<X509Certificate> certs3 = CertificateTestUtils.newChain(1, null).getCertificateChain();
        byte[] fingerprint2 = MessageDigest.getInstance("SHA1").digest(certs3.get(0).getEncoded());
        km.setSha1Fingerprint(fingerprint2);
        CertificateProvider cp = mock(CertificateProvider.class);
        when(cp.lookupCertificateBySha256Fingerprint(fingerprint1)).thenReturn(certs2);
        when(cp.lookupCertificateBySha1Fingerprint(fingerprint2)).thenReturn(certs3);
        km.setCertificateProvider(cp);
        WebPublicKey key3 = mock(WebPublicKey.class);
        when(key3.getKeyType()).thenReturn(KeyType.RSA);
        KeyProvider kp = mock(KeyProvider.class);
        when(kp.lookupKey("test")).thenReturn(Arrays.asList(new WebKey[] {key3}));
        km.setKeyProvider(kp);
        assertEquals(km.match(), key1);
        km.setWebKey(null);
        assertEquals(km.match(), certs1);
        km.setCertificateChain(null);
        assertEquals(km.match(), key2);
        km.setWebKeySet(null);
        assertEquals(km.match(), certs2);
        km.setSha256Fingerprint(null);
        assertEquals(km.match(), certs3);
        km.setSha1Fingerprint(null);
        assertEquals(km.match(), key3);
    }
}

Answer 1

I really like the small test methods. But without seeing the actual implementation, it is very hard to tell, if a test case makes sense or does what it should do.

Small improvements:

• Split your test methods into three blocks, when-given-then and use a empty line between those. It can help a lot, not always, but I recommend to do it. It's like using your indicator: Even though noone is around (= quite an easy test case), it's a good habit to use it always, so you will use it, when it's actually needed.
• You can make WebPublicKey an instance variable and use the @Mock annotation. In the setup (@Before), you can use MockitoAnnotations.initMocks(this);. So you can save the first line of every test.
• I have a hard time to understand, what match() does, or what it should do (= the intention is not clear). Why must match() equal key? When I read matches(), I expect to have a boolean returned. Shouldn't it be something like findMatchingKey() or something?
• The test-prefix of your test cases aren't needed, it's used "back in the day", before annotations were a thing in java/junit. Instead of testExplicitKeyMatchWithAlgorithm, you can write explicitKeyMatchesWithAlgorithm
• The JavaDoc for the methods are first of all, most of the times it is JavaDoc, but not always. 2nd: I'm 99% sure, noone will ever read those java docs (you guys do even generate those for test cases?). 3rd: "Test for matching an explicitly given key." vs "testExplicitKeyMatchWithAlgorithm". So, you have a comment, a method name, and the actual code. Rhetorical question: which one is true? The java doc does not talk about algorithm, the method name does.
• The "test" algorithm: I usually declare those explicitly as variable in the test case, so the reader sees where it's used (WebPublicKey and KeyMatcher)
• There's a lof of repetition of the WebPublicKey instantiation (RSA KeyType and test algorithm), you might want to add a static helper method for that, something like rsaWithTestAlgorithmKeyMatcher(), or even add a constant. For the other creations, I'd provide a method like keyMatcher(keyType: KeyType, algorithm: String): KeyMatcher.
• testMatchingOrder: Now, that's quite the confusing test method. You wrote, that the implementation does not exist. The actual test driven approach would be "write failing test case, implement, refactor". Now important is, you write one failing test case. And you only implement what is needed for the test case to run ("Know when to stop."), so you do not implement too much. And an important thing, too, is: "What do I need to change, to make my test fail". I mention all that especially because of the last test case. Do you actually have to use four different Keys, wouldn't be two enough to ensure the correct return value? If not: Then something's different, you have to consider writing two different test cases.
• testMatchingOrder 2: Also to point out the other points I've mentioned, especially the helper-methods and then give-when-then block: Those applied should make this method a lot easier to understand. Beside that: There's nothing wrong to write keyMatcher instead of km, certificateProvider instead of cp and so on. It certainly would have helped me.
• testMatchingOrder 3: After reading that, I still do not understand the expected behavior of the KeyMatcher, especiall the latest part. You call match, and expect key1. Why? Then you set the webKey to null (why?), and then you expect certs1 (why?). The setup of the test doesn't really help to understand it either.

Hope that helps,...