Commit 266db39e authored by James Lopez's avatar James Lopez

Refactor code to use SCIM token

parent 95167417
...@@ -10,6 +10,13 @@ class ScimOauthAccessToken < ApplicationRecord ...@@ -10,6 +10,13 @@ class ScimOauthAccessToken < ApplicationRecord
validates :group, presence: true validates :group, presence: true
before_save :ensure_token before_save :ensure_token
def self.token_matches_for_group?(token, group)
# Necessary to call `TokenAuthenticatableStrategies::Encrypted.find_token_authenticatable`
token = find_by_token(token)
token && (token.group_id == group.id)
end
def as_entity_json def as_entity_json
ScimOauthAccessTokenEntity.new(self).as_json ScimOauthAccessTokenEntity.new(self).as_json
end end
......
# frozen_string_literal: true # frozen_string_literal: true
class ScimOauthAccessTokenEntity < Grape::Entity class ScimOauthAccessTokenEntity < Grape::Entity
include ::API::Helpers::RelatedResourcesHelpers
SCIM_PATH = '/api/scim/v2/groups'
expose :scim_api_url do |scim| expose :scim_api_url do |scim|
expose_url("#{SCIM_PATH}/#{scim.group.full_path}") Gitlab::Routing.url_helpers.group_scim_oauth_url(scim.group)
end end
expose :token, as: :scim_token expose :token, as: :scim_token
......
...@@ -34,6 +34,20 @@ module API ...@@ -34,6 +34,20 @@ module API
error!({ with: EE::Gitlab::Scim::Error }.merge(detail: message), 409) error!({ with: EE::Gitlab::Scim::Error }.merge(detail: message), 409)
end end
def find_and_authenticate_group!(group_path)
group = find_group(group_path)
scim_not_found!(message: "Group #{group_path} not found") unless group
token = Doorkeeper::OAuth::Token.from_request(current_request, *Doorkeeper.configuration.access_token_methods)
unauthorized! unless token
scim_token = ScimOauthAccessToken.token_matches_for_group?(token, group)
unauthorized! unless scim_token
group
end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def email_taken?(email, identity) def email_taken?(email, identity)
return unless email return unless email
...@@ -53,7 +67,7 @@ module API ...@@ -53,7 +67,7 @@ module API
detail 'This feature was introduced in GitLab 11.9.' detail 'This feature was introduced in GitLab 11.9.'
end end
get do get do
group = find_group(params[:group]) group = find_and_authenticate_group!(params[:group])
scim_error!(message: 'Missing filter params') unless params[:filter] scim_error!(message: 'Missing filter params') unless params[:filter]
...@@ -69,7 +83,7 @@ module API ...@@ -69,7 +83,7 @@ module API
detail 'This feature was introduced in GitLab 11.9.' detail 'This feature was introduced in GitLab 11.9.'
end end
get ':id' do get ':id' do
group = find_group(params[:group]) group = find_and_authenticate_group!(params[:group])
identity = GroupSamlIdentityFinder.find_by_group_and_uid(group: group, uid: params[:id]) identity = GroupSamlIdentityFinder.find_by_group_and_uid(group: group, uid: params[:id])
...@@ -87,7 +101,7 @@ module API ...@@ -87,7 +101,7 @@ module API
patch ':id' do patch ':id' do
scim_error!(message: 'Missing ID') unless params[:id] scim_error!(message: 'Missing ID') unless params[:id]
group = find_group(params[:group]) group = find_and_authenticate_group!(params[:group])
parser = EE::Gitlab::Scim::ParamsParser.new(params) parser = EE::Gitlab::Scim::ParamsParser.new(params)
parsed_hash = parser.to_hash parsed_hash = parser.to_hash
...@@ -125,7 +139,7 @@ module API ...@@ -125,7 +139,7 @@ module API
delete ":id" do delete ":id" do
scim_error!(message: 'Missing ID') unless params[:id] scim_error!(message: 'Missing ID') unless params[:id]
group = find_group(params[:group]) group = find_and_authenticate_group!(params[:group])
identity = GroupSamlIdentityFinder.find_by_group_and_uid(group: group, uid: params[:id]) identity = GroupSamlIdentityFinder.find_by_group_and_uid(group: group, uid: params[:id])
scim_not_found!(message: "Resource #{params[:id]} not found") unless identity scim_not_found!(message: "Resource #{params[:id]} not found") unless identity
......
...@@ -77,7 +77,7 @@ describe Groups::ScimOauthController do ...@@ -77,7 +77,7 @@ describe Groups::ScimOauthController do
end end
it 'shows the url' do it 'shows the url' do
expect(json_response['scim_api_url']).not_to be_empty expect(json_response['scim_api_url']).to eq("http://localhost/groups/#{group.full_path}/-/scim_oauth")
end end
end end
end end
......
...@@ -11,6 +11,18 @@ describe ScimOauthAccessToken do ...@@ -11,6 +11,18 @@ describe ScimOauthAccessToken do
it { is_expected.to validate_presence_of(:group) } it { is_expected.to validate_presence_of(:group) }
end end
describe '.token_matches_for_group?' do
it 'finds the token' do
group = create(:group)
scim_token = create(:scim_oauth_access_token, group: group)
token_value = scim_token.token
expect(described_class.token_matches_for_group?(token_value, group)).to be true
end
end
describe '#token' do describe '#token' do
it 'generates a token on creation' do it 'generates a token on creation' do
scim_token = described_class.create(group: create(:group)) scim_token = described_class.create(group: create(:group))
......
...@@ -6,6 +6,7 @@ describe API::Scim do ...@@ -6,6 +6,7 @@ describe API::Scim do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:identity) { create(:group_saml_identity, user: user) } let(:identity) { create(:group_saml_identity, user: user) }
let(:group) { identity.saml_provider.group } let(:group) { identity.saml_provider.group }
let(:scim_token) { create(:scim_oauth_access_token, group: group) }
before do before do
stub_licensed_features(group_saml: true) stub_licensed_features(group_saml: true)
...@@ -14,15 +15,23 @@ describe API::Scim do ...@@ -14,15 +15,23 @@ describe API::Scim do
end end
describe 'GET api/scim/v2/groups/:group/Users' do describe 'GET api/scim/v2/groups/:group/Users' do
context 'without token auth' do
it 'responds with 401' do
get scim_api("scim/v2/groups/#{group.full_path}/Users?filter=id eq \"#{identity.extern_uid}\"", token: false)
expect(response).to have_gitlab_http_status(401)
end
end
it 'responds with an error if there is no filter' do it 'responds with an error if there is no filter' do
get api("scim/v2/groups/#{group.full_path}/Users", user, version: '') get scim_api("scim/v2/groups/#{group.full_path}/Users")
expect(response).to have_gitlab_http_status(409) expect(response).to have_gitlab_http_status(409)
end end
context 'existing user' do context 'existing user' do
it 'responds with 200' do it 'responds with 200' do
get api("scim/v2/groups/#{group.full_path}/Users?filter=id eq \"#{identity.extern_uid}\"", user, version: '') get scim_api("scim/v2/groups/#{group.full_path}/Users?filter=id eq \"#{identity.extern_uid}\"")
expect(response).to have_gitlab_http_status(200) expect(response).to have_gitlab_http_status(200)
expect(json_response['Resources']).not_to be_empty expect(json_response['Resources']).not_to be_empty
...@@ -32,7 +41,7 @@ describe API::Scim do ...@@ -32,7 +41,7 @@ describe API::Scim do
context 'no user' do context 'no user' do
it 'responds with 200' do it 'responds with 200' do
get api("scim/v2/groups/#{group.full_path}/Users?filter=id eq \"nonexistent\"", user, version: '') get scim_api("scim/v2/groups/#{group.full_path}/Users?filter=id eq \"nonexistent\"")
expect(response).to have_gitlab_http_status(200) expect(response).to have_gitlab_http_status(200)
expect(json_response['Resources']).to be_empty expect(json_response['Resources']).to be_empty
...@@ -43,14 +52,14 @@ describe API::Scim do ...@@ -43,14 +52,14 @@ describe API::Scim do
describe 'GET api/scim/v2/groups/:group/Users/:id' do describe 'GET api/scim/v2/groups/:group/Users/:id' do
it 'responds with 404 if there is no user' do it 'responds with 404 if there is no user' do
get api("scim/v2/groups/#{group.full_path}/Users/123", user, version: '') get scim_api("scim/v2/groups/#{group.full_path}/Users/123")
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(404)
end end
context 'existing user' do context 'existing user' do
it 'responds with 200' do it 'responds with 200' do
get api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}", user, version: '') get scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}")
expect(response).to have_gitlab_http_status(200) expect(response).to have_gitlab_http_status(200)
expect(json_response['id']).to eq(identity.extern_uid) expect(json_response['id']).to eq(identity.extern_uid)
...@@ -60,7 +69,7 @@ describe API::Scim do ...@@ -60,7 +69,7 @@ describe API::Scim do
describe 'PATCH api/scim/v2/groups/:group/Users/:id' do describe 'PATCH api/scim/v2/groups/:group/Users/:id' do
it 'responds with 404 if there is no user' do it 'responds with 404 if there is no user' do
patch api("scim/v2/groups/#{group.full_path}/Users/123", user, version: '') patch scim_api("scim/v2/groups/#{group.full_path}/Users/123")
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(404)
end end
...@@ -70,7 +79,7 @@ describe API::Scim do ...@@ -70,7 +79,7 @@ describe API::Scim do
before do before do
params = { Operations: [{ 'op': 'Replace', 'path': 'id', 'value': 'new_uid' }] }.to_query params = { Operations: [{ 'op': 'Replace', 'path': 'id', 'value': 'new_uid' }] }.to_query
patch api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}?#{params}", user, version: '') patch scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}?#{params}")
end end
it 'responds with 204' do it 'responds with 204' do
...@@ -86,7 +95,7 @@ describe API::Scim do ...@@ -86,7 +95,7 @@ describe API::Scim do
before do before do
params = { Operations: [{ 'op': 'Replace', 'path': 'name.formatted', 'value': 'new_name' }] }.to_query params = { Operations: [{ 'op': 'Replace', 'path': 'name.formatted', 'value': 'new_name' }] }.to_query
patch api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}?#{params}", user, version: '') patch scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}?#{params}")
end end
it 'responds with 204' do it 'responds with 204' do
...@@ -102,7 +111,7 @@ describe API::Scim do ...@@ -102,7 +111,7 @@ describe API::Scim do
before do before do
params = { Operations: [{ 'op': 'Replace', 'path': 'active', 'value': 'False' }] }.to_query params = { Operations: [{ 'op': 'Replace', 'path': 'active', 'value': 'False' }] }.to_query
patch api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}?#{params}", user, version: '') patch scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}?#{params}")
end end
it 'responds with 204' do it 'responds with 204' do
...@@ -119,7 +128,7 @@ describe API::Scim do ...@@ -119,7 +128,7 @@ describe API::Scim do
describe 'DELETE/scim/v2/groups/:group/Users/:id' do describe 'DELETE/scim/v2/groups/:group/Users/:id' do
context 'existing user' do context 'existing user' do
before do before do
delete api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}", user, version: '') delete scim_api("scim/v2/groups/#{group.full_path}/Users/#{identity.extern_uid}")
end end
it 'responds with 204' do it 'responds with 204' do
...@@ -132,9 +141,13 @@ describe API::Scim do ...@@ -132,9 +141,13 @@ describe API::Scim do
end end
it 'responds with 404 if there is no user' do it 'responds with 404 if there is no user' do
delete api("scim/v2/groups/#{group.full_path}/Users/123", user, version: '') delete scim_api("scim/v2/groups/#{group.full_path}/Users/123")
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(404)
end end
end end
def scim_api(url, token: true)
api(url, user, version: '', oauth_access_token: token ? scim_token : nil)
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment