Design for authorization page when client_name not providen + XSS spec

app/assets/stylesheets/mobile/settings.scss

@@ -33,3 +33,15 @@
 .applications-page .applications-explanation {
   margin-bottom: 15px;
 }
+
+.application-img {
+  margin: auto;
+  max-width: 150px;
+  text-align: center;
+
+  .entypo-browser {
+    font-size: 137px;
+    height: 160px;
+    margin-top: -45px;
+  }
+}

app/controllers/api/openid_connect/authorizations_controller.rb

@@ -114,11 +114,7 @@ module Api
         ]
         save_request_parameters
 
-        @app = {
+        @app = UserApplicationPresenter.new @o_auth_application, @scopes
-          name:           @o_auth_application.client_name,
-          image:          @o_auth_application.image_uri,
-          authorizations: @scopes
-        }
 
         render :new
       end

app/helpers/user_applications_helper.rb

@@ -0,0 +1,9 @@
+module UserApplicationsHelper
+  def user_application_name(app)
+    if app.name?
+      "#{app.name} (#{link_to(app.url, app.url)})"
+    else
+      link_to(app.url, app.url)
+    end
+  end
+end

app/presenters/user_application_presenter.rb

@@ -0,0 +1,36 @@
+class UserApplicationPresenter
+  def initialize(application, scopes, authorization_id=nil)
+    @app = application
+    @scopes = scopes
+    @authorization_id = authorization_id
+  end
+
+  def scopes
+    @scopes
+  end
+
+  def id
+    @authorization_id
+  end
+
+  def name
+    @app.client_name
+  end
+
+  def image
+    @app.image_uri
+  end
+
+  def name?
+    if @app.client_name
+      true
+    else
+      false
+    end
+  end
+
+  def url
+    client_redirect = URI(@app.redirect_uris[0])
+    "#{client_redirect.scheme}://#{client_redirect.host}"
+  end
+end

app/presenters/user_applications_presenter.rb

@@ -4,7 +4,10 @@ class UserApplicationsPresenter
   end
 
   def user_applications
-    @applications ||= @user.o_auth_applications.map {|app| app_as_json(app) }
+    @applications ||= @user.o_auth_applications.map do |app|
+      authorization = Api::OpenidConnect::Authorization.find_by_client_id_and_user(app.client_id, @user)
+      UserApplicationPresenter.new app, authorization.scopes, authorization.id
+    end
   end
 
   def applications_count
@@ -14,27 +17,4 @@ class UserApplicationsPresenter
   def applications?
     applications_count > 0
   end
-
-  private
-
-  def app_as_json(application)
-    {
-      id:             find_id(application),
-      name:           application.client_name,
-      image:          application.image_uri,
-      authorizations: find_scopes(application)
-    }
-  end
-
-  def find_scopes(application)
-    find_auth(application).scopes
-  end
-
-  def find_id(application)
-    find_auth(application).id
-  end
-
-  def find_auth(application)
-    Api::OpenidConnect::Authorization.find_by_client_id_and_user(application.client_id, @user)
-  end
 end

app/views/api/openid_connect/authorizations/_grants_list.haml

@@ -1,16 +1,17 @@
 .application-img
-  - if app[:image]
+  - if app.image
-    = image_tag app[:image], class: "img-responsive"
+    = image_tag app.image, class: "img-responsive"
   - else
     %i.entypo-browser
 .application-authorizations
-  - if app[:authorizations].count > 0
+  - if app.scopes.count > 0
-    %h4= t("api.openid_connect.authorizations.new.access", name: app[:name])
+    %h4
+      = t("api.openid_connect.authorizations.new.access", name: user_application_name(app)).html_safe
     %ul
-      - app[:authorizations].each do |authorization|
+      - app.scopes.each do |scope|
         %li
-          %b= t("api.openid_connect.scopes.#{authorization}.name")
+          %b= t("api.openid_connect.scopes.#{scope}.name")
-          %p= t("api.openid_connect.scopes.#{authorization}.description")
+          %p= t("api.openid_connect.scopes.#{scope}.description")
   - else
     .well
-      = t("api.openid_connect.authorizations.new.no_requirement", name: app[:name])
+      = t("api.openid_connect.authorizations.new.no_requirement", name: user_application_name(app)).html_safe

app/views/api/openid_connect/authorizations/new.html.haml

@@ -1,6 +1,6 @@
-.user-consent.col-md-6.col-md-offset-1
+.user-consent.col-md-10.col-md-offset-1
   %ul.list-group
-    %li.list-group-item.authorized-application
+    %li.list-group-item.authorized-application.clearfix
       = render "grants_list", app: @app
 
       .clearfix.pull-right

app/views/api/openid_connect/user_applications/_add_remove_applications.haml

@@ -3,7 +3,7 @@
     - @user_apps.user_applications.each do |app|
       %li.list-group-item.authorized-application
         = render "grants_list", app: app
-        = form_for "application", url: "#{api_openid_connect_authorizations_path}/#{app[:id]}",
+        = form_for "application", url: "#{api_openid_connect_authorization_path(app.id)}",
             html: {method: :delete, class: "form-horizontal"} do |f|
           .clearfix= f.submit t("api.openid_connect.user_applications.revoke_autorization"),
             class: "btn btn-danger pull-right app-revoke"

app/views/api/openid_connect/user_applications/_grants_list.haml

@@ -1,16 +1,16 @@
 .application-img
-  - if app[:image]
+  - if app.image
-    = image_tag app[:image], class: "img-responsive"
+    = image_tag app.image, class: "img-responsive"
   - else
     %i.entypo-browser
 .application-authorizations
-  - if app[:authorizations].count > 0
+  - if app.scopes.count > 0
-    %h4= t("api.openid_connect.user_applications.index.access", name: app[:name])
+    %h4= t("api.openid_connect.user_applications.index.access", name: user_application_name(app)).html_safe
     %ul
-      - app[:authorizations].each do |authorization|
+      - app.scopes.each do |scope|
         %li
-          %b= t("api.openid_connect.scopes.#{authorization}.name")
+          %b= t("api.openid_connect.scopes.#{scope}.name")
-          %p= t("api.openid_connect.scopes.#{authorization}.description")
+          %p= t("api.openid_connect.scopes.#{scope}.description")
   - else
     .well
-      = t("api.openid_connect.user_applications.index.no_requirement", name: app[:name])
+      = t("api.openid_connect.user_applications.index.no_requirement", name: user_application_name(app)).html_safe

app/views/api/openid_connect/user_applications/index.html.haml

@@ -2,16 +2,12 @@
   = t(".edit_applications")
 
 .container-fluid.applications-page
-  = render "shared/settings_nav"
+  .row
-  .container-fluid
+    .col-lg-10.col-lg-offset-1
-    .row
+      = render "shared/settings_nav"
-      .col-lg-8.col-lg-offset-2
+  .row
-        %h3= t(".title")
+    .col-lg-8.col-lg-offset-2
-        %p.visible-sm-block.visible-xs-block
+      %h3= t(".title")
-          = t(".applications_explanation")
+      .row
-          .row
+        .col-md-12
-            .col-md-7
+          = render "add_remove_applications"
-              = render "add_remove_applications"
-            .col-md-5
-              %p.hidden-sm.hidden-xs
-                = t(".applications_explanation")

app/views/api/openid_connect/user_applications/index.mobile.haml

@@ -1,13 +1,9 @@
-.settings_container.applications-page
+.container-fluid.settings_container.applications-page
-  - content_for :page_title do
+  .row
-    = t(".edit_applications")
+    .col-lg-10.col-lg-offset-1
-
+      - content_for :page_title do
-  = render "shared/settings_nav"
+        = t(".edit_applications")
-
+      = render "shared/settings_nav"
-  .container-fluid
+  .row
-    .row
+    .col-md-12
-      .col-md-12.applications-explanation
+      = render "add_remove_applications"
-        = t(".applications_explanation")
-      .col-md-12
-        = render "add_remove_applications"
-

config/locales/diaspora/en.yml

@@ -898,7 +898,6 @@ en:
           title: "Authorized applications"
           access: "%{name} has access to:"
           no_requirement: "%{name} requires no permissions"
-          applications_explanation: "Here is a list of applications you have authorized"
         no_applications: "You have no authorized applications"
         revoke_autorization: "Revoke"
       scopes:

features/desktop/user_applications.feature

@@ -21,3 +21,7 @@ Feature: managing authorized applications
     Then I should see 1 authorized applications
     And I revoke the first authorization
     Then I should see 0 authorized applications
+
+  Scenario: XSS escaping
+    When An application manually registers
+    Then I should not see "<script>alert(0);</script>"

features/step_definitions/user_applications_steps.rb

@@ -14,3 +14,9 @@ end
 When /^I revoke the first authorization$/ do
   find(".app-revoke", match: :first).click
 end
+
+When /^An application manually registers$/ do
+  post api_openid_connect_authorizations_new_path, client_name: "<script>alert(0);</script>",
+       redirect_uri: "http://example.org/", response_type: "id_token", scope: "openid",
+       state: 1234, display: "page", prompt: "none"
+end

spec/controllers/api/openid_connect/authorizations_controller_spec.rb

@@ -130,6 +130,7 @@ describe Api::OpenidConnect::AuthorizationsController, type: :controller do
         end
       end
     end
+
     context "when already authorized" do
       let!(:auth) {
         Api::OpenidConnect::Authorization.find_or_create_by(o_auth_application: client, user: alice,