Commit f8700d07 authored by James Lopez's avatar James Lopez Committed by Douglas Barbosa Alexandre

Resolve "Create a user via SCIM"

parent b7ac0167
......@@ -26,7 +26,7 @@ module Users
end
end
identity_attrs = params.slice(:extern_uid, :provider)
identity_attrs = params.slice(*identity_params)
unless identity_attrs.empty?
user.identities.build(identity_attrs)
......@@ -37,6 +37,10 @@ module Users
private
def identity_params
[:extern_uid, :provider]
end
def can_create_user?
(current_user.nil? && Gitlab::CurrentSettings.allow_signup?) || current_user&.admin?
end
......
......@@ -49,6 +49,8 @@ Example response:
"id": "0b1d561c-21ff-4092-beab-8154b17f82f2",
"active": true,
"name.formatted": "Test User",
"userName": "username",
"meta": { "resourceType":"User" },
"emails": [
{
"type": "work",
......@@ -90,6 +92,8 @@ Example response:
"id": "0b1d561c-21ff-4092-beab-8154b17f82f2",
"active": true,
"name.formatted": "Test User",
"userName": "username",
"meta": { "resourceType":"User" },
"emails": [
{
"type": "work",
......@@ -100,16 +104,63 @@ Example response:
}
```
## Create a SAML user
```text
POST /api/scim/v2/groups/:group_path/Users/
```
Parameters:
| Attribute | Type | Required | Description |
|:---------------|:----------|:----|:--------------------------|
| `externalId` | string | yes | External UID of the user. |
| `userName` | string | yes | Username of the user. |
| `emails` | JSON string | yes | Work email. |
| `name` | JSON string | yes | Name of the user. |
| `meta` | string | no | Resource type (`User'). |
Example request:
```sh
curl --verbose --request POST 'https://example.gitlab.com/api/scim/v2/groups/test_group/Users' --data '{"externalId":"test_uid","active":null,"userName":"username","emails":[{"primary":true,"type":"work","value":"name@example.com"}],"name":{"formatted":"Test User","familyName":"User","givenName":"Test"},"schemas":["urn:ietf:params:scim:schemas:core:2.0:User"],"meta":{"resourceType":"User"}}' --header "Authorization: Bearer <your_scim_token>" --header "Content-Type: application/scim+json"
```
Example response:
```json
{
"schemas": [
"urn:ietf:params:scim:schemas:core:2.0:User"
],
"id": "0b1d561c-21ff-4092-beab-8154b17f82f2",
"active": true,
"name.formatted": "Test User",
"userName": "username",
"meta": { "resourceType":"User" },
"emails": [
{
"type": "work",
"value": "name@example.com",
"primary": true
}
]
}
```
Returns a `201` status code if successful.
## Update a single SAML user
Fields that can be updated are:
| SCIM/IdP field | GitLab field |
|:----------|:--------|
| id | extern_uid |
| id/externalId | extern_uid |
| name.formatted | name |
| emails\[type eq "work"\].value | email |
| active | Identity removal if `active = false` |
| userName | username |
```text
PATCH /api/scim/v2/groups/:group_path/Users/:id
......
......@@ -6,6 +6,7 @@ GitLab's [SCIM API](../../../api/scim.md) implements part of [the RFC7644 protoc
Currently, the following actions are available:
- CREATE
- UPDATE
- DELETE (deprovisioning)
......@@ -71,8 +72,10 @@ You can then test the connection clicking on `Test Connection`.
1. Click on `Synchronize Azure Active Directory Users to AppName`, to configure
the attribute mapping.
1. Select the unique identifier (in the example `objectId`) as the `id` and enable
the`Update` and `Create` actions.
1. Select the unique identifier (in the example `objectId`) as the `id` and `externalId`,
and enable the `Create`, `Update`, and `Delete` actions.
1. Map the `userPricipalName` to `emails[type eq "work"].value` and `mailNickname` to
`userName`.
Example configuration:
......
......@@ -16,5 +16,11 @@ module EE
scope :non_owners, -> { where("members.access_level < ?", ::Gitlab::Access::OWNER) }
end
class_methods do
def member_of_group?(group, user)
exists?(group: group, user: user)
end
end
end
end
......@@ -30,6 +30,11 @@ module EE
]
end
override :identity_params
def identity_params
super.push(:saml_provider_id)
end
def build_smartcard_identity(user, params)
smartcard_identity_attrs = params.slice(:certificate_subject, :certificate_issuer)
......
---
title: Create a user via SCIM
merge_request: 10456
author:
type: added
......@@ -26,12 +26,20 @@ module API
false
end
def render_scim_error(error_class, message)
error!({ with: error_class }.merge(detail: message), error_class::STATUS)
end
def scim_not_found!(message:)
error!({ with: EE::Gitlab::Scim::NotFound }.merge(detail: message), 404)
render_scim_error(EE::Gitlab::Scim::NotFound, message)
end
def scim_error!(message:)
error!({ with: EE::Gitlab::Scim::Error }.merge(detail: message), 409)
render_scim_error(EE::Gitlab::Scim::Error, message)
end
def scim_conflict!(message:)
render_scim_error(EE::Gitlab::Scim::Conflict, message)
end
def find_and_authenticate_group!(group_path)
......@@ -51,14 +59,14 @@ module API
# rubocop: disable CodeReuse/ActiveRecord
def update_scim_user(identity)
parser = EE::Gitlab::Scim::ParamsParser.new(params)
parsed_hash = parser.to_hash
parsed_hash = parser.result
if parser.deprovision_user?
destroy_identity(identity)
elsif parsed_hash[:extern_uid]
identity.update(parsed_hash.slice(:extern_uid))
else
scim_error!(message: 'Email has already been taken') if email_taken?(parsed_hash[:email], identity)
scim_conflict!(message: 'Email has already been taken') if email_taken?(parsed_hash[:email], identity)
result = ::Users::UpdateService.new(identity.user,
parsed_hash.except(:extern_uid)
......@@ -67,15 +75,12 @@ module API
result[:status] == :success
end
end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def email_taken?(email, identity)
return unless email
User.by_any_email(email.downcase).where.not(id: identity.user.id).exists?
end
# rubocop: enable CodeReuse/ActiveRecord
end
resource :Users do
......@@ -88,11 +93,10 @@ module API
detail 'This feature was introduced in GitLab 11.10.'
end
get do
group = find_and_authenticate_group!(params[:group])
scim_error!(message: 'Missing filter params') unless params[:filter]
parsed_hash = EE::Gitlab::Scim::ParamsParser.new(params).to_hash
group = find_and_authenticate_group!(params[:group])
parsed_hash = EE::Gitlab::Scim::ParamsParser.new(params).result
identity = GroupSamlIdentityFinder.find_by_group_and_uid(group: group, uid: parsed_hash[:extern_uid])
status 200
......@@ -114,6 +118,27 @@ module API
present identity, with: ::EE::Gitlab::Scim::User
end
desc 'Create a SAML user' do
detail 'This feature was introduced in GitLab 11.10.'
end
post do
group = find_and_authenticate_group!(params[:group])
parser = EE::Gitlab::Scim::ParamsParser.new(params)
result = EE::Gitlab::Scim::ProvisioningService.new(group, parser.result).execute
case result.status
when :success
status 201
present result.identity, with: ::EE::Gitlab::Scim::User
when :conflict
scim_conflict!(message: "Error saving user with #{params.inspect}: #{result.message}")
when :error
scim_error!(message: ["Error saving user with #{params.inspect}", result.message].compact.join(": "))
end
end
desc 'Updates a SAML user' do
detail 'This feature was introduced in GitLab 11.10.'
end
......
# frozen_string_literal: true
module EE
module Gitlab
module Scim
class Conflict < Error
STATUS = 409
end
end
end
end
......@@ -4,6 +4,8 @@ module EE
module Gitlab
module Scim
class Error < Grape::Entity
STATUS = 412
expose :schemas
expose :detail, safe: true
expose :status
......@@ -17,7 +19,7 @@ module EE
end
def status
409
self.class::STATUS
end
end
end
......
......@@ -4,9 +4,7 @@ module EE
module Gitlab
module Scim
class NotFound < EE::Gitlab::Scim::Error
def status
404
end
STATUS = 404
end
end
end
......
......@@ -11,7 +11,9 @@ module EE
id: :extern_uid,
'name.formatted': :name,
'emails[type eq "work"].value': :email,
active: :active
active: :active,
externalId: :extern_uid,
userName: :username
}.with_indifferent_access.freeze
COERCED_VALUES = {
......@@ -20,46 +22,42 @@ module EE
}.freeze
def initialize(params)
@filter = params[:filter]
@operations = params[:Operations]
@params = params.with_indifferent_access
@hash = {}
end
def deprovision_user?
data[:active] == false
result[:active] == false
end
def to_hash
@data ||=
begin
hash = {}
process_filter(hash)
process_operations(hash)
hash
end
def result
@result ||= process
end
alias_method :data, :to_hash
private :data
private
def process_filter(hash)
return unless @filter
def process
if @params[:filter]
process_filter
elsif @params[:Operations]
process_operations
else
# SCIM POST params
process_params
end
end
attribute, operator, value = @filter.split(' ')
def process_filter
attribute, operator, value = @params[:filter].split(' ')
return unless FILTER_OPERATORS.include?(operator)
return unless ATTRIBUTE_MAP[attribute]
return {} unless FILTER_OPERATORS.include?(operator)
return {} unless ATTRIBUTE_MAP[attribute]
hash[ATTRIBUTE_MAP[attribute]] = coerce(value.delete('\"'))
{ ATTRIBUTE_MAP[attribute] => coerce(value) }
end
def process_operations(hash)
return unless @operations
@operations.each do |operation|
def process_operations
@params[:Operations].each_with_object({}) do |operation, hash|
next unless OPERATIONS_OPERATORS.include?(operation[:op])
attribute = ATTRIBUTE_MAP[operation[:path]]
......@@ -68,7 +66,43 @@ module EE
end
end
def process_params
parse_params.merge(
email: parse_emails,
name: parse_name
).compact # so if parse_emails returns nil, it'll be removed from the hash
end
# rubocop: disable CodeReuse/ActiveRecord
def parse_params
# compact can remove :active if the value for that is nil
@params.except(:email, :name).compact.each_with_object({}) do |(param, value), hash|
attribute = ATTRIBUTE_MAP[param]
hash[attribute] = coerce(value) if attribute
end
end
# rubocop: enable CodeReuse/ActiveRecord
def parse_emails
emails = @params[:emails]
return unless emails
email = emails.find { |email| email[:type] == 'work' || email[:primary] }
email[:value] if email
end
def parse_name
name = @params.delete(:name)
@hash[:name] = name[:formatted] if name
end
def coerce(value)
return value unless value.is_a?(String)
value = value.delete('\"')
coerced = COERCED_VALUES[value]
coerced.nil? ? value : coerced
......
# frozen_string_literal: true
module EE
module Gitlab
module Scim
class ProvisioningResponse
attr_reader :status, :message, :identity
def initialize(status:, message: nil, identity: nil)
@status = status
@message = message
@identity = identity
end
end
end
end
end
# frozen_string_literal: true
module EE
module Gitlab
module Scim
class ProvisioningService
include ::Gitlab::Utils::StrongMemoize
IDENTITY_PROVIDER = 'group_saml'
PASSWORD_AUTOMATICALLY_SET = true
SKIP_EMAIL_CONFIRMATION = true
DEFAULT_ACCESS = :guest
def initialize(group, parsed_hash)
@group = group
@parsed_hash = parsed_hash.dup
end
def execute
return success_response if existing_member?
clear_memoization(:identity)
if user.save && member.errors.empty?
success_response
else
error_response
end
rescue => e
logger.error(error: e.class.name, message: e.message, source: "#{__FILE__}:#{__LINE__}")
error_response(errors: [e.message])
end
private
def success_response
ProvisioningResponse.new(status: :success, identity: identity)
end
def identity
strong_memoize(:identity) do
::Identity.with_extern_uid(IDENTITY_PROVIDER, @parsed_hash[:extern_uid]).first
end
end
def user
@user ||= ::Users::BuildService.new(nil, user_params).execute(skip_authorization: true)
end
def error_response(errors: nil)
errors ||= [user, identity, member].compact.map { |obj| obj.errors.full_messages }.flatten
conflict = errors.any? { |error| error.include?('has already been taken') }
ProvisioningResponse.new(status: conflict ? :conflict : :error, message: errors.to_sentence)
rescue => e
logger.error(error: e.class.name, message: e.message, source: "#{__FILE__}:#{__LINE__}")
ProvisioningResponse.new(status: :error, message: e.message)
end
def logger
::API::API.logger
end
def user_params
@parsed_hash.tap do |hash|
hash[:skip_confirmation] = SKIP_EMAIL_CONFIRMATION
hash[:saml_provider_id] = @group.saml_provider.id
hash[:provider] = IDENTITY_PROVIDER
hash[:email_confirmation] = hash[:email]
hash[:username] = valid_username
hash[:password] = hash[:password_confirmation] = random_password
hash[:password_automatically_set] = PASSWORD_AUTOMATICALLY_SET
end
end
def random_password
Devise.friendly_token.first(Devise.password_length.min)
end
def valid_username
clean_username = ::Namespace.clean_path(@parsed_hash[:username])
Uniquify.new.string(clean_username) { |s| !NamespacePathValidator.valid_path?(s) }
end
def member
@member ||= @group.add_user(user, DEFAULT_ACCESS)
end
def existing_member?
identity && ::GroupMember.member_of_group?(@group, identity.user)
end
end
end
end
end
......@@ -7,11 +7,17 @@ module EE
expose :schemas
expose :extern_uid, as: :id
expose :active
expose :email_user, as: :emails, using: '::EE::Gitlab::Scim::Emails'
expose 'name.formatted' do |identity, _options|
identity.user.name
end
expose :email_user, as: :emails, using: '::EE::Gitlab::Scim::Emails'
expose :meta do
expose :resource_type, as: :resourceType
end
expose :username, as: :userName do |identity, _options|
identity.user.username
end
private
......@@ -38,6 +44,10 @@ module EE
def email_user
[object.user]
end
def resource_type
'User'
end
end
end
end
......
# frozen_string_literal: true
require 'spec_helper'
describe ::EE::Gitlab::Scim::Conflict do
let(:params) { { detail: 'error' } }
let(:entity) do
described_class.new(params)
end
subject { entity.as_json }
it 'contains the schemas' do
expect(subject[:schemas]).not_to be_empty
end
it 'contains the detail' do
expect(subject[:detail]).to eq(params[:detail])
end
it 'contains the status' do
expect(subject[:status]).to eq(409)
end
end
......@@ -19,6 +19,6 @@ describe ::EE::Gitlab::Scim::Error do
end
it 'contains the status' do
expect(subject[:status]).to eq(409)
expect(subject[:status]).to eq(412)
end
end
......@@ -3,41 +3,60 @@
require 'spec_helper'
describe EE::Gitlab::Scim::ParamsParser do
describe '#to_hash' do
describe '#result' do
it 'returns the correct filter attributes' do
filter = 'id eq "6ba81b08-77da"'
expect(described_class.new(filter: filter).to_hash).to eq(extern_uid: '6ba81b08-77da')
expect(described_class.new(filter: filter).result).to eq(extern_uid: '6ba81b08-77da')
end
it 'returns an empty hash for the wrong filter' do
filter = 'blah eq "6ba81b08-77da"'
expect(described_class.new(filter: filter).to_hash).to eq({})
expect(described_class.new(filter: filter).result).to eq({})
end
it 'returns the correct operation attributes' do
operations = [{ "op": "Replace", "path": "active", "value": "False" }]
operations = [{ 'op': 'Replace', 'path': 'active', 'value': 'False' }]
expect(described_class.new(Operations: operations).to_hash).to eq(active: false)
expect(described_class.new(Operations: operations).result).to eq(active: false)
end
it 'returns an empty hash for the wrong operations' do
operations = [{ "op": "Replace", "path": "test", "value": "False" }]
operations = [{ 'op': 'Replace', 'path': 'test', 'value': 'False' }]
expect(described_class.new(Operations: operations).to_hash).to eq({})
expect(described_class.new(Operations: operations).result).to eq({})
end
it 'returns a parsed hash for POST params' do
params = {
externalId: 'test',
active: nil,
userName: 'username',
emails: [
{ primary: nil, type: 'work', value: 'work@example.com' },
{ primary: nil, type: 'home', value: 'home@example.com' }
],
name: { formatted: 'Test Name', familyName: 'Name', givenName: 'Test' },
extra: true
}
expect(described_class.new(params).result).to eq(email: 'work@example.com',
extern_uid: 'test',
name: 'Test Name',
username: 'username')
end
end
describe '#deprovision_user?' do
it 'returns true when deprovisioning' do
operations = [{ "op": "Replace", "path": "active", "value": "False" }]
operations = [{ 'op': 'Replace', 'path': 'active', 'value': 'False' }]
expect(described_class.new(Operations: operations).deprovision_user?).to be true
end
it 'returns false when not deprovisioning' do
operations = [{ "op": "Replace", "path": "active", "value": "True" }]
operations = [{ 'op': 'Replace', 'path': 'active', 'value': 'True' }]
expect(described_class.new(Operations: operations).deprovision_user?).to be false
end
......
# frozen_string_literal: true
require 'spec_helper'
describe ::EE::Gitlab::Scim::ProvisioningService do
describe '#execute' do
let(:group) { create(:group) }
let(:service) { described_class.new(group, service_params) }
before do
stub_licensed_features(group_saml: true)
create(:saml_provider, group: group)
end
context 'valid params' do
let(:service_params) do
{
email: 'work@example.com',
name: 'Test Name',
extern_uid: 'test_uid',
username: 'username'
}.freeze
end
it 'succeeds' do
expect(service.execute.status).to eq(:success)
end
it 'creates the identity' do
expect { service.execute }.to change { Identity.count }.by(1)
end
it 'creates the user' do
expect { service.execute }.to change { User.count }.by(1)
end
it 'creates the group member' do
expect { service.execute }.to change { GroupMember.count }.by(1)
end
it 'creates the correct user attributes' do
service.execute
expect(User.find_by(service_params.except(:extern_uid))).to be_a(User)
end
context 'existing user' do
before do
create(:user, email: 'work@example.com')
end
it 'does not create a new user' do
expect { service.execute }.not_to change { User.count }
end
it 'fails with conflict' do
expect(service.execute.status).to eq(:conflict)
end
end
end
context 'invalid params' do
let(:service_params) do
{
email: 'work@example.com',
name: 'Test Name',
extern_uid: 'test_uid'
}.freeze
end
it 'fails with error' do
expect(service.execute.status).to eq(:error)
end
end
end
end
......@@ -31,4 +31,12 @@ describe ::EE::Gitlab::Scim::User do
it 'contains the email' do
expect(subject[:emails].first[:value]).to eq(user.email)
end
it 'contains the username' do
expect(subject[:userName]).to eq(user.username)
end
it 'contains the resource type' do
expect(subject[:meta][:resourceType]).to eq('User')
end
end
......@@ -26,7 +26,7 @@ describe API::Scim do
it 'responds with an error if there is no filter' do
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(412)
end
context 'existing user' do
......@@ -67,6 +67,78 @@ describe API::Scim do
end
end
describe 'POST api/scim/v2/groups/:group/Users' do
let(:post_params) do
{
externalId: 'test_uid',
active: nil,
userName: 'username',
emails: [
{ primary: true, type: 'work', value: 'work@example.com' }
],
name: { formatted: 'Test Name', familyName: 'Name', givenName: 'Test' }
}.to_query
end
context 'without an existing user' do
let(:new_user) { User.find_by_email('work@example.com') }
let(:member) { GroupMember.find_by(user: new_user, group: group) }
before do
post scim_api("scim/v2/groups/#{group.full_path}/Users?params=#{post_params}")
end
it 'responds with 201' do
expect(response).to have_gitlab_http_status(201)
end
it 'has the user external ID' do
expect(json_response['id']).to eq('test_uid')
end
it 'has the email' do
expect(json_response['emails'].first['value']).to eq('work@example.com')
end
it 'created the identity' do
expect(Identity.find_by_extern_uid(:group_saml, 'test_uid')).not_to be_nil
end
it 'has the right saml provider' do
identity = Identity.find_by_extern_uid(:group_saml, 'test_uid')
expect(identity.saml_provider_id).to eq(group.saml_provider.id)
end
it 'created the user' do
expect(new_user).not_to be_nil
end
it 'created the right member' do
expect(member.access_level).to eq(::Gitlab::Access::GUEST)
end
end
context 'existing user' do
before do
old_user = create(:user, email: 'work@example.com')
create(:group_saml_identity, user: old_user, extern_uid: 'test_uid')
group.add_guest(old_user)
post scim_api("scim/v2/groups/#{group.full_path}/Users?params=#{post_params}")
end
it 'responds with 201' do
expect(response).to have_gitlab_http_status(201)
end
it 'has the user external ID' do
expect(json_response['id']).to eq('test_uid')
end
end
end
describe 'PATCH api/scim/v2/groups/:group/Users/:id' do
it 'responds with 404 if there is no user' do
patch scim_api("scim/v2/groups/#{group.full_path}/Users/123")
......
# frozen_string_literal: true
require 'spec_helper'
describe Users::BuildService do
describe '#execute' do
let(:params) do
{ name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: 'mydummypass' }
end
context 'with an admin user' do
let!(:admin_user) { create(:admin) }
let(:service) { described_class.new(admin_user, ActionController::Parameters.new(params).permit!) }
context 'allowed params' do
context 'with identity' do
let(:provider) { create(:saml_provider) }
let(:identity_params) { { extern_uid: 'uid', provider: 'group_saml', saml_provider_id: provider.id } }
before do
params.merge!(identity_params)
end
it 'sets all allowed attributes' do
expect(Identity).to receive(:new).with(hash_including(identity_params)).and_call_original
service.execute
end
end
end
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