From c7b932148807c191099ba37c9e694ded5ed5187f Mon Sep 17 00:00:00 2001 From: Andreas Maierhofer Date: Wed, 22 May 2024 16:58:32 +0200 Subject: [PATCH] Add additional oidc claims, (#428) --- app/domain/sac_cas/oidc_claim_setup.rb | 76 ++++++++ config/locales/wagon.de.yml | 4 + lib/hitobito_sac_cas/wagon.rb | 23 +-- .../oauth/userinfo_controller_spec.rb | 41 +++-- spec/domain/oidc_claim_setup_spec.rb | 170 ++++++++++++++++++ spec/requests/oauth_profile_spec.rb | 151 +++++++++------- 6 files changed, 370 insertions(+), 95 deletions(-) create mode 100644 app/domain/sac_cas/oidc_claim_setup.rb create mode 100644 spec/domain/oidc_claim_setup_spec.rb diff --git a/app/domain/sac_cas/oidc_claim_setup.rb b/app/domain/sac_cas/oidc_claim_setup.rb new file mode 100644 index 000000000..2c3577d2d --- /dev/null +++ b/app/domain/sac_cas/oidc_claim_setup.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +# Copyright (c) 2024, Schweizer Alpen-Club. This file is part of +# hitobito_sac_cas and licensed under the Affero General Public License version 3 +# or later. See the COPYING file at the top-level directory or at +# https://github.com/hitobito/hitobito_sac_cas + +module SacCas::OidcClaimSetup + + extend ActiveSupport::Concern + + PHONE_NUMBER_LABEL = 'Haupt-Telefonnummer'.freeze + INFERRED_ROLE_LABLES = { + section_functionary: Group::SektionsFunktionaere.roles, + section_president: [Group::SektionsFunktionaere::Praesidium], + SAC_employee: Group::Geschaeftsstelle.roles, + SAC_management: Group::Geschaeftsleitung.roles, + SAC_member: [Group::SektionsMitglieder::Mitglied], + SAC_member_additional: [Group::SektionsMitglieder::MitgliedZusatzsektion], + SAC_central_board_member: Group::Zentralvorstand.roles, + SAC_commission_member: Group::Kommission.roles, + SAC_tourenportal_subscriber: Group::AboTourenPortal.roles, + section_commission_member: Group::SektionsKommission.roles, + huts_functionary: Group::SektionsHuettenkommission.roles, + tourenportal_author: [Group::AboTourenPortal::Autor], + tourenportal_community: [Group::AboTourenPortal::Community], + tourenportal_administrator: [Group::AboTourenPortal::Admin], + magazin_subscriber: Group::AboMagazin.roles, + section_tour_functionary: Group::SektionsTourenkommission.roles, + } + + def run + super + + add_claim(:picture_url, scope: [:name, :with_roles]) do |owner| + owner.decorate.picture_full_url + end + + add_claim(:phone_number, scope: [:name, :with_roles]) do |owner| + phone_number(owner) + end + + add_claim(:membership_years, scope: :with_roles) + add_claim(:user_groups, scope: :with_groups) do |owner| + inferred_role_strings(owner) + formatted_active_roles(owner) + end + end + + private + + def inferred_role_strings(owner) + INFERRED_ROLE_LABLES.select do |_, roles| + (owner.roles.map(&:class) & roles).any? + end.keys.map(&:to_s) + end + + def formatted_active_roles(owner) + owner.roles.map { |r| "#{r.type}##{r.group_id}" } + end + + def phone_number(owner) + owner.phone_numbers.order(:id).find_by(label: PHONE_NUMBER_LABEL)&.number + end + + def membership_years(owner) + Person.with_membership_years.find_by(id: owner.id).membership_years + end + + def section_functionary(owner) + owner.roles.any? + end + + def section_president(owner) + owner.roles.any? { |r| } + end +end diff --git a/config/locales/wagon.de.yml b/config/locales/wagon.de.yml index 2b0afd1f4..467381913 100644 --- a/config/locales/wagon.de.yml +++ b/config/locales/wagon.de.yml @@ -598,6 +598,10 @@ de: form: login_identity: Haupt‑E‑Mail / Mitglied‑Nr + doorkeeper: + scopes: + with_groups: Lesen deiner Gruppen, Rollen und berechneter Felder. + dropdown/people_export: recipients: Empfänger diff --git a/lib/hitobito_sac_cas/wagon.rb b/lib/hitobito_sac_cas/wagon.rb index 5c4cdb011..3171e32a6 100644 --- a/lib/hitobito_sac_cas/wagon.rb +++ b/lib/hitobito_sac_cas/wagon.rb @@ -80,6 +80,7 @@ class Wagon < Rails::Engine Event::ParticipationDecorator.prepend SacCas::Event::ParticipationDecorator ## Domain + OidcClaimSetup.prepend SacCas::OidcClaimSetup SearchStrategies::SqlConditionBuilder.matchers.merge!( 'people.id' => SearchStrategies::SqlConditionBuilder::IdMatcher, 'people.birthday' => SearchStrategies::SqlConditionBuilder::BirthdayMatcher @@ -175,26 +176,8 @@ class Wagon < Rails::Engine end end - initializer 'sac_cas.add_oidc_claims' do |_app| - Doorkeeper::OpenidConnect.configuration.claims[:picture_url] = - Doorkeeper::OpenidConnect::Claims::NormalClaim.new( - name: :picture_url, - scope: :name, - response: [:user_info], - generator: proc do |resource_owner| - resource_owner.decorate.picture_full_url - end - ) - - Doorkeeper::OpenidConnect.configuration.claims[:with_roles_picture_url] = - Doorkeeper::OpenidConnect::Claims::NormalClaim.new( - name: :picture_url, - scope: :with_roles, - response: [:user_info], - generator: proc do |resource_owner| - resource_owner.decorate.picture_full_url - end - ) + initializer 'sac_cas.append_doorkeeper_scope' do |_app| + Doorkeeper.configuration.scopes.add "with_groups" end private diff --git a/spec/controllers/oauth/userinfo_controller_spec.rb b/spec/controllers/oauth/userinfo_controller_spec.rb index f9acd5d7a..bb8165c91 100644 --- a/spec/controllers/oauth/userinfo_controller_spec.rb +++ b/spec/controllers/oauth/userinfo_controller_spec.rb @@ -11,6 +11,7 @@ let(:user) { people(:admin) } let(:app) { Oauth::Application.create!(name: 'MyApp', redirect_uri: redirect_uri) } let(:redirect_uri) { 'urn:ietf:wg:oauth:2.0:oob' } + let(:data) { JSON.parse(response.body) } describe 'GET#show' do context 'with name scope' do @@ -22,17 +23,18 @@ it 'shows the userinfo' do get :show, params: { access_token: token.token } expect(response.status).to eq 200 - expect(JSON.parse(response.body)).to match({ - sub: user.id.to_s, - first_name: user.first_name, - last_name: user.last_name, - nickname: user.nickname, - address: user.address, - zip_code: user.zip_code, - town: user.town, - country: user.country, - picture_url: /\/packs-test\/media\/images\/profile-.*\.svg/, - }.deep_stringify_keys) + expect(data).to match({ + sub: user.id.to_s, + first_name: user.first_name, + last_name: user.last_name, + nickname: user.nickname, + address: user.address, + zip_code: user.zip_code, + town: user.town, + country: user.country, + phone_number: nil, + picture_url: /\/packs-test\/media\/images\/profile-.*\.svg/, + }.deep_stringify_keys) end end @@ -45,7 +47,7 @@ it 'shows the userinfo' do get :show, params: { access_token: token.token } expect(response.status).to eq 200 - expect(JSON.parse(response.body)).to match({ + expect(data).to match({ sub: user.id.to_s, first_name: user.first_name, last_name: user.last_name, @@ -61,6 +63,8 @@ birthday: user.birthday.to_s.presence, primary_group_id: user.primary_group_id, language: user.language, + phone_number: nil, + membership_years: 0, picture_url: /\/packs-test\/media\/images\/profile-.*\.svg/, roles: [ { @@ -75,6 +79,19 @@ ] }.deep_stringify_keys) end + + end + context 'with with_groups scope' do + let(:token) do + app.access_tokens.create!(resource_owner_id: user.id, + scopes: 'openid with_groups', expires_in: 2.hours) + end + + it 'has user_groups key' do + get :show, params: { access_token: token.token } + expect(response.status).to eq 200 + expect(data['user_groups']).to include 'SAC_employee' + end end end end diff --git a/spec/domain/oidc_claim_setup_spec.rb b/spec/domain/oidc_claim_setup_spec.rb new file mode 100644 index 000000000..71fbdb021 --- /dev/null +++ b/spec/domain/oidc_claim_setup_spec.rb @@ -0,0 +1,170 @@ +# frozen_string_literal: true + +# Copyright (c) 2024, Schweizer Alpen-Club. This file is part of +# hitobito_sac_cas and licensed under the Affero General Public License version 3 +# or later. See the COPYING file at the top-level directory or at +# https://github.com/hitobito/hitobito_sac_cas + +require 'spec_helper' + +describe OidcClaimSetup do + let(:owner) { people(:admin) } + let(:response) { :user_info } + let(:token) { Doorkeeper::AccessToken.new(resource_owner_id: owner.id, scopes: scope) } + let(:claim_keys) { claims.stringify_keys.keys } + + subject(:claims) { Doorkeeper::OpenidConnect::ClaimsBuilder.generate(token, response) } + + shared_examples 'shared claims' do + describe 'phone_number' do + it 'is blank when no matching number exists' do + expect(claim_keys).to include('phone_number') + end + + it 'returns first number with matching label' do + owner.phone_numbers.create!(label: 'Haupt-Telefonnummer', number: '0791234560') + owner.phone_numbers.create!(label: 'Haupt-Telefonnummer', number: '0791234561') + expect(claims[:phone_number]).to eq '+41 79 123 45 60' + end + end + end + + context 'name' do + let(:scope) { :name } + + it 'picture_url is present' do + expect(claims[:picture_url]).to eq owner.decorate.picture_full_url + end + + it_behaves_like 'shared claims' + end + + context 'with_roles' do + let(:scope) { :with_roles } + + describe 'membership_years'do + it 'is blank when no matching number exists' do + expect(claim_keys).to include('membership_years') + end + + it 'is blank when no matching number exists' do + expect(claims[:membership_years]).to eq 0 + end + end + it_behaves_like 'shared claims' + end + + context 'with_groups' do + let(:role) { roles(:admin) } + let(:scope) { :with_groups } + let(:user_groups) { claims[:user_groups] } + + def create_role(key, role) + group = key.is_a?(Group) ? key : groups(key) + role_type = group.class.const_get(role) + Fabricate(role_type.sti_name, group: group, person: owner) + end + + it 'includes SAC_employee key when matching role exists' do + expect(user_groups).to include 'SAC_employee' + expect(user_groups).to include 'Group::Geschaeftsstelle::Admin#384133472' + end + + it 'includes section_functionary key when matching role exists' do + role = create_role(:bluemlisalp_funktionaere, 'AdministrationReadOnly') + expect(user_groups).to include "section_functionary" + expect(user_groups).to include "Group::Geschaeftsstelle::Admin#384133472" + expect(user_groups).to include format('%s#%d' % [role.type, role.group_id]) + end + + it 'includes section_president key when matching role exists' do + role = create_role(:bluemlisalp_funktionaere, 'Praesidium') + expect(user_groups).to include 'section_president' + end + + it 'includes SAC_management key when matching role exists' do + group = Fabricate(Group::Geschaeftsleitung.sti_name, parent: groups(:root)) + + role = create_role(group, 'Ressortleitung') + expect(user_groups).to include 'SAC_management' + end + + it 'includes SAC_member key when matching role exists' do + role = create_role(:bluemlisalp_mitglieder, 'Mitglied') + expect(user_groups).to include 'SAC_member' + expect(user_groups).not_to include 'SAC_member_additional' + end + + it 'includes SAC_member key when matching role exists' do + create_role(:bluemlisalp_mitglieder, 'Mitglied') + role = create_role(:matterhorn_mitglieder, 'MitgliedZusatzsektion') + expect(user_groups).to include 'SAC_member' + expect(user_groups).to include 'SAC_member_additional' + end + + it 'includes SAC_central_board_member key when matching role exists' do + group = Fabricate(Group::Zentralvorstand.sti_name, parent: groups(:root)) + create_role(group, 'Praesidium') + expect(user_groups).to include 'SAC_central_board_member' + end + + it 'includes SAC_central_board_member key when matching role exists' do + group = Fabricate(Group::Kommission.sti_name, parent: groups(:root)) + create_role(group, 'Praesidium') + expect(user_groups).to include 'SAC_commission_member' + end + + it 'includes SAC_tourenportal_subscriber key when matching role exists' do + group = Fabricate(Group::AboTourenPortal.sti_name, parent: groups(:abonnenten)) + create_role(group, 'Autor') + expect(user_groups).to include 'SAC_tourenportal_subscriber' + end + + it 'includes section_commission_member key when matching role exists' do + group = Fabricate(Group::SektionsKommission.sti_name, parent: groups(:bluemlisalp)) + create_role(group, 'Mitglied') + expect(user_groups).to include 'section_commission_member' + end + + it 'includes huts_functionary key when matching role exists' do + group = Fabricate(Group::SektionsHuettenkommission.sti_name, parent: groups(:bluemlisalp)) + create_role(group, 'Huettenobmann') + expect(user_groups).to include 'huts_functionary' + end + + it 'includes tourenportal_author key when matching role exists' do + group = Fabricate(Group::AboTourenPortal.sti_name, parent: groups(:abonnenten)) + create_role(group, 'Autor') + expect(user_groups).to include 'tourenportal_author' + end + + it 'includes tourenportal_community key when matching role exists' do + group = Fabricate(Group::AboTourenPortal.sti_name, parent: groups(:abonnenten)) + create_role(group, 'Community') + expect(user_groups).to include 'tourenportal_community' + end + + it 'includes tourenportal_community key when matching role exists' do + group = Fabricate(Group::AboTourenPortal.sti_name, parent: groups(:abonnenten)) + create_role(group, 'Community') + expect(user_groups).to include 'tourenportal_community' + end + + it 'includes tourenportal_administrator key when matching role exists' do + group = Fabricate(Group::AboTourenPortal.sti_name, parent: groups(:abonnenten)) + create_role(group, 'Admin') + expect(user_groups).to include 'tourenportal_administrator' + end + + it 'includes magazin_subscriber key when matching role exists' do + group = Fabricate(Group::AboMagazin.sti_name, parent: groups(:abonnenten)) + create_role(group, 'Andere') + expect(user_groups).to include 'magazin_subscriber' + end + + it 'includes section_tour_functionary key when matching role exists' do + create_role(:bluemlisalp_ortsgruppe_ausserberg_tourenkommission, 'JoChef') + expect(user_groups).to include 'section_tour_functionary' + end + end +end diff --git a/spec/requests/oauth_profile_spec.rb b/spec/requests/oauth_profile_spec.rb index 00c261f61..52ffeca9f 100644 --- a/spec/requests/oauth_profile_spec.rb +++ b/spec/requests/oauth_profile_spec.rb @@ -10,75 +10,100 @@ RSpec.describe 'GET oauth/profile', type: :request do let(:application) { Fabricate(:application) } let(:user) { people(:mitglied) } + let(:json) { JSON.parse(response.body) } + let(:token) { + Fabricate(:access_token, application: application, scopes: "name #{scope}", + resource_owner_id: user.id ) } - context 'with all scopes in token' do - let(:token) { Fabricate(:access_token, application: application, scopes: 'email name with_roles', resource_owner_id: user.id ) } - context 'with scope "name" in request' do - it 'succeeds' do - get '/oauth/profile', headers: { 'Authorization': 'Bearer ' + token.token, 'X-Scope': 'name' } - expect(response).to have_http_status(:ok) - expect(response.content_type).to eq('application/json; charset=utf-8') - json = JSON.parse(response.body) - expect(json).to match({ - id: user.id, - email: user.email, - first_name: user.first_name, - last_name: user.last_name, - nickname: user.nickname, - address: user.address, - zip_code: user.zip_code, - town: user.town, - country: user.country, - picture_url: /\/packs-test\/media\/images\/profile-.*\.svg/, - }.deep_stringify_keys) - end + def make_request(skip_checks: true) + get '/oauth/profile', headers: { 'Authorization': 'Bearer ' + token.token, 'X-Scope': scope } + return if skip_checks + + expect(response).to have_http_status(:ok) + expect(response.content_type).to eq('application/json; charset=utf-8') + end + + context 'with scope "name" in request' do + let(:scope) { :name } + + it 'succeeds' do + make_request + expect(json).to match({ + id: user.id, + email: user.email, + first_name: user.first_name, + last_name: user.last_name, + nickname: user.nickname, + address: user.address, + zip_code: user.zip_code, + town: user.town, + country: user.country, + picture_url: /\/packs-test\/media\/images\/profile-.*\.svg/, + phone_number: nil, + }.deep_stringify_keys) + end + end + + context 'with scope "with_roles" in request' do + let(:scope) { 'with_roles' } + + it 'succeeds' do + make_request + expect(json).to match({ + id: user.id, + first_name: user.first_name, + last_name: user.last_name, + nickname: user.nickname, + company_name: user.company_name, + company: user.company, + email: user.email, + address: user.address, + zip_code: user.zip_code, + town: user.town, + country: user.country, + gender: user.gender, + birthday: user.birthday.to_s.presence, + primary_group_id: user.primary_group_id, + language: user.language, + picture_url: /\/packs-test\/media\/images\/profile-.*\.svg/, + phone_number: nil, + membership_years: 1, + roles: [{ + group_id: user.roles.first.group_id, + group_name: user.roles.first.group.name, + role: 'Group::SektionsMitglieder::Mitglied', + role_class: 'Group::SektionsMitglieder::Mitglied', + role_name: 'Mitglied (Stammsektion)', + permissions: [], + layer_group_id: user.roles.first.group.layer_group_id + },{ + group_id: user.roles.second.group_id, + group_name: user.roles.second.group.name, + role: 'Group::SektionsMitglieder::MitgliedZusatzsektion', + role_class: 'Group::SektionsMitglieder::MitgliedZusatzsektion', + role_name: 'Mitglied (Zusatzsektion)', + permissions: [], + layer_group_id: user.roles.second.group.layer_group_id + }] + }.deep_stringify_keys) end + end - context 'with scope "with_roles" in request' do - it 'succeeds' do - get '/oauth/profile', headers: { 'Authorization': 'Bearer ' + token.token, 'X-Scope': 'with_roles' } + context 'with scope "with_groups" in request' do + let(:scope) { 'with_groups' } - expect(response).to have_http_status(:ok) - expect(response.content_type).to eq('application/json; charset=utf-8') - json = JSON.parse(response.body) - expect(json).to match({ - id: user.id, - first_name: user.first_name, - last_name: user.last_name, - nickname: user.nickname, - company_name: user.company_name, - company: user.company, - email: user.email, - address: user.address, - zip_code: user.zip_code, - town: user.town, - country: user.country, - gender: user.gender, - birthday: user.birthday.to_s.presence, - primary_group_id: user.primary_group_id, - language: user.language, - picture_url: /\/packs-test\/media\/images\/profile-.*\.svg/, - roles: [{ - group_id: user.roles.first.group_id, - group_name: user.roles.first.group.name, - role: 'Group::SektionsMitglieder::Mitglied', - role_class: 'Group::SektionsMitglieder::Mitglied', - role_name: 'Mitglied (Stammsektion)', - permissions: [], - layer_group_id: user.roles.first.group.layer_group_id - },{ - group_id: user.roles.second.group_id, - group_name: user.roles.second.group.name, - role: 'Group::SektionsMitglieder::MitgliedZusatzsektion', - role_class: 'Group::SektionsMitglieder::MitgliedZusatzsektion', - role_name: 'Mitglied (Zusatzsektion)', - permissions: [], - layer_group_id: user.roles.second.group.layer_group_id - }] - }.deep_stringify_keys) - end + it 'succeeds' do + make_request + expect(json['user_groups']).to include 'SAC_member' + end + + it 'is forbidden to be used without names scope on token' do + token.update!(scopes: 'with_groups') + make_request + expect(response).to have_http_status(:forbidden) end end + end