[rb] update virtual auth credential implementation to match conventions

titusfortner · titusfortner · commit 930edbd8611e · 2022-08-04T15:45:26.000-05:00
diff --git a/rb/.rubocop.yml b/rb/.rubocop.yml
@@ -358,7 +358,7 @@ RSpec/Rails/AvoidSetupHook: # new in 2.4
 Style/EnvHome: # new in 1.29
   Enabled: true
 Layout/LineContinuationLeadingSpace: # new in 1.31
-  Enabled: true
+  Enabled: false
 Layout/LineContinuationSpacing: # new in 1.31
   Enabled: true
 Lint/ConstantOverwrittenInRescue: # new in 1.31
diff --git a/rb/lib/selenium/webdriver/common/virtual_authenticator/credential.rb b/rb/lib/selenium/webdriver/common/virtual_authenticator/credential.rb
@@ -25,65 +25,59 @@
 module Selenium
   module WebDriver
     class Credential
-      def initialize(id:, is_resident_credential:, rp_id:, user_handle:, private_key:, sign_count:)
+      class << self
+        def resident(**opts)
+          Credential.new(resident_credential: true, **opts)
+        end
+
+        def non_resident(**opts)
+          Credential.new(resident_credential: false, **opts)
+        end
+
+        def encode(byte_array)
+          Base64.urlsafe_encode64(byte_array&.pack('C*'))
+        end
+
+        def decode(base64)
+          Base64.urlsafe_decode64(base64).unpack('C*')
+        end
+
+        def from_json(opts)
+          user_handle = opts['userHandle'] ? decode(opts['userHandle']) : nil
+          new(id: decode(opts["credentialId"]),
+              resident_credential: opts["isResidentCredential"],
+              rp_id: opts['rpId'],
+              private_key: opts['privateKey'],
+              sign_count: opts['signCount'],
+              user_handle: user_handle)
+        end
+      end
+
+      attr_reader :id, :resident_credential, :rp_id, :user_handle, :private_key, :sign_count
+      alias_method :resident_credential?, :resident_credential
+
+      def initialize(id:, resident_credential:, rp_id:, private_key:, user_handle: nil, sign_count: 0)
         @id = id
-        @is_resident_credential = is_resident_credential
+        @resident_credential = resident_credential
         @rp_id = rp_id
         @user_handle = user_handle
         @private_key = private_key
         @sign_count = sign_count
       end
 
-      attr_reader :id, :is_resident_credential, :rp_id, :user_handle, :private_key, :sign_count
-
-      def self.create_resident_credential(id, rp_id, user_handle, private_key, sign_count)
-        Credential.new(id: id, is_resident_credential: true, rp_id: rp_id, user_handle: user_handle,
-                       private_key: private_key, sign_count: sign_count)
-      end
-
-      def self.create_non_resident_credential(id, rp_id, private_key, sign_count)
-        Credential.new(id: id, is_resident_credential: false, rp_id: rp_id, user_handle: nil,
-                       private_key: private_key, sign_count: sign_count)
-      end
-
       #
       # @api private
       #
 
       def as_json(*)
-        credential_data = {
-          credentialId: Base64.urlsafe_encode64(@id&.pack('C*')),
-          isResidentCredential: @is_resident_credential,
-          rpId: @rp_id,
-          privateKey: Base64.urlsafe_encode64(@private_key),
-          signCount: @sign_count
-        }
-
-        credential_data[:userHandle] = Base64.urlsafe_encode64(@user_handle&.pack('C*')) unless user_handle.nil?
+        credential_data = {'credentialId' => Credential.encode(id),
+                           'isResidentCredential' => resident_credential?,
+                           'rpId' => rp_id,
+                           'privateKey' => Credential.encode(private_key),
+                           'signCount' => sign_count}
+        credential_data['userHandle'] = Credential.encode(user_handle) if user_handle
         credential_data
       end
-
-      #
-      # @api private
-      #
-
-      def self.from_json(data)
-        id = Base64.urlsafe_decode64(data["credentialId"]).unpack('C*')
-        is_resident_credential = data["isResidentCredential"]
-        rp_id = data['rpId']
-        private_key = Base64.urlsafe_decode64(data["privateKey"])
-        sign_count = data['signCount']
-        user_handle = (Base64.urlsafe_decode64(data["userHandle"]).unpack('C*') if data.key?("userHandle"))
-
-        Credential.new(
-          id: id,
-          is_resident_credential: is_resident_credential,
-          rp_id: rp_id,
-          user_handle: user_handle,
-          private_key: private_key,
-          sign_count: sign_count
-        )
-      end
     end # Credential
   end # WebDriver
 end # Selenium
diff --git a/rb/spec/integration/selenium/webdriver/virtual_authenticator_spec.rb b/rb/spec/integration/selenium/webdriver/virtual_authenticator_spec.rb
@@ -142,11 +142,11 @@ def extract_id_from(response)
         # Add a non-resident credential using the testing API.
         #
         @authenticator = create_rk_disabled_ctap2_authenticator
-        credential = Credential.create_non_resident_credential(
-          [1, 2, 3, 4],
-          'localhost',
-          Base64.urlsafe_decode64(base64_encoded_pk),
-          0
+        credential = Credential.non_resident(
+          id: [1, 2, 3, 4],
+          rp_id: 'localhost',
+          private_key: Credential.decode(base64_encoded_pk),
+          sign_count: 0
         )
 
         @authenticator.add_credential(credential)
@@ -160,15 +160,15 @@ def extract_id_from(response)
       it 'should test add non-resident credential when authenticator uses U2F protocol' do
         @authenticator = create_rk_disabled_u2f_authenticator
         base64_enc_pk =
-          "MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQg8_zMDQDYAxlU-Q"\
-          "hk1Dwkf0v18GZca1DMF3SaJ9HPdmShRANCAASNYX5lyVCOZLzFZzrIKmeZ2jwU"\
+          "MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQg8_zMDQDYAxlU-Q" \
+          "hk1Dwkf0v18GZca1DMF3SaJ9HPdmShRANCAASNYX5lyVCOZLzFZzrIKmeZ2jwU" \
           "RmgsJYxGP__fWN_S-j5sN4tT15XEpN_7QZnt14YvI6uvAgO0uJEboFaZlOEB"
 
-        credential = Credential.create_non_resident_credential(
-          [1, 2, 3, 4],
-          'localhost',
-          Base64.urlsafe_decode64(base64_enc_pk),
-          0
+        credential = Credential.non_resident(
+          id: [1, 2, 3, 4],
+          rp_id: 'localhost',
+          private_key: Credential.decode(base64_enc_pk),
+          sign_count: 0
         )
         @authenticator.add_credential(credential)
         response = get_assertion_for([1, 2, 3, 4])
@@ -177,12 +177,12 @@ def extract_id_from(response)
 
       it 'should test add resident credential' do
         @authenticator = create_rk_enabled_ctap2_authenticator
-        credential = Credential.create_resident_credential(
-          [1, 2, 3, 4],
-          'localhost',
-          [1],
-          Base64.urlsafe_decode64(base64_encoded_pk),
-          0
+        credential = Credential.resident(
+          id: [1, 2, 3, 4],
+          rp_id: 'localhost',
+          user_handle: [1],
+          private_key: Credential.decode(base64_encoded_pk),
+          sign_count: 0
         )
         @authenticator.add_credential(credential)
         #
@@ -198,16 +198,16 @@ def extract_id_from(response)
       it 'should test add resident credential not supported when authenticator uses U2F protocol' do
         @authenticator = create_rk_enabled_u2f_authenticator
         base64_enc_pk =
-          "MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQg8_zMDQDYAxlU-Q"\
-          "hk1Dwkf0v18GZca1DMF3SaJ9HPdmShRANCAASNYX5lyVCOZLzFZzrIKmeZ2jwU"\
+          "MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQg8_zMDQDYAxlU-Q" \
+          "hk1Dwkf0v18GZca1DMF3SaJ9HPdmShRANCAASNYX5lyVCOZLzFZzrIKmeZ2jwU" \
           "RmgsJYxGP__fWN_S-j5sN4tT15XEpN_7QZnt14YvI6uvAgO0uJEboFaZlOEB"
 
-        credential = Credential.create_resident_credential(
-          [1, 2, 3, 4],
-          'localhost',
-          [1],
-          Base64.urlsafe_decode64(base64_enc_pk),
-          0
+        credential = Credential.resident(
+          id: [1, 2, 3, 4],
+          rp_id: 'localhost',
+          user_handle: [1],
+          private_key: Credential.decode(base64_enc_pk),
+          sign_count: 0
         )
 
         #
@@ -266,13 +266,13 @@ def extract_id_from(response)
           end
         end
 
-        expect(credential1.is_resident_credential).to eq(true)
+        expect(credential1.resident_credential?).to eq(true)
         expect(credential1.private_key).not_to eq(nil)
         expect(credential1.rp_id).to eq('localhost')
         expect(credential1.user_handle).to eq([1])
         expect(credential1.sign_count).to eq(1)
 
-        expect(credential2.is_resident_credential).to eq(false)
+        expect(credential2.resident_credential?).to eq(false)
         expect(credential2.private_key).not_to eq(nil)
         #
         # Non resident keys do not store raw RP IDs or user handles.
@@ -341,7 +341,7 @@ def extract_id_from(response)
         #
 
         response = driver.execute_async_script(
-          "getCredential([{"\
+          "getCredential([{" \
           "  \"type\": \"public-key\"," \
           "  \"id\": Int8Array.from(arguments[0])," \
           "}, {" \
@@ -362,7 +362,7 @@ def extract_id_from(response)
         #
 
         response = driver.execute_async_script(
-          "registerCredential({authenticatorSelection: {userVerification: 'required'}})"\
+          "registerCredential({authenticatorSelection: {userVerification: 'required'}})" \
           "  .then(arguments[arguments.length - 1]);"
         )
         expect(response['status']).to eq('OK')
diff --git a/rb/spec/unit/selenium/webdriver/common/credentials_spec.rb b/rb/spec/unit/selenium/webdriver/common/credentials_spec.rb
