From d08b31f2eddeb54cb42157d9c82114289115cb79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonne=20Ha=C3=9F?= Date: Sun, 28 Apr 2019 17:00:16 +0200 Subject: [PATCH] OpenID: remove private profile data claims that are not returned anyway and fix return values for profile and picture --- app/models/api/openid_connect/authorization.rb | 5 ----- app/serializers/user_info_serializer.rb | 4 ++-- config/locales/diaspora/en.yml | 12 ------------ features/step_definitions/oidc_common_steps.rb | 2 +- .../openid_connect/authorization_point/endpoint.rb | 1 - .../endpoint_confirmation_point.rb | 4 ---- .../authorization_point/endpoint_start_point.rb | 6 ------ spec/factories.rb | 4 ++-- spec/integration/api/user_info_controller_spec.rb | 4 +++- 9 files changed, 8 insertions(+), 34 deletions(-) diff --git a/app/models/api/openid_connect/authorization.rb b/app/models/api/openid_connect/authorization.rb index 6a89d63aa..1391fc4d3 100644 --- a/app/models/api/openid_connect/authorization.rb +++ b/app/models/api/openid_connect/authorization.rb @@ -19,14 +19,11 @@ module Api scope :with_redirect_uri, ->(given_uri) { where redirect_uri: given_uri } SCOPES = %w[ - birthdate contacts:modify contacts:read conversations email - gender interactions - locale name nickname notifications @@ -35,14 +32,12 @@ module Api private:modify private:read profile - profile profile:modify public:modify public:read sub tags:modify tags:read - updated_at ].freeze def setup diff --git a/app/serializers/user_info_serializer.rb b/app/serializers/user_info_serializer.rb index 7410d60cf..965e6a47d 100644 --- a/app/serializers/user_info_serializer.rb +++ b/app/serializers/user_info_serializer.rb @@ -17,10 +17,10 @@ class UserInfoSerializer < ActiveModel::Serializer end def profile - File.join(AppConfig.environment.url, "people", object.guid).to_s + api_v1_user_url end def picture - File.join(AppConfig.environment.url, object.image_url).to_s + object.image_url(fallback_to_default: false) end end diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index de863855c..1815700f0 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -937,18 +937,6 @@ en: picture: name: "picture" description: "This grants read access to user's profile picture data to the application." - gender: - name: "gender" - description: "This grants read access to the user's gendere data to the application." - birthdate: - name: "birthdate" - description: "This grants read access ot the user's birthdate to the application." - locale: - name: "locale" - description: "This grants read access ot the user's locale to the application." - updated_at: - name: "updated_at" - description: "This grants read access to the user's profile update time to the application." 'contacts:read': name: "contacts:read" description: "This grants read permissions to contacts and related data (like aspects) to the application." diff --git a/features/step_definitions/oidc_common_steps.rb b/features/step_definitions/oidc_common_steps.rb index 0952a0293..c95125374 100644 --- a/features/step_definitions/oidc_common_steps.rb +++ b/features/step_definitions/oidc_common_steps.rb @@ -28,7 +28,7 @@ Then /^I should receive "([^\"]*)"'s id, username, and email$/ do |username| user_info_json = JSON.parse(last_response.body) user = User.find_by_username(username) user_profile_url = File.join(AppConfig.environment.url, "people", user.guid).to_s - expect(user_info_json["profile"]).to have_content(user_profile_url) + expect(user_info_json["profile"]).to have_content(api_v1_user_path) end Then /^I should receive an "([^\"]*)" error$/ do |error_message| diff --git a/lib/api/openid_connect/authorization_point/endpoint.rb b/lib/api/openid_connect/authorization_point/endpoint.rb index ce108ce82..396149cf5 100644 --- a/lib/api/openid_connect/authorization_point/endpoint.rb +++ b/lib/api/openid_connect/authorization_point/endpoint.rb @@ -47,7 +47,6 @@ module Api end def build_scopes(req) - replace_profile_scope_with_specific_claims(req) @scopes = req.scope.map {|scope| scope.tap do |scope_name| req.invalid_scope! I18n.t("api.openid_connect.authorizations.new.unknown_scope") \ diff --git a/lib/api/openid_connect/authorization_point/endpoint_confirmation_point.rb b/lib/api/openid_connect/authorization_point/endpoint_confirmation_point.rb index 9c4eadeea..7cdd02143 100644 --- a/lib/api/openid_connect/authorization_point/endpoint_confirmation_point.rb +++ b/lib/api/openid_connect/authorization_point/endpoint_confirmation_point.rb @@ -21,10 +21,6 @@ module Api end end - def replace_profile_scope_with_specific_claims(_req) - # Empty - end - def build_from_request_object(_req) # Empty end diff --git a/lib/api/openid_connect/authorization_point/endpoint_start_point.rb b/lib/api/openid_connect/authorization_point/endpoint_start_point.rb index 4496eb53a..f0dd6a6dc 100644 --- a/lib/api/openid_connect/authorization_point/endpoint_start_point.rb +++ b/lib/api/openid_connect/authorization_point/endpoint_start_point.rb @@ -16,12 +16,6 @@ module Api @response_type = req.response_type end - def replace_profile_scope_with_specific_claims(req) - profile_claims = %w[sub name nickname profile picture gender birthdate locale updated_at] - scopes_as_claims = req.scope.flat_map {|scope| scope == "profile" ? profile_claims : [scope] }.uniq - req.update_param("scope", scopes_as_claims) - end - private def build_request_object(req) diff --git a/spec/factories.rb b/spec/factories.rb index 611ee1723..a9b7b4516 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -448,7 +448,7 @@ FactoryGirl.define do factory :auth_with_read_scopes, class: Api::OpenidConnect::Authorization do o_auth_application association :user, factory: :user_with_aspect - scopes %w[openid sub name nickname profile picture gender birthdate locale updated_at contacts:read conversations + scopes %w[openid sub name nickname profile picture contacts:read conversations email interactions notifications private:read public:read profile tags:read] after(:build) {|m| m.redirect_uri = m.o_auth_application.redirect_uris[0] @@ -458,7 +458,7 @@ FactoryGirl.define do factory :auth_with_read_scopes_not_private, class: Api::OpenidConnect::Authorization do o_auth_application association :user, factory: :user_with_aspect - scopes %w[openid sub name nickname profile picture gender birthdate locale updated_at contacts:read conversations + scopes %w[openid sub name nickname profile picture gender contacts:read conversations email interactions notifications public:read profile tags:read] after(:build) {|m| m.redirect_uri = m.o_auth_application.redirect_uris[0] diff --git a/spec/integration/api/user_info_controller_spec.rb b/spec/integration/api/user_info_controller_spec.rb index dcae99918..97834a05c 100644 --- a/spec/integration/api/user_info_controller_spec.rb +++ b/spec/integration/api/user_info_controller_spec.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true describe Api::OpenidConnect::UserInfoController do + include Rails.application.routes.url_helpers + let!(:auth_with_read_and_ppid) { FactoryGirl.create(:auth_with_profile_and_ppid) } @@ -19,7 +21,7 @@ describe Api::OpenidConnect::UserInfoController do @user.pairwise_pseudonymous_identifiers.find_or_create_by(identifier: "https://example.com/uri").guid expect(json_body["sub"]).to eq(expected_sub) expect(json_body["nickname"]).to eq(@user.name) - expect(json_body["profile"]).to eq(File.join(AppConfig.environment.url, "people", @user.guid).to_s) + expect(json_body["profile"]).to end_with(api_v1_user_path) end end end