From 4fb6ddfe06164c211f22e69fdec0b248bc61f6b4 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 29 Jul 2015 15:40:08 +0200 Subject: [PATCH 1/3] Add ability to manage user email addresses via the API. --- CHANGELOG | 1 + doc/api/users.md | 132 ++++++++++++++++++++++++ lib/api/entities.rb | 4 + lib/api/users.rb | 111 +++++++++++++++++++++ spec/requests/api/users_spec.rb | 171 ++++++++++++++++++++++++++++++++ 5 files changed, 419 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 7ca450d423c..ab46ef3b169 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -17,6 +17,7 @@ v 7.14.0 (unreleased) - Add fetch command to the MR page - Fix bug causing Bitbucket importer to crash when OAuth application had been removed. - Add fetch command to the MR page. + - Add ability to manage user email addresses via the API. v 7.13.2 - Fix randomly failed spec diff --git a/doc/api/users.md b/doc/api/users.md index 5dca77b5c7b..9ac55d3f09e 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -397,6 +397,138 @@ Parameters: Will return `200 OK` on success, or `404 Not found` if either user or key cannot be found. +## List emails + +Get a list of currently authenticated user's emails. + +``` +GET /user/emails +``` + +```json +[ + { + "id": 1, + "email": "email@example.com" + }, + { + "id": 3, + "email": "email2@example.com" + } +] +``` + +Parameters: + +- **none** + +## List emails for user + +Get a list of a specified user's emails. Available only for admin + +``` +GET /users/:uid/emails +``` + +Parameters: + +- `uid` (required) - id of specified user + +## Single SSH key + +Get a single key. + +``` +GET /user/emails/:id +``` + +Parameters: + +- `id` (required) - The ID of an SSH key + +```json +{ + "id": 1, + "email": "email@example.com" +} +``` + +## Add email + +Creates a new email owned by the currently authenticated user. + +``` +POST /user/emails +``` + +Parameters: + +- `email` (required) - email address + +```json +{ + "id": 4, + "email": "email@example.com" +} +``` + +Will return created key with status `201 Created` on success. If an +error occurs a `400 Bad Request` is returned with a message explaining the error: + +```json +{ + "message": { + "email": [ + "has already been taken" + ] + } +} +``` + +## Add email for user + +Create new email owned by specified user. Available only for admin + +``` +POST /users/:id/emails +``` + +Parameters: + +- `id` (required) - id of specified user +- `email` (required) - email address + +Will return created key with status `201 Created` on success, or `404 Not found` on fail. + +## Delete email for current user + +Deletes email owned by currently authenticated user. +This is an idempotent function and calling it on a email that is already deleted +or not available results in `200 OK`. + +``` +DELETE /user/emails/:id +``` + +Parameters: + +- `id` (required) - email ID + +## Delete email for given user + +Deletes email owned by a specified user. Available only for admin. + +``` +DELETE /users/:uid/emails/:id +``` + +Parameters: + +- `uid` (required) - id of specified user +- `id` (required) - email ID + +Will return `200 OK` on success, or `404 Not found` if either user or key cannot be found. + ## Block user Blocks the specified user. Available only for admin. diff --git a/lib/api/entities.rb b/lib/api/entities.rb index ecf1412dee5..ce3d09a32cd 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -31,6 +31,10 @@ module API expose :private_token end + class Email < Grape::Entity + expose :id, :email + end + class Hook < Grape::Entity expose :id, :url, :created_at end diff --git a/lib/api/users.rb b/lib/api/users.rb index c468371d3d4..bd8cc9f16a8 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -185,6 +185,65 @@ module API end end + # Add email to a specified user. Only available to admin users. + # + # Parameters: + # id (required) - The ID of a user + # email (required) - Email address + # Example Request: + # POST /users/:id/emails + post ":id/emails" do + authenticated_as_admin! + required_attributes! [:email] + + user = User.find(params[:id]) + attrs = attributes_for_keys [:email] + email = user.emails.new attrs + if email.save + NotificationService.new.new_email(email) + present email, with: Entities::Email + else + render_validation_error!(email) + end + end + + # Get emails of a specified user. Only available to admin users. + # + # Parameters: + # uid (required) - The ID of a user + # Example Request: + # GET /users/:uid/emails + get ':uid/emails' do + authenticated_as_admin! + user = User.find_by(id: params[:uid]) + not_found!('User') unless user + + present user.emails, with: Entities::Email + end + + # Delete existing email of a specified user. Only available to admin + # users. + # + # Parameters: + # uid (required) - The ID of a user + # id (required) - Email ID + # Example Request: + # DELETE /users/:uid/emails/:id + delete ':uid/emails/:id' do + authenticated_as_admin! + user = User.find_by(id: params[:uid]) + not_found!('User') unless user + + begin + email = user.emails.find params[:id] + email.destroy + + user.update_secondary_emails! + rescue ActiveRecord::RecordNotFound + not_found!('Email') + end + end + # Delete user. Available only for admin # # Example Request: @@ -289,6 +348,58 @@ module API rescue end end + + # Get currently authenticated user's emails + # + # Example Request: + # GET /user/emails + get "emails" do + present current_user.emails, with: Entities::Email + end + + # Get single email owned by currently authenticated user + # + # Example Request: + # GET /user/emails/:id + get "emails/:id" do + email = current_user.emails.find params[:id] + present email, with: Entities::Email + end + + # Add new email to currently authenticated user + # + # Parameters: + # email (required) - Email address + # Example Request: + # POST /user/emails + post "emails" do + required_attributes! [:email] + + attrs = attributes_for_keys [:email] + email = current_user.emails.new attrs + if email.save + NotificationService.new.new_email(email) + present email, with: Entities::Email + else + render_validation_error!(email) + end + end + + # Delete existing email of currently authenticated user + # + # Parameters: + # id (required) - EMail ID + # Example Request: + # DELETE /user/emails/:id + delete "emails/:id" do + begin + email = current_user.emails.find params[:id] + email.destroy + + current_user.update_secondary_emails! + rescue + end + end end end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index c4dd1f76cf2..7fa6aebca0b 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -6,6 +6,7 @@ describe API::API, api: true do let(:user) { create(:user) } let(:admin) { create(:admin) } let(:key) { create(:key, user: user) } + let(:email) { create(:email, user: user) } describe "GET /users" do context "when unauthenticated" do @@ -384,6 +385,87 @@ describe API::API, api: true do end end + describe "POST /users/:id/emails" do + before { admin } + + it "should not create invalid email" do + post api("/users/#{user.id}/emails", admin), { } + expect(response.status).to eq(400) + expect(json_response['message']).to eq('400 (Bad request) "email" not given') + end + + it "should create email" do + email_attrs = attributes_for :email + expect do + post api("/users/#{user.id}/emails", admin), email_attrs + end.to change{ user.emails.count }.by(1) + end + end + + describe 'GET /user/:uid/emails' do + before { admin } + + context 'when unauthenticated' do + it 'should return authentication error' do + get api("/users/#{user.id}/emails") + expect(response.status).to eq(401) + end + end + + context 'when authenticated' do + it 'should return 404 for non-existing user' do + get api('/users/999999/emails', admin) + expect(response.status).to eq(404) + expect(json_response['message']).to eq('404 User Not Found') + end + + it 'should return array of emails' do + user.emails << email + user.save + get api("/users/#{user.id}/emails", admin) + expect(response.status).to eq(200) + expect(json_response).to be_an Array + expect(json_response.first['email']).to eq(email.email) + end + end + end + + describe 'DELETE /user/:uid/emails/:id' do + before { admin } + + context 'when unauthenticated' do + it 'should return authentication error' do + delete api("/users/#{user.id}/emails/42") + expect(response.status).to eq(401) + end + end + + context 'when authenticated' do + it 'should delete existing email' do + user.emails << email + user.save + expect do + delete api("/users/#{user.id}/emails/#{email.id}", admin) + end.to change { user.emails.count }.by(-1) + expect(response.status).to eq(200) + end + + it 'should return 404 error if user not found' do + user.emails << email + user.save + delete api("/users/999999/emails/#{email.id}", admin) + expect(response.status).to eq(404) + expect(json_response['message']).to eq('404 User Not Found') + end + + it 'should return 404 error if email not foud' do + delete api("/users/#{user.id}/emails/42", admin) + expect(response.status).to eq(404) + expect(json_response['message']).to eq('404 Email Not Found') + end + end + end + describe "DELETE /users/:id" do before { admin } @@ -528,6 +610,95 @@ describe API::API, api: true do end end + describe "GET /user/emails" do + context "when unauthenticated" do + it "should return authentication error" do + get api("/user/emails") + expect(response.status).to eq(401) + end + end + + context "when authenticated" do + it "should return array of emails" do + user.emails << email + user.save + get api("/user/emails", user) + expect(response.status).to eq(200) + expect(json_response).to be_an Array + expect(json_response.first["email"]).to eq(email.email) + end + end + end + + describe "GET /user/emails/:id" do + it "should return single email" do + user.emails << email + user.save + get api("/user/emails/#{email.id}", user) + expect(response.status).to eq(200) + expect(json_response["email"]).to eq(email.email) + end + + it "should return 404 Not Found within invalid ID" do + get api("/user/emails/42", user) + expect(response.status).to eq(404) + expect(json_response['message']).to eq('404 Not found') + end + + it "should return 404 error if admin accesses user's email" do + user.emails << email + user.save + admin + get api("/user/emails/#{email.id}", admin) + expect(response.status).to eq(404) + expect(json_response['message']).to eq('404 Not found') + end + end + + describe "POST /user/emails" do + it "should create email" do + email_attrs = attributes_for :email + expect do + post api("/user/emails", user), email_attrs + end.to change{ user.emails.count }.by(1) + expect(response.status).to eq(201) + end + + it "should return a 401 error if unauthorized" do + post api("/user/emails"), email: 'some email' + expect(response.status).to eq(401) + end + + it "should not create email with invalid email" do + post api("/user/emails", user), {} + expect(response.status).to eq(400) + expect(json_response['message']).to eq('400 (Bad request) "email" not given') + end + end + + describe "DELETE /user/emails/:id" do + it "should delete existed email" do + user.emails << email + user.save + expect do + delete api("/user/emails/#{email.id}", user) + end.to change{user.emails.count}.by(-1) + expect(response.status).to eq(200) + end + + it "should return success if email ID not found" do + delete api("/user/emails/42", user) + expect(response.status).to eq(200) + end + + it "should return 401 error if unauthorized" do + user.emails << email + user.save + delete api("/user/emails/#{email.id}") + expect(response.status).to eq(401) + end + end + describe 'PUT /user/:id/block' do before { admin } it 'should block existing user' do From 1c7a8b8c27398250983bf4329007f6971df65f34 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 30 Jul 2015 11:41:59 +0200 Subject: [PATCH 2/3] Fix docs --- doc/api/users.md | 12 ++++++------ spec/requests/api/users_spec.rb | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/doc/api/users.md b/doc/api/users.md index 9ac55d3f09e..7ba2db248ff 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -434,9 +434,9 @@ Parameters: - `uid` (required) - id of specified user -## Single SSH key +## Single email -Get a single key. +Get a single email. ``` GET /user/emails/:id @@ -444,7 +444,7 @@ GET /user/emails/:id Parameters: -- `id` (required) - The ID of an SSH key +- `id` (required) - email ID ```json { @@ -472,7 +472,7 @@ Parameters: } ``` -Will return created key with status `201 Created` on success. If an +Will return created email with status `201 Created` on success. If an error occurs a `400 Bad Request` is returned with a message explaining the error: ```json @@ -498,7 +498,7 @@ Parameters: - `id` (required) - id of specified user - `email` (required) - email address -Will return created key with status `201 Created` on success, or `404 Not found` on fail. +Will return created email with status `201 Created` on success, or `404 Not found` on fail. ## Delete email for current user @@ -527,7 +527,7 @@ Parameters: - `uid` (required) - id of specified user - `id` (required) - email ID -Will return `200 OK` on success, or `404 Not found` if either user or key cannot be found. +Will return `200 OK` on success, or `404 Not found` if either user or email cannot be found. ## Block user diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 7fa6aebca0b..f2aa369985e 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -389,7 +389,7 @@ describe API::API, api: true do before { admin } it "should not create invalid email" do - post api("/users/#{user.id}/emails", admin), { } + post api("/users/#{user.id}/emails", admin), {} expect(response.status).to eq(400) expect(json_response['message']).to eq('400 (Bad request) "email" not given') end From 8802846565f382f4bf21ff7e08a4e9c459bb10d6 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 31 Jul 2015 14:35:32 +0200 Subject: [PATCH 3/3] Fix indentation --- lib/api/users.rb | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/api/users.rb b/lib/api/users.rb index bd8cc9f16a8..ee29f952246 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -131,11 +131,11 @@ module API # Add ssh key to a specified user. Only available to admin users. # # Parameters: - # id (required) - The ID of a user - # key (required) - New SSH Key - # title (required) - New SSH Key's title + # id (required) - The ID of a user + # key (required) - New SSH Key + # title (required) - New SSH Key's title # Example Request: - # POST /users/:id/keys + # POST /users/:id/keys post ":id/keys" do authenticated_as_admin! required_attributes! [:title, :key] @@ -153,9 +153,9 @@ module API # Get ssh keys of a specified user. Only available to admin users. # # Parameters: - # uid (required) - The ID of a user + # uid (required) - The ID of a user # Example Request: - # GET /users/:uid/keys + # GET /users/:uid/keys get ':uid/keys' do authenticated_as_admin! user = User.find_by(id: params[:uid]) @@ -188,10 +188,10 @@ module API # Add email to a specified user. Only available to admin users. # # Parameters: - # id (required) - The ID of a user - # email (required) - Email address + # id (required) - The ID of a user + # email (required) - Email address # Example Request: - # POST /users/:id/emails + # POST /users/:id/emails post ":id/emails" do authenticated_as_admin! required_attributes! [:email] @@ -210,9 +210,9 @@ module API # Get emails of a specified user. Only available to admin users. # # Parameters: - # uid (required) - The ID of a user + # uid (required) - The ID of a user # Example Request: - # GET /users/:uid/emails + # GET /users/:uid/emails get ':uid/emails' do authenticated_as_admin! user = User.find_by(id: params[:uid])