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
This commit is contained in:
Angus McLeod 2021-12-09 20:30:27 +08:00 committed by GitHub
parent 347669ef04
commit df3886d6e5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
44 changed files with 926 additions and 16 deletions

View File

@ -1,11 +1,13 @@
import Component from "@ember/component"; import Component from "@ember/component";
import I18n from "I18n"; import I18n from "I18n";
import { computed } from "@ember/object"; 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 discourseComputed from "discourse-common/utils/decorators";
import AssociatedGroup from "discourse/models/associated-group";
export default Component.extend({ export default Component.extend({
tokenSeparator: "|", tokenSeparator: "|",
showAssociatedGroups: readOnly("site.can_associate_groups"),
init() { init() {
this._super(...arguments); this._super(...arguments);
@ -20,6 +22,10 @@ export default Component.extend({
{ name: 3, value: 3 }, { name: 3, value: 3 },
{ name: 4, value: 4 }, { name: 4, value: 4 },
]; ];
if (this.showAssociatedGroups) {
this.loadAssociatedGroups();
}
}, },
canEdit: not("model.automatic"), canEdit: not("model.automatic"),
@ -54,6 +60,10 @@ export default Component.extend({
return this.model.emailDomains.split(this.tokenSeparator).filter(Boolean); return this.model.emailDomains.split(this.tokenSeparator).filter(Boolean);
}), }),
loadAssociatedGroups() {
AssociatedGroup.list().then((ags) => this.set("associatedGroups", ags));
},
actions: { actions: {
onChangeEmailDomainsSetting(value) { onChangeEmailDomainsSetting(value) {
this.set( this.set(

View File

@ -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;

View File

@ -29,6 +29,11 @@ const Group = RestModel.extend({
return isEmpty(value) ? "" : value; return isEmpty(value) ? "" : value;
}, },
@discourseComputed("associated_group_ids")
associatedGroupIds(value) {
return isEmpty(value) ? [] : value;
},
@discourseComputed("automatic") @discourseComputed("automatic")
type(automatic) { type(automatic) {
return automatic ? "automatic" : "custom"; 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") { if (this.flair_type === "icon") {
attrs["flair_icon"] = this.flair_icon; attrs["flair_icon"] = this.flair_icon;
} else if (this.flair_type === "image") { } else if (this.flair_type === "image") {

View File

@ -59,6 +59,23 @@
onChange=(action "onChangeEmailDomainsSetting") onChange=(action "onChangeEmailDomainsSetting")
options=(hash allowAny=true) options=(hash allowAny=true)
}} }}
{{#if showAssociatedGroups}}
<label for="automatic_membership_associated_groups">
{{i18n "admin.groups.manage.membership.automatic_membership_associated_groups"}}
</label>
{{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}}
</div> </div>
{{plugin-outlet name="groups-form-membership-below-automatic" {{plugin-outlet name="groups-form-membership-below-automatic"

View File

@ -7,9 +7,24 @@ import {
import { click, visit } from "@ember/test-helpers"; import { click, visit } from "@ember/test-helpers";
import selectKit from "discourse/tests/helpers/select-kit-helper"; import selectKit from "discourse/tests/helpers/select-kit-helper";
import { test } from "qunit"; import { test } from "qunit";
import Site from "discourse/models/site";
acceptance("Managing Group Membership", function (needs) { acceptance("Managing Group Membership", function (needs) {
needs.user(); 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) { test("As an admin", async function (assert) {
updateCurrentUser({ can_create_group: true }); updateCurrentUser({ can_create_group: true });
@ -94,6 +109,36 @@ acceptance("Managing Group Membership", function (needs) {
assert.strictEqual(emailDomains.header().value(), "foo.com"); 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) { test("As a group owner", async function (assert) {
updateCurrentUser({ moderator: false, admin: false }); updateCurrentUser({ moderator: false, admin: false });
@ -104,6 +149,11 @@ acceptance("Managing Group Membership", function (needs) {
"it should not display automatic membership label" "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( assert.ok(
!exists(".groups-form-automatic-membership-retroactive"), !exists(".groups-form-automatic-membership-retroactive"),
"it should not display automatic membership retroactive checkbox" "it should not display automatic membership retroactive checkbox"

View File

@ -180,6 +180,10 @@ class Admin::GroupsController < Admin::AdminController
custom_fields = DiscoursePluginRegistry.editable_group_custom_fields custom_fields = DiscoursePluginRegistry.editable_group_custom_fields
permitted << { custom_fields: custom_fields } unless custom_fields.blank? permitted << { custom_fields: custom_fields } unless custom_fields.blank?
if guardian.can_associate_groups?
permitted << { associated_group_ids: [] }
end
permitted = permitted | DiscoursePluginRegistry.group_params permitted = permitted | DiscoursePluginRegistry.group_params
params.require(:group).permit(permitted) params.require(:group).permit(permitted)

View File

@ -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

View File

@ -736,6 +736,10 @@ class GroupsController < ApplicationController
end end
end end
if guardian.can_associate_groups?
permitted_params << { associated_group_ids: [] }
end
permitted_params = permitted_params | DiscoursePluginRegistry.group_params permitted_params = permitted_params | DiscoursePluginRegistry.group_params
params.require(:group).permit(*permitted_params) params.require(:group).permit(*permitted_params)

View File

@ -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 elsif Guardian.new(user).can_access_forum? && user.active # log on any account that is active with forum access
begin begin
user.save! if @auth_result.apply_user_attributes! user.save! if @auth_result.apply_user_attributes!
@auth_result.apply_associated_attributes!
rescue ActiveRecord::RecordInvalid => e rescue ActiveRecord::RecordInvalid => e
@auth_result.failed = true @auth_result.failed = true
@auth_result.failed_reason = e.record.errors.full_messages.join(", ") @auth_result.failed_reason = e.record.errors.full_messages.join(", ")

View File

@ -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

View File

@ -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
#

View File

@ -21,6 +21,7 @@ class Group < ActiveRecord::Base
has_many :group_users, dependent: :destroy has_many :group_users, dependent: :destroy
has_many :group_requests, dependent: :destroy has_many :group_requests, dependent: :destroy
has_many :group_mentions, dependent: :destroy has_many :group_mentions, dependent: :destroy
has_many :group_associated_groups, dependent: :destroy
has_many :group_archived_messages, 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 :reviewables, foreign_key: :reviewable_by_group_id, dependent: :nullify
has_many :group_category_notification_defaults, dependent: :destroy has_many :group_category_notification_defaults, dependent: :destroy
has_many :group_tag_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 :flair_upload, class_name: 'Upload'
belongs_to :smtp_updated_by, class_name: 'User' belongs_to :smtp_updated_by, class_name: 'User'
@ -768,6 +770,20 @@ class Group < ActiveRecord::Base
self self
end 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? def staff?
STAFF_GROUPS.include?(self.name.to_sym) STAFF_GROUPS.include?(self.name.to_sym)
end end

View File

@ -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)
#

View File

@ -38,6 +38,7 @@ class User < ActiveRecord::Base
has_many :reviewable_scores, dependent: :destroy has_many :reviewable_scores, dependent: :destroy
has_many :invites, foreign_key: :invited_by_id, dependent: :destroy has_many :invites, foreign_key: :invited_by_id, dependent: :destroy
has_many :user_custom_fields, 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_many :pending_posts, -> { merge(Reviewable.pending) }, class_name: 'ReviewableQueuedPost', foreign_key: :created_by_id
has_one :user_option, dependent: :destroy 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 :topics_allowed, through: :topic_allowed_users, source: :topic
has_many :groups, through: :group_users has_many :groups, through: :group_users
has_many :secure_categories, through: :groups, source: :categories has_many :secure_categories, through: :groups, source: :categories
has_many :associated_groups, through: :user_associated_groups, dependent: :destroy
# deleted in user_second_factors relationship # deleted in user_second_factors relationship
has_many :totps, -> { has_many :totps, -> {

View File

@ -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)
#

View File

@ -0,0 +1,8 @@
# frozen_string_literal: true
class AssociatedGroupSerializer < ApplicationSerializer
attributes :id,
:name,
:provider_name,
:label
end

View File

@ -36,7 +36,8 @@ class GroupShowSerializer < BasicGroupSerializer
:imap_old_emails, :imap_old_emails,
:imap_new_emails, :imap_new_emails,
:message_count, :message_count,
:allow_unknown_sender_topic_replies :allow_unknown_sender_topic_replies,
:associated_group_ids
def self.admin_or_owner_attributes(*attrs) def self.admin_or_owner_attributes(*attrs)
attributes(*attrs) attributes(*attrs)
@ -121,6 +122,14 @@ class GroupShowSerializer < BasicGroupSerializer
end end
end end
def associated_group_ids
object.associated_groups.map(&:id)
end
def include_associated_group_ids?
scope.can_associate_groups?
end
private private
def authenticated? def authenticated?

View File

@ -21,6 +21,7 @@ class SiteSerializer < ApplicationSerializer
:can_tag_pms, :can_tag_pms,
:tags_filter_regexp, :tags_filter_regexp,
:top_tags, :top_tags,
:can_associate_groups,
:wizard_required, :wizard_required,
:topic_featured_link_allowed_category_ids, :topic_featured_link_allowed_category_ids,
:user_themes, :user_themes,
@ -134,6 +135,14 @@ class SiteSerializer < ApplicationSerializer
scope.can_tag_pms? scope.can_tag_pms?
end end
def can_associate_groups
scope.can_associate_groups?
end
def include_can_associate_groups?
scope.is_admin?
end
def include_tags_filter_regexp? def include_tags_filter_regexp?
SiteSetting.tagging_enabled SiteSetting.tagging_enabled
end end

View File

@ -21,17 +21,19 @@ class GroupActionLogger
)) ))
end end
def log_add_user_to_group(target_user) def log_add_user_to_group(target_user, subject = nil)
GroupHistory.create!(default_params.merge( GroupHistory.create!(default_params.merge(
action: GroupHistory.actions[:add_user_to_group], action: GroupHistory.actions[:add_user_to_group],
target_user: target_user target_user: target_user,
subject: subject
)) ))
end end
def log_remove_user_from_group(target_user) def log_remove_user_from_group(target_user, subject = nil)
GroupHistory.create!(default_params.merge( GroupHistory.create!(default_params.merge(
action: GroupHistory.actions[:remove_user_from_group], action: GroupHistory.actions[:remove_user_from_group],
target_user: target_user target_user: target_user,
subject: subject
)) ))
end end

View File

@ -4124,6 +4124,7 @@ en:
automatic_membership_user_count: automatic_membership_user_count:
one: "%{count} user has the new email domains and will be added to the group." 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." 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" primary_group: "Automatically set as primary group"
name_placeholder: "Group name, no spaces, same as username rule" name_placeholder: "Group name, no spaces, same as username rule"
primary: "Primary Group" primary: "Primary Group"

View File

@ -1724,6 +1724,7 @@ en:
google_oauth2_client_secret: "Client secret of your Google application." 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 <a href='https://developers.google.com/identity/protocols/OpenIDConnect#prompt' target='_blank'>https://developers.google.com/identity/protocols/OpenIDConnect#prompt</a> for the possible values." 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 <a href='https://developers.google.com/identity/protocols/OpenIDConnect#prompt' target='_blank'>https://developers.google.com/identity/protocols/OpenIDConnect#prompt</a> for the possible values."
google_oauth2_hd: "An optional Google Apps Hosted domain that the sign-in will be limited to. See <a href='https://developers.google.com/identity/protocols/OpenIDConnect#hd-param' target='_blank'>https://developers.google.com/identity/protocols/OpenIDConnect#hd-param</a> for more details." google_oauth2_hd: "An optional Google Apps Hosted domain that the sign-in will be limited to. See <a href='https://developers.google.com/identity/protocols/OpenIDConnect#hd-param' target='_blank'>https://developers.google.com/identity/protocols/OpenIDConnect#hd-param</a> 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 <a href='https://meta.discourse.org/t/13395' target='_blank'>Configuring Twitter login (and rich embeds) for Discourse</a>." enable_twitter_logins: "Enable Twitter authentication, requires twitter_consumer_key and twitter_consumer_secret. See <a href='https://meta.discourse.org/t/13395' target='_blank'>Configuring Twitter login (and rich embeds) for Discourse</a>."
twitter_consumer_key: "Consumer key for Twitter authentication, registered at <a href='https://developer.twitter.com/apps' target='_blank'>https://developer.twitter.com/apps</a>" twitter_consumer_key: "Consumer key for Twitter authentication, registered at <a href='https://developer.twitter.com/apps' target='_blank'>https://developer.twitter.com/apps</a>"
@ -2390,6 +2391,7 @@ en:
leading_trailing_slash: "The regular expression must not start and end with a slash." 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." unicode_usernames_avatars: "The internal system avatars do not support Unicode usernames."
list_value_count: "The list must contain exactly %{count} values." 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: placeholder:
discourse_connect_provider_secrets: discourse_connect_provider_secrets:

View File

@ -634,6 +634,8 @@ Discourse::Application.routes.draw do
end end
end end
resources :associated_groups, only: %i[index], constraints: AdminConstraint.new
# aliases so old API code works # aliases so old API code works
delete "admin/groups/:id/members" => "groups#remove_member", constraints: AdminConstraint.new delete "admin/groups/:id/members" => "groups#remove_member", constraints: AdminConstraint.new
put "admin/groups/:id/members" => "groups#add_members", constraints: AdminConstraint.new put "admin/groups/:id/members" => "groups#add_members", constraints: AdminConstraint.new

View File

@ -411,6 +411,9 @@ login:
- "select_account" - "select_account"
google_oauth2_hd: google_oauth2_hd:
default: "" default: ""
google_oauth2_hd_groups:
default: false
validator: GoogleOauth2HdGroupsValidator
enable_twitter_logins: enable_twitter_logins:
default: false default: false
twitter_consumer_key: twitter_consumer_key:

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -6,6 +6,7 @@ require 'auth/auth_provider'
require 'auth/result' require 'auth/result'
require 'auth/authenticator' require 'auth/authenticator'
require 'auth/managed_authenticator' require 'auth/managed_authenticator'
require 'auth/omniauth_strategies/discourse_google_oauth2'
require 'auth/facebook_authenticator' require 'auth/facebook_authenticator'
require 'auth/github_authenticator' require 'auth/github_authenticator'
require 'auth/twitter_authenticator' require 'auth/twitter_authenticator'

View File

@ -65,4 +65,9 @@ class Auth::Authenticator
def revoke(user, skip_remote: false) def revoke(user, skip_remote: false)
raise NotImplementedError raise NotImplementedError
end end
# provider has implemented user group membership (or equivalent) request
def provides_groups?
false
end
end end

View File

@ -16,6 +16,7 @@ class Auth::GoogleOAuth2Authenticator < Auth::ManagedAuthenticator
end end
def register_middleware(omniauth) def register_middleware(omniauth)
strategy_class = Auth::OmniAuthStrategies::DiscourseGoogleOauth2
options = { options = {
setup: lambda { |env| setup: lambda { |env|
strategy = env["omniauth.strategy"] 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. # the JWT can fail due to clock skew, so let's skip it completely.
# https://github.com/zquestz/omniauth-google-oauth2/pull/392 # https://github.com/zquestz/omniauth-google-oauth2/pull/392
strategy.options[:skip_jwt] = true 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
end end

View File

@ -113,14 +113,16 @@ class Auth::ManagedAuthenticator < Auth::Authenticator
result result
end end
def after_create_account(user, auth) def after_create_account(user, auth_result)
auth_token = auth[:extra_data] auth_token = auth_result[:extra_data]
association = UserAssociatedAccount.find_or_initialize_by(provider_name: auth_token[:provider], provider_uid: auth_token[:uid]) association = UserAssociatedAccount.find_or_initialize_by(provider_name: auth_token[:provider], provider_uid: auth_token[:uid])
association.user = user association.user = user
association.save! association.save!
retrieve_avatar(user, association.info["image"]) retrieve_avatar(user, association.info["image"])
retrieve_profile(user, association.info) retrieve_profile(user, association.info)
auth_result.apply_associated_attributes!
end end
def find_user_by_email(auth_token) def find_user_by_email(auth_token)

View File

@ -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

View File

@ -21,7 +21,8 @@ class Auth::Result
:omniauth_disallow_totp, :omniauth_disallow_totp,
:failed, :failed,
:failed_reason, :failed_reason,
:failed_code :failed_code,
:associated_groups
] ]
attr_accessor *ATTRIBUTES attr_accessor *ATTRIBUTES
@ -36,7 +37,8 @@ class Auth::Result
:name, :name,
:authenticator_name, :authenticator_name,
:extra_data, :extra_data,
:skip_email_validation :skip_email_validation,
:associated_groups
] ]
def [](key) def [](key)
@ -94,6 +96,29 @@ class Auth::Result
change_made change_made
end 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 def can_edit_name
!SiteSetting.auth_overrides_name !SiteSetting.auth_overrides_name
end end
@ -167,6 +192,10 @@ class Auth::Result
username || name || email username || name || email
end end
def authenticator
@authenticator ||= Discourse.enabled_authenticators.find { |a| a.name == authenticator_name }
end
def resolve_username def resolve_username
if staged_user if staged_user
if !username.present? || UserNameSuggester.fix_username(username) == staged_user.username if !username.present? || UserNameSuggester.fix_username(username) == staged_user.username

View File

@ -36,4 +36,8 @@ module GroupGuardian
SiteSetting.enable_personal_messages? && group.users.include?(user) SiteSetting.enable_personal_messages? && group.users.include?(user)
end end
def can_associate_groups?
is_admin? && AssociatedGroup.has_provider?
end
end end

View File

@ -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

View File

@ -3,7 +3,6 @@
require 'rails_helper' require 'rails_helper'
describe Auth::GoogleOAuth2Authenticator do describe Auth::GoogleOAuth2Authenticator do
it 'does not look up user unless email is verified' do it 'does not look up user unless email is verified' do
# note, emails that come back from google via omniauth are always valid # note, emails that come back from google via omniauth are always valid
# this protects against future regressions # this protects against future regressions
@ -113,6 +112,61 @@ describe Auth::GoogleOAuth2Authenticator do
expect(result.user).to eq(nil) expect(result.user).to eq(nil)
expect(result.name).to eq("Jane Doe") expect(result.name).to eq("Jane Doe")
end 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 end
context 'revoke' do context 'revoke' do

View File

@ -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 describe 'after_authenticate' do
it 'can match account from an existing association' do it 'can match account from an existing association' do
user = Fabricate(:user) user = Fabricate(:user)
@ -250,14 +256,14 @@ describe Auth::ManagedAuthenticator do
let!(:association) { UserAssociatedAccount.create!(provider_name: 'myauth', provider_uid: "1234") } let!(:association) { UserAssociatedAccount.create!(provider_name: 'myauth', provider_uid: "1234") }
it "doesn't schedule with no image" do 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) .to change { Jobs::DownloadAvatarFromUrl.jobs.count }.by(0)
end end
it "schedules with image" do it "schedules with image" do
association.info["image"] = "https://some.domain/image.jpg" association.info["image"] = "https://some.domain/image.jpg"
association.save! 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) .to change { Jobs::DownloadAvatarFromUrl.jobs.count }.by(1)
end end
end end
@ -267,14 +273,14 @@ describe Auth::ManagedAuthenticator do
let!(:association) { UserAssociatedAccount.create!(provider_name: 'myauth', provider_uid: "1234") } let!(:association) { UserAssociatedAccount.create!(provider_name: 'myauth', provider_uid: "1234") }
it "doesn't explode without profile" do 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 end
it "works with profile" do it "works with profile" do
association.info["location"] = "DiscourseVille" association.info["location"] = "DiscourseVille"
association.info["description"] = "Online forum expert" association.info["description"] = "Online forum expert"
association.save! 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.bio_raw).to eq("Online forum expert")
expect(user.user_profile.location).to eq("DiscourseVille") expect(user.user_profile.location).to eq("DiscourseVille")
end end

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -251,6 +251,12 @@
"allow_unknown_sender_topic_replies": { "allow_unknown_sender_topic_replies": {
"type": "boolean" "type": "boolean"
}, },
"associated_group_ids": {
"type": "array",
"items": [
]
},
"watching_category_ids": { "watching_category_ids": {
"type": "array", "type": "array",
"items": [ "items": [

View File

@ -358,6 +358,9 @@
"wizard_required": { "wizard_required": {
"type": "boolean" "type": "boolean"
}, },
"can_associate_groups": {
"type": "boolean"
},
"topic_featured_link_allowed_category_ids": { "topic_featured_link_allowed_category_ids": {
"type": "array", "type": "array",
"items": [ "items": [

View File

@ -694,6 +694,91 @@ RSpec.describe Users::OmniauthCallbacksController do
expect(data["username"]).to eq(fixed_username) expect(data["username"]).to eq(fixed_username)
end 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 end
context 'when attempting reconnect' do context 'when attempting reconnect' do