From df3886d6e5fca3e1ac2bb961997fcc52bfeff8c1 Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Thu, 9 Dec 2021 20:30:27 +0800 Subject: [PATCH] FEATURE: Experimental support for group membership via google auth (#14835) This commit introduces a new site setting "google_oauth2_hd_groups". If enabled, group information will be fetched from Google during authentication, and stored in the Discourse database. These 'associated groups' can be connected to a Discourse group via the "Membership" tab of the group preferences UI. The majority of the implementation is generic, so we will be able to add support to more authentication methods in the near future. https://meta.discourse.org/t/managing-group-membership-via-authentication/175950 --- .../groups-form-membership-fields.js | 12 +- .../discourse/app/models/associated-group.js | 17 +++ .../javascripts/discourse/app/models/group.js | 10 ++ .../groups-form-membership-fields.hbs | 17 +++ .../group-manage-membership-test.js | 50 +++++++ app/controllers/admin/groups_controller.rb | 4 + .../associated_groups_controller.rb | 10 ++ app/controllers/groups_controller.rb | 4 + .../users/omniauth_callbacks_controller.rb | 1 + .../scheduled/clean_up_associated_groups.rb | 11 ++ app/models/associated_group.rb | 38 ++++++ app/models/group.rb | 16 +++ app/models/group_associated_group.rb | 61 +++++++++ app/models/user.rb | 2 + app/models/user_associated_group.rb | 46 +++++++ .../associated_group_serializer.rb | 8 ++ app/serializers/group_show_serializer.rb | 11 +- app/serializers/site_serializer.rb | 9 ++ app/services/group_action_logger.rb | 10 +- config/locales/client.en.yml | 1 + config/locales/server.en.yml | 2 + config/routes.rb | 2 + config/site_settings.yml | 3 + ...20211106085344_create_associated_groups.rb | 15 +++ ...106085527_create_user_associated_groups.rb | 13 ++ ...06085605_create_group_associated_groups.rb | 13 ++ lib/auth.rb | 1 + lib/auth/authenticator.rb | 5 + lib/auth/google_oauth2_authenticator.rb | 20 ++- lib/auth/managed_authenticator.rb | 6 +- .../discourse_google_oauth2.rb | 45 +++++++ lib/auth/result.rb | 33 ++++- lib/guardian/group_guardian.rb | 4 + .../google_oauth2_hd_groups_validator.rb | 15 +++ .../auth/google_oauth2_authenticator_spec.rb | 56 +++++++- .../auth/managed_authenticator_spec.rb | 14 +- .../discourse_google_oauth2_spec.rb | 126 ++++++++++++++++++ .../associated_group_fabricator.rb | 7 + spec/models/associated_group_spec.rb | 48 +++++++ spec/models/group_associated_group_spec.rb | 41 ++++++ spec/models/user_associated_group_spec.rb | 41 ++++++ .../api/schemas/json/group_response.json | 6 + .../api/schemas/json/site_response.json | 3 + .../omniauth_callbacks_controller_spec.rb | 85 ++++++++++++ 44 files changed, 926 insertions(+), 16 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/models/associated-group.js create mode 100644 app/controllers/associated_groups_controller.rb create mode 100644 app/jobs/scheduled/clean_up_associated_groups.rb create mode 100644 app/models/associated_group.rb create mode 100644 app/models/group_associated_group.rb create mode 100644 app/models/user_associated_group.rb create mode 100644 app/serializers/associated_group_serializer.rb create mode 100644 db/migrate/20211106085344_create_associated_groups.rb create mode 100644 db/migrate/20211106085527_create_user_associated_groups.rb create mode 100644 db/migrate/20211106085605_create_group_associated_groups.rb create mode 100644 lib/auth/omniauth_strategies/discourse_google_oauth2.rb create mode 100644 lib/validators/google_oauth2_hd_groups_validator.rb create mode 100644 spec/components/auth/omniauth_strategies/discourse_google_oauth2_spec.rb create mode 100644 spec/fabricators/associated_group_fabricator.rb create mode 100644 spec/models/associated_group_spec.rb create mode 100644 spec/models/group_associated_group_spec.rb create mode 100644 spec/models/user_associated_group_spec.rb diff --git a/app/assets/javascripts/discourse/app/components/groups-form-membership-fields.js b/app/assets/javascripts/discourse/app/components/groups-form-membership-fields.js index 7603df532c5..5292efa5858 100644 --- a/app/assets/javascripts/discourse/app/components/groups-form-membership-fields.js +++ b/app/assets/javascripts/discourse/app/components/groups-form-membership-fields.js @@ -1,11 +1,13 @@ import Component from "@ember/component"; import I18n from "I18n"; import { computed } from "@ember/object"; -import { not } from "@ember/object/computed"; +import { not, readOnly } from "@ember/object/computed"; import discourseComputed from "discourse-common/utils/decorators"; +import AssociatedGroup from "discourse/models/associated-group"; export default Component.extend({ tokenSeparator: "|", + showAssociatedGroups: readOnly("site.can_associate_groups"), init() { this._super(...arguments); @@ -20,6 +22,10 @@ export default Component.extend({ { name: 3, value: 3 }, { name: 4, value: 4 }, ]; + + if (this.showAssociatedGroups) { + this.loadAssociatedGroups(); + } }, canEdit: not("model.automatic"), @@ -54,6 +60,10 @@ export default Component.extend({ return this.model.emailDomains.split(this.tokenSeparator).filter(Boolean); }), + loadAssociatedGroups() { + AssociatedGroup.list().then((ags) => this.set("associatedGroups", ags)); + }, + actions: { onChangeEmailDomainsSetting(value) { this.set( diff --git a/app/assets/javascripts/discourse/app/models/associated-group.js b/app/assets/javascripts/discourse/app/models/associated-group.js new file mode 100644 index 00000000000..e914aaf824e --- /dev/null +++ b/app/assets/javascripts/discourse/app/models/associated-group.js @@ -0,0 +1,17 @@ +import EmberObject from "@ember/object"; +import { ajax } from "discourse/lib/ajax"; +import { popupAjaxError } from "discourse/lib/ajax-error"; + +const AssociatedGroup = EmberObject.extend(); + +AssociatedGroup.reopenClass({ + list() { + return ajax("/associated_groups") + .then((result) => { + return result.associated_groups.map((ag) => AssociatedGroup.create(ag)); + }) + .catch(popupAjaxError); + }, +}); + +export default AssociatedGroup; diff --git a/app/assets/javascripts/discourse/app/models/group.js b/app/assets/javascripts/discourse/app/models/group.js index 2f597b4dbac..a360f8c3d89 100644 --- a/app/assets/javascripts/discourse/app/models/group.js +++ b/app/assets/javascripts/discourse/app/models/group.js @@ -29,6 +29,11 @@ const Group = RestModel.extend({ return isEmpty(value) ? "" : value; }, + @discourseComputed("associated_group_ids") + associatedGroupIds(value) { + return isEmpty(value) ? [] : value; + }, + @discourseComputed("automatic") type(automatic) { return automatic ? "automatic" : "custom"; @@ -277,6 +282,11 @@ const Group = RestModel.extend({ } ); + let agIds = this.associated_group_ids; + if (agIds) { + attrs["associated_group_ids"] = agIds.length ? agIds : [null]; + } + if (this.flair_type === "icon") { attrs["flair_icon"] = this.flair_icon; } else if (this.flair_type === "image") { diff --git a/app/assets/javascripts/discourse/app/templates/components/groups-form-membership-fields.hbs b/app/assets/javascripts/discourse/app/templates/components/groups-form-membership-fields.hbs index bce462add00..98e85f36a9f 100644 --- a/app/assets/javascripts/discourse/app/templates/components/groups-form-membership-fields.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/groups-form-membership-fields.hbs @@ -59,6 +59,23 @@ onChange=(action "onChangeEmailDomainsSetting") options=(hash allowAny=true) }} + + {{#if showAssociatedGroups}} + + + {{list-setting + name="automatic_membership_associated_groups" + class="group-form-automatic-membership-associated-groups" + value=model.associatedGroupIds + choices=associatedGroups + settingName="name" + nameProperty="label" + valueProperty="id" + onChange=(action (mut model.associated_group_ids)) + }} + {{/if}} {{plugin-outlet name="groups-form-membership-below-automatic" diff --git a/app/assets/javascripts/discourse/tests/acceptance/group-manage-membership-test.js b/app/assets/javascripts/discourse/tests/acceptance/group-manage-membership-test.js index 15c5c0d56fe..ffe5b0bf251 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/group-manage-membership-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/group-manage-membership-test.js @@ -7,9 +7,24 @@ import { import { click, visit } from "@ember/test-helpers"; import selectKit from "discourse/tests/helpers/select-kit-helper"; import { test } from "qunit"; +import Site from "discourse/models/site"; acceptance("Managing Group Membership", function (needs) { needs.user(); + needs.pretender((server, helper) => { + server.get("/associated_groups", () => + helper.response({ + associated_groups: [ + { + id: 123, + name: "test-group", + provider_name: "google_oauth2", + label: "google_oauth2:test-group", + }, + ], + }) + ); + }); test("As an admin", async function (assert) { updateCurrentUser({ can_create_group: true }); @@ -94,6 +109,36 @@ acceptance("Managing Group Membership", function (needs) { assert.strictEqual(emailDomains.header().value(), "foo.com"); }); + test("As an admin on a site that can associate groups", async function (assert) { + let site = Site.current(); + site.set("can_associate_groups", true); + updateCurrentUser({ can_create_group: true }); + + await visit("/g/alternative-group/manage/membership"); + + const associatedGroups = selectKit( + ".group-form-automatic-membership-associated-groups" + ); + await associatedGroups.expand(); + await associatedGroups.selectRowByName("google_oauth2:test-group"); + await associatedGroups.keyboard("enter"); + + assert.equal(associatedGroups.header().name(), "google_oauth2:test-group"); + }); + + test("As an admin on a site that can't associate groups", async function (assert) { + let site = Site.current(); + site.set("can_associate_groups", false); + updateCurrentUser({ can_create_group: true }); + + await visit("/g/alternative-group/manage/membership"); + + assert.ok( + !exists('label[for="automatic_membership_associated_groups"]'), + "it should not display associated groups automatic membership label" + ); + }); + test("As a group owner", async function (assert) { updateCurrentUser({ moderator: false, admin: false }); @@ -104,6 +149,11 @@ acceptance("Managing Group Membership", function (needs) { "it should not display automatic membership label" ); + assert.ok( + !exists('label[for="automatic_membership_associated_groups"]'), + "it should not display associated groups automatic membership label" + ); + assert.ok( !exists(".groups-form-automatic-membership-retroactive"), "it should not display automatic membership retroactive checkbox" diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index f80dfe27c5e..33944246e8e 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -180,6 +180,10 @@ class Admin::GroupsController < Admin::AdminController custom_fields = DiscoursePluginRegistry.editable_group_custom_fields permitted << { custom_fields: custom_fields } unless custom_fields.blank? + if guardian.can_associate_groups? + permitted << { associated_group_ids: [] } + end + permitted = permitted | DiscoursePluginRegistry.group_params params.require(:group).permit(permitted) diff --git a/app/controllers/associated_groups_controller.rb b/app/controllers/associated_groups_controller.rb new file mode 100644 index 00000000000..cf82ae20c7d --- /dev/null +++ b/app/controllers/associated_groups_controller.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class AssociatedGroupsController < ApplicationController + requires_login + + def index + guardian.ensure_can_associate_groups! + render_serialized(AssociatedGroup.all, AssociatedGroupSerializer, root: 'associated_groups') + end +end diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 6fbb21044f6..6a2d6d37d11 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -736,6 +736,10 @@ class GroupsController < ApplicationController end end + if guardian.can_associate_groups? + permitted_params << { associated_group_ids: [] } + end + permitted_params = permitted_params | DiscoursePluginRegistry.group_params params.require(:group).permit(*permitted_params) diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index 57c794f9cc2..1047d6f9ff6 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -171,6 +171,7 @@ class Users::OmniauthCallbacksController < ApplicationController elsif Guardian.new(user).can_access_forum? && user.active # log on any account that is active with forum access begin user.save! if @auth_result.apply_user_attributes! + @auth_result.apply_associated_attributes! rescue ActiveRecord::RecordInvalid => e @auth_result.failed = true @auth_result.failed_reason = e.record.errors.full_messages.join(", ") diff --git a/app/jobs/scheduled/clean_up_associated_groups.rb b/app/jobs/scheduled/clean_up_associated_groups.rb new file mode 100644 index 00000000000..9b3514798b8 --- /dev/null +++ b/app/jobs/scheduled/clean_up_associated_groups.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Jobs + class CleanUpAssociatedGroups < ::Jobs::Scheduled + every 1.day + + def execute(args) + AssociatedGroup.cleanup! + end + end +end diff --git a/app/models/associated_group.rb b/app/models/associated_group.rb new file mode 100644 index 00000000000..9d6e9365dcd --- /dev/null +++ b/app/models/associated_group.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true +class AssociatedGroup < ActiveRecord::Base + has_many :user_associated_groups, dependent: :destroy + has_many :users, through: :user_associated_groups + has_many :group_associated_groups, dependent: :destroy + has_many :groups, through: :group_associated_groups + + def label + "#{provider_name}:#{name}" + end + + def self.has_provider? + Discourse.enabled_authenticators.any? { |a| a.provides_groups? } + end + + def self.cleanup! + AssociatedGroup.left_joins(:group_associated_groups, :user_associated_groups) + .where("group_associated_groups.id IS NULL AND user_associated_groups.id IS NULL") + .where("last_used < ?", 1.week.ago).delete_all + end +end + +# == Schema Information +# +# Table name: associated_groups +# +# id :bigint not null, primary key +# name :string not null +# provider_name :string not null +# provider_id :string not null +# last_used :datetime not null +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# associated_groups_provider_id (provider_name,provider_id) UNIQUE +# diff --git a/app/models/group.rb b/app/models/group.rb index 37e08a23db8..ebb703e88cd 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -21,6 +21,7 @@ class Group < ActiveRecord::Base has_many :group_users, dependent: :destroy has_many :group_requests, dependent: :destroy has_many :group_mentions, dependent: :destroy + has_many :group_associated_groups, dependent: :destroy has_many :group_archived_messages, dependent: :destroy @@ -32,6 +33,7 @@ class Group < ActiveRecord::Base has_many :reviewables, foreign_key: :reviewable_by_group_id, dependent: :nullify has_many :group_category_notification_defaults, dependent: :destroy has_many :group_tag_notification_defaults, dependent: :destroy + has_many :associated_groups, through: :group_associated_groups, dependent: :destroy belongs_to :flair_upload, class_name: 'Upload' belongs_to :smtp_updated_by, class_name: 'User' @@ -768,6 +770,20 @@ class Group < ActiveRecord::Base self end + def add_automatically(user, subject: nil) + if users.exclude?(user) && add(user) + logger = GroupActionLogger.new(Discourse.system_user, self) + logger.log_add_user_to_group(user, subject) + end + end + + def remove_automatically(user, subject: nil) + if users.include?(user) && remove(user) + logger = GroupActionLogger.new(Discourse.system_user, self) + logger.log_remove_user_from_group(user, subject) + end + end + def staff? STAFF_GROUPS.include?(self.name.to_sym) end diff --git a/app/models/group_associated_group.rb b/app/models/group_associated_group.rb new file mode 100644 index 00000000000..be71f09eb08 --- /dev/null +++ b/app/models/group_associated_group.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true +class GroupAssociatedGroup < ActiveRecord::Base + belongs_to :group + belongs_to :associated_group + + after_commit :add_associated_users, on: [:create, :update] + before_destroy :remove_associated_users + + def add_associated_users + with_mutex do + associated_group.users.in_batches do |users| + users.each do |user| + group.add_automatically(user, subject: associated_group.label) + end + end + end + end + + def remove_associated_users + with_mutex do + User.where("NOT EXISTS( + SELECT 1 + FROM user_associated_groups uag + JOIN group_associated_groups gag + ON gag.associated_group_id = uag.associated_group_id + WHERE uag.user_id = users.id + AND gag.id != :gag_id + AND gag.group_id = :group_id + )", gag_id: id, group_id: group_id).in_batches do |users| + users.each do |user| + group.remove_automatically(user, subject: associated_group.label) + end + end + end + end + + private + + def with_mutex + DistributedMutex.synchronize("group_associated_group_#{group_id}_#{associated_group_id}") do + yield + end + end +end + +# == Schema Information +# +# Table name: group_associated_groups +# +# id :bigint not null, primary key +# group_id :bigint not null +# associated_group_id :bigint not null +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# index_group_associated_groups (group_id,associated_group_id) UNIQUE +# index_group_associated_groups_on_associated_group_id (associated_group_id) +# index_group_associated_groups_on_group_id (group_id) +# diff --git a/app/models/user.rb b/app/models/user.rb index fc62d990479..d610cc524f7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -38,6 +38,7 @@ class User < ActiveRecord::Base has_many :reviewable_scores, dependent: :destroy has_many :invites, foreign_key: :invited_by_id, dependent: :destroy has_many :user_custom_fields, dependent: :destroy + has_many :user_associated_groups, dependent: :destroy has_many :pending_posts, -> { merge(Reviewable.pending) }, class_name: 'ReviewableQueuedPost', foreign_key: :created_by_id has_one :user_option, dependent: :destroy @@ -83,6 +84,7 @@ class User < ActiveRecord::Base has_many :topics_allowed, through: :topic_allowed_users, source: :topic has_many :groups, through: :group_users has_many :secure_categories, through: :groups, source: :categories + has_many :associated_groups, through: :user_associated_groups, dependent: :destroy # deleted in user_second_factors relationship has_many :totps, -> { diff --git a/app/models/user_associated_group.rb b/app/models/user_associated_group.rb new file mode 100644 index 00000000000..1413efb7c67 --- /dev/null +++ b/app/models/user_associated_group.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +class UserAssociatedGroup < ActiveRecord::Base + belongs_to :user + belongs_to :associated_group + + after_commit :add_to_associated_groups, on: [:create, :update] + before_destroy :remove_from_associated_groups + + def add_to_associated_groups + associated_group.groups.each do |group| + group.add_automatically(user, subject: associated_group.label) + end + end + + def remove_from_associated_groups + Group.where("NOT EXISTS( + SELECT 1 + FROM user_associated_groups uag + JOIN group_associated_groups gag + ON gag.associated_group_id = uag.associated_group_id + WHERE uag.user_id = :user_id + AND uag.id != :uag_id + AND gag.group_id = groups.id + )", uag_id: id, user_id: user_id).each do |group| + group.remove_automatically(user, subject: associated_group.label) + end + end +end + +# == Schema Information +# +# Table name: user_associated_groups +# +# id :bigint not null, primary key +# user_id :bigint not null +# associated_group_id :bigint not null +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# index_user_associated_groups (user_id,associated_group_id) UNIQUE +# index_user_associated_groups_on_associated_group_id (associated_group_id) +# index_user_associated_groups_on_user_id (user_id) +# diff --git a/app/serializers/associated_group_serializer.rb b/app/serializers/associated_group_serializer.rb new file mode 100644 index 00000000000..db7dec87238 --- /dev/null +++ b/app/serializers/associated_group_serializer.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class AssociatedGroupSerializer < ApplicationSerializer + attributes :id, + :name, + :provider_name, + :label +end diff --git a/app/serializers/group_show_serializer.rb b/app/serializers/group_show_serializer.rb index 826b2c6533f..0b51aed01bf 100644 --- a/app/serializers/group_show_serializer.rb +++ b/app/serializers/group_show_serializer.rb @@ -36,7 +36,8 @@ class GroupShowSerializer < BasicGroupSerializer :imap_old_emails, :imap_new_emails, :message_count, - :allow_unknown_sender_topic_replies + :allow_unknown_sender_topic_replies, + :associated_group_ids def self.admin_or_owner_attributes(*attrs) attributes(*attrs) @@ -121,6 +122,14 @@ class GroupShowSerializer < BasicGroupSerializer end end + def associated_group_ids + object.associated_groups.map(&:id) + end + + def include_associated_group_ids? + scope.can_associate_groups? + end + private def authenticated? diff --git a/app/serializers/site_serializer.rb b/app/serializers/site_serializer.rb index 29972ae8f57..c65628748f3 100644 --- a/app/serializers/site_serializer.rb +++ b/app/serializers/site_serializer.rb @@ -21,6 +21,7 @@ class SiteSerializer < ApplicationSerializer :can_tag_pms, :tags_filter_regexp, :top_tags, + :can_associate_groups, :wizard_required, :topic_featured_link_allowed_category_ids, :user_themes, @@ -134,6 +135,14 @@ class SiteSerializer < ApplicationSerializer scope.can_tag_pms? end + def can_associate_groups + scope.can_associate_groups? + end + + def include_can_associate_groups? + scope.is_admin? + end + def include_tags_filter_regexp? SiteSetting.tagging_enabled end diff --git a/app/services/group_action_logger.rb b/app/services/group_action_logger.rb index 9fb65afbdec..d38d0e7c5a7 100644 --- a/app/services/group_action_logger.rb +++ b/app/services/group_action_logger.rb @@ -21,17 +21,19 @@ class GroupActionLogger )) end - def log_add_user_to_group(target_user) + def log_add_user_to_group(target_user, subject = nil) GroupHistory.create!(default_params.merge( action: GroupHistory.actions[:add_user_to_group], - target_user: target_user + target_user: target_user, + subject: subject )) end - def log_remove_user_from_group(target_user) + def log_remove_user_from_group(target_user, subject = nil) GroupHistory.create!(default_params.merge( action: GroupHistory.actions[:remove_user_from_group], - target_user: target_user + target_user: target_user, + subject: subject )) end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 4c5c10671e8..05a88bd7994 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -4124,6 +4124,7 @@ en: automatic_membership_user_count: one: "%{count} user has the new email domains and will be added to the group." other: "%{count} users have the new email domains and will be added to the group." + automatic_membership_associated_groups: "Users who are members of a group on a service listed here will be automatically added to this group when they log in with the service." primary_group: "Automatically set as primary group" name_placeholder: "Group name, no spaces, same as username rule" primary: "Primary Group" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index b46fb39daca..a85fe23ffbc 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1724,6 +1724,7 @@ en: google_oauth2_client_secret: "Client secret of your Google application." google_oauth2_prompt: "An optional space-delimited list of string values that specifies whether the authorization server prompts the user for reauthentication and consent. See https://developers.google.com/identity/protocols/OpenIDConnect#prompt for the possible values." google_oauth2_hd: "An optional Google Apps Hosted domain that the sign-in will be limited to. See https://developers.google.com/identity/protocols/OpenIDConnect#hd-param for more details." + google_oauth2_hd_groups: "(experimental) Retrieve users' Google groups on the hosted domain on authentication. Retrieved Google groups can be used to grant automatic Discourse group membership (see group settings)." enable_twitter_logins: "Enable Twitter authentication, requires twitter_consumer_key and twitter_consumer_secret. See Configuring Twitter login (and rich embeds) for Discourse." twitter_consumer_key: "Consumer key for Twitter authentication, registered at https://developer.twitter.com/apps" @@ -2390,6 +2391,7 @@ en: leading_trailing_slash: "The regular expression must not start and end with a slash." unicode_usernames_avatars: "The internal system avatars do not support Unicode usernames." list_value_count: "The list must contain exactly %{count} values." + google_oauth2_hd_groups: "You must first set 'google oauth2 hd' before enabling this setting." placeholder: discourse_connect_provider_secrets: diff --git a/config/routes.rb b/config/routes.rb index 041b713b7d2..1f75aaa30ce 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -634,6 +634,8 @@ Discourse::Application.routes.draw do end end + resources :associated_groups, only: %i[index], constraints: AdminConstraint.new + # aliases so old API code works delete "admin/groups/:id/members" => "groups#remove_member", constraints: AdminConstraint.new put "admin/groups/:id/members" => "groups#add_members", constraints: AdminConstraint.new diff --git a/config/site_settings.yml b/config/site_settings.yml index f2019ac4356..ff5e68b3597 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -411,6 +411,9 @@ login: - "select_account" google_oauth2_hd: default: "" + google_oauth2_hd_groups: + default: false + validator: GoogleOauth2HdGroupsValidator enable_twitter_logins: default: false twitter_consumer_key: diff --git a/db/migrate/20211106085344_create_associated_groups.rb b/db/migrate/20211106085344_create_associated_groups.rb new file mode 100644 index 00000000000..72c441fe5fa --- /dev/null +++ b/db/migrate/20211106085344_create_associated_groups.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true +class CreateAssociatedGroups < ActiveRecord::Migration[6.1] + def change + create_table :associated_groups do |t| + t.string :name, null: false + t.string :provider_name, null: false + t.string :provider_id, null: false + t.datetime :last_used, null: false, default: -> { "CURRENT_TIMESTAMP" } + + t.timestamps + end + + add_index :associated_groups, %i[provider_name provider_id], unique: true, name: 'associated_groups_provider_id' + end +end diff --git a/db/migrate/20211106085527_create_user_associated_groups.rb b/db/migrate/20211106085527_create_user_associated_groups.rb new file mode 100644 index 00000000000..73464b66f47 --- /dev/null +++ b/db/migrate/20211106085527_create_user_associated_groups.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true +class CreateUserAssociatedGroups < ActiveRecord::Migration[6.1] + def change + create_table :user_associated_groups do |t| + t.references :user, null: false + t.references :associated_group, null: false + + t.timestamps + end + + add_index :user_associated_groups, %i[user_id associated_group_id], unique: true, name: 'index_user_associated_groups' + end +end diff --git a/db/migrate/20211106085605_create_group_associated_groups.rb b/db/migrate/20211106085605_create_group_associated_groups.rb new file mode 100644 index 00000000000..281adb11b17 --- /dev/null +++ b/db/migrate/20211106085605_create_group_associated_groups.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true +class CreateGroupAssociatedGroups < ActiveRecord::Migration[6.1] + def change + create_table :group_associated_groups do |t| + t.references :group, null: false + t.references :associated_group, null: false + + t.timestamps + end + + add_index :group_associated_groups, %i[group_id associated_group_id], unique: true, name: 'index_group_associated_groups' + end +end diff --git a/lib/auth.rb b/lib/auth.rb index f501d901574..21e2716250f 100644 --- a/lib/auth.rb +++ b/lib/auth.rb @@ -6,6 +6,7 @@ require 'auth/auth_provider' require 'auth/result' require 'auth/authenticator' require 'auth/managed_authenticator' +require 'auth/omniauth_strategies/discourse_google_oauth2' require 'auth/facebook_authenticator' require 'auth/github_authenticator' require 'auth/twitter_authenticator' diff --git a/lib/auth/authenticator.rb b/lib/auth/authenticator.rb index 2302d3bce00..4333e87d631 100644 --- a/lib/auth/authenticator.rb +++ b/lib/auth/authenticator.rb @@ -65,4 +65,9 @@ class Auth::Authenticator def revoke(user, skip_remote: false) raise NotImplementedError end + + # provider has implemented user group membership (or equivalent) request + def provides_groups? + false + end end diff --git a/lib/auth/google_oauth2_authenticator.rb b/lib/auth/google_oauth2_authenticator.rb index 59b0caa2c78..02e015cd10c 100644 --- a/lib/auth/google_oauth2_authenticator.rb +++ b/lib/auth/google_oauth2_authenticator.rb @@ -16,6 +16,7 @@ class Auth::GoogleOAuth2Authenticator < Auth::ManagedAuthenticator end def register_middleware(omniauth) + strategy_class = Auth::OmniAuthStrategies::DiscourseGoogleOauth2 options = { setup: lambda { |env| strategy = env["omniauth.strategy"] @@ -35,8 +36,25 @@ class Auth::GoogleOAuth2Authenticator < Auth::ManagedAuthenticator # the JWT can fail due to clock skew, so let's skip it completely. # https://github.com/zquestz/omniauth-google-oauth2/pull/392 strategy.options[:skip_jwt] = true + strategy.options[:request_groups] = provides_groups? + + if provides_groups? + strategy.options[:scope] = "#{strategy_class::DEFAULT_SCOPE},#{strategy_class::GROUPS_SCOPE}" + end } } - omniauth.provider :google_oauth2, options + omniauth.provider strategy_class, options + end + + def after_authenticate(auth_token, existing_account: nil) + result = super + if provides_groups? && (groups = auth_token[:extra][:raw_groups]) + result.associated_groups = groups.map { |group| group.slice(:id, :name) } + end + result + end + + def provides_groups? + SiteSetting.google_oauth2_hd.present? && SiteSetting.google_oauth2_hd_groups end end diff --git a/lib/auth/managed_authenticator.rb b/lib/auth/managed_authenticator.rb index e82f1de96e1..a7753050b8c 100644 --- a/lib/auth/managed_authenticator.rb +++ b/lib/auth/managed_authenticator.rb @@ -113,14 +113,16 @@ class Auth::ManagedAuthenticator < Auth::Authenticator result end - def after_create_account(user, auth) - auth_token = auth[:extra_data] + def after_create_account(user, auth_result) + auth_token = auth_result[:extra_data] association = UserAssociatedAccount.find_or_initialize_by(provider_name: auth_token[:provider], provider_uid: auth_token[:uid]) association.user = user association.save! retrieve_avatar(user, association.info["image"]) retrieve_profile(user, association.info) + + auth_result.apply_associated_attributes! end def find_user_by_email(auth_token) diff --git a/lib/auth/omniauth_strategies/discourse_google_oauth2.rb b/lib/auth/omniauth_strategies/discourse_google_oauth2.rb new file mode 100644 index 00000000000..326e26d81be --- /dev/null +++ b/lib/auth/omniauth_strategies/discourse_google_oauth2.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +class Auth::OmniAuthStrategies + class DiscourseGoogleOauth2 < OmniAuth::Strategies::GoogleOauth2 + GROUPS_SCOPE ||= "admin.directory.group.readonly" + GROUPS_DOMAIN ||= "admin.googleapis.com" + GROUPS_PATH ||= "/admin/directory/v1/groups" + + def extra + hash = {} + hash[:raw_info] = raw_info + hash[:raw_groups] = raw_groups if options[:request_groups] + hash + end + + def raw_groups + @raw_groups ||= begin + groups = [] + page_token = nil + groups_url = "https://#{GROUPS_DOMAIN}#{GROUPS_PATH}" + + loop do + params = { + userKey: uid + } + params[:pageToken] = page_token if page_token + + response = access_token.get(groups_url, params: params, raise_errors: false) + + if response.status == 200 + response = response.parsed + groups.push(*response['groups']) + page_token = response['nextPageToken'] + break if page_token.nil? + else + Rails.logger.error("[Discourse Google OAuth2] failed to retrieve groups for #{uid} - status #{response.status}") + break + end + end + + groups + end + end + end +end diff --git a/lib/auth/result.rb b/lib/auth/result.rb index 2b0ba30c2f8..c63fb928696 100644 --- a/lib/auth/result.rb +++ b/lib/auth/result.rb @@ -21,7 +21,8 @@ class Auth::Result :omniauth_disallow_totp, :failed, :failed_reason, - :failed_code + :failed_code, + :associated_groups ] attr_accessor *ATTRIBUTES @@ -36,7 +37,8 @@ class Auth::Result :name, :authenticator_name, :extra_data, - :skip_email_validation + :skip_email_validation, + :associated_groups ] def [](key) @@ -94,6 +96,29 @@ class Auth::Result change_made end + def apply_associated_attributes! + if authenticator&.provides_groups? && !associated_groups.nil? + associated_group_ids = [] + + associated_groups.uniq.each do |associated_group| + begin + associated_group = AssociatedGroup.find_or_create_by( + name: associated_group[:name], + provider_id: associated_group[:id], + provider_name: extra_data[:provider] + ) + rescue ActiveRecord::RecordNotUnique + retry + end + + associated_group_ids.push(associated_group.id) + end + + user.update(associated_group_ids: associated_group_ids) + AssociatedGroup.where(id: associated_group_ids).update_all("last_used = CURRENT_TIMESTAMP") + end + end + def can_edit_name !SiteSetting.auth_overrides_name end @@ -167,6 +192,10 @@ class Auth::Result username || name || email end + def authenticator + @authenticator ||= Discourse.enabled_authenticators.find { |a| a.name == authenticator_name } + end + def resolve_username if staged_user if !username.present? || UserNameSuggester.fix_username(username) == staged_user.username diff --git a/lib/guardian/group_guardian.rb b/lib/guardian/group_guardian.rb index 36d4efc4cc4..1a869e4e7fe 100644 --- a/lib/guardian/group_guardian.rb +++ b/lib/guardian/group_guardian.rb @@ -36,4 +36,8 @@ module GroupGuardian SiteSetting.enable_personal_messages? && group.users.include?(user) end + + def can_associate_groups? + is_admin? && AssociatedGroup.has_provider? + end end diff --git a/lib/validators/google_oauth2_hd_groups_validator.rb b/lib/validators/google_oauth2_hd_groups_validator.rb new file mode 100644 index 00000000000..b4c3f91431e --- /dev/null +++ b/lib/validators/google_oauth2_hd_groups_validator.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class GoogleOauth2HdGroupsValidator + def initialize(opts = {}) + @opts = opts + end + + def valid_value?(value) + @valid = value == "f" || SiteSetting.google_oauth2_hd.present? + end + + def error_message + I18n.t("site_settings.errors.google_oauth2_hd_groups") if !@valid + end +end diff --git a/spec/components/auth/google_oauth2_authenticator_spec.rb b/spec/components/auth/google_oauth2_authenticator_spec.rb index 71bd5ccbc02..b82649070d8 100644 --- a/spec/components/auth/google_oauth2_authenticator_spec.rb +++ b/spec/components/auth/google_oauth2_authenticator_spec.rb @@ -3,7 +3,6 @@ require 'rails_helper' describe Auth::GoogleOAuth2Authenticator do - it 'does not look up user unless email is verified' do # note, emails that come back from google via omniauth are always valid # this protects against future regressions @@ -113,6 +112,61 @@ describe Auth::GoogleOAuth2Authenticator do expect(result.user).to eq(nil) expect(result.name).to eq("Jane Doe") end + + context "provides groups" do + before do + SiteSetting.google_oauth2_hd = "domain.com" + group1 = OmniAuth::AuthHash.new(id: "12345", name: "group1") + group2 = OmniAuth::AuthHash.new(id: "67890", name: "group2") + @groups = [group1, group2] + @groups_hash = OmniAuth::AuthHash.new( + provider: "google_oauth2", + uid: "123456789", + info: { + first_name: "Jane", + last_name: "Doe", + name: "Jane Doe", + email: "jane.doe@the.google.com" + }, + extra: { + raw_info: { + email: "jane.doe@the.google.com", + email_verified: true, + name: "Jane Doe" + }, + raw_groups: @groups + } + ) + end + + context "enabled" do + before do + SiteSetting.google_oauth2_hd_groups = true + end + + it "adds associated groups" do + result = described_class.new.after_authenticate(@groups_hash) + expect(result.associated_groups).to eq(@groups) + end + + it "handles a blank groups array" do + @groups_hash[:extra][:raw_groups] = [] + result = described_class.new.after_authenticate(@groups_hash) + expect(result.associated_groups).to eq([]) + end + end + + context "disabled" do + before do + SiteSetting.google_oauth2_hd_groups = false + end + + it "doesnt add associated groups" do + result = described_class.new.after_authenticate(@groups_hash) + expect(result.associated_groups).to eq(nil) + end + end + end end context 'revoke' do diff --git a/spec/components/auth/managed_authenticator_spec.rb b/spec/components/auth/managed_authenticator_spec.rb index f98419ac67b..2f6e371a0ec 100644 --- a/spec/components/auth/managed_authenticator_spec.rb +++ b/spec/components/auth/managed_authenticator_spec.rb @@ -38,6 +38,12 @@ describe Auth::ManagedAuthenticator do ) } + def create_auth_result(attrs) + auth_result = Auth::Result.new + attrs.each { |k, v| auth_result.send("#{k}=", v) } + auth_result + end + describe 'after_authenticate' do it 'can match account from an existing association' do user = Fabricate(:user) @@ -250,14 +256,14 @@ describe Auth::ManagedAuthenticator do let!(:association) { UserAssociatedAccount.create!(provider_name: 'myauth', provider_uid: "1234") } it "doesn't schedule with no image" do - expect { result = authenticator.after_create_account(user, extra_data: create_hash) } + expect { result = authenticator.after_create_account(user, create_auth_result(extra_data: create_hash)) } .to change { Jobs::DownloadAvatarFromUrl.jobs.count }.by(0) end it "schedules with image" do association.info["image"] = "https://some.domain/image.jpg" association.save! - expect { result = authenticator.after_create_account(user, extra_data: create_hash) } + expect { result = authenticator.after_create_account(user, create_auth_result(extra_data: create_hash)) } .to change { Jobs::DownloadAvatarFromUrl.jobs.count }.by(1) end end @@ -267,14 +273,14 @@ describe Auth::ManagedAuthenticator do let!(:association) { UserAssociatedAccount.create!(provider_name: 'myauth', provider_uid: "1234") } it "doesn't explode without profile" do - authenticator.after_create_account(user, extra_data: create_hash) + authenticator.after_create_account(user, create_auth_result(extra_data: create_hash)) end it "works with profile" do association.info["location"] = "DiscourseVille" association.info["description"] = "Online forum expert" association.save! - authenticator.after_create_account(user, extra_data: create_hash) + authenticator.after_create_account(user, create_auth_result(extra_data: create_hash)) expect(user.user_profile.bio_raw).to eq("Online forum expert") expect(user.user_profile.location).to eq("DiscourseVille") end diff --git a/spec/components/auth/omniauth_strategies/discourse_google_oauth2_spec.rb b/spec/components/auth/omniauth_strategies/discourse_google_oauth2_spec.rb new file mode 100644 index 00000000000..e5eda8ff6b0 --- /dev/null +++ b/spec/components/auth/omniauth_strategies/discourse_google_oauth2_spec.rb @@ -0,0 +1,126 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Auth::OmniAuthStrategies::DiscourseGoogleOauth2 do + let(:response_hash) do + { + email: 'user@domain.com', + email_verified: true + } + end + let(:groups) do + [ + { + id: "12345", + name: "group1" + }, + { + id: "67890", + name: "group2" + } + ] + end + let(:uid) { "12345" } + let(:domain) { "domain.com" } + + def build_response(body, code = 200) + [code, { 'Content-Type' => 'application/json' }, body.to_json] + end + + def build_client(groups_response) + OAuth2::Client.new('abc', 'def') do |builder| + builder.request :url_encoded + builder.adapter :test do |stub| + stub.get('/oauth2/v3/userinfo') { build_response(response_hash) } + stub.get(described_class::GROUPS_PATH) { groups_response } + end + end + end + + let(:successful_groups_client) do + build_client( + build_response( + groups: groups + ) + ) + end + + let(:unsuccessful_groups_client) do + build_client( + build_response( + error: { + code: 403, + message: "Not Authorized to access this resource/api" + } + ) + ) + end + + let(:successful_groups_token) do + OAuth2::AccessToken.from_hash(successful_groups_client, {}) + end + + let(:unsuccessful_groups_token) do + OAuth2::AccessToken.from_hash(unsuccessful_groups_client, {}) + end + + def app + lambda do |_env| + [200, {}, ["Hello."]] + end + end + + def build_strategy(access_token) + strategy = described_class.new(app, 'appid', 'secret', @options) + strategy.stubs(:uid).returns(uid) + strategy.stubs(:access_token).returns(access_token) + strategy + end + + before do + @options = {} + OmniAuth.config.test_mode = true + end + + after do + OmniAuth.config.test_mode = false + end + + context 'request_groups is true' do + before do + @options[:request_groups] = true + end + + context 'groups request successful' do + before do + @strategy = build_strategy(successful_groups_token) + end + + it 'should include users groups' do + expect(@strategy.extra[:raw_groups].map(&:symbolize_keys)).to eq(groups) + end + end + + context 'groups request unsuccessful' do + before do + @strategy = build_strategy(unsuccessful_groups_token) + end + + it 'users groups should be empty' do + expect(@strategy.extra[:raw_groups].empty?).to eq(true) + end + end + end + + context 'request_groups is not true' do + before do + @options[:request_groups] = false + @strategy = build_strategy(successful_groups_token) + end + + it 'should not include users groups' do + expect(@strategy.extra).not_to have_key(:raw_groups) + end + end +end diff --git a/spec/fabricators/associated_group_fabricator.rb b/spec/fabricators/associated_group_fabricator.rb new file mode 100644 index 00000000000..f7aff76893c --- /dev/null +++ b/spec/fabricators/associated_group_fabricator.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +Fabricator(:associated_group) do + name { sequence(:name) { |n| "group_#{n}" } } + provider_name 'google' + provider_id { SecureRandom.hex(20) } +end diff --git a/spec/models/associated_group_spec.rb b/spec/models/associated_group_spec.rb new file mode 100644 index 00000000000..3127b35e29d --- /dev/null +++ b/spec/models/associated_group_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe AssociatedGroup do + let(:user) { Fabricate(:user) } + let(:associated_group) { Fabricate(:associated_group) } + let(:group) { Fabricate(:group) } + + it "generates a label" do + ag = described_class.new(name: "group1", provider_name: "google") + expect(ag.label).to eq("google:group1") + end + + it "detects whether any auth providers provide associated groups" do + SiteSetting.enable_google_oauth2_logins = true + SiteSetting.google_oauth2_hd = 'domain.com' + SiteSetting.google_oauth2_hd_groups = false + expect(described_class.has_provider?).to eq(false) + + SiteSetting.google_oauth2_hd_groups = true + expect(described_class.has_provider?).to eq(true) + end + + context "cleanup!" do + before do + associated_group.last_used = 8.days.ago + associated_group.save + end + + it "deletes associated groups not used in over a week" do + described_class.cleanup! + expect(described_class.exists?(associated_group.id)).to eq(false) + end + + it "doesnt delete associated groups associated with groups" do + GroupAssociatedGroup.create(group_id: group.id, associated_group_id: associated_group.id) + described_class.cleanup! + expect(described_class.exists?(associated_group.id)).to eq(true) + end + + it "doesnt delete associated groups associated with users" do + UserAssociatedGroup.create(user_id: user.id, associated_group_id: associated_group.id) + described_class.cleanup! + expect(described_class.exists?(associated_group.id)).to eq(true) + end + end +end diff --git a/spec/models/group_associated_group_spec.rb b/spec/models/group_associated_group_spec.rb new file mode 100644 index 00000000000..213a7afecfc --- /dev/null +++ b/spec/models/group_associated_group_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe GroupAssociatedGroup do + let(:user) { Fabricate(:user) } + let(:group) { Fabricate(:group) } + let(:group2) { Fabricate(:group) } + let(:associated_group) { Fabricate(:associated_group) } + let(:associated_group2) { Fabricate(:associated_group) } + + before do + UserAssociatedGroup.create(user_id: user.id, associated_group_id: associated_group.id) + @gag = described_class.create(group_id: group.id, associated_group_id: associated_group.id) + end + + it "adds users to group when created" do + expect(group.users.include?(user)).to eq(true) + end + + it "removes users from group when destroyed" do + @gag.destroy! + expect(group.users.include?(user)).to eq(false) + end + + it "does not remove users with multiple associations to group when destroyed" do + UserAssociatedGroup.create(user_id: user.id, associated_group_id: associated_group2.id) + described_class.create(group_id: group.id, associated_group_id: associated_group2.id) + + @gag.destroy! + expect(group.users.include?(user)).to eq(true) + end + + it "removes users with multiple associations to other groups when destroyed" do + UserAssociatedGroup.create(user_id: user.id, associated_group_id: associated_group2.id) + described_class.create(group_id: group2.id, associated_group_id: associated_group2.id) + + @gag.destroy! + expect(group.users.include?(user)).to eq(false) + end +end diff --git a/spec/models/user_associated_group_spec.rb b/spec/models/user_associated_group_spec.rb new file mode 100644 index 00000000000..c686cea9386 --- /dev/null +++ b/spec/models/user_associated_group_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe UserAssociatedGroup do + let(:user) { Fabricate(:user) } + let(:group) { Fabricate(:group) } + let(:group2) { Fabricate(:group) } + let(:associated_group) { Fabricate(:associated_group) } + let(:associated_group2) { Fabricate(:associated_group) } + + before do + GroupAssociatedGroup.create(group_id: group.id, associated_group_id: associated_group.id) + @uag = described_class.create(user_id: user.id, associated_group_id: associated_group.id) + end + + it "adds user to group when created" do + expect(group.users.include?(user)).to eq(true) + end + + it "removes user from group when destroyed" do + @uag.destroy! + expect(group.users.include?(user)).to eq(false) + end + + it "does not remove user with multiple associations from group when destroyed" do + GroupAssociatedGroup.create(group_id: group.id, associated_group_id: associated_group2.id) + described_class.create(user_id: user.id, associated_group_id: associated_group2.id) + + @uag.destroy! + expect(group.users.include?(user)).to eq(true) + end + + it "removes users with multiple associations to other groups when destroyed" do + GroupAssociatedGroup.create(group_id: group2.id, associated_group_id: associated_group2.id) + described_class.create(user_id: user.id, associated_group_id: associated_group2.id) + + @uag.destroy! + expect(group.users.include?(user)).to eq(false) + end +end diff --git a/spec/requests/api/schemas/json/group_response.json b/spec/requests/api/schemas/json/group_response.json index c4b31112f60..a3bf6d151c4 100644 --- a/spec/requests/api/schemas/json/group_response.json +++ b/spec/requests/api/schemas/json/group_response.json @@ -251,6 +251,12 @@ "allow_unknown_sender_topic_replies": { "type": "boolean" }, + "associated_group_ids": { + "type": "array", + "items": [ + + ] + }, "watching_category_ids": { "type": "array", "items": [ diff --git a/spec/requests/api/schemas/json/site_response.json b/spec/requests/api/schemas/json/site_response.json index 82ae9485d0c..ce4bb9c6f1c 100644 --- a/spec/requests/api/schemas/json/site_response.json +++ b/spec/requests/api/schemas/json/site_response.json @@ -358,6 +358,9 @@ "wizard_required": { "type": "boolean" }, + "can_associate_groups": { + "type": "boolean" + }, "topic_featured_link_allowed_category_ids": { "type": "array", "items": [ diff --git a/spec/requests/omniauth_callbacks_controller_spec.rb b/spec/requests/omniauth_callbacks_controller_spec.rb index 8c594d555b2..8d2a0c2a5ad 100644 --- a/spec/requests/omniauth_callbacks_controller_spec.rb +++ b/spec/requests/omniauth_callbacks_controller_spec.rb @@ -694,6 +694,91 @@ RSpec.describe Users::OmniauthCallbacksController do expect(data["username"]).to eq(fixed_username) end + + context "groups are enabled" do + let(:strategy_class) { Auth::OmniAuthStrategies::DiscourseGoogleOauth2 } + let(:groups_url) { "#{strategy_class::GROUPS_DOMAIN}#{strategy_class::GROUPS_PATH}" } + let(:groups_scope) { strategy_class::DEFAULT_SCOPE + strategy_class::GROUPS_SCOPE } + let(:group1) { { id: "12345", name: "group1" } } + let(:group2) { { id: "67890", name: "group2" } } + let(:uid) { "12345" } + let(:token) { "1245678" } + let(:domain) { "mydomain.com" } + + def mock_omniauth_for_groups(groups) + raw_groups = groups.map { |group| OmniAuth::AuthHash.new(group) } + mock_auth = OmniAuth.config.mock_auth[:google_oauth2] + mock_auth[:extra][:raw_groups] = raw_groups + OmniAuth.config.mock_auth[:google_oauth2] = mock_auth + Rails.application.env_config["omniauth.auth"] = mock_auth + end + + before do + SiteSetting.google_oauth2_hd = domain + SiteSetting.google_oauth2_hd_groups = true + end + + it "updates associated groups" do + mock_omniauth_for_groups([group1, group2]) + get "/auth/google_oauth2/callback.json", params: { + scope: groups_scope.split(' '), + code: 'abcde', + hd: domain + } + expect(response.status).to eq(302) + + associated_groups = AssociatedGroup.where(provider_name: 'google_oauth2') + expect(associated_groups.length).to eq(2) + expect(associated_groups.exists?(name: group1[:name])).to eq(true) + expect(associated_groups.exists?(name: group2[:name])).to eq(true) + + user_associated_groups = UserAssociatedGroup.where(user_id: user.id) + expect(user_associated_groups.length).to eq(2) + expect(user_associated_groups.exists?(associated_group_id: associated_groups.first.id)).to eq(true) + expect(user_associated_groups.exists?(associated_group_id: associated_groups.second.id)).to eq(true) + + mock_omniauth_for_groups([group1]) + get "/auth/google_oauth2/callback.json", params: { + scope: groups_scope.split(' '), + code: 'abcde', + hd: domain + } + expect(response.status).to eq(302) + + user_associated_groups = UserAssociatedGroup.where(user_id: user.id) + expect(user_associated_groups.length).to eq(1) + expect(user_associated_groups.exists?(associated_group_id: associated_groups.first.id)).to eq(true) + expect(user_associated_groups.exists?(associated_group_id: associated_groups.second.id)).to eq(false) + + mock_omniauth_for_groups([]) + get "/auth/google_oauth2/callback.json", params: { + scope: groups_scope.split(' '), + code: 'abcde', + hd: domain + } + expect(response.status).to eq(302) + + user_associated_groups = UserAssociatedGroup.where(user_id: user.id) + expect(user_associated_groups.length).to eq(0) + expect(user_associated_groups.exists?(associated_group_id: associated_groups.first.id)).to eq(false) + expect(user_associated_groups.exists?(associated_group_id: associated_groups.second.id)).to eq(false) + end + + it "handles failure to retrieve groups" do + mock_omniauth_for_groups([]) + + get "/auth/google_oauth2/callback.json", params: { + scope: groups_scope.split(' '), + code: 'abcde', + hd: domain + } + + expect(response.status).to eq(302) + + associated_groups = AssociatedGroup.where(provider_name: 'google_oauth2') + expect(associated_groups.exists?).to eq(false) + end + end end context 'when attempting reconnect' do