From 7b2be0d3c6154e91e713193a306f17cdd7c58c00 Mon Sep 17 00:00:00 2001 From: augier Date: Sat, 10 Oct 2015 14:41:16 +0200 Subject: [PATCH] Support displaying TOS and policy --- app/assets/stylesheets/user_applications.scss | 5 ++++ .../authorizations_controller.rb | 2 +- app/helpers/user_applications_helper.rb | 2 +- app/presenters/user_application_presenter.rb | 24 ++++++++++++++----- .../authorizations/_grants_list.haml | 14 +++++++++++ .../user_applications/_grants_list.haml | 14 +++++++++++ config/locales/diaspora/en.yml | 2 ++ db/schema.rb | 2 +- .../authorizations_controller_spec.rb | 10 ++++++++ 9 files changed, 66 insertions(+), 9 deletions(-) diff --git a/app/assets/stylesheets/user_applications.scss b/app/assets/stylesheets/user_applications.scss index 2b49bc5e4..39c94fc75 100644 --- a/app/assets/stylesheets/user_applications.scss +++ b/app/assets/stylesheets/user_applications.scss @@ -25,5 +25,10 @@ float: right; } +.application-tos-policy > b { + &:first-child { margin-right: 5px; } + &:nth-child(2) { margin-left: 5px; } +} + .user-consent { margin-top: 20px; } .approval-button { display: inline; } diff --git a/app/controllers/api/openid_connect/authorizations_controller.rb b/app/controllers/api/openid_connect/authorizations_controller.rb index e8369e372..0ad312708 100644 --- a/app/controllers/api/openid_connect/authorizations_controller.rb +++ b/app/controllers/api/openid_connect/authorizations_controller.rb @@ -189,7 +189,7 @@ module Api redirect_prompt_error_display(error, error_description) else render json: {error: "bad_request", - description: "No client with client_id " + params[:client_id] + " found"} + description: "No client with client_id #{params[:client_id]} found"} end else render json: {error: "bad_request", description: "Missing client id or redirect URI"} diff --git a/app/helpers/user_applications_helper.rb b/app/helpers/user_applications_helper.rb index 6fe2a7f57..190c43580 100644 --- a/app/helpers/user_applications_helper.rb +++ b/app/helpers/user_applications_helper.rb @@ -1,7 +1,7 @@ module UserApplicationsHelper def user_application_name(app) if app.name? - "#{app.name} (#{link_to(app.url, app.url)})" + "#{html_escape app.name} (#{link_to(app.url, app.url)})" else link_to(app.url, app.url) end diff --git a/app/presenters/user_application_presenter.rb b/app/presenters/user_application_presenter.rb index 8a0015622..7930d0808 100644 --- a/app/presenters/user_application_presenter.rb +++ b/app/presenters/user_application_presenter.rb @@ -14,19 +14,31 @@ class UserApplicationPresenter end def name - CGI::escape @app.client_name + @app.client_name end def image @app.image_uri end + def terms_of_services + @app.tos_uri + end + + def policy + @app.policy_uri + end + def name? - if @app.client_name - true - else - false - end + @app.client_name.present? + end + + def terms_of_services? + @app.tos_uri.present? + end + + def policy? + @app.policy_uri.present? end def url diff --git a/app/views/api/openid_connect/authorizations/_grants_list.haml b/app/views/api/openid_connect/authorizations/_grants_list.haml index 4f858457a..60cbbe4dd 100644 --- a/app/views/api/openid_connect/authorizations/_grants_list.haml +++ b/app/views/api/openid_connect/authorizations/_grants_list.haml @@ -15,3 +15,17 @@ - else .well = t("api.openid_connect.authorizations.new.no_requirement", name: user_application_name(app)).html_safe + + .small-horizontal-spacer + .application-tos-policy + - if app.terms_of_services? + %b= link_to t("api.openid_connect.user_applications.tos"), app.terms_of_services + + - if app.policy? && app.terms_of_services? + | + + - if app.policy? + %b= link_to t("api.openid_connect.user_applications.policy"), app.policy + + - if app.policy? || app.terms_of_services? + .small-horizontal-spacer diff --git a/app/views/api/openid_connect/user_applications/_grants_list.haml b/app/views/api/openid_connect/user_applications/_grants_list.haml index eb874053c..1ef5d12dc 100644 --- a/app/views/api/openid_connect/user_applications/_grants_list.haml +++ b/app/views/api/openid_connect/user_applications/_grants_list.haml @@ -14,3 +14,17 @@ - else .well = t("api.openid_connect.user_applications.index.no_requirement", name: user_application_name(app)).html_safe + + .small-horizontal-spacer + .application-tos-policy + - if app.terms_of_services? + %b= link_to t("api.openid_connect.user_applications.tos"), app.terms_of_services + + - if app.policy? && app.terms_of_services? + | + + - if app.policy? + %b= link_to t("api.openid_connect.user_applications.policy"), app.policy + + - if app.policy? || app.terms_of_services? + .small-horizontal-spacer diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index 3539d69fd..48bc58a44 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -900,6 +900,8 @@ en: no_requirement: "%{name} requires no permissions" no_applications: "You have no authorized applications" revoke_autorization: "Revoke" + tos: "See the application's ToS" + policy: "See the application's privacy policy" scopes: openid: name: "basic profile" diff --git a/db/schema.rb b/db/schema.rb index a7232a417..d9e539bc8 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150828132451) do +ActiveRecord::Schema.define(version: 20151003142048) do create_table "account_deletions", force: :cascade do |t| t.string "diaspora_handle", limit: 255 diff --git a/spec/controllers/api/openid_connect/authorizations_controller_spec.rb b/spec/controllers/api/openid_connect/authorizations_controller_spec.rb index 7527bacce..a871996d3 100644 --- a/spec/controllers/api/openid_connect/authorizations_controller_spec.rb +++ b/spec/controllers/api/openid_connect/authorizations_controller_spec.rb @@ -2,6 +2,7 @@ require "spec_helper" describe Api::OpenidConnect::AuthorizationsController, type: :controller do let!(:client) { FactoryGirl.create(:o_auth_application) } + let!(:client_with_xss) { FactoryGirl.create(:o_auth_application_with_xss) } let!(:client_with_multiple_redirects) { FactoryGirl.create(:o_auth_application_with_multiple_redirects) } let!(:auth_with_read) { FactoryGirl.create(:auth_with_read) } @@ -129,7 +130,16 @@ describe Api::OpenidConnect::AuthorizationsController, type: :controller do expect(json_body["error"]).to match("bad_request") end end + + context "when XSS script is passed as name" do + it "should escape html" do + post :new, client_id: client_with_xss.client_id, redirect_uri: "http://localhost:3000/", + response_type: "id_token", scope: "openid", nonce: SecureRandom.hex(16), state: SecureRandom.hex(16) + expect(response.body).to_not include("") + end + end end + context "when already authorized" do let!(:auth) { Api::OpenidConnect::Authorization.find_or_create_by(o_auth_application: client, user: alice,