Improved otpauth uri generation to elide defaults

src/URI/Otpauth.php

@@ -6,6 +6,12 @@ use InvalidArgumentException;
 
 class Otpauth
 {
+    const DEFAULTS = [
+            "algorithm" => "SHA1",
+            "period" => 30,
+            "digits" => 6,
+    ];
+
     private readonly string $secret;
 
     public function __construct(
@@ -71,12 +77,9 @@ class Otpauth
         $parsedQuery = [];
         \parse_str($parsedUri['query'], $parsedQuery);
 
-        $queryDefaults = [
+        $queryDefaults = \array_merge (self::DEFAULTS, [
-            "algorithm" => "SHA1",
-            "period" => "30",
-            "digits" => "6",
             "issuer" => \rawurldecode(\ltrim($label[0], "/")),
-        ];
+        ]);
         $parsedQuery = \array_merge($queryDefaults, $parsedQuery);
         $runCheck($queryChecks, $parsedQuery);
 
@@ -115,20 +118,44 @@ class Otpauth
         );
     }
 
-    public function emitStr(): string
+    private function buildUri(array $queryValues): string
     {
         $encodeIssuer = \rawurlencode($this->issuer);
         $label = $encodeIssuer . ":" . \rawurlencode($this->userid);
-        $issuer = "issuer=" . $encodeIssuer;
-        $algo = "algorithm=" . $this->algorithm;
-        $digits = "digits=" . $this->digits;
-        $period = "period=" . $this->period;
-        $secret = "secret=" . \rawurlencode($this->secret);
-        $query = \implode("&", [$secret, $issuer, $algo, $period, $digits]);
 
+        $query = \http_build_query($queryValues);
         return "otpauth://totp/" . $label . "?" . $query;
     }
 
+    //TODO: Add a logger in each of the following
+    public function uriString(): string
+    {
+        $queryValues = [
+            'issuer' => $this->issuer,
+            'secret' => $this->secret,
+            'algorithm' => ($this->algorithm == self::DEFAULTS['algorithm']) ? null : $this->algorithm,
+            'period' => ($this->period == self::DEFAULTS['period']) ? null : $this->period,
+            'digits' => ($this->digits == self::DEFAULTS['digits']) ? null : $this->digits,
+        ];
+        return $this->buildUri($queryValues);
+    }
+
+    public function uriStringExplicit(): string
+    {
+        $queryValues = [
+            'issuer' => $this->issuer,
+            'secret' => $this->secret,
+            'algorithm' => $this->algorithm,
+            'period' => $this->period,
+            'digits' => $this->digits,
+        ];
+        return $this->buildUri($queryValues);
+    }
+
+    public function getSecret(): string
+    {
+        return $this->secret;
+    }
 }
 
 ?>

src/Workflow/UserManagement.php

@@ -44,15 +44,17 @@ class UserManagement implements WorkflowInterface
         $html .= "<input type=\"submit\" value=\"Submit\" />";
         $html .= "</form></div>";
 
-        $provisioningUri = (new Otpauth(
+        $otpauthURI = new Otpauth(
             $this->db->userString($this->userIndex),
             "taatp",
             $this->hash->keygen(),
             $this->hash->hashType(),
             30,
             6
-        ))->emitStr();
+        );
-        $this->session->store("secret", $provisioningUri);
+        $provisioningUri = $otpauthURI->uriString();
+        $persistentUri = $otpauthURI->uriStringExplicit();
+        $this->session->store("secret", $persistentUri);
 
         $values = [
             "%frm" => $this->request->formProps("enroll"),
@@ -98,7 +100,7 @@ class UserManagement implements WorkflowInterface
         $enrollFlag && $enrollFlag = $this->session->get('secret');
 
         $totp = _6238(
-            $pUri.secret,
+            $pUri->getSecret(),
             $pUri.period,
             $enrollFlag? 0:$this->db->getLastTime($this->userIndex),
             2,

src/WorkflowInterface.php

@@ -13,7 +13,7 @@ interface WorkflowInterface
      * returns the workflow's relevant form.
      * @return string
      */
-    public function emitStr():string;
+    public function emitStr(): string;
     /**
      * Handles any response given to the workflow's form.
      * @return bool

tests/URITest.php

@@ -9,77 +9,42 @@ use BradyMcD\TAATP\URI\Otpauth;
 /** @SuppressWarnings(PHPMD.StaticAccess)*/
 final class URITest extends TestCase
 {
-    public function testRejectsInvalidUri(): void
+    private static $defaults;
-    {
+    private static $exampleUri;
-        $string = 'totp/ACME%20Co:john.doe@email.com?secret=HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ&issuer=ACME%20Co&algorithm=MD5&digits=8&period=60';
+    private static $exampleComponents;
-
-        $this->expectException(\InvalidArgumentException::class);
-
-        Otpauth::fromString($string);
-    }
 
-
+    public static function setUpBeforeClass(): void
-    public function testRejectsMissingSecretInformation(): void
     {
-        $string = 'otpauth://totp/ACME%20Co:john.doe@email.com?issuer=ACME%20Co&algorithm=MD5&digits=8&period=60';
+        self::$defaults = Otpauth::DEFAULTS;
-
+        self::$exampleUri = 'otpauth://totp/ACME%20Co:john.doe@email.com?secret=HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ&issuer=ACME%20Co&algorithm=MD5&digits=8&period=60';
-        $this->expectException(\InvalidArgumentException::class);
+        \parse_str(\parse_url(self::$exampleUri)['query'], self::$exampleComponents);
 
-        Otpauth::fromString($string);
     }
 
-    public function testPreservesAllUriFields(): void
+    /** @SuppressWarnings(PHPMD.EmptyCatchBlock)*/
+    public function testRejectsInvalidUris(): void
     {
-        $string = 'otpauth://totp/ACME%20Co:john.doe@email.com?secret=HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ&issuer=ACME%20Co&algorithm=MD5&digits=8&period=60';
+        $invalids = [
-
+            "https://www.example.org",
-         $queryComponents = [ "secret" => "HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ",
+            "Just a normal string",
-                              "issuer" => "ACME Co",
+            "otpauth://totp/NOSECRETS:john.doe@email.com?issuer=ACME%20Co&algorithm=MD5&digits=8&period=60",
-                              "algorithm" => "MD5",
+            "otpauth://totp/MISS:john.doe@email.com?secret=HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ&issuer=MATCH&algorithm=MD5&digits=8&period=60",
-                              "digits" => 8,
+            "otpauth://totp/vendor:john.doe@email.com?secret=HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ&issuer=vendohr&algorithm=NotAnAlgo&digits=8&period=60",
-                              "period" => 60,];
+            "",
-
-       $provisioningUri = Otpauth::fromString($string);
 
-        $this->assertSame($provisioningUri->issuer, $queryComponents['issuer']);
+        ];
-        $this->assertSame($provisioningUri->digits, $queryComponents['digits']);
+        $pUri;
-        $this->assertSame($provisioningUri->period, $queryComponents['period']);
-        $this->assertSame($provisioningUri->algorithm, $queryComponents['algorithm']);
 
-        $calculatedUrl = $provisioningUri->emitStr();
+        foreach ($invalids as $invalid)
+        {
+            $this->expectException(\InvalidArgumentException::class);
 
-        $parsedOtp = \parse_url($calculatedUrl);
+            $pUri = Otpauth::fromString($invalid);
-        $parsedTest = \parse_url($string);
 
-        $parsedOtpQuery = [];
+            $this->assertNull($pUri);
-        $parsedTestQuery = [];
+        }
-
-        \parse_str($parsedOtp['query'], $parsedOtpQuery);
-        unset($parsedOtp['query']);
-        \parse_str($parsedTest['query'], $parsedTestQuery);
-        unset($parsedTest['query']);
-
-        $this->assertEqualsCanonicalizing($parsedOtp, $parsedTest);
-        $this->assertEqualsCanonicalizing($parsedOtpQuery, $parsedTestQuery);
     }
 
-    public function testCanFallbackToDefaults(): void
-    {
-        $string = 'otpauth://totp/Example:john.doe@email.com?secret=HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ';
-        $queryComponents = [ "secret" => "HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ",
-                             "issuer" => "Example",
-                             "algorithm" => "SHA1",
-                             "digits" => 6,
-                             "period" => 30,];
-
-
-
-        $provisioningUri = Otpauth::fromString($string);
-
-        $this->assertSame($provisioningUri->algorithm, $queryComponents['algorithm']);
-        $this->assertSame($provisioningUri->digits, $queryComponents['digits']);
-        $this->assertSame($provisioningUri->period, $queryComponents['period']);
-        $this->assertSame($provisioningUri->issuer, $queryComponents['issuer']);
-    }
 }
 
 ?>