Exposed OTP secret for not-yet serialised Transaction entities

Author: Michael Mroz

src/main/java/com/auth0/guardian/Transaction.java

@@ -40,12 +40,12 @@ public class Transaction implements Serializable {
 
     private String transactionToken;
     private String recoveryCode;
-    private transient String otpSecret;
+    private transient String totpSecret;
 
-    Transaction(String transactionToken, String recoveryCode, String otpSecret) {
+    Transaction(String transactionToken, String recoveryCode, String totpSecret) {
         this.transactionToken = transactionToken;
         this.recoveryCode = recoveryCode;
-        this.otpSecret = otpSecret;
+        this.totpSecret = totpSecret;
     }
 
     public String getTransactionToken() {
@@ -56,6 +56,21 @@ public String getRecoveryCode() {
         return recoveryCode;
     }
 
+    /**
+     * Returns the TOTP secret to be encoded in a URI for QR code generation,
+     * or manually entered if a camera is not available to the enrollment device.
+     *
+     * @return the TOTP secret
+     * @throws IllegalStateException when there is no OTP secret
+     */
+    public String getTotpSecret() throws IllegalStateException {
+        if (totpSecret == null) {
+            throw new IllegalStateException("There is no OTP Secret for this transaction");
+        }
+
+        return totpSecret;
+    }
+
     /**
      * Returns the TOTP enrollment URI to be displayed in the QR code
      *
@@ -65,16 +80,12 @@ public String getRecoveryCode() {
      * @throws IllegalStateException when there is no OTP secret
      */
     public String totpURI(String user, String issuer) throws IllegalStateException {
-        if (otpSecret == null) {
-            throw new IllegalStateException("There is no OTP Secret for this transaction");
-        }
-
         // use HttpUrl to build the URI, java.net.URL really sucks
         return new HttpUrl.Builder()
                 .scheme("https") // HttpUrl only allows http/https so I replace it afterwards
                 .host("totp")
                 .addPathSegment(String.format("%s:%s", issuer, user))
-                .addQueryParameter("secret", otpSecret)
+                .addQueryParameter("secret", getTotpSecret())
                 .addQueryParameter("issuer", issuer)
                 .build()
                 .toString()

src/test/java/com/auth0/guardian/GuardianTest.java

@@ -98,6 +98,7 @@ public void shouldRequestEnrollWithTOTP() throws Exception {
         assertThat(transaction, is(notNullValue()));
         assertThat(transaction.getTransactionToken(), is(equalTo("THE_TRANSACTION_TOKEN")));
         assertThat(transaction.getRecoveryCode(), is(equalTo("THE_RECOVERY_CODE")));
+        assertThat(transaction.getTotpSecret(), is(equalTo("THE_OTP_SECRET")));
         assertThat(transaction.totpURI("user", "issuer"), is(equalTo("otpauth://totp/issuer:user?secret=THE_OTP_SECRET&issuer=issuer")));
     }
 
@@ -130,6 +131,7 @@ public void shouldRequestEnrollWithSMS() throws Exception {
         assertThat(transaction, is(notNullValue()));
         assertThat(transaction.getTransactionToken(), is(equalTo("THE_TRANSACTION_TOKEN")));
         assertThat(transaction.getRecoveryCode(), is(equalTo("THE_RECOVERY_CODE")));
+        assertThat(transaction.getTotpSecret(), is(equalTo("THE_OTP_SECRET")));
         assertThat(transaction.totpURI("user", "issuer"), is(equalTo("otpauth://totp/issuer:user?secret=THE_OTP_SECRET&issuer=issuer")));
     }
 

src/test/java/com/auth0/guardian/TransactionTest.java

@@ -46,6 +46,7 @@ public void shouldCreateCorrectly() throws Exception {
 
         assertThat(transaction.getTransactionToken(), is(equalTo("TRANSACTION_TOKEN")));
         assertThat(transaction.getRecoveryCode(), is(equalTo("RECOVERY_CODE")));
+        assertThat(transaction.getTotpSecret(), is(equalTo("OTP_SECRET")));
         assertThat(transaction.totpURI("username", "company"), is(equalTo("otpauth://totp/company:username?secret=OTP_SECRET&issuer=company")));
     }
 
@@ -55,6 +56,7 @@ public void shouldCreateCorrectlyWithSpaces() throws Exception {
 
         assertThat(transaction.getTransactionToken(), is(equalTo("TRANSACTION_TOKEN")));
         assertThat(transaction.getRecoveryCode(), is(equalTo("RECOVERY_CODE")));
+        assertThat(transaction.getTotpSecret(), is(equalTo("OTP_SECRET")));
         assertThat(transaction.totpURI("user name", "company name"), is(equalTo("otpauth://totp/company%20name:user%20name?secret=OTP_SECRET&issuer=company%20name")));
     }
 
@@ -64,6 +66,7 @@ public void shouldCreateCorrectlyWithWeirdCharacters() throws Exception {
 
         assertThat(transaction.getTransactionToken(), is(equalTo("TRANSACTION_TOKEN")));
         assertThat(transaction.getRecoveryCode(), is(equalTo("RECOVERY_CODE")));
+        assertThat(transaction.getTotpSecret(), is(equalTo("OTP_SECRET")));
         assertThat(transaction.totpURI("user%name", "compañy?!"), is(equalTo("otpauth://totp/compa%C3%B1y%3F!:user%25name?secret=OTP_SECRET&issuer=compa%C3%B1y?!")));
     }
 
@@ -91,6 +94,32 @@ public void shouldSerializeCorrectly() throws Exception {
         assertThat(restoredTransaction.getRecoveryCode(), is(equalTo("RECOVERY_CODE")));
     }
 
+    @Test
+    public void shouldThrowWhenRequestingOtpSecretAfterSerialization() throws Exception {
+        exception.expect(IllegalStateException.class);
+        exception.expectMessage("There is no OTP Secret for this transaction");
+
+        Transaction transaction = new Transaction("TRANSACTION_TOKEN", "RECOVERY_CODE", "OTP_SECRET");
+
+        // save
+        ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+        ObjectOutputStream objectOutputStream = new ObjectOutputStream(outputStream);
+        objectOutputStream.writeObject(transaction);
+        objectOutputStream.close();
+        outputStream.close();
+
+        byte[] binaryData = outputStream.toByteArray();
+
+        // restore
+        ByteArrayInputStream inputStream = new ByteArrayInputStream(binaryData);
+        ObjectInputStream objectInputStream = new ObjectInputStream(inputStream);
+        Transaction restoredTransaction = (Transaction) objectInputStream.readObject();
+        objectInputStream.close();
+        inputStream.close();
+
+        restoredTransaction.getTotpSecret();
+    }
+
     @Test
     public void shouldThrowWhenRequestingTotpUriAfterSerialization() throws Exception {
         exception.expect(IllegalStateException.class);