diff --git a/doc/api/group_service_accounts.md b/doc/api/group_service_accounts.md index 2c4ac724781e1f49de062a1780a9f5e3409a5165..d90ed60efe4854879f6545256bd619cf600f59a8 100644 --- a/doc/api/group_service_accounts.md +++ b/doc/api/group_service_accounts.md @@ -116,6 +116,52 @@ Example response: } ``` +## Update a service account user + +{{< history >}} + +- [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/182607/) in GitLab 17.10. + +{{< /history >}} + +Updates a service account user in a given top-level group. + +{{< alert type="note" >}} + +This endpoint only works on top-level groups. + +{{< /alert >}} + +```plaintext +PATCH /groups/:id/service_accounts/:user_id +``` + +Parameters: + +| Attribute | Type | Required | Description | +|:-----------|:---------------|:---------|:----------------------------------------------------------------| +| `id` | integer/string | yes | The ID or [URL-encoded path of the target group](rest/_index.md#namespaced-paths). | +| `user_id` | integer | yes | The ID of the service account user. | +| `name` | string | no | Name of the user. | +| `username` | string | no | Username of the user. | + +Example request: + +```shell +curl --request PATCH --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/groups/345/service_accounts/57" --data "name=Updated Service Account" +``` + +Example response: + +```json +{ + "id": 57, + "username": "service_account_group_345_6018816a18e515214e0c34c2b33523fc", + "name": "Updated Service Account", + "email": "service_account_group_345_<random_hash>@noreply.gitlab.example.com" +} +``` + ## Delete a service account user {{< history >}} diff --git a/ee/app/services/namespaces/service_accounts/update_service.rb b/ee/app/services/namespaces/service_accounts/update_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..54dabcd1d9e44a0e7dd2379d0c666c2974df1318 --- /dev/null +++ b/ee/app/services/namespaces/service_accounts/update_service.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +module Namespaces + module ServiceAccounts + class UpdateService + attr_reader :current_user, :user, :params + + ALLOWED_PARAMS = [:username, :name].freeze + + def initialize(current_user, user, params = {}) + @current_user = current_user + @user = user + @params = params.slice(*ALLOWED_PARAMS) + end + + def execute + return error(_('User is not a service account'), :bad_request) unless user.service_account? + + unless can_update_service_account? + return error( + s_('ServiceAccount|You are not authorized to update service accounts in this namespace.'), + :forbidden + ) + end + + user_update = update_user + + if user_update[:status] == :success + success + else + error(user_update[:message], :bad_request) + end + end + + private + + def can_update_service_account? + Ability.allowed?(current_user, :admin_service_accounts, user.provisioned_by_group) + end + + def update_user + Users::UpdateService.new(current_user, update_params.merge(user: user, force_name_change: true)).execute + end + + def update_params + params + end + + def error(message, reason) + ServiceResponse.error(message: message, reason: reason) + end + + def success + ServiceResponse.success(message: _('Service account was successfully updated.'), payload: { user: user }) + end + end + end +end diff --git a/ee/lib/api/group_service_accounts.rb b/ee/lib/api/group_service_accounts.rb index 09125897501101b3f7d888c3964c193ee4658dc6..76fa72eddce35d1ca90ada7dc702db1ec6efdcb3 100644 --- a/ee/lib/api/group_service_accounts.rb +++ b/ee/lib/api/group_service_accounts.rb @@ -32,7 +32,7 @@ def validate_service_account_user resource :service_accounts do desc 'Create a service account user' do detail 'Create a service account user' - success Entities::UserSafe + success Entities::ServiceAccount failure [ { code: 400, message: '400 Bad request' }, { code: 401, message: '401 Unauthorized' }, @@ -66,7 +66,7 @@ def validate_service_account_user desc 'Get list of service account users' do detail 'Get list of service account users' - success Entities::UserSafe + success Entities::ServiceAccount failure [ { code: 400, message: '400 Bad request' }, { code: 401, message: '401 Unauthorized' }, @@ -122,6 +122,39 @@ def validate_service_account_user end end + desc 'Update a service account user' do + detail 'Update a service account user' + success Entities::ServiceAccount + failure [ + { code: 400, message: '400 Bad request' }, + { code: 401, message: '401 Unauthorized' }, + { code: 403, message: '403 Forbidden' }, + { code: 404, message: '404 User not found' } + ] + end + + params do + requires :user_id, type: Integer, desc: 'The ID of the service account user' + optional :name, type: String, desc: 'Name of the user' + optional :username, type: String, desc: 'Username of the user' + end + + patch ":user_id" do + validate_service_account_user + + update_params = declared_params(include_missing: false) + + response = ::Namespaces::ServiceAccounts::UpdateService + .new(current_user, user, update_params) + .execute + + if response.success? + present response.payload[:user], with: Entities::ServiceAccount, current_user: current_user + else + render_api_error!(response.message, response.reason) + end + end + resource ":user_id/personal_access_tokens" do desc 'Create a personal access token. Available only for group owners.' do detail 'This feature was introduced in GitLab 16.1' diff --git a/ee/spec/requests/api/group_service_accounts_spec.rb b/ee/spec/requests/api/group_service_accounts_spec.rb index f51f7c31e99e68b03fd2ef171e101982b82b3197..384f96529f3b31d0deb2a248e589a4624882fc6b 100644 --- a/ee/spec/requests/api/group_service_accounts_spec.rb +++ b/ee/spec/requests/api/group_service_accounts_spec.rb @@ -253,6 +253,68 @@ end end + RSpec.shared_examples "service account user update" do + context 'when the group exists' do + let(:group_id) { group.id } + + it 'updates the service account user' do + perform_request + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.keys).to match_array(%w[id name username email]) + expect(json_response['name']).to eq(params[:name]) + expect(json_response['username']).to eq(params[:username]) + end + + context 'when user with the username already exists' do + let(:existing_user) { create(:user, username: 'existing_user') } + let(:params) { { username: existing_user.username } } + + it 'returns error' do + perform_request + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to include('Username has already been taken') + end + end + + it "returns 404 for non-existing user" do + patch api("/groups/#{group_id}/service_accounts/#{non_existing_record_id}", user), params: params + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 User Not Found') + end + + it "returns a 400 for invalid user ID" do + patch api("/groups/#{group_id}/service_accounts/ASDF", user), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + end + + context 'when target user is not a service account' do + let(:regular_user) { create(:user, provisioned_by_group: group) } + + it 'returns bad request error' do + patch api("/groups/#{group_id}/service_accounts/#{regular_user.id}", user), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to include('User is not of type Service Account') + end + end + end + + context 'when the group does not exist' do + let(:group_id) { non_existing_record_id } + + it "returns error" do + perform_request + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to include('404 Group Not Found') + end + end + end + describe "POST /groups/:id/service_accounts" do subject(:perform_request) { post api("/groups/#{group_id}/service_accounts", user), params: params } @@ -451,6 +513,79 @@ end end + describe "PATCH /groups/:id/service_accounts/:user_id" do + let(:group_id) { group.id } + let(:params) { { name: 'Updated Name', username: 'updated_username' } } + + subject(:perform_request) do + patch api("/groups/#{group_id}/service_accounts/#{service_account_user.id}", user), params: params + end + + context 'when feature is licensed' do + before do + stub_licensed_features(service_accounts: true) + end + + context 'when current user is an admin' do + let(:user) { create(:admin) } + + context 'when admin mode is not enabled' do + it "returns forbidden error" do + perform_request + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when admin mode is enabled', :enable_admin_mode do + it_behaves_like "service account user update" + end + end + + context 'when current user is a group owner' do + before do + group.add_owner(user) + end + + it_behaves_like "service account user update" + + context 'when saas', :saas do + it_behaves_like "service account user update" + end + end + + context 'when current user is not a group owner' do + before do + group.add_maintainer(user) + end + + it "returns forbidden error" do + perform_request + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + + context 'when feature is not licensed' do + before do + stub_licensed_features(service_accounts: false) + end + + context 'when current user is an admin' do + let(:current_user) { admin } + + context 'when admin mode is enabled', :enable_admin_mode do + it "returns forbidden error" do + perform_request + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + end + end + describe "DELETE /groups/:id/service_accounts/:user_id" do let(:issue) { create(:issue, author: service_account_user) } let(:group_id) { group.id } diff --git a/ee/spec/services/namespaces/service_accounts/update_service_spec.rb b/ee/spec/services/namespaces/service_accounts/update_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..8a72f62ac7832b1087b1bf3427ad5c1a16a1e246 --- /dev/null +++ b/ee/spec/services/namespaces/service_accounts/update_service_spec.rb @@ -0,0 +1,150 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Namespaces::ServiceAccounts::UpdateService, feature_category: :user_management do + let_it_be(:organization) { create(:organization) } + let(:group) { create(:group, organization: organization) } + let(:subgroup) { create(:group, :private, parent: group) } + let(:admin) { create(:admin) } + let(:owner) { create(:user) } + let(:maintainer) { create(:user) } + let(:service_account_user) { create(:user, :service_account, provisioned_by_group: group) } + let(:regular_user) { create(:user) } + + let(:user) { service_account_user } + let(:params) { { name: FFaker::Name.name, username: FFaker::Internet.unique.user_name } } + let(:current_user) { owner } + + subject(:result) { described_class.new(current_user, user, params).execute } + + before do + group.add_owner(owner) + group.add_maintainer(maintainer) + end + + shared_examples 'not authorized to update' do + it 'returns an error', :aggregate_failures do + expect(result.status).to eq(:error) + expect(result.message).to eq( + s_('ServiceAccount|You are not authorized to update service accounts in this namespace.') + ) + expect(result.reason).to eq(:forbidden) + end + end + + shared_examples 'authorized to update' do + it 'updates the service account', :aggregate_failures do + expect(result.status).to eq(:success) + expect(result.message).to eq(_('Service account was successfully updated.')) + expect(result.payload[:user]).to eq(service_account_user) + expect(result.payload[:user].name).to eq(params[:name]) + expect(result.payload[:user].username).to eq(params[:username]) + end + + context 'when the ability to update name for users is disabled' do + before do + stub_application_setting(updating_name_disabled_for_users: true) + end + + it 'updates the service account name', :aggregate_failures do + expect(result.status).to eq(:success) + expect(result.message).to eq(_('Service account was successfully updated.')) + expect(result.payload[:user]).to eq(service_account_user) + expect(result.payload[:user].name).to eq(params[:name]) + end + end + + context 'when user is not a service account' do + let(:user) { regular_user } + + it 'returns an error', :aggregate_failures do + expect(result.status).to eq(:error) + expect(result.message).to eq('User is not a service account') + expect(result.reason).to eq(:bad_request) + end + end + + context 'when params are empty' do + let(:params) { {} } + + it 'returns success', :aggregate_failures do + expect(result.status).to eq(:success) + expect(result.message).to eq(_('Service account was successfully updated.')) + end + end + + context 'when username is already taken' do + let(:existing_user) { create(:user, username: 'existing_username') } + let(:params) { { username: existing_user.username } } + + it 'returns an error', :aggregate_failures do + expect(result.status).to eq(:error) + expect(result.message).to include('Username has already been taken') + expect(result.reason).to eq(:bad_request) + end + end + + context 'when user update fails' do + before do + allow_next_instance_of(Users::UpdateService) do |update_service| + allow(update_service).to receive(:execute).and_return(ServiceResponse.error(message: 'Update failed')) + end + end + + it 'returns an error', :aggregate_failures do + expect(result.status).to eq(:error) + expect(result.message).to eq('Update failed') + expect(result.reason).to eq(:bad_request) + end + end + end + + context 'when feature is licensed' do + before do + stub_licensed_features(service_accounts: true) + end + + context 'when current user is an admin' do + let(:current_user) { admin } + + context 'when admin mode is not enabled' do + it_behaves_like 'not authorized to update' + end + + context 'when admin mode is enabled', :enable_admin_mode do + it_behaves_like 'authorized to update' + end + end + + context 'when current user is a group owner' do + let(:current_user) { owner } + + it_behaves_like 'authorized to update' + + context 'when saas', :saas do + it_behaves_like 'authorized to update' + end + end + + context 'when current user is not a group owner' do + let(:current_user) { maintainer } + + it_behaves_like 'not authorized to update' + end + end + + context 'when feature is not licensed' do + before do + stub_licensed_features(service_accounts: false) + end + + context 'when current user is an admin' do + let(:current_user) { admin } + + context 'when admin mode is enabled', :enable_admin_mode do + it_behaves_like 'not authorized to update' + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 621c134ccece390bd0e83d5dd536d8361d51de4d..76a79326b7305f89faad23829881a1d75ddf26d6 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -54673,6 +54673,9 @@ msgstr "" msgid "Service account token expiration" msgstr "" +msgid "Service account was successfully updated." +msgstr "" + msgid "Service accounts" msgstr "" @@ -54715,6 +54718,9 @@ msgstr "" msgid "ServiceAccount|User does not have permission to delete a service account." msgstr "" +msgid "ServiceAccount|You are not authorized to update service accounts in this namespace." +msgstr "" + msgid "ServiceDesk|%{customEmail} with SMTP host %{smtpAddress} is %{badgeStart}verified%{badgeEnd}" msgstr "" @@ -63362,6 +63368,9 @@ msgstr[1] "" msgid "User is blocked" msgstr "" +msgid "User is not a service account" +msgstr "" + msgid "User is not allowed to resolve thread" msgstr ""