From 7fb65e308cbf687ce35eeeb0e0eae8673cc29d86 Mon Sep 17 00:00:00 2001 From: Jakub Jirutka Date: Thu, 6 Apr 2023 00:54:56 +0200 Subject: [PATCH 1/2] Add support for synchronizing groups --- README.rdoc | 9 +++++++ app/controllers/oidc_controller.rb | 29 +++++++++++++++++++++++ app/models/oidc_session.rb | 10 ++++++++ app/views/settings/_redmine_oidc.html.erb | 8 +++++++ config/locales/de.yml | 3 +++ config/locales/en.yml | 3 +++ lib/redmine_oidc/settings.rb | 10 ++++++++ 7 files changed, 72 insertions(+) diff --git a/README.rdoc b/README.rdoc index 9c1f3e6..cba5cd8 100644 --- a/README.rdoc +++ b/README.rdoc @@ -87,6 +87,15 @@ admin_role:: +realm_access.roles+. Example: ROLES/REDMINE/ADMIN +groups_claim:: + The claim which provides the set of groups (optional). + + Example: +groups+ +group_names_pattern:: + A regex pattern to filter names of user groups that should be synchronized. + Default is +.*+ (match all). + + Example: +^(Red|Blue)$+ == Mapping users diff --git a/app/controllers/oidc_controller.rb b/app/controllers/oidc_controller.rb index ee6b586..ff7355f 100644 --- a/app/controllers/oidc_controller.rb +++ b/app/controllers/oidc_controller.rb @@ -86,6 +86,7 @@ def create_user user.activate user.random_password user.last_login_on = Time.now + update_groups(user) user.save ? successful_login(user) : unsuccessful_login(user) end @@ -93,9 +94,34 @@ def update_user(user) user.update(@oidc_session.user_attributes) user.activate user.update_last_login_on! + update_groups(user) user.save ? successful_login(user) : unsuccessful_login(user) end + def update_groups(user) + return unless settings.update_groups? + + current = user.groups.select { |group| settings.group_names_regexp.match?(group.name) } + + target = @oidc_session.groups.map do |name| + begin + Group.named(name).first_or_create!(name: name) + rescue ActiveRecord::RecordInvalid => e + logger.error "Failed to create group #{name}: #{e}" + end + end + + unless (added = target - current).empty? + logger.info "Adding user #{user.login} to groups: #{added.map(&:name).join(', ')}" + user.groups += added + end + + unless (removed = current - target).empty? + logger.info "Removing user #{user.login} from groups: #{removed.map(&:name).join(', ')}" + user.groups -= removed + end + end + def successful_login(user) logger.info "Successful authentication for '#{user.login}' from #{request.remote_ip} at #{Time.now.utc}" oidc_session = OidcSession.spawn(session) @@ -110,4 +136,7 @@ def unsuccessful_login(user) end end + def settings + @settings ||= RedmineOidc.settings + end end diff --git a/app/models/oidc_session.rb b/app/models/oidc_session.rb index 3c61cbc..a9320ac 100644 --- a/app/models/oidc_session.rb +++ b/app/models/oidc_session.rb @@ -103,6 +103,16 @@ def user_attributes } end + def groups + @groups ||= if settings.update_groups? + (decoded_id_token.raw_attributes[settings.groups_claim] || []) + .select { |name| settings.group_names_regexp.match?(name) } + .to_set + else + Set.new + end + end + def refresh_token_expiration_timestamp decoded_refresh_token['exp'] end diff --git a/app/views/settings/_redmine_oidc.html.erb b/app/views/settings/_redmine_oidc.html.erb index c77d19f..9eb7f93 100644 --- a/app/views/settings/_redmine_oidc.html.erb +++ b/app/views/settings/_redmine_oidc.html.erb @@ -38,6 +38,14 @@ <%= label_tag 'settings[admin_role]', l('oidc.settings.admin_role') %> <%= text_field_tag 'settings[admin_role]', oidc_settings.admin_role, size: 60 %>

+

+ <%= label_tag 'settings[groups_claim]', l('oidc.settings.groups_claim') %> + <%= text_field_tag 'settings[groups_claim]', oidc_settings.groups_claim, size: 60 %> +

+

+ <%= label_tag 'settings[group_names_pattern]', l('oidc.settings.group_names_pattern') %> + <%= text_field_tag 'settings[group_names_pattern]', oidc_settings.group_names_pattern, size: 60, placeholder: l('oidc.settings.group_names_pattern_placeholder') %> +

<%= label_tag 'settings[session_check_enabled]', l('oidc.settings.session_check_enabled') %> <%= check_box_tag 'settings[session_check_enabled]', 1, oidc_settings.session_check_enabled %> diff --git a/config/locales/de.yml b/config/locales/de.yml index 66adf98..fc85fe8 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -34,6 +34,9 @@ de: roles_claim_placeholder: roles access_roles: Leerzeichen-separierte Liste der autorisierten Rollen admin_role: Administrationsrolle + groups_claim: Gruppe-Claim (optional) + group_names_pattern: Regex zum Filtern der Namen von Gruppen, die synchronisiert werden sollen + group_names_pattern_placeholder: .* session_check_enabled: Session Check aktivieren session_check_users_csv: Komma-separierte Liste der Logins mit Session Check (* = alle) error: diff --git a/config/locales/en.yml b/config/locales/en.yml index cedad2a..b9e30d1 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -34,6 +34,9 @@ en: roles_claim_placeholder: roles access_roles: Space-separated list of authorized roles admin_role: Administration role + groups_claim: Groups claim (optional) + group_names_pattern: Regex to filter names of groups that should be synchronized + group_names_pattern_placeholder: .* session_check_enabled: Enable session check session_check_users_csv: Comma-separated list of logins with session check (* = all) error: diff --git a/lib/redmine_oidc/settings.rb b/lib/redmine_oidc/settings.rb index 39f7444..f8b76ca 100644 --- a/lib/redmine_oidc/settings.rb +++ b/lib/redmine_oidc/settings.rb @@ -31,6 +31,8 @@ class Settings roles_claim access_roles admin_role + groups_claim + group_names_pattern session_check_enabled session_check_users_csv ) @@ -70,6 +72,14 @@ def to_h serializable_hash end + def group_names_regexp + @group_names_regexp ||= Regexp.new(@group_names_pattern || '.*', Regexp::IGNORECASE) + end + + def update_groups? + !!@groups_claim + end + def session_check_users @session_check_users ||= @session_check_users_csv.split(',').map(&:strip) end From bf0a24821163a527f5c75de0f6c4aa32e5333b9a Mon Sep 17 00:00:00 2001 From: Jakub Jirutka Date: Fri, 21 Apr 2023 13:25:50 +0200 Subject: [PATCH 2/2] Change default group_names_pattern to ^$ (match nothing) The current default (match all) is dangerous, I got burned on it myself. --- README.rdoc | 4 +++- config/locales/de.yml | 2 +- config/locales/en.yml | 2 +- lib/redmine_oidc/settings.rb | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/README.rdoc b/README.rdoc index cba5cd8..23920ad 100644 --- a/README.rdoc +++ b/README.rdoc @@ -93,7 +93,9 @@ groups_claim:: Example: +groups+ group_names_pattern:: A regex pattern to filter names of user groups that should be synchronized. - Default is +.*+ (match all). + Default is +^$+ - match nothing. Note that if you're using local groups (not + synchronised with OIDC), they should not match this pattern, otherwise users + will be removed from these groups when they log in. Example: +^(Red|Blue)$+ diff --git a/config/locales/de.yml b/config/locales/de.yml index fc85fe8..904e4d2 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -36,7 +36,7 @@ de: admin_role: Administrationsrolle groups_claim: Gruppe-Claim (optional) group_names_pattern: Regex zum Filtern der Namen von Gruppen, die synchronisiert werden sollen - group_names_pattern_placeholder: .* + group_names_pattern_placeholder: ^$ session_check_enabled: Session Check aktivieren session_check_users_csv: Komma-separierte Liste der Logins mit Session Check (* = alle) error: diff --git a/config/locales/en.yml b/config/locales/en.yml index b9e30d1..f03a82a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -36,7 +36,7 @@ en: admin_role: Administration role groups_claim: Groups claim (optional) group_names_pattern: Regex to filter names of groups that should be synchronized - group_names_pattern_placeholder: .* + group_names_pattern_placeholder: ^$ session_check_enabled: Enable session check session_check_users_csv: Comma-separated list of logins with session check (* = all) error: diff --git a/lib/redmine_oidc/settings.rb b/lib/redmine_oidc/settings.rb index f8b76ca..8108900 100644 --- a/lib/redmine_oidc/settings.rb +++ b/lib/redmine_oidc/settings.rb @@ -73,7 +73,7 @@ def to_h end def group_names_regexp - @group_names_regexp ||= Regexp.new(@group_names_pattern || '.*', Regexp::IGNORECASE) + @group_names_regexp ||= Regexp.new(@group_names_pattern || '^$', Regexp::IGNORECASE) end def update_groups?