Fix hostname ip addr checking.

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    atproto_auth (0.2.2)
+    atproto_auth (0.2.3)
       jose (~> 1.2)
       jwt (~> 2.9)
       redis (~> 5.3)

examples/confidential_client/Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: ../..
   specs:
-    atproto_auth (0.2.2)
+    atproto_auth (0.2.3)
       jose (~> 1.2)
       jwt (~> 2.9)
       redis (~> 5.3)

lib/atproto_auth/http_client.rb

@@ -108,9 +108,7 @@ def validate_ip!(uri)
       if uri.host =~ /^(\d{1,3}\.){3}\d{1,3}$/
         begin
           ip = IPAddr.new(uri.host)
-          if forbidden_ip?(ip)
-            raise SSRFError, "Request to forbidden IP address"
-          end
+          raise SSRFError, "Request to forbidden IP address" if forbidden_ip?(ip)
         rescue IPAddr::InvalidAddressError
           # Not a valid IP, will be handled as hostname below
         end
@@ -119,15 +117,11 @@ def validate_ip!(uri)
       # Also check resolved IPs for hostnames
       begin
         ips = Resolv::DNS.new.getaddresses(uri.host)
-        ips.each do |ip|
-          begin
-            ip_addr = IPAddr.new(ip.to_s)
-            if forbidden_ip?(ip_addr)
-              raise SSRFError, "Request to forbidden IP address"
-            end
-          rescue IPAddr::InvalidAddressError
-            next
-          end
+        ips.each do |x|
+          ip_addr = IPAddr.new(x.to_s)
+          raise SSRFError, "Request to forbidden IP address" if forbidden_ip?(ip_addr)
+        rescue IPAddr::InvalidAddressError
+          next
         end
       rescue Resolv::ResolvError
         raise SSRFError, "Could not resolve hostname"

lib/atproto_auth/identity/resolver.rb

@@ -62,7 +62,11 @@ def get_did_info(did)
       # @raise [ValidationError] if verification fails
       def verify_pds_binding(did, pds_url)
         info = get_did_info(did)
-        normalize_url(info[:pds]) == normalize_url(pds_url)
+        if normalize_url(info[:pds]) != normalize_url(pds_url)
+          raise ValidationError, "PDS #{pds_url} is not authorized for DID #{did}"
+        end
+
+        true
       rescue StandardError => e
         raise ValidationError, "Failed to verify PDS binding: #{e.message}"
       end
@@ -82,7 +86,11 @@ def verify_issuer_binding(did, issuer)
         auth_server_url = resource_server.authorization_servers.first
 
         # Compare normalized URLs
-        normalize_url(auth_server_url) == normalize_url(issuer)
+        if normalize_url(auth_server_url) != normalize_url(issuer)
+          raise ValidationError, "Issuer #{issuer} is not authorized for DID #{did}"
+        end
+
+        true
       rescue StandardError => e
         raise ValidationError, "Failed to verify issuer binding: #{e.message}"
       end
@@ -94,7 +102,13 @@ def verify_issuer_binding(did, issuer)
       # @raise [ValidationError] if verification fails
       def verify_handle_binding(handle, did)
         info = get_did_info(did)
-        info[:document].has_handle?(handle)
+
+        unless info[:document].has_handle?(handle)
+          raise ValidationError,
+                "Handle #{handle} does not belong to DID #{did}"
+        end
+
+        true
       rescue StandardError => e
         raise ValidationError, "Failed to verify handle binding: #{e.message}"
       end
@@ -137,14 +151,7 @@ def resolve_handle_dns(handle)
       def extract_domain(handle)
         # Remove @ prefix if present
         handle = handle[1..] if handle.start_with?("@")
-
-        # Handle could be user.domain.com or domain.com format
-        # We just need the domain portion
-        if handle.count(".") == 1
-          handle
-        else
-          handle.split(".", 2)[1]
-        end
+        handle
       end
 
       def fetch_txt_records(domain)

lib/atproto_auth/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module AtprotoAuth
-  VERSION = "0.2.2"
+  VERSION = "0.2.3"
 end

test/atproto_auth/identity/resolver_test.rb

@@ -7,6 +7,7 @@
   let(:handle) { "user.test.com" }
   let(:did) { "did:plc:test123" }
   let(:pds_url) { "https://pds.test.com" }
+
   before do
     AtprotoAuth.configure do |configuration|
       configuration.http_client = AtprotoAuth::HttpClient.new
@@ -15,22 +16,56 @@
 
   describe "#resolve_handle" do
     it "resolves handle via DNS" do
-      stub_dns_records(["did=#{did}"])
-      stub_plc_document(did, pds_url)
+      # Create mock resolver
+      mock_dns = Minitest::Mock.new
 
-      result = resolver.resolve_handle(handle)
-      assert_equal did, result[:did]
-      assert_equal pds_url, result[:pds]
+      def mock_dns.timeouts=(val); end
+
+      def mock_dns.close; end
+
+      def mock_dns.getresources(name, type)
+        if name == "_atproto.user.test.com" && type == Resolv::DNS::Resource::IN::TXT
+          txt_record = Minitest::Mock.new
+
+          def txt_record.strings
+            ["did=did:plc:test123"]
+          end
+
+          [txt_record]
+        else
+          []
+        end
+      end
+
+      def mock_dns.getaddresses(_name)
+        []
+      end
+
+      # Stub DNS resolution and PLC document
+      Resolv::DNS.stub :new, mock_dns do
+        stub_plc_document(did, pds_url)
+
+        result = resolver.resolve_handle(handle)
+        assert_equal did, result[:did]
+        assert_equal pds_url, result[:pds]
+      end
     end
 
-    it "falls back to HTTP resolution" do
-      stub_dns_empty
-      stub_request(:get, "https://#{handle}/.well-known/atproto-did")
-        .to_return(body: did)
-      stub_plc_document(did, pds_url)
+    it "falls back to HTTP resolution when DNS fails" do
+      stub_dns_empty do
+        stub_request(:get, "https://#{handle}/.well-known/atproto-did")
+          .to_return(
+            status: 200,
+            body: did,
+            headers: { "Content-Type": "text/plain" }
+          )
 
-      result = resolver.resolve_handle(handle)
-      assert_equal did, result[:did]
+        stub_plc_document(did, pds_url)
+
+        result = resolver.resolve_handle(handle)
+        assert_equal did, result[:did]
+        assert_equal pds_url, result[:pds]
+      end
     end
 
     it "handles @ prefix in handle" do
@@ -52,6 +87,7 @@
       result = resolver.get_did_info(did)
       assert_equal did, result[:did]
       assert_equal pds_url, result[:pds]
+      assert_kind_of AtprotoAuth::Identity::Document, result[:document]
     end
 
     it "requires HTTPS for PDS URL" do
@@ -71,7 +107,14 @@
     it "fetches and validates web DID document" do
       web_did = "did:web:example.com"
       stub_request(:get, "https://example.com/.well-known/did.json")
-        .to_return(body: { id: web_did, pds: "https://pds.example.com" }.to_json)
+        .to_return(
+          status: 200,
+          body: {
+            id: web_did,
+            pds: "https://pds.example.com"
+          }.to_json,
+          headers: { "Content-Type": "application/json" }
+        )
 
       result = resolver.get_did_info(web_did)
       assert_equal web_did, result[:did]
@@ -81,7 +124,14 @@
     it "handles web DID with path component" do
       web_did = "did:web:example.com:user:alice"
       stub_request(:get, "https://example.com/user/alice/did.json")
-        .to_return(body: { id: web_did, pds: "https://pds.example.com" }.to_json)
+        .to_return(
+          status: 200,
+          body: {
+            id: web_did,
+            pds: "https://pds.example.com"
+          }.to_json,
+          headers: { "Content-Type": "application/json" }
+        )
 
       result = resolver.get_did_info(web_did)
       assert_equal web_did, result[:did]
@@ -100,6 +150,17 @@
 
       assert resolver.verify_pds_binding(did, "#{pds_url}/")
     end
+
+    it "raises error for non-matching PDS" do
+      stub_plc_document(did, pds_url)
+      different_pds = "https://different-pds.com"
+
+      error = assert_raises(AtprotoAuth::Identity::ValidationError) do
+        resolver.verify_pds_binding(did, different_pds)
+      end
+
+      assert_match(/PDS .* is not authorized for DID .*/, error.message)
+    end
   end
 
   describe "#verify_issuer_binding" do
@@ -111,6 +172,24 @@
 
       assert resolver.verify_issuer_binding(did, issuer)
     end
+
+    it "normalizes URLs for comparison" do
+      stub_plc_document(did, pds_url)
+      stub_resource_server_metadata(pds_url, issuer)
+
+      assert resolver.verify_issuer_binding(did, "#{issuer}/")
+    end
+
+    it "fails for non-matching issuer" do
+      stub_plc_document(did, pds_url)
+      stub_resource_server_metadata(pds_url, issuer)
+
+      error = assert_raises(AtprotoAuth::Identity::ValidationError) do
+        resolver.verify_issuer_binding(did, "https://wrong-issuer.com")
+      end
+
+      assert_match(/Issuer .* is not authorized for DID .*/, error.message)
+    end
   end
 
   describe "#verify_handle_binding" do
@@ -121,48 +200,78 @@
         "pds" => pds_url
       }
       stub_request(:get, "#{resolver.instance_variable_get(:@plc_directory)}/#{did}")
-        .to_return(body: doc.to_json)
+        .to_return(
+          status: 200,
+          body: doc.to_json,
+          headers: { "Content-Type": "application/json" }
+        )
 
       assert resolver.verify_handle_binding(handle, did)
     end
+
+    it "fails for non-matching handle" do
+      doc = {
+        "id" => did,
+        "alsoKnownAs" => ["at://other.handle.com"],
+        "pds" => pds_url
+      }
+      stub_request(:get, "#{resolver.instance_variable_get(:@plc_directory)}/#{did}")
+        .to_return(
+          status: 200,
+          body: doc.to_json,
+          headers: { "Content-Type": "application/json" }
+        )
+
+      error = assert_raises(AtprotoAuth::Identity::ValidationError) do
+        resolver.verify_handle_binding(handle, did)
+      end
+
+      assert_match(/Handle .* does not belong to DID .*/, error.message)
+    end
   end
 
   private
 
-  def stub_dns_records(records)
-    Resolv::DNS.stubs(:new).returns(mock_resolver(records))
-  end
+  def stub_dns_empty(&block)
+    mock_dns = Minitest::Mock.new
 
-  def stub_dns_empty
-    Resolv::DNS.stubs(:new).returns(mock_resolver([]))
-  end
+    def mock_dns.timeouts=(val); end
 
-  def mock_resolver(records)
-    resolver = Minitest::Mock.new
-    def resolver.timeouts=(val); end
-    def resolver.close; end
+    def mock_dns.close; end
 
-    txt_resources = records.map do |record|
-      resource = Minitest::Mock.new
-      resource.expect :strings, [record]
-      resource
+    def mock_dns.getresources(*)
+      []
     end
 
-    resolver.expect :getresources, txt_resources, [String, Resolv::DNS::Resource::IN::TXT]
-    resolver
+    def mock_dns.getaddresses(*)
+      []
+    end
+
+    Resolv::DNS.stub :new, mock_dns do
+      block&.call
+    end
   end
 
   def stub_plc_document(did, pds_url)
-    doc = {
-      "id" => did,
-      "pds" => pds_url
-    }
     stub_request(:get, "#{resolver.instance_variable_get(:@plc_directory)}/#{did}")
-      .to_return(body: doc.to_json)
+      .to_return(
+        status: 200,
+        body: {
+          "id" => did,
+          "pds" => pds_url
+        }.to_json,
+        headers: { "Content-Type": "application/json" }
+      )
   end
 
   def stub_resource_server_metadata(pds_url, auth_server)
     stub_request(:get, "#{pds_url}/.well-known/oauth-protected-resource")
-      .to_return(body: { authorization_servers: [auth_server] }.to_json)
+      .to_return(
+        status: 200,
+        body: {
+          authorization_servers: [auth_server]
+        }.to_json,
+        headers: { "Content-Type": "application/json" }
+      )
   end
 end