diff --git a/app/assets/stylesheets/misc.css b/app/assets/stylesheets/misc.css index 9b4cecb..e350060 100644 --- a/app/assets/stylesheets/misc.css +++ b/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 { diff --git a/app/controllers/indieauth_controller.rb b/app/controllers/indieauth_controller.rb index 56da242..8d15440 100644 --- a/app/controllers/indieauth_controller.rb +++ b/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: "

Bad Request

IndieAuth client ID doesn't match the provided redirect url.

".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 diff --git a/app/views/indieauth/_client.html.erb b/app/views/indieauth/_client.html.erb index 6b3a1f6..97b084c 100644 --- a/app/views/indieauth/_client.html.erb +++ b/app/views/indieauth/_client.html.erb @@ -1,4 +1 @@ - -

<%=@client.name%>

-

DEBUG

- + diff --git a/app/views/indieauth/authorization.html.erb b/app/views/indieauth/authorization.html.erb index 6318330..77797a7 100644 --- a/app/views/indieauth/authorization.html.erb +++ b/app/views/indieauth/authorization.html.erb @@ -1,9 +1,9 @@

You are logged in as <%= current_user.name %> with email <%= current_user.email %>

-<% render 'client' %> +<%= render 'client' %> <% if @auth_request.scope.size > 0 %> -

<%= @auth_request.client_id %> 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.

+

<%= @client.name %> (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.

<% else %>

<%= @auth_request.client_id %> 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 %> diff --git a/lib/indie_auth_client.rb b/lib/indie_auth_client.rb index 5d29f93..564d14c 100644 --- a/lib/indie_auth_client.rb +++ b/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