diff --git a/doc/api/users.md b/doc/api/users.md index e5893638fdc..96aebffafd8 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -88,36 +88,47 @@ Return values: ## User creation -Create user. Available only for admin + +Creates a new user. Note only administrators can create new users. ``` POST /users ``` Parameters: -+ `email` (required) - Email -+ `password` (required) - Password -+ `username` (required) - Username -+ `name` (required) - Name -+ `skype` - Skype ID -+ `linkedin` - Linkedin -+ `twitter` - Twitter account -+ `projects_limit` - Number of projects user can create -+ `extern_uid` - External UID -+ `provider` - External provider name -+ `bio` - User's bio -Will return created user with status `201 Created` on success, or `404 Not -found` on fail. ++ `email` (required) - Email ++ `password` (required) - Password ++ `username` (required) - Username ++ `name` (required) - Name ++ `skype` (optional) - Skype ID ++ `linkedin` (optional) - Linkedin ++ `twitter` (optional) - Twitter account ++ `projects_limit` (optional) - Number of projects user can create ++ `extern_uid` (optional) - External UID ++ `provider` (optional) - External provider name ++ `bio` (optional) - User's bio + +Return values: + ++ `201 Created` on success and returns the new user ++ `400 Bad Request` if one of the required attributes is missing from the request ++ `401 Unauthorized` if the user is not authorized ++ `403 Forbidden` if the user is not allowed to create a new user (must be admin) ++ `404 Not Found` if something else fails ++ `409 Conflict` if a user with the same email address or username already exists + ## User modification -Modify user. Available only for admin + +Modifies an existing user. Only administrators can change attributes of a user. ``` PUT /users/:id ``` Parameters: + + `email` - Email + `username` - Username + `name` - Name @@ -130,23 +141,42 @@ Parameters: + `provider` - External provider name + `bio` - User's bio +Return values: + ++ `200 Ok` on success and returns the new user ++ `401 Unauthorized` if the user is not authorized ++ `403 Forbidden` if the user is not allowed to create a new user (must be admin) ++ `404 Not Found` if something else fails + +Note, at the moment this method does only return a 404 error, even in cases where a 409 (Conflict) would +be more appropriate, e.g. when renaming the email address to some exsisting one. -Will return created user with status `200 OK` on success, or `404 Not -found` on fail. ## User deletion -Delete user. Available only for admin + +Deletes a user. Available only for administrators. This is an idempotent function, calling this function +for a non-existent user id still returns a status code `200 Ok`. The JSON response differs if the user +was actually deleted or not. In the former the user is returned and in the latter not. ``` DELETE /users/:id ``` -Will return deleted user with status `200 OK` on success, or `404 Not -found` on fail. +Parameters: + ++ `id` (required) - The ID of the user + +Return values: + ++ `200 Ok` on success and returns the deleted user ++ `401 Unauthorized` if the user is not authorized ++ `403 Forbidden` if the user is not allowed to create a new user (must be admin) ++ `404 Not Found` if user with ID not found or something else fails + ## Current user -Get currently authenticated user. +Gets currently authenticated user. ``` GET /user @@ -169,6 +199,13 @@ GET /user } ``` +Return values: + ++ `200 Ok` on success and returns the current user ++ `401 Unauthorized` if the user is not authorized ++ `404 Not Found` if something else fails + + ## List SSH keys Get a list of currently authenticated user's SSH keys. @@ -196,6 +233,17 @@ GET /user/keys ] ``` +Parameters: + ++ **none** + +Return values: + ++ `200 Ok` on success and a list of ssh keys ++ `401 Unauthorized` if the user is not authenticated ++ `404 Not Found` if something else fails + + ## Single SSH key Get a single key. @@ -217,9 +265,17 @@ Parameters: soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=" } ``` + +Return values: + ++ `200 Ok` on success and the ssh key with ID ++ `401 Unauthorized` if it is not allowed to access the user ++ `404 Not Found` if the ssh key with ID not found + + ## Add SSH key -Create new key owned by currently authenticated user +Creates a new key owned by the currently authenticated user. ``` POST /user/keys @@ -230,12 +286,18 @@ Parameters: + `title` (required) - new SSH Key's title + `key` (required) - new SSH key -Will return created key with status `201 Created` on success, or `404 Not -found` on fail. +Return values: + ++ `201 Created` on success and the added key ++ `400 Bad Request` if one of the required attributes is not given ++ `401 Unauthorized` if user is not authorized to add ssh key ++ `404 Not Found` if something else fails + ## Delete SSH key -Delete key owned by currently authenticated user +Deletes key owned by currently authenticated user. This is an idempotent function and calling it on a key that is already +deleted or not available results in `200 Ok`. ``` DELETE /user/keys/:id @@ -245,4 +307,8 @@ Parameters: + `id` (required) - SSH key ID -Will return `200 OK` on success, or `404 Not Found` on fail. +Return values: + ++ `200 Ok` on success ++ `401 Unauthorized` if user is not allowed to delete they key ++ `404 Not Found` if something else fails diff --git a/lib/api/users.rb b/lib/api/users.rb index 7ea90c75e9e..b9dce58a13d 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -41,6 +41,12 @@ module Gitlab # POST /users post do authenticated_as_admin! + + bad_request!(:email) if !params.has_key? :email + bad_request!(:password) if !params.has_key? :password + bad_request!(:name) if !params.has_key? :name + bad_request!(:username) if !params.has_key? :username + attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :projects_limit, :username, :extern_uid, :provider, :bio] user = User.new attrs, as: :admin if user.save @@ -67,10 +73,12 @@ module Gitlab # PUT /users/:id put ":id" do authenticated_as_admin! - attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :projects_limit, :username, :extern_uid, :provider, :bio] - user = User.find_by_id(params[:id]) - if user && user.update_attributes(attrs) + attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :projects_limit, :username, :extern_uid, :provider, :bio] + user = User.find(params[:id]) + not_found!("User not found") unless user + + if user.update_attributes(attrs) present user, with: Entities::User else not_found! @@ -127,6 +135,9 @@ module Gitlab # Example Request: # POST /user/keys post "keys" do + bad_request!(:title) unless params[:title].present? + bad_request!(:key) unless params[:key].present? + attrs = attributes_for_keys [:title, :key] key = current_user.keys.new attrs if key.save @@ -136,15 +147,18 @@ module Gitlab end end - # Delete existed ssh key of currently authenticated user + # Delete existing ssh key of currently authenticated user # # Parameters: # id (required) - SSH Key ID # Example Request: # DELETE /user/keys/:id delete "keys/:id" do - key = current_user.keys.find params[:id] - key.delete + begin + key = current_user.keys.find params[:id] + key.delete + rescue + end end end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 1645117e231..b0cf12654be 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -31,26 +31,69 @@ describe Gitlab::API do response.status.should == 200 json_response['email'].should == user.email end + + it "should return a 401 if unauthenticated" do + get api("/users/9998") + response.status.should == 401 + end + + it "should return a 404 error if user id not found" do + get api("/users/9999", user) + response.status.should == 404 + end end describe "POST /users" do before{ admin } - it "should not create invalid user" do - post api("/users", admin), { email: "invalid email" } - response.status.should == 404 - end - it "should create user" do expect { post api("/users", admin), attributes_for(:user, projects_limit: 3) }.to change { User.count }.by(1) end + it "should return 201 Created on success" do + post api("/users", admin), attributes_for(:user, projects_limit: 3) + response.status.should == 201 + end + + it "should not create user with invalid email" do + post api("/users", admin), { email: "invalid email", password: 'password' } + response.status.should == 400 + end + + it "should return 400 error if password not given" do + post api("/users", admin), { email: 'test@example.com' } + response.status.should == 400 + end + + it "should return 400 error if email not given" do + post api("/users", admin), { password: 'pass1234' } + response.status.should == 400 + end + it "shouldn't available for non admin users" do post api("/users", user), attributes_for(:user) response.status.should == 403 end + + context "with existing user" do + before { post api("/users", admin), { email: 'test@example.com', password: 'password', username: 'test' } } + + it "should not create user with same email" do + expect { + post api("/users", admin), { email: 'test@example.com', password: 'password' } + }.to change { User.count }.by(0) + end + + it "should return 409 conflict error if user with email exists" do + post api("/users", admin), { email: 'test@example.com', password: 'password' } + end + + it "should return 409 conflict error if same username exists" do + post api("/users", admin), { email: 'foo@example.com', password: 'pass', username: 'test' } + end + end end describe "GET /users/sign_up" do @@ -86,7 +129,7 @@ describe Gitlab::API do describe "PUT /users/:id" do before { admin } - it "should update user" do + it "should update user with new bio" do put api("/users/#{user.id}", admin), {bio: 'new test bio'} response.status.should == 200 json_response['bio'].should == 'new test bio' @@ -108,6 +151,25 @@ describe Gitlab::API do put api("/users/999999", admin), {bio: 'update should fail'} response.status.should == 404 end + + context "with existing user" do + before { + post api("/users", admin), { email: 'test@example.com', password: 'password', username: 'test', name: 'test' } + post api("/users", admin), { email: 'foo@bar.com', password: 'password', username: 'john', name: 'john' } + @user_id = User.all.last.id + } + +# it "should return 409 conflict error if email address exists" do +# put api("/users/#{@user_id}", admin), { email: 'test@example.com' } +# response.status.should == 409 +# end +# +# it "should return 409 conflict error if username taken" do +# @user_id = User.all.last.id +# put api("/users/#{@user_id}", admin), { username: 'test' } +# response.status.should == 409 +# end + end end describe "DELETE /users/:id" do @@ -120,6 +182,11 @@ describe Gitlab::API do json_response['email'].should == user.email end + it "should not delete for unauthenticated user" do + delete api("/users/#{user.id}") + response.status.should == 401 + end + it "shouldn't available for non admin users" do delete api("/users/#{user.id}", user) response.status.should == 403 @@ -137,6 +204,11 @@ describe Gitlab::API do response.status.should == 200 json_response['email'].should == user.email end + + it "should return 401 error if user is unauthenticated" do + get api("/user") + response.status.should == 401 + end end describe "GET /user/keys" do @@ -172,19 +244,38 @@ describe Gitlab::API do get api("/user/keys/42", user) response.status.should == 404 end + + it "should return 404 error if admin accesses user's ssh key" do + user.keys << key + user.save + admin + get api("/user/keys/#{key.id}", admin) + response.status.should == 404 + end end describe "POST /user/keys" do - it "should not create invalid ssh key" do - post api("/user/keys", user), { title: "invalid key" } - response.status.should == 404 - end - it "should create ssh key" do key_attrs = attributes_for :key expect { post api("/user/keys", user), key_attrs }.to change{ user.keys.count }.by(1) + response.status.should == 201 + end + + it "should return a 401 error if unauthorized" do + post api("/user/keys"), title: 'some title', key: 'some key' + response.status.should == 401 + end + + it "should not create ssh key without key" do + post api("/user/keys", user), title: 'title' + response.status.should == 400 + end + + it "should not create ssh key without title" do + post api("/user/keys", user), key: "somekey" + response.status.should == 400 end end @@ -195,11 +286,19 @@ describe Gitlab::API do expect { delete api("/user/keys/#{key.id}", user) }.to change{user.keys.count}.by(-1) + response.status.should == 200 end - it "should return 404 Not Found within invalid ID" do + it "should return sucess if key ID not found" do delete api("/user/keys/42", user) - response.status.should == 404 + response.status.should == 200 + end + + it "should return 401 error if unauthorized" do + user.keys << key + user.save + delete api("/user/keys/#{key.id}") + response.status.should == 401 end end end