From 07c12ba0579f05fe481b1c12310dcb0521840e25 Mon Sep 17 00:00:00 2001 From: augier Date: Sun, 2 Aug 2015 12:48:41 +0200 Subject: [PATCH] Using Camo for the application logo --- app/assets/stylesheets/mobile/settings.scss | 16 ++++++------ app/assets/stylesheets/user_applications.scss | 16 ++++++------ .../authorizations_controller.rb | 2 +- .../api/openid_connect/o_auth_application.rb | 4 +++ app/presenters/user_applications_presenter.rb | 2 +- .../_add_remove_applications.haml | 4 +-- features/desktop/user_applications.feature | 24 ++++++++++++++++++ features/mobile/user_applications.feature | 25 +++++++++++++++++++ .../step_definitions/oidc_common_steps.rb | 11 ++++++++ .../user_applications_steps.rb | 16 ++++++++++++ features/support/paths.rb | 2 ++ spec/factories.rb | 6 +++++ 12 files changed, 106 insertions(+), 22 deletions(-) create mode 100644 features/desktop/user_applications.feature create mode 100644 features/mobile/user_applications.feature create mode 100644 features/step_definitions/user_applications_steps.rb diff --git a/app/assets/stylesheets/mobile/settings.scss b/app/assets/stylesheets/mobile/settings.scss index c45814a7d..9b614aa66 100644 --- a/app/assets/stylesheets/mobile/settings.scss +++ b/app/assets/stylesheets/mobile/settings.scss @@ -36,19 +36,17 @@ } .application-img { - display: inline; - padding: 9px 0; + margin: 9px 0; float: left; + width: 60px; + max-height: 60px; text-align: center; - &, img { - width: 70px; - max-height: 70px; - } - [class^="entypo-"] { font-size: 60px; - height: 0; + height: 60px; + margin: 0; + padding: 0; width: 100%; &::before { position: relative; @@ -58,7 +56,7 @@ } .application-authorizations { - width: calc(100% - 70px); + width: calc(100% - 60px); padding: 0 0 15px 15px; display: inline-block; float: right; diff --git a/app/assets/stylesheets/user_applications.scss b/app/assets/stylesheets/user_applications.scss index 1290a5abd..c99ed3672 100644 --- a/app/assets/stylesheets/user_applications.scss +++ b/app/assets/stylesheets/user_applications.scss @@ -4,19 +4,17 @@ } .application-img { - display: inline; - padding: 9px 0; + margin: 9px 0; float: left; + width: 60px; + max-height: 60px; text-align: center; - &, img { - width: 70px; - max-height: 70px; - } - [class^="entypo-"] { font-size: 60px; - height: 0; + height: 60px; + margin: 0; + padding: 0; width: 100%; &::before { position: relative; @@ -26,7 +24,7 @@ } .application-authorizations { - width: calc(100% - 70px); + width: calc(100% - 60px); padding: 0 0 15px 15px; display: inline-block; float: right; diff --git a/app/controllers/api/openid_connect/authorizations_controller.rb b/app/controllers/api/openid_connect/authorizations_controller.rb index 3a57a0832..706062744 100644 --- a/app/controllers/api/openid_connect/authorizations_controller.rb +++ b/app/controllers/api/openid_connect/authorizations_controller.rb @@ -22,7 +22,7 @@ module Api begin Api::OpenidConnect::Authorization.find_by(id: params[:id]).destroy rescue - # TODO: Log something here? + logger.error "Error while trying revoke inexistant authorization #{params[:id]}" end redirect_to user_applications_url end diff --git a/app/models/api/openid_connect/o_auth_application.rb b/app/models/api/openid_connect/o_auth_application.rb index 5ecf8ea69..58a03b3ca 100644 --- a/app/models/api/openid_connect/o_auth_application.rb +++ b/app/models/api/openid_connect/o_auth_application.rb @@ -19,6 +19,10 @@ module Api self.client_secret = SecureRandom.hex(32) end + def image_uri + logo_uri ? Diaspora::Camo.image_url(logo_uri) : nil + end + class << self def available_response_types ["id_token", "id_token token", "code"] diff --git a/app/presenters/user_applications_presenter.rb b/app/presenters/user_applications_presenter.rb index 5d400822d..5a0ad9cfc 100644 --- a/app/presenters/user_applications_presenter.rb +++ b/app/presenters/user_applications_presenter.rb @@ -23,7 +23,7 @@ class UserApplicationsPresenter { id: find_id(application), name: application.client_name, - image: application.logo_uri, + image: application.image_uri, authorizations: find_scopes(application) } end diff --git a/app/views/user_applications/_add_remove_applications.haml b/app/views/user_applications/_add_remove_applications.haml index db71c35c9..9ca522541 100644 --- a/app/views/user_applications/_add_remove_applications.haml +++ b/app/views/user_applications/_add_remove_applications.haml @@ -1,7 +1,7 @@ - if @user_apps.applications? %ul.list-group - @user_apps.user_applications.each do |app| - %li.list-group-item + %li.list-group-item.authorized-application .application-img - if app[:image] = image_tag app[:image], class: "img-responsive" @@ -20,7 +20,7 @@ =t("user_applications.show.no_requirement") = form_for "application", url: "#{api_openid_connect_authorizations_path}/#{app[:id]}", html: { method: :delete, class: "form-horizontal"} do |f| - .clearfix= f.submit t("user_applications.revoke_autorization"), class: "btn btn-primary pull-right" + .clearfix= f.submit t("user_applications.revoke_autorization"), class: "btn btn-danger pull-right app-revoke" - else .well diff --git a/features/desktop/user_applications.feature b/features/desktop/user_applications.feature new file mode 100644 index 000000000..99a2f8843 --- /dev/null +++ b/features/desktop/user_applications.feature @@ -0,0 +1,24 @@ +@javascript +Feature: managing authorized applications + Background: + Given following users exist: + | username | email | + | Augier | augier@example.org | + And all scopes exist + And a client with a provided picture exists for user "augier@example.org" + And a client exists for user "augier@example.org" + + Scenario: displaying authorizations + When I sign in as "augier@example.org" + And I go to the user applications page + Then I should see 2 authorized applications + And I should see 1 authorized applications with no provided image + And I should see 1 authorized applications with an image + + Scenario: revoke an authorization + When I sign in as "augier@example.org" + And I go to the user applications page + And I revoke the first authorization + Then I should see 1 authorized applications + And I revoke the first authorization + Then I should see 0 authorized applications diff --git a/features/mobile/user_applications.feature b/features/mobile/user_applications.feature new file mode 100644 index 000000000..18895e3ce --- /dev/null +++ b/features/mobile/user_applications.feature @@ -0,0 +1,25 @@ +@mobile +@javascript +Feature: managing authorized applications + Background: + Given following users exist: + | username | email | + | Augier | augier@example.org | + And all scopes exist + And a client with a provided picture exists for user "augier@example.org" + And a client exists for user "augier@example.org" + + Scenario: displaying authorizations + When I sign in as "augier@example.org" + And I go to the user applications page + Then I should see 2 authorized applications + And I should see 1 authorized applications with no provided image + And I should see 1 authorized applications with an image + + Scenario: revoke an authorization + When I sign in as "augier@example.org" + And I go to the user applications page + And I revoke the first authorization + Then I should see 1 authorized applications + And I revoke the first authorization + Then I should see 0 authorized applications diff --git a/features/step_definitions/oidc_common_steps.rb b/features/step_definitions/oidc_common_steps.rb index bcf8be96e..e9a812d41 100644 --- a/features/step_definitions/oidc_common_steps.rb +++ b/features/step_definitions/oidc_common_steps.rb @@ -3,6 +3,17 @@ Given(/^all scopes exist$/) do Api::OpenidConnect::Scope.find_or_create_by(name: "read") end +Given /^a client with a provided picture exists for user "([^\"]*)"$/ do |email| + app = FactoryGirl.create(:o_auth_application_with_image) + user = User.find_by(email: email) + FactoryGirl.create(:auth_with_read, user: user, o_auth_application: app) +end + +Given /^a client exists for user "([^\"]*)"$/ do |email| + user = User.find_by(email: email) + FactoryGirl.create(:auth_with_read, user: user) +end + When /^I register a new client$/ do post api_openid_connect_clients_path, redirect_uris: ["http://localhost:3000"], client_name: "diaspora client" end diff --git a/features/step_definitions/user_applications_steps.rb b/features/step_definitions/user_applications_steps.rb new file mode 100644 index 000000000..7cef79050 --- /dev/null +++ b/features/step_definitions/user_applications_steps.rb @@ -0,0 +1,16 @@ +Then /^I should see (\d+) authorized applications$/ do |num| + expect(page).to have_selector(".applications-page", count: 1) + expect(page).to have_selector(".authorized-application", count: num.to_i) +end + +Then /^I should see (\d+) authorized applications with no provided image$/ do |num| + expect(page).to have_selector(".application-img > .entypo-browser", count: num.to_i) +end + +Then /^I should see (\d+) authorized applications with an image$/ do |num| + expect(page).to have_selector(".application-img > .img-responsive", count: num.to_i) +end + +When /^I revoke the first authorization$/ do + find(".app-revoke", match: :first).click +end diff --git a/features/support/paths.rb b/features/support/paths.rb index 1d512dc43..19ada8e40 100644 --- a/features/support/paths.rb +++ b/features/support/paths.rb @@ -36,6 +36,8 @@ module NavigationHelpers edit_user_path when /^forgot password page$/ new_user_password_path + when /^user applications page$/ + user_applications_path when %r{^"(/.*)"} Regexp.last_match(1) else diff --git a/spec/factories.rb b/spec/factories.rb index 946985a95..ce54a4aa4 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -315,6 +315,12 @@ FactoryGirl.define do redirect_uris ["http://localhost:3000/"] end + factory :o_auth_application_with_image, class: Api::OpenidConnect::OAuthApplication do + client_name "Diaspora Test Client" + redirect_uris ["http://localhost:3000/"] + logo_uri "/assets/user/default.png" + end + factory :o_auth_application_with_ppid, class: Api::OpenidConnect::OAuthApplication do client_name "Diaspora Test Client" redirect_uris ["http://localhost:3000/"]