Fix Indieauth

app/assets/stylesheets/misc.css

@@ -11,14 +11,19 @@ a.feed_entry_skip {
 }
 
 img.indie-client-logo {
-  display: inline-block;
-  width: 80px;
-  height: 80px;
-  margin-bottom: 1em;
-}
-
-p.indie-client-logo {
-  text-align: center;
+  display: block;
+  max-width: 100%;
+  max-height: 100%;
+  object-fit: contain;
+}
+
+div.indie-client-logo {
+  object-fit: contain;
+  overflow: hidden;
+  width: 120px;
+  height: 120px;
+  flex-shrink: 0;
+  margin: 0 auto;
 }
 
 p.indie-client-name {

app/controllers/indieauth_controller.rb

@@ -35,7 +35,7 @@ def authorization
     @auth_request = IndieAuthRequestObj.new(params)
     @client = IndieAuthClient.new(@auth_request.client_id)
     if !@client.valid_redirect?(@auth_request.redirect_uri)
-      head :bad_request
+      render html: "<h1>Bad Request</h1><p>IndieAuth client ID doesn't match the provided redirect url.</p>".html_safe, status: :bad_request
     end
   end
 
@@ -59,7 +59,7 @@ def approval
       redirect_to redirect_url, allow_other_host: true
     elsif params[:commit] == 'Deny'
       flash[:alert] = "You denied the request"
-      redirect_to root
+      redirect_to posts_url
     end
   end
 

app/views/indieauth/_client.html.erb

@@ -1,4 +1 @@
-<p class="indie-client-logo"><img class="indie-client-logo" src="<%=@client.logo%>"/></p>
-<p class="indie-client-name"><%=@client.name%></p>
-<p>DEBUG</p>
-
+<div class="indie-client-logo"><img class="indie-client-logo" src="<%=@client.logo%>"/></div>

app/views/indieauth/authorization.html.erb

@@ -1,9 +1,9 @@
 <p>You are logged in as <%= current_user.name %> with email <%= current_user.email %> </p>
 
-<% render 'client' %>
+<%= render 'client' %>
 
 <% if @auth_request.scope.size > 0 %>
-  <p><span class="indie-auth-client"><%= @auth_request.client_id %></span> is requesting access to your Haven.  It it requesting permision to take the following actions on your behalf.  You can select each permission you want to grant individually.  Removing a permission will prevent the application from taking that action.</p>
+  <p><span class="indie-auth-client"><%= @client.name %></span> (at <%= URI(@auth_request.client_id).host %>) is requesting access to your Haven.  It it requesting permision to take the following actions on your behalf.  You can select each permission you want to grant individually.  Removing a permission will prevent the application from taking that action.</p>
 <% else %>
   <p><span class="indie-auth-client"><%= @auth_request.client_id %></span> is requesting access to your Haven.  By approving the request, it will learn that you are able to sign into this Haven, but will not be able to access anything else about your Haven.
 <% end %>

lib/indie_auth_client.rb

@@ -13,11 +13,20 @@ def to_html
     return output
   end
 
-  def valid_redirect? (redirect)
-    return true if redirect.start_with? @client_id
-    return true if @redirects.include? redirect
-    #puts "redirect validation error.  Indie auth client #{@client_id} requesting redirect to #{redirect} which does not have #{@client_id} as a prefix, and is not in list of valid redirects fetched: #{@redirects.join(",")}"
-    return false
+  def valid_redirect?(redirect)
+    return true if @redirects.include?(redirect)
+    begin
+      client_uri = URI.parse(@client_id)
+      redirect_uri = URI.parse(redirect)
+      return true if redirect_uri.host && 
+                     client_uri.scheme == redirect_uri.scheme &&
+                     client_uri.host == redirect_uri.host &&
+                     client_uri.port == redirect_uri.port
+    rescue URI::InvalidURIError
+      return false
+    end
+    # puts "Redirect validation error: Client #{@client_id} requested invalid redirect #{redirect}."
+    false
   end
 
   def initialize(client_id)
@@ -26,10 +35,51 @@ def initialize(client_id)
     @logo = "/warning.svg"
     @url = @client_id
     @redirects = []
-    if should_fetch?
+    if should_fetch? @client_id
       begin
         URI(client_id).open do |f|
-          doc = Nokogiri::HTML(f.read)
+          if f.content_type == 'application/json' || client_id.end_with?('.json')
+            parse_json_client(f.read)
+          else
+            parse_html_client(f.read)
+          end
+          @logo = "/warning.svg" unless should_fetch? @logo
+        end
+      rescue
+        # do we need to complain loudly if unable to fetch client_id?
+      end
+    else
+      @name = "A test application at #{@client_id}"
+    end
+  end
+
+  private
+
+  def parse_json_client(raw_content)
+    data = JSON.parse(raw_content)
+
+    @name = data['client_name'] unless data['client_name'].nil? || data['client_name'].empty?
+
+    # Handle optional relative paths for URL and Logo, though JSON usually uses absolute URIs
+    @url = data['client_uri']
+    @url = client_id.chomp("/") + @url if @url&.start_with?("/")
+
+    @logo = data['logo_uri']
+    @logo = client_id.chomp("/") + @logo if @logo&.start_with?("/")
+
+    raw_redirects = data['redirect_uris'] || []
+    raw_redirects.each do |rr|
+      if rr.start_with?("/")
+        @redirects << client_id.chomp("/") + rr
+      else
+        @redirects << rr
+      end
+    end
+  end
+
+  def parse_html_client(raw_content)
+
+          doc = Nokogiri::HTML(raw_content)
 
           url = ""
           doc.xpath("//div[@class='h-app']//a[contains(@class, 'u-url')]/@href").each {|u| url = u.value}
@@ -43,7 +93,7 @@ def initialize(client_id)
           doc.xpath("//div[@class='h-app']//img[contains(@class, 'u-logo')]/@src").each {|l| logo = l.value}
           @logo = client_id.chomp("/") + logo if logo.start_with? "/"
 
-          raw_redirect = []
+          raw_redirects = []
           doc.xpath("//link[@rel='redirect_uri']").each {|l| raw_redirects << l["href"]}
           raw_redirects.each do |rr|
             if rr.start_with? "/"
@@ -52,22 +102,18 @@ def initialize(client_id)
               @redirects << rr
             end
           end
-        end
-      rescue
-        # do we need to complain loudly if unable to fetch client_id?
-      end
-    else
-      @name = "A test application at #{@client_id}"
-    end
   end
 
-  private
-
-  def should_fetch?
-    clean_host = parse_hostname(@client_id)
+  ## Protect against server-side request forgery (SSRF) by validating the IP address
+  def should_fetch?(suspect_url)
+    return false unless URI(suspect_url).scheme=="https"
+    clean_host = URI.parse(suspect_url).host
+    puts "DEBUG: clean host: #{clean_host}"
     begin
-      client_id_uri = URI.parse(@client_id) #for validation only
+      client_id_uri = URI.parse(suspect_url) #for validation only
+      puts "DEBUG: client_id_uri: #{client_id_uri}"
       ip = Resolv.getaddress(clean_host)
+      puts "DEBUG: ip resolved: #{ip}"
       return safe_ip? ip
     rescue
       return false
@@ -89,20 +135,4 @@ def safe_ip?(ip)
     return false if ip.start_with? "fe80::"
     return true
   end
-
-  def parse_hostname(client_id)
-    no_scheme = client_id
-    if no_scheme.include? "://"
-      scheme, no_scheme = client_id.split("://",2)
-    end
-    no_auth = no_scheme
-    if (no_auth.include?(":") and no_auth.include?("@")) #user:pass@host
-      auth, no_auth = no_scheme.split("@",2)
-    end
-    no_port = no_auth
-    if no_port.include? ":"
-      no_port, port = no_auth.split(":",2)
-    end
-    return no_port
-  end
 end