Skip to content
体验新版
项目
组织
正在加载...
登录
切换导航
打开侧边栏
李少辉-开发者
gitlab-foss
提交
88d610c6
G
gitlab-foss
项目概览
李少辉-开发者
/
gitlab-foss
通知
15
Star
0
Fork
0
代码
文件
提交
分支
Tags
贡献者
分支图
Diff
Issue
0
列表
看板
标记
里程碑
合并请求
0
Wiki
0
Wiki
分析
仓库
DevOps
项目成员
Pages
G
gitlab-foss
项目概览
项目概览
详情
发布
仓库
仓库
文件
提交
分支
标签
贡献者
分支图
比较
Issue
0
Issue
0
列表
看板
标记
里程碑
合并请求
0
合并请求
0
Pages
分析
分析
仓库分析
DevOps
Wiki
0
Wiki
成员
成员
收起侧边栏
关闭侧边栏
动态
分支图
创建新Issue
提交
Issue看板
体验新版 GitCode,发现更多精彩内容 >>
提交
88d610c6
编写于
2月 09, 2017
作者:
J
Jarka Kadlecova
浏览文件
操作
浏览文件
下载
电子邮件补丁
差异文件
Add member: Always return 409 when a member exists
上级
0fddece7
变更
7
隐藏空白更改
内联
并排
Showing
7 changed file
with
487 addition
and
12 deletion
+487
-12
changelogs/unreleased/20732_member_exists_409.yml
changelogs/unreleased/20732_member_exists_409.yml
+4
-0
doc/api/v3_to_v4.md
doc/api/v3_to_v4.md
+1
-0
lib/api/api.rb
lib/api/api.rb
+1
-0
lib/api/members.rb
lib/api/members.rb
+3
-10
lib/api/v3/members.rb
lib/api/v3/members.rb
+134
-0
spec/requests/api/members_spec.rb
spec/requests/api/members_spec.rb
+2
-2
spec/requests/api/v3/members_spec.rb
spec/requests/api/v3/members_spec.rb
+342
-0
未找到文件。
changelogs/unreleased/20732_member_exists_409.yml
0 → 100644
浏览文件 @
88d610c6
---
title
:
'
Add
member:
Always
return
409
when
a
member
exists'
merge_request
:
author
:
doc/api/v3_to_v4.md
浏览文件 @
88d610c6
...
...
@@ -12,3 +12,4 @@ changes are in V4:
-
Endpoints under
`projects/merge_request/:id`
have been removed (use:
`projects/merge_requests/:id`
)
-
Project snippets do not return deprecated field
`expires_at`
-
Endpoints under
`projects/:id/keys`
have been removed (use
`projects/:id/deploy_keys`
)
-
Status 409 returned for POST
`project/:id/members`
when a member already exists
lib/api/api.rb
浏览文件 @
88d610c6
...
...
@@ -7,6 +7,7 @@ module API
version
'v3'
,
using: :path
do
mount
::
API
::
V3
::
DeployKeys
mount
::
API
::
V3
::
Issues
mount
::
API
::
V3
::
Members
mount
::
API
::
V3
::
MergeRequests
mount
::
API
::
V3
::
Projects
mount
::
API
::
V3
::
ProjectSnippets
...
...
lib/api/members.rb
浏览文件 @
88d610c6
...
...
@@ -56,16 +56,9 @@ module API
member
=
source
.
members
.
find_by
(
user_id:
params
[
:user_id
])
# We need this explicit check because `source.add_user` doesn't
# currently return the member created so it would return 201 even if
# the member already existed...
# The `source_type == 'group'` check is to ensure back-compatibility
# but 409 behavior should be used for both project and group members in 9.0!
conflict!
(
'Member already exists'
)
if
source_type
==
'group'
&&
member
unless
member
member
=
source
.
add_user
(
params
[
:user_id
],
params
[
:access_level
],
current_user:
current_user
,
expires_at:
params
[
:expires_at
])
end
conflict!
(
'Member already exists'
)
if
member
member
=
source
.
add_user
(
params
[
:user_id
],
params
[
:access_level
],
current_user:
current_user
,
expires_at:
params
[
:expires_at
])
if
member
.
persisted?
&&
member
.
valid?
present
member
.
user
,
with:
Entities
::
Member
,
member:
member
...
...
lib/api/v3/members.rb
0 → 100644
浏览文件 @
88d610c6
module
API
module
V3
class
Members
<
Grape
::
API
include
PaginationParams
before
{
authenticate!
}
helpers
::
API
::
Helpers
::
MembersHelpers
%w[group project]
.
each
do
|
source_type
|
params
do
requires
:id
,
type:
String
,
desc:
"The
#{
source_type
}
ID"
end
resource
source_type
.
pluralize
do
desc
'Gets a list of group or project members viewable by the authenticated user.'
do
success
::
API
::
Entities
::
Member
end
params
do
optional
:query
,
type:
String
,
desc:
'A query string to search for members'
use
:pagination
end
get
":id/members"
do
source
=
find_source
(
source_type
,
params
[
:id
])
users
=
source
.
users
users
=
users
.
merge
(
User
.
search
(
params
[
:query
]))
if
params
[
:query
]
present
paginate
(
users
),
with:
::
API
::
Entities
::
Member
,
source:
source
end
desc
'Gets a member of a group or project.'
do
success
::
API
::
Entities
::
Member
end
params
do
requires
:user_id
,
type:
Integer
,
desc:
'The user ID of the member'
end
get
":id/members/:user_id"
do
source
=
find_source
(
source_type
,
params
[
:id
])
members
=
source
.
members
member
=
members
.
find_by!
(
user_id:
params
[
:user_id
])
present
member
.
user
,
with:
::
API
::
Entities
::
Member
,
member:
member
end
desc
'Adds a member to a group or project.'
do
success
::
API
::
Entities
::
Member
end
params
do
requires
:user_id
,
type:
Integer
,
desc:
'The user ID of the new member'
requires
:access_level
,
type:
Integer
,
desc:
'A valid access level (defaults: `30`, developer access level)'
optional
:expires_at
,
type:
DateTime
,
desc:
'Date string in the format YEAR-MONTH-DAY'
end
post
":id/members"
do
source
=
find_source
(
source_type
,
params
[
:id
])
authorize_admin_source!
(
source_type
,
source
)
member
=
source
.
members
.
find_by
(
user_id:
params
[
:user_id
])
# We need this explicit check because `source.add_user` doesn't
# currently return the member created so it would return 201 even if
# the member already existed...
# The `source_type == 'group'` check is to ensure back-compatibility
# but 409 behavior should be used for both project and group members in 9.0!
conflict!
(
'Member already exists'
)
if
source_type
==
'group'
&&
member
unless
member
member
=
source
.
add_user
(
params
[
:user_id
],
params
[
:access_level
],
current_user:
current_user
,
expires_at:
params
[
:expires_at
])
end
if
member
.
persisted?
&&
member
.
valid?
present
member
.
user
,
with:
::
API
::
Entities
::
Member
,
member:
member
else
# This is to ensure back-compatibility but 400 behavior should be used
# for all validation errors in 9.0!
render_api_error!
(
'Access level is not known'
,
422
)
if
member
.
errors
.
key?
(
:access_level
)
render_validation_error!
(
member
)
end
end
desc
'Updates a member of a group or project.'
do
success
::
API
::
Entities
::
Member
end
params
do
requires
:user_id
,
type:
Integer
,
desc:
'The user ID of the new member'
requires
:access_level
,
type:
Integer
,
desc:
'A valid access level'
optional
:expires_at
,
type:
DateTime
,
desc:
'Date string in the format YEAR-MONTH-DAY'
end
put
":id/members/:user_id"
do
source
=
find_source
(
source_type
,
params
[
:id
])
authorize_admin_source!
(
source_type
,
source
)
member
=
source
.
members
.
find_by!
(
user_id:
params
[
:user_id
])
attrs
=
attributes_for_keys
[
:access_level
,
:expires_at
]
if
member
.
update_attributes
(
attrs
)
present
member
.
user
,
with:
::
API
::
Entities
::
Member
,
member:
member
else
# This is to ensure back-compatibility but 400 behavior should be used
# for all validation errors in 9.0!
render_api_error!
(
'Access level is not known'
,
422
)
if
member
.
errors
.
key?
(
:access_level
)
render_validation_error!
(
member
)
end
end
desc
'Removes a user from a group or project.'
params
do
requires
:user_id
,
type:
Integer
,
desc:
'The user ID of the member'
end
delete
":id/members/:user_id"
do
source
=
find_source
(
source_type
,
params
[
:id
])
# This is to ensure back-compatibility but find_by! should be used
# in that casse in 9.0!
member
=
source
.
members
.
find_by
(
user_id:
params
[
:user_id
])
# This is to ensure back-compatibility but this should be removed in
# favor of find_by! in 9.0!
not_found!
(
"Member: user_id:
#{
params
[
:user_id
]
}
"
)
if
source_type
==
'group'
&&
member
.
nil?
# This is to ensure back-compatibility but 204 behavior should be used
# for all DELETE endpoints in 9.0!
if
member
.
nil?
{
message:
"Access revoked"
,
id:
params
[
:user_id
].
to_i
}
else
::
Members
::
DestroyService
.
new
(
source
,
current_user
,
declared_params
).
execute
present
member
.
user
,
with:
::
API
::
Entities
::
Member
,
member:
member
end
end
end
end
end
end
end
spec/requests/api/members_spec.rb
浏览文件 @
88d610c6
...
...
@@ -145,11 +145,11 @@ describe API::Members, api: true do
end
end
it
"returns
#{
source_type
==
'project'
?
201
:
409
}
if member already exists"
do
it
"returns
409
if member already exists"
do
post
api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members"
,
master
),
user_id:
master
.
id
,
access_level:
Member
::
MASTER
expect
(
response
).
to
have_http_status
(
source_type
==
'project'
?
201
:
409
)
expect
(
response
).
to
have_http_status
(
409
)
end
it
'returns 400 when user_id is not given'
do
...
...
spec/requests/api/v3/members_spec.rb
0 → 100644
浏览文件 @
88d610c6
require
'spec_helper'
describe
API
::
Members
,
api:
true
do
include
ApiHelpers
let
(
:master
)
{
create
(
:user
)
}
let
(
:developer
)
{
create
(
:user
)
}
let
(
:access_requester
)
{
create
(
:user
)
}
let
(
:stranger
)
{
create
(
:user
)
}
let
(
:project
)
do
create
(
:empty_project
,
:public
,
:access_requestable
,
creator_id:
master
.
id
,
namespace:
master
.
namespace
)
do
|
project
|
project
.
team
<<
[
developer
,
:developer
]
project
.
team
<<
[
master
,
:master
]
project
.
request_access
(
access_requester
)
end
end
let!
(
:group
)
do
create
(
:group
,
:public
,
:access_requestable
)
do
|
group
|
group
.
add_developer
(
developer
)
group
.
add_owner
(
master
)
group
.
request_access
(
access_requester
)
end
end
shared_examples
'GET /:sources/:id/members'
do
|
source_type
|
context
"with :sources ==
#{
source_type
.
pluralize
}
"
do
it_behaves_like
'a 404 response when source is private'
do
let
(
:route
)
{
get
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members"
,
stranger
)
}
end
%i[master developer access_requester stranger]
.
each
do
|
type
|
context
"when authenticated as a
#{
type
}
"
do
it
'returns 200'
do
user
=
public_send
(
type
)
get
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members"
,
user
)
expect
(
response
).
to
have_http_status
(
200
)
expect
(
json_response
.
size
).
to
eq
(
2
)
expect
(
json_response
.
map
{
|
u
|
u
[
'id'
]
}).
to
match_array
[
master
.
id
,
developer
.
id
]
end
end
end
it
'does not return invitees'
do
create
(
:"
#{
source_type
}
_member"
,
invite_token:
'123'
,
invite_email:
'test@abc.com'
,
source:
source
,
user:
nil
)
get
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members"
,
developer
)
expect
(
response
).
to
have_http_status
(
200
)
expect
(
json_response
.
size
).
to
eq
(
2
)
expect
(
json_response
.
map
{
|
u
|
u
[
'id'
]
}).
to
match_array
[
master
.
id
,
developer
.
id
]
end
it
'finds members with query string'
do
get
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members"
,
developer
),
query:
master
.
username
expect
(
response
).
to
have_http_status
(
200
)
expect
(
json_response
.
count
).
to
eq
(
1
)
expect
(
json_response
.
first
[
'username'
]).
to
eq
(
master
.
username
)
end
end
end
shared_examples
'GET /:sources/:id/members/:user_id'
do
|
source_type
|
context
"with :sources ==
#{
source_type
.
pluralize
}
"
do
it_behaves_like
'a 404 response when source is private'
do
let
(
:route
)
{
get
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members/
#{
developer
.
id
}
"
,
stranger
)
}
end
context
'when authenticated as a non-member'
do
%i[access_requester stranger]
.
each
do
|
type
|
context
"as a
#{
type
}
"
do
it
'returns 200'
do
user
=
public_send
(
type
)
get
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members/
#{
developer
.
id
}
"
,
user
)
expect
(
response
).
to
have_http_status
(
200
)
# User attributes
expect
(
json_response
[
'id'
]).
to
eq
(
developer
.
id
)
expect
(
json_response
[
'name'
]).
to
eq
(
developer
.
name
)
expect
(
json_response
[
'username'
]).
to
eq
(
developer
.
username
)
expect
(
json_response
[
'state'
]).
to
eq
(
developer
.
state
)
expect
(
json_response
[
'avatar_url'
]).
to
eq
(
developer
.
avatar_url
)
expect
(
json_response
[
'web_url'
]).
to
eq
(
Gitlab
::
Routing
.
url_helpers
.
user_url
(
developer
))
# Member attributes
expect
(
json_response
[
'access_level'
]).
to
eq
(
Member
::
DEVELOPER
)
end
end
end
end
end
end
shared_examples
'POST /:sources/:id/members'
do
|
source_type
|
context
"with :sources ==
#{
source_type
.
pluralize
}
"
do
it_behaves_like
'a 404 response when source is private'
do
let
(
:route
)
do
post
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members"
,
stranger
),
user_id:
access_requester
.
id
,
access_level:
Member
::
MASTER
end
end
context
'when authenticated as a non-member or member with insufficient rights'
do
%i[access_requester stranger developer]
.
each
do
|
type
|
context
"as a
#{
type
}
"
do
it
'returns 403'
do
user
=
public_send
(
type
)
post
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members"
,
user
),
user_id:
access_requester
.
id
,
access_level:
Member
::
MASTER
expect
(
response
).
to
have_http_status
(
403
)
end
end
end
end
context
'when authenticated as a master/owner'
do
context
'and new member is already a requester'
do
it
'transforms the requester into a proper member'
do
expect
do
post
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members"
,
master
),
user_id:
access_requester
.
id
,
access_level:
Member
::
MASTER
expect
(
response
).
to
have_http_status
(
201
)
end
.
to
change
{
source
.
members
.
count
}.
by
(
1
)
expect
(
source
.
requesters
.
count
).
to
eq
(
0
)
expect
(
json_response
[
'id'
]).
to
eq
(
access_requester
.
id
)
expect
(
json_response
[
'access_level'
]).
to
eq
(
Member
::
MASTER
)
end
end
it
'creates a new member'
do
expect
do
post
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members"
,
master
),
user_id:
stranger
.
id
,
access_level:
Member
::
DEVELOPER
,
expires_at:
'2016-08-05'
expect
(
response
).
to
have_http_status
(
201
)
end
.
to
change
{
source
.
members
.
count
}.
by
(
1
)
expect
(
json_response
[
'id'
]).
to
eq
(
stranger
.
id
)
expect
(
json_response
[
'access_level'
]).
to
eq
(
Member
::
DEVELOPER
)
expect
(
json_response
[
'expires_at'
]).
to
eq
(
'2016-08-05'
)
end
end
it
"returns
#{
source_type
==
'project'
?
201
:
409
}
if member already exists"
do
post
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members"
,
master
),
user_id:
master
.
id
,
access_level:
Member
::
MASTER
expect
(
response
).
to
have_http_status
(
source_type
==
'project'
?
201
:
409
)
end
it
'returns 400 when user_id is not given'
do
post
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members"
,
master
),
access_level:
Member
::
MASTER
expect
(
response
).
to
have_http_status
(
400
)
end
it
'returns 400 when access_level is not given'
do
post
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members"
,
master
),
user_id:
stranger
.
id
expect
(
response
).
to
have_http_status
(
400
)
end
it
'returns 422 when access_level is not valid'
do
post
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members"
,
master
),
user_id:
stranger
.
id
,
access_level:
1234
expect
(
response
).
to
have_http_status
(
422
)
end
end
end
shared_examples
'PUT /:sources/:id/members/:user_id'
do
|
source_type
|
context
"with :sources ==
#{
source_type
.
pluralize
}
"
do
it_behaves_like
'a 404 response when source is private'
do
let
(
:route
)
do
put
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members/
#{
developer
.
id
}
"
,
stranger
),
access_level:
Member
::
MASTER
end
end
context
'when authenticated as a non-member or member with insufficient rights'
do
%i[access_requester stranger developer]
.
each
do
|
type
|
context
"as a
#{
type
}
"
do
it
'returns 403'
do
user
=
public_send
(
type
)
put
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members/
#{
developer
.
id
}
"
,
user
),
access_level:
Member
::
MASTER
expect
(
response
).
to
have_http_status
(
403
)
end
end
end
end
context
'when authenticated as a master/owner'
do
it
'updates the member'
do
put
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members/
#{
developer
.
id
}
"
,
master
),
access_level:
Member
::
MASTER
,
expires_at:
'2016-08-05'
expect
(
response
).
to
have_http_status
(
200
)
expect
(
json_response
[
'id'
]).
to
eq
(
developer
.
id
)
expect
(
json_response
[
'access_level'
]).
to
eq
(
Member
::
MASTER
)
expect
(
json_response
[
'expires_at'
]).
to
eq
(
'2016-08-05'
)
end
end
it
'returns 409 if member does not exist'
do
put
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members/123"
,
master
),
access_level:
Member
::
MASTER
expect
(
response
).
to
have_http_status
(
404
)
end
it
'returns 400 when access_level is not given'
do
put
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members/
#{
developer
.
id
}
"
,
master
)
expect
(
response
).
to
have_http_status
(
400
)
end
it
'returns 422 when access level is not valid'
do
put
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members/
#{
developer
.
id
}
"
,
master
),
access_level:
1234
expect
(
response
).
to
have_http_status
(
422
)
end
end
end
shared_examples
'DELETE /:sources/:id/members/:user_id'
do
|
source_type
|
context
"with :sources ==
#{
source_type
.
pluralize
}
"
do
it_behaves_like
'a 404 response when source is private'
do
let
(
:route
)
{
delete
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members/
#{
developer
.
id
}
"
,
stranger
)
}
end
context
'when authenticated as a non-member or member with insufficient rights'
do
%i[access_requester stranger]
.
each
do
|
type
|
context
"as a
#{
type
}
"
do
it
'returns 403'
do
user
=
public_send
(
type
)
delete
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members/
#{
developer
.
id
}
"
,
user
)
expect
(
response
).
to
have_http_status
(
403
)
end
end
end
end
context
'when authenticated as a member and deleting themself'
do
it
'deletes the member'
do
expect
do
delete
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members/
#{
developer
.
id
}
"
,
developer
)
expect
(
response
).
to
have_http_status
(
200
)
end
.
to
change
{
source
.
members
.
count
}.
by
(
-
1
)
end
end
context
'when authenticated as a master/owner'
do
context
'and member is a requester'
do
it
"returns
#{
source_type
==
'project'
?
200
:
404
}
"
do
expect
do
delete
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members/
#{
access_requester
.
id
}
"
,
master
)
expect
(
response
).
to
have_http_status
(
source_type
==
'project'
?
200
:
404
)
end
.
not_to
change
{
source
.
requesters
.
count
}
end
end
it
'deletes the member'
do
expect
do
delete
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members/
#{
developer
.
id
}
"
,
master
)
expect
(
response
).
to
have_http_status
(
200
)
end
.
to
change
{
source
.
members
.
count
}.
by
(
-
1
)
end
end
it
"returns
#{
source_type
==
'project'
?
200
:
404
}
if member does not exist"
do
delete
v3_api
(
"/
#{
source_type
.
pluralize
}
/
#{
source
.
id
}
/members/123"
,
master
)
expect
(
response
).
to
have_http_status
(
source_type
==
'project'
?
200
:
404
)
end
end
end
it_behaves_like
'GET /:sources/:id/members'
,
'project'
do
let
(
:source
)
{
project
}
end
it_behaves_like
'GET /:sources/:id/members'
,
'group'
do
let
(
:source
)
{
group
}
end
it_behaves_like
'GET /:sources/:id/members/:user_id'
,
'project'
do
let
(
:source
)
{
project
}
end
it_behaves_like
'GET /:sources/:id/members/:user_id'
,
'group'
do
let
(
:source
)
{
group
}
end
it_behaves_like
'POST /:sources/:id/members'
,
'project'
do
let
(
:source
)
{
project
}
end
it_behaves_like
'POST /:sources/:id/members'
,
'group'
do
let
(
:source
)
{
group
}
end
it_behaves_like
'PUT /:sources/:id/members/:user_id'
,
'project'
do
let
(
:source
)
{
project
}
end
it_behaves_like
'PUT /:sources/:id/members/:user_id'
,
'group'
do
let
(
:source
)
{
group
}
end
it_behaves_like
'DELETE /:sources/:id/members/:user_id'
,
'project'
do
let
(
:source
)
{
project
}
end
it_behaves_like
'DELETE /:sources/:id/members/:user_id'
,
'group'
do
let
(
:source
)
{
group
}
end
context
'Adding owner to project'
do
it
'returns 403'
do
expect
do
post
v3_api
(
"/projects/
#{
project
.
id
}
/members"
,
master
),
user_id:
stranger
.
id
,
access_level:
Member
::
OWNER
expect
(
response
).
to
have_http_status
(
422
)
end
.
to
change
{
project
.
members
.
count
}.
by
(
0
)
end
end
end
编辑
预览
Markdown
is supported
0%
请重试
或
添加新附件
.
添加附件
取消
You are about to add
0
people
to the discussion. Proceed with caution.
先完成此消息的编辑!
取消
想要评论请
注册
或
登录