mirror of
https://github.com/discourse/discourse.git
synced 2025-06-01 09:08:10 +08:00
Use service account credentials for fetching google hd groups (#18329)
The previous implementation would attempt to fetch groups using the end-user's Google auth token. This only worked for admin accounts, or users with 'delegated' access to the `admin.directory.group.readonly` API. This commit changes the approach to use a single 'service account' for fetching the groups. This removes the need to add permissions to all regular user accounts. I'll be updating the [meta docs](https://meta.discourse.org/t/226850) with instructions on setting up the service account. This is technically a breaking change in behavior, but the existing implementation was marked experimental, and is currently unusable in production google workspace environments.
This commit is contained in:
David Taylor
@ -117,7 +117,7 @@ RSpec.describe Auth::GoogleOAuth2Authenticator do
|
||||
group1 = OmniAuth::AuthHash.new(id: "12345", name: "group1")
|
||||
group2 = OmniAuth::AuthHash.new(id: "67890", name: "group2")
|
||||
@groups = [group1, group2]
|
||||
@groups_hash = OmniAuth::AuthHash.new(
|
||||
@auth_hash = OmniAuth::AuthHash.new(
|
||||
provider: "google_oauth2",
|
||||
uid: "123456789",
|
||||
info: {
|
||||
@ -132,26 +132,86 @@ RSpec.describe Auth::GoogleOAuth2Authenticator do
|
||||
email_verified: true,
|
||||
name: "Jane Doe"
|
||||
},
|
||||
raw_groups: @groups
|
||||
}
|
||||
)
|
||||
end
|
||||
|
||||
context "when enabled" do
|
||||
let(:private_key) { OpenSSL::PKey::RSA.generate(2048) }
|
||||
|
||||
let(:group_response) {
|
||||
{
|
||||
groups: [
|
||||
{
|
||||
id: "12345",
|
||||
name: "group1"
|
||||
},
|
||||
{
|
||||
id: "67890",
|
||||
name: "group2"
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
before do
|
||||
SiteSetting.google_oauth2_hd_groups_service_account_admin_email = "admin@example.com"
|
||||
SiteSetting.google_oauth2_hd_groups_service_account_json = {
|
||||
"private_key" => private_key.to_s,
|
||||
"client_email": "discourse-group-sync@example.iam.gserviceaccount.com",
|
||||
}.to_json
|
||||
SiteSetting.google_oauth2_hd_groups = true
|
||||
|
||||
token = "abcde"
|
||||
|
||||
stub_request(:post, "https://oauth2.googleapis.com/token").to_return do |request|
|
||||
jwt = Rack::Utils.parse_query(request.body)["assertion"]
|
||||
decoded_token = JWT.decode(jwt, private_key.public_key, true, { algorithm: 'RS256' })
|
||||
{
|
||||
status: 200,
|
||||
body: { "access_token" => token, "type" => "bearer" }.to_json,
|
||||
headers: { "Content-Type" => "application/json" }
|
||||
}
|
||||
rescue JWT::VerificationError
|
||||
{
|
||||
status: 403,
|
||||
body: "Invalid JWT"
|
||||
}
|
||||
end
|
||||
|
||||
stub_request(:get, "https://admin.googleapis.com/admin/directory/v1/groups?userKey=#{@auth_hash.uid}").
|
||||
with(headers: { "Authorization" => "Bearer #{token}" }).
|
||||
to_return do
|
||||
{
|
||||
status: 200,
|
||||
body: group_response.to_json,
|
||||
headers: {
|
||||
"Content-Type" => "application/json"
|
||||
}
|
||||
}
|
||||
end
|
||||
end
|
||||
|
||||
it "adds associated groups" do
|
||||
result = described_class.new.after_authenticate(@groups_hash)
|
||||
result = described_class.new.after_authenticate(@auth_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)
|
||||
group_response[:groups] = []
|
||||
result = described_class.new.after_authenticate(@auth_hash)
|
||||
expect(result.associated_groups).to eq([])
|
||||
end
|
||||
|
||||
it "doesn't explode with invalid credentials" do
|
||||
SiteSetting.google_oauth2_hd_groups_service_account_json = {
|
||||
"private_key" => OpenSSL::PKey::RSA.generate(2048).to_s,
|
||||
"client_email": "discourse-group-sync@example.iam.gserviceaccount.com",
|
||||
}.to_json
|
||||
|
||||
result = described_class.new.after_authenticate(@auth_hash)
|
||||
expect(result.associated_groups).to eq(nil)
|
||||
end
|
||||
end
|
||||
|
||||
context "when disabled" do
|
||||
@ -160,7 +220,7 @@ RSpec.describe Auth::GoogleOAuth2Authenticator do
|
||||
end
|
||||
|
||||
it "doesnt add associated groups" do
|
||||
result = described_class.new.after_authenticate(@groups_hash)
|
||||
result = described_class.new.after_authenticate(@auth_hash)
|
||||
expect(result.associated_groups).to eq(nil)
|
||||
end
|
||||
end
|
||||
|
||||
@ -1,124 +0,0 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
RSpec.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 'when request_groups is true' do
|
||||
before do
|
||||
@options[:request_groups] = true
|
||||
end
|
||||
|
||||
context 'when 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 'when 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 'when 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
|
||||
Reference in New Issue
Block a user